-
-
Notifications
You must be signed in to change notification settings - Fork 216
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
[no-release-notes] test for left join bug #2840
Conversation
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.
LGTM, see comments
}, | ||
tests: []JoinOpTests{ | ||
{ | ||
Query: "select /*+ JOIN_ORDER(ab,xy) */ x from xy left join ab on x = a and z = 5 where a is null order by x ", |
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.
Kind of surprised this join hint does anything, isn't it ignored? Maybe add a comment
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.
for these tests join order does not appear to be respected, the hint and join code has a lot of change, i imagine it worked at some point, we should probably fix that
"insert into ab values (0,1,0), (1,2,1), (2,3,2), (3,4,3);", | ||
}, | ||
}, | ||
tests: []JoinOpTests{ |
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.
Because of the z=5 join condition, you're getting every row in the left table in the result set. You should have a variant where not every row in the left corner is included (because there is a match for at least one of the rows in the right-hand side of the join).
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.
added more tests
}, | ||
}, | ||
{ | ||
name: "type conversion panic bug", |
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.
Should add a comment with details on what the error is here (comparing varchar to int literal)
No description provided.