-
Notifications
You must be signed in to change notification settings - Fork 2.5k
[CALCITE-7382] The TopDownGeneralDecorrelator returns an error result when a subquery contains a LIMIT 1 #4754
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
base: main
Are you sure you want to change the base?
Conversation
… when a subquery contains a LIMIT 1
|
|
||
| !ok | ||
|
|
||
| select dname, (select empno from emp where dept.deptno = emp.deptno limit 1) from dept where deptno = 10; |
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.
Shouldn't we have an ORDER BY in the sub-query (before the LIMIT 1) to make sure that the query result is deterministic?
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 test is intended to demonstrate that Sort needs to be rewritten as a [Filter-]Window regardless of whether it has an ORDER BY or LIMIT/OFFSET. As I updated in the comment
because now the order/limit/offset has to be enforced per value of the outer bindings instead of globally.
This is exactly what @asolimando was concerned about in his comment below. Perhaps in practical applications there's almost no need to write it this way; can I call this a edge case?
|
|
||
| # [CALCITE-7382] The TopDownGeneralDecorrelator returns an error result when a subquery contains a LIMIT 1 | ||
| !use scott | ||
| SELECT dname, (SELECT emp.comm FROM "scott".emp where dept.deptno = emp.deptno ORDER BY emp.comm limit 1) FROM "scott".dept; |
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.
| SELECT dname, (SELECT emp.comm FROM "scott".emp where dept.deptno = emp.deptno ORDER BY emp.comm limit 1) FROM "scott".dept; | |
| SELECT dname, (SELECT emp.comm FROM "scott".emp where dept.deptno = emp.deptno ORDER BY emp.comm LIMIT 1) FROM "scott".dept; |
| builder.filter(conditions); | ||
| } else { | ||
| builder.sortLimit(sort.offset, sort.fetch, builder.fields(shiftCollation)); | ||
| // the Sort have to be changed during rewriting because now the order/limit/offset has to be |
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.
Q: was this a shortcoming of the original paper or an implementation issue?
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.
The paper's original wording is:
Subqueries with ORDER BY and LIMIT or OFFSET have to be changed....
It doesn't say what to do when only LIMIT or OFFSET is present. However, I believe that even with only LIMIT or OFFSET, it should be rewritten as a window function without ORDER clause.
This matches the test case that @rubenada mentioned in his comment. I performed the same test in the umbra-db interface, and it indeed rewrite the LIMIT as a window function. It seems our approach aligns with the paper's intent.

|
After using the changes made in this PR, this case no longer works properly.
|
|
@xiedeyantu Hi, I don't think this is caused by the decorrelation algorithm. It looks like a loss of significant digits during type casting. The failure seems to come from the
and it fails the same way. Perhaps a new ticket needs to be created to record it. |
Thanks! I have file a new jira CALCITE-7384 to log this issue. |
|
CALCITE-7384 This is an old problem that can be solved using cast(col as bigint), and it has been discussed many times. We can ignore this problem. |
| !ok | ||
|
|
||
| # [CALCITE-7382] The TopDownGeneralDecorrelator returns an error result when a subquery contains a LIMIT 1 | ||
| !use scott |
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.
We can add a note to record where the currently processed case originated.
# This case comes from sub-query.iq [CALCITE-6652]
|



This PR makes two changes:
Sortwill always be rewritten as[Filter-]Window, not only when bothORDER BYandLIMIT/OFFSETare present, becauseORDER BY,LIMITandOFFSEThave to be enforced per value of the outer bindings instead of globally.SortintoFilter-Window, the decorrelator forgot to set the frame clause for the window function, causingROW_NUMBER()to be computed incorrectly. This has now been fixed.