Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/22813 )
Change subject: IMPALA-13584: Add option to shows num row report in impala-shell ...................................................................... Patch Set 3: (6 comments) http://gerrit.cloudera.org:8080/#/c/22813/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/22813/3//COMMIT_MSG@10 PS3, Line 10: Fetched X row(s) I'd include the part about the elapsed time in the example because that is the motivation for adding this in HS2 too. Something like "Fetched X row(s) in Ys". http://gerrit.cloudera.org:8080/#/c/22813/3//COMMIT_MSG@16 PS3, Line 16: Added --beeswax_compat option in impala-shell. If this option is set, We could add that it is set by default. http://gerrit.cloudera.org:8080/#/c/22813/3/shell/impala_client.py File shell/impala_client.py: http://gerrit.cloudera.org:8080/#/c/22813/3/shell/impala_client.py@1400 PS3, Line 1400: if True in set(map(query_substr.startswith, excluded_query_types)): Now that we've modified this, we could also use "any" instead of creating a set to have short circuiting. "if any(map(query_substr.startswith, excluded_query_types)): ..." http://gerrit.cloudera.org:8080/#/c/22813/3/shell/option_parser.py File shell/option_parser.py: http://gerrit.cloudera.org:8080/#/c/22813/3/shell/option_parser.py@365 PS3, Line 365: beeswax_compat I think 'beeswax_compat' may be too broad a term. There may be other aspects where we may want to introduce compatibility with beeswax, and it may become misleading then. An example from the past is IMPALA-10660. We could for example use 'beeswax_compat_num_rows' or something similar. http://gerrit.cloudera.org:8080/#/c/22813/3/tests/shell/test_shell_interactive.py File tests/shell/test_shell_interactive.py: http://gerrit.cloudera.org:8080/#/c/22813/3/tests/shell/test_shell_interactive.py@1075 PS3, Line 1075: use Nit: uses. http://gerrit.cloudera.org:8080/#/c/22813/3/tests/shell/test_shell_interactive.py@1077 PS3, Line 1077: test Nit: tests. -- To view, visit http://gerrit.cloudera.org:8080/22813 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id76ede98c514f73ff1dfa123a0d951e80e7508b4 Gerrit-Change-Number: 22813 Gerrit-PatchSet: 3 Gerrit-Owner: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Comment-Date: Fri, 25 Apr 2025 10:06:12 +0000 Gerrit-HasComments: Yes