-
-
Notifications
You must be signed in to change notification settings - Fork 238
fix db qualified column names in order by #2059
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
Conversation
…-server into james/qualified
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is solid! I had a couple more ideas reading through your tests. If prisma is generating fully qualified names it might be worth following this up this PR with another that targets the skipped tests you've added here.
| }, | ||
| }, | ||
| { | ||
| Name: "Multi-db Aliasing", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe test join's ON, USING condition?
| var foundCand bool | ||
| for _, c := range s.cols { | ||
| if strings.EqualFold(c.col, col) && (c.table == table || table == "") { | ||
| if strings.EqualFold(c.col, col) && (c.table == table || table == "") && (c.db == db || db == "") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i wonder if there's any weird behavior with col/db non-nil, but table nil....so like select * from xy where mydb.``.x == 1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
idk what this is doing, but it's doing the same thing as mysql
tmp> select db1.t1.i from db1.t1 where db1.``.i > 0;
+---+
| i |
+---+
| 1 |
+---+
1 row in set (0.00 sec)
We did not handle the case where database qualified column names would be in the order by clause.
This PR fixes that issue and adds a variety of tests with database qualified column names and database qualified table names.
There is still a case where joining two tables with the same name from two different databases results in an error; there's a skipped test for this, and a workaround is to alias the tables.
fixes dolthub/dolt#6773