Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/20524 )
Change subject: IMPALA-12426: Adds the backend InternalServer class. ...................................................................... Patch Set 16: (2 comments) http://gerrit.cloudera.org:8080/#/c/20524/16/be/src/runtime/query-driver.h File be/src/runtime/query-driver.h: http://gerrit.cloudera.org:8080/#/c/20524/16/be/src/runtime/query-driver.h@75 PS16, Line 75: /// Wraps a `std:shared_ptr<QueryHandle>` to make it easier to access the query handle : /// methods. The `QueryHandle` struct overloads the member access operator which makes : /// working with shared_ptrs very awkward. This class restores the ability to cleanly : /// use the member access operator. : class QueryHandlePtr { : private: : std::shared_ptr<QueryHandle> ptr_; : : public: : QueryHandlePtr() { : this->ptr_ = make_shared<QueryHandle>(); : } : : QueryHandlePtr(QueryHandle* new_ptr) { : this->ptr_.reset(new_ptr); : } : : ClientRequestState* operator->() const { : return this->ptr_->operator->(); : } : : QueryHandle* get() { : return this->ptr_.get(); : } : : void reset(QueryHandle* new_ptr) { : this->ptr_.reset(new_ptr); : } : }; // class QueryHandlePtr > What is the shared_ptr for? As a reference-counted pointer, when would the I like the idea of just passing query id instead of passing around the query handle. I'm going to switch the functions to take a TUniqueId instead of a QueryHandlePtr. That will also handle the situation where Impala does automatic retries. http://gerrit.cloudera.org:8080/#/c/20524/16/be/src/service/internal-server.h File be/src/service/internal-server.h: http://gerrit.cloudera.org:8080/#/c/20524/16/be/src/service/internal-server.h@171 PS16, Line 171: virtual Status ExecuteAndWait(const std::string& user_name, : const std::string& sql, QueryHandlePtr& query_handle) = 0; > Nit: I think it would be a bit cleaner to avoid having an execute function Good point. I renamed it to "SubmitAndWait" -- To view, visit http://gerrit.cloudera.org:8080/20524 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I27686aa563fac87429657e4980b29b0da91eb9e1 Gerrit-Change-Number: 20524 Gerrit-PatchSet: 16 Gerrit-Owner: Jason Fehr <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Andrew Sherman <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Comment-Date: Tue, 07 Nov 2023 23:27:18 +0000 Gerrit-HasComments: Yes
