feat: add a config to disable subquery_sort_elimination#21614
feat: add a config to disable subquery_sort_elimination#21614alamb merged 7 commits intoapache:mainfrom
Conversation
asolimando
left a comment
There was a problem hiding this comment.
Nice improvement!
In Apache Calcite we had a similar discussion a while ago (CALCITE-4160 for the details) and we reached the same conclusion, preserving ordering for subqueries is an optional feature and it's technically safe to always remove it, but it's good to let the user control this behavior.
There is a typo in the PR title where "disalbe" should be "disable", as this goes into the commit message we should fix it before merging.
Nit: it would be nice to have an SLT test with SET datafusion.sql_parser.enable_subquery_sort_elimination = false in addition to the unit tests, to check if the user-facing propagation works as expected.
|
thanks for your reviews @asolimando and the information from Apache Calcite, i added the .slt test case |
## Which issue does this PR close? - Closes apache#15886 ## Rationale for this change - see apache#15886 (comment) ## What changes are included in this PR? - Added a new optimizer config option: `datafusion.sql_parser.enable_subquery_sort_elimination` - Added a `SessionConfig` builder method: `with_enable_subquery_sort_elimination(...)` - Updated SQL relation planning so subquery/CTE `ORDER BY` elimination only happens when this config is enabled - Kept the current behavior as the default to avoid changing existing behavior unexpectedly ## Are these changes tested? Added SQL integration tests that verify: - subquery `ORDER BY` is removed by default - subquery `ORDER BY` is preserved when `enable_subquery_sort_elimination` is set to `false` - add slt test case ## Are there any user-facing changes? no
Which issue does this PR close?
Rationale for this change
What changes are included in this PR?
datafusion.sql_parser.enable_subquery_sort_eliminationSessionConfigbuilder method:with_enable_subquery_sort_elimination(...)ORDER BYelimination only happens when this config is enabledAre these changes tested?
Added SQL integration tests that verify:
ORDER BYis removed by defaultORDER BYis preserved whenenable_subquery_sort_eliminationis set tofalseAre there any user-facing changes?
no