Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/20495 )
Change subject: PROTOTYPE: Update TCLIService.thrift to a more recent version ...................................................................... Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/20495/2/be/src/service/impala-hs2-server.cc File be/src/service/impala-hs2-server.cc: http://gerrit.cloudera.org:8080/#/c/20495/2/be/src/service/impala-hs2-server.cc@832 PS2, Line 832: return_val.__set_hasResultSet(query_handle->returns_result_set()); > This doesn't seem to be setting the flag in a place where the result is act The way I understand it is that hasResultSet is not about whether the result is available. It is about whether the statement produces a result at all. In Impala's current version of the protocol, the TExecuteStatementResp for the ExecuteStatement RPC contains the hasResultSet field on the TOperationHandle. Impala does compilation during ExecuteStatement and knows by the end whether there is a result set, so we were fine with this. However, if the ExecuteStatement is doing async compilation (which Hive does), it may not know if the statement produces a result set when it is returning from ExecuteStatement. So, Hive added a hasResultSet field on GetOperationStatus so it could find out later as the statement compiles. I think it is valid for us to set it to true here even if the results aren't available yet and the statement is not in a terminal state. I can look at what Hive does to double check that I'm not misunderstanding it. That said, the Hive JDBC logic shouldn't be looking at the hasResultSet field the way it does. It seems like the fix for HIVE-20621 never needed to reference hasResultSet. -- To view, visit http://gerrit.cloudera.org:8080/20495 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I02320f982e86f539d8eed259a7c5adef49f81de6 Gerrit-Change-Number: 20495 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Comment-Date: Wed, 20 Sep 2023 15:39:07 +0000 Gerrit-HasComments: Yes
