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

Reply via email to