Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/17023 )
Change subject: IMPALA-10473: Fix wrong analytic results on constant partition/order by exprs ...................................................................... Patch Set 2: (5 comments) Thanks for the quick review! Addressed the comments. http://gerrit.cloudera.org:8080/#/c/17023/2/testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test File testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test: http://gerrit.cloudera.org:8080/#/c/17023/2/testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test@3210 PS2, Line 3210: plased > nit: change to 'placed' Done http://gerrit.cloudera.org:8080/#/c/17023/2/testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test@3212 PS2, Line 3212: select row_number() over (order by 'a') from functional.alltypes > One other suggestion for the test since we are concerned about correctness Good point! Added more tests. http://gerrit.cloudera.org:8080/#/c/17023/2/testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test@3229 PS2, Line 3229: plased > nit: same as above Done http://gerrit.cloudera.org:8080/#/c/17023/2/testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test@3261 PS2, Line 3261: plased > nit: same as above Done http://gerrit.cloudera.org:8080/#/c/17023/2/testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test File testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test: http://gerrit.cloudera.org:8080/#/c/17023/2/testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test@2200 PS2, Line 2200: select row_number() over (order by 'a'), count() over (order by 0) > This query actually produces the right results without the patch because al Thanks for the suggestion! I guess you are running the query without "set exec_single_node_rows_threshold=0". It can hit the bug as long as the table contains several files because this test is ran with exec_single_node_rows_threshold=0 which is in the default test dimension: https://github.com/apache/impala/blob/60f8f87b09a27618df2ac73c1cc6dcd052f8c60d/tests/common/test_dimensions.py#L169 I run the test using impala-py.test tests/query_test/test_queries.py::TestQueries::test_analytic_fns The ouputs show me that tests are ran with exec_single_node_rows_threshold=0: tests/query_test/test_queries.py::TestQueries::test_analytic_fns[protocol: beeswax | exec_option: {'batch_size': 0, 'num_nodes': 0, 'disable_codegen_rows_threshold': 0, 'disable_codegen': False, 'abort_on_error': 1, 'exec_single_node_rows_threshold': 0} | table_format: parquet/none] PASSED tests/query_test/test_queries.py::TestQueries::test_analytic_fns[protocol: hs2 | exec_option: {'batch_size': 0, 'num_nodes': 0, 'disable_codegen_rows_threshold': 0, 'disable_codegen': False, 'abort_on_error': 1, 'exec_single_node_rows_threshold': 0} | table_format: parquet/none] PASSED tests/query_test/test_queries.py::TestQueries::test_analytic_fns[protocol: hs2-http | exec_option: {'batch_size': 0, 'num_nodes': 0, 'disable_codegen_rows_threshold': 0, 'disable_codegen': False, 'abort_on_error': 1, 'exec_single_node_rows_threshold': 0} | table_format: parquet/none] PASSED Reverting changes in FE can fail this test. -- To view, visit http://gerrit.cloudera.org:8080/17023 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibc88a410dab984ff37e27dc635bee5f289003a2a Gerrit-Change-Number: 17023 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Fri, 05 Feb 2021 01:40:34 +0000 Gerrit-HasComments: Yes
