Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22358 )
Change subject: IMPALA-13672: Migrate query_test/test_kudu.py to use hs2 protocol ...................................................................... Patch Set 11: Code-Review+2 (5 comments) Thank you Csaba and Jason. Carry +2. http://gerrit.cloudera.org:8080/#/c/22358/10//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/22358/10//COMMIT_MSG@15 PS10, Line 15: clien > Nit: client Done http://gerrit.cloudera.org:8080/#/c/22358/10//COMMIT_MSG@38 PS10, Line 38: Fixed a > Nit: Fixed Done http://gerrit.cloudera.org:8080/#/c/22358/10/fe/src/main/java/org/apache/impala/util/TResultRowBuilder.java File fe/src/main/java/org/apache/impala/util/TResultRowBuilder.java: http://gerrit.cloudera.org:8080/#/c/22358/10/fe/src/main/java/org/apache/impala/util/TResultRowBuilder.java@30 PS10, Line 30: public TResultRowBuilder add(int val) { > Is this function still needed since this patch made changes to cast ints to This is specifically needed if TColumn has Type.INT https://gerrit.cloudera.org/c/22358/10/fe/src/main/java/org/apache/impala/catalog/FeKuduTable.java#151 http://gerrit.cloudera.org:8080/#/c/22358/10/tests/common/impala_test_suite.py File tests/common/impala_test_suite.py: http://gerrit.cloudera.org:8080/#/c/22358/10/tests/common/impala_test_suite.py@360 PS10, Line 360: def create_impala_client(cls, host_port=None, protocol=None, is_hive=False): > Should we deprecate this method and document to use create_impala_client_fr create_impala_client_from_vector() reuse default impala clients of ImpalaTestSuite. I think there are legit cases where a test need to create a new fresh clients instead of using a default one. For example, running multiple concurrent clients/queries in separate sessions. http://gerrit.cloudera.org:8080/#/c/22358/10/tests/common/kudu_test_suite.py File tests/common/kudu_test_suite.py: http://gerrit.cloudera.org:8080/#/c/22358/10/tests/common/kudu_test_suite.py@189 PS10, Line 189: > Should the sql also have an "if exists" clause on it? Done. Also marking this as DEPRECATED. -- To view, visit http://gerrit.cloudera.org:8080/22358 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5f38baf5a0bbde1a1ad0bb4666c300f4f3cabd33 Gerrit-Change-Number: 22358 Gerrit-PatchSet: 11 Gerrit-Owner: Riza Suminto <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Wenzhe Zhou <[email protected]> Gerrit-Comment-Date: Fri, 07 Feb 2025 00:12:30 +0000 Gerrit-HasComments: Yes
