Skip to content

Conversation

@jycor
Copy link
Contributor

@jycor jycor commented Oct 4, 2023

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

Copy link
Contributor

@max-hoffman max-hoffman left a 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",
Copy link
Contributor

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 == "") {
Copy link
Contributor

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

Copy link
Contributor Author

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)

@jycor jycor merged commit 2a440c2 into main Oct 5, 2023
@jycor jycor deleted the james/qualified branch October 5, 2023 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Column could not be found when using fully qualified table name in ORDER BY

2 participants