Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/13645 )
Change subject: IMPALA-6590: Disable expr rewrites and codegen for VALUES() statements ...................................................................... Patch Set 11: (3 comments) http://gerrit.cloudera.org:8080/#/c/13645/10/be/src/exec/union-node.cc File be/src/exec/union-node.cc: http://gerrit.cloudera.org:8080/#/c/13645/10/be/src/exec/union-node.cc@44 PS10, Line 44: const int64_t num_nonconst_scalar_expr_to_be_codegened = > Nit: Should we call this "num_nonconst_scalar_expr_to_be_codegened"? It mig Renamed variable as suggested http://gerrit.cloudera.org:8080/#/c/13645/10/be/src/exprs/scalar-expr.h File be/src/exprs/scalar-expr.h: http://gerrit.cloudera.org:8080/#/c/13645/10/be/src/exprs/scalar-expr.h@259 PS10, Line 259: bool is_codegen_disabled > Nit: One option is to give is_codegen_disabled a default value of false. Th Good suggestion. Set default value as false. http://gerrit.cloudera.org:8080/#/c/13645/10/tests/query_test/test_codegen.py File tests/query_test/test_codegen.py: http://gerrit.cloudera.org:8080/#/c/13645/10/tests/query_test/test_codegen.py@96 PS10, Line 96: def test_const_scalar_expr_in_union(self, vector, > Nit: Since this creates tables, it would be good to use a unique_database s done -- To view, visit http://gerrit.cloudera.org:8080/13645 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I229d67b821968321abd8f97f7c89cf2617000d8d Gerrit-Change-Number: 13645 Gerrit-PatchSet: 11 Gerrit-Owner: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Wenzhe Zhou <[email protected]> Gerrit-Comment-Date: Wed, 15 Dec 2021 22:54:59 +0000 Gerrit-HasComments: Yes
