sankalpsthakur opened a new pull request, #6798: URL: https://github.com/apache/hive/pull/6798
### What changes were proposed in this pull request? When `hive.cbo.enable=true` (the default), `SemanticAnalyzer.processPositionAlias` skips ORDER BY ordinal substitution because CBO is expected to resolve `ORDER BY 1` in `CalcitePlanner.genSortByKey`. If CBO then **declines** the statement (TABLESAMPLE, SORT BY + LIMIT in a subquery, TRANSFORM, UNIQUEJOIN, charset literals, bucket sample, …), the unsubstituted ordinal is handed to the legacy planner, compiled as a constant sort key, and dropped from the ReduceSink. The query returns rows in an order other than the one requested, with no error or warning. This patch substitutes ORDER BY ordinals on that decline path before `super.genOPTree`, so the planner that actually runs is the one that sees the resolved columns. ```sql create table ob_t (d int); insert into ob_t values (1), (2), (3); set hive.fetch.task.conversion=none; -- CBO declines TABLESAMPLE; before this change the DESC is silently ignored select d from ob_t tablesample (5 rows) s order by 1 desc; -- expected: 3, 2, 1 ``` JIRA: https://issues.apache.org/jira/browse/HIVE-30037 ### Why are the changes needed? Silent wrong order is a correctness bug. The exception-driven CBO fallback (`recompile_without_cbo`) is not affected because it reruns with `hive.cbo.enable=false`. Only the decline path (CBO never invoked) was missing the substitution. HIVE-28725 fixed the mirror case on the CBO-success path. ### Does this PR introduce _any_ user-facing change? Yes: queries that CBO declines and that use positional `ORDER BY n` now sort as requested (matching `hive.cbo.enable=false`). Previously they could return unsorted/wrongly ordered rows. ### How was this patch tested? Unit tests in `TestSemanticAnalyzer`: - TABLESAMPLE (CBO-declined) + `ORDER BY 1 DESC` substitutes the ordinal (`Number` -> `TOK_TABLE_OR_COL`) - subquery `SORT BY … LIMIT` (CBO-declined) does the same - CBO-handled `ORDER BY 1 DESC` still reports "Plan optimized by CBO" (no regression on the success path) `mvn test -pl ql -Dtest=TestSemanticAnalyzer` — 25 tests, 0 failures. ### ICLA I have an Apache ICLA on file. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
