Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable table name uniquification, fix column uniquification #14055

Closed
AndriySvyryd opened this issue Nov 30, 2018 · 6 comments · Fixed by #14818
Closed

Enable table name uniquification, fix column uniquification #14055

AndriySvyryd opened this issue Nov 30, 2018 · 6 comments · Fixed by #14818
Labels
breaking-change closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Milestone

Comments

@AndriySvyryd
Copy link
Member

Currently table names longer than MaxIdentifierLength are just truncated.

There's also a bug in column uniquification that prefixes duplicate columns with the type name even on the same type.

@ajcvickers ajcvickers added this to the 3.0.0 milestone Dec 3, 2018
AndriySvyryd added a commit that referenced this issue Feb 25, 2019
Don't prefix columns on the same type when uniquifying

Fixes #14055
AndriySvyryd added a commit that referenced this issue Feb 25, 2019
Don't prefix columns on the same type when uniquifying

Fixes #14055
@AndriySvyryd AndriySvyryd reopened this Feb 25, 2019
@AndriySvyryd AndriySvyryd added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Feb 25, 2019
@AndriySvyryd AndriySvyryd removed their assignment Mar 28, 2019
@ajcvickers ajcvickers modified the milestones: 3.0.0, 3.0.0-preview4 Apr 15, 2019
@roji
Copy link
Member

roji commented Apr 20, 2019

@AndriySvyryd, after updating Npgsql to preview4 I get:

2019-04-20 14:00:03.826 CEST [6256] npgsql_tests@PartialUpdateNpgsqlTest LOG:  execute <unnamed>: CREATE UNIQUE INDEX "IX_LoginEntityTypeWithAnExtremelyLongAndOverlyConvolutedNameTh~" ON "LoginEntityTypeWithAnExtremelyLongAndOverlyConvolutedNameThat~1" ("ProfileId", "ProfileId1", "ProfileId3", "ProfileId4", "ProfileId5", "ProfileId6", "ProfileId7", "ProfileId8", "ProfileId9", "ProfileId10", "ProfileId11", "ProfileId12", "ProfileId13", "ProfileId14")                                                                                        
2019-04-20 14:00:03.840 CEST [6256] npgsql_tests@PartialUpdateNpgsqlTest LOG:  execute <unnamed>: CREATE INDEX "IX_LoginEntityTypeWithAnExtremelyLongAndOverlyConvolutedNameTh~" ON "LoginEntityTypeWithAnExtremelyLongAndOverlyConvolutedNameThatI~" ("ProfileId", "ProfileId1", "ProfileId3", "ProfileId4", "ProfileId5", "ProfileId6", "ProfileId7", "ProfileId8", "ProfileId9", "ProfileId10", "ProfileId11", "ProfileId12", "ProfileId13", "ProfileId14", "ExtraProperty")                                                                              
2019-04-20 14:00:03.841 CEST [6256] npgsql_tests@PartialUpdateNpgsqlTest ERROR:  relation "IX_LoginEntityTypeWithAnExtremelyLongAndOverlyConvolutedNameTh~" already exists                                                                                                    

Seems like there may be an issue with the uniquification for the indexes? Not sure why two are being created (one UNIQUE, one non-UNIQUE)... Anything I'm doing wrong?

@roji
Copy link
Member

roji commented Apr 20, 2019

Note that the max identifier length on PostgreSQL is 63

@ErikEJ
Copy link
Contributor

ErikEJ commented Apr 20, 2019

Should that limit not be set in the test?

@roji
Copy link
Member

roji commented Apr 20, 2019

I haven't had time to dive in, but I don't think so - at least up to preview4 there was nothing specifically needed to be set up in the tests...

@AndriySvyryd
Copy link
Member Author

@roji This is a bug and precisely the kind this test should catch. The indexes are being created on different tables, but the uniquification code only looks at the indexes in any single table.

@roji
Copy link
Member

roji commented Apr 20, 2019

Thanks @AndriySvyryd, opened #15425 to track. Am curious why this didn't fail on SqlServer/Sqlite (identifier limitation wasn't hit for some reason?)

@ajcvickers ajcvickers modified the milestones: 3.0.0-preview4, 3.0.0 Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants