Xiang Yang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20344 )
Change subject: IMPALA-10120: Add required fields for TGetInfoResp when error. ...................................................................... Patch Set 5: (2 comments) Hi quanlong, thanks for pointing this out. Here's a small query we could discuss. http://gerrit.cloudera.org:8080/#/c/20344/5/be/src/service/impala-hs2-server.cc File be/src/service/impala-hs2-server.cc: http://gerrit.cloudera.org:8080/#/c/20344/5/be/src/service/impala-hs2-server.cc@1011 PS5, Line 1011: return_val > It seems TGetLogResp could have the same issue in error handling. Its 'log' It seems that the GetLog() interface is only used by the cloudera private driver, and it doesn't exist in high version of Hive thrift protocol, see :https://github.com/apache/impala/blob/4.3.0/common/thrift/hive-1-api/TCLIService.thrift#L1232 and https://github.com/apache/hive/blob/rel/release-3.1.3/service-rpc/if/TCLIService.thrift . Howerver it wouldn't hurt to fix it. http://gerrit.cloudera.org:8080/#/c/20344/5/tests/common/impala_test_suite.py File tests/common/impala_test_suite.py: http://gerrit.cloudera.org:8080/#/c/20344/5/tests/common/impala_test_suite.py@1019 PS5, Line 1019: run_stmt_in_impala > nit: the method name doesn't indicate it's using beeline, what about 'run_i It really isn't a good name, but it can align with 'run_stmt_in_hive' which we don't need to modify all the references of it. So do you think it's not necessary to algin with the name 'run_stmt_in_hive'? or we can bear the cost of modify all the references of it? -- To view, visit http://gerrit.cloudera.org:8080/20344 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib42bb82735fb4a8e6911b6a19adb8bd84973300b Gerrit-Change-Number: 20344 Gerrit-PatchSet: 5 Gerrit-Owner: Xiang Yang <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Xiang Yang <[email protected]> Gerrit-Comment-Date: Sat, 07 Oct 2023 04:23:15 +0000 Gerrit-HasComments: Yes
