Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/22101 )

Change subject: PROTOTYPE: Add Calcite end-to-end tests
......................................................................


Patch Set 2:

(5 comments)

Approach generally makes sense to me.

http://gerrit.cloudera.org:8080/#/c/22101/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/22101/2//COMMIT_MSG@24
PS2, Line 24:    sections in this cae. First, it produces the regular test 
section
typo: case


http://gerrit.cloudera.org:8080/#/c/22101/2//COMMIT_MSG@33
PS2, Line 33: 6. bin/run-all-tests.sh modfies the EE_TEST section to restart 
with
typo: modifies


http://gerrit.cloudera.org:8080/#/c/22101/2/bin/run-all-tests.sh
File bin/run-all-tests.sh:

http://gerrit.cloudera.org:8080/#/c/22101/2/bin/run-all-tests.sh@390
PS2, Line 390:       USE_CALCITE_PLANNER_SAVE="${USE_CALCITE_PLANNER}"
Could also use parens to start a subshell, as in 
https://github.com/cloudera/native-toolchain/blob/master/buildall.sh#L191.


http://gerrit.cloudera.org:8080/#/c/22101/2/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/22101/2/bin/start-impala-cluster.py@58
PS2, Line 58: BOOLEAN_STRINGS = ["true", "false"]
Why do we use this instead of action="store_true"?


http://gerrit.cloudera.org:8080/#/c/22101/2/testdata/workloads/functional-query/queries/QueryTest/top-n.test
File testdata/workloads/functional-query/queries/QueryTest/top-n.test:

http://gerrit.cloudera.org:8080/#/c/22101/2/testdata/workloads/functional-query/queries/QueryTest/top-n.test@898
PS2, Line 898: # can't handle alias is values clause
typo: in?



--
To view, visit http://gerrit.cloudera.org:8080/22101
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I87400a9bc602b6320c1625448874e76c6caad392
Gerrit-Change-Number: 22101
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Comment-Date: Tue, 26 Nov 2024 00:57:24 +0000
Gerrit-HasComments: Yes

Reply via email to