Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/17104 )
Change subject: IMPALA-10535: Add interface to ImpalaServer for execution of externally compiled statements ...................................................................... Patch Set 10: (3 comments) http://gerrit.cloudera.org:8080/#/c/17104/10/be/src/service/impala-server.h File be/src/service/impala-server.h: http://gerrit.cloudera.org:8080/#/c/17104/10/be/src/service/impala-server.h@679 PS10, Line 679: external_exec_request is a statement that was prepared by an external frontend using : /// Impala PlanNodes. or null if the external frontend isn't being used http://gerrit.cloudera.org:8080/#/c/17104/10/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/17104/10/be/src/service/impala-server.cc@1207 PS10, Line 1207: (*query_handle)->SetExecRequestKrpcAddress(ExecEnv::GetInstance()->krpc_address()); I think you misunderstood my previous comment here, sorry if I was unclear. I was suggesting moving all of these calls to set other values on the TExecRequest into SetExternalPlan() (not just the call to set local_time, which you did move). If you do that, then I think you can eliminate most of the extra ClientRequestState functions that you added - SetExecRequestQueryId/SetExecRequestHostname/etc. http://gerrit.cloudera.org:8080/#/c/17104/10/fe/src/main/java/org/apache/impala/analysis/Analyzer.java File fe/src/main/java/org/apache/impala/analysis/Analyzer.java: http://gerrit.cloudera.org:8080/#/c/17104/10/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@1080 PS10, Line 1080: false could you add a "/* must_exist */" before this, for readability, ie: tbl = getTable(tblName.getDb(), tblName.getTbl(), /* must_exist */ false); here and elsewhere in this patch -- To view, visit http://gerrit.cloudera.org:8080/17104 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iace716dd67290f08441857dc02d2428b0e335eaa Gerrit-Change-Number: 17104 Gerrit-PatchSet: 10 Gerrit-Owner: Kurt Deschler <kdesc...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: John Sherman <j...@cloudera.com> Gerrit-Reviewer: Kurt Deschler <kdesc...@cloudera.com> Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-Comment-Date: Tue, 02 Mar 2021 18:55:05 +0000 Gerrit-HasComments: Yes
