Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/20850 )
Change subject: IMPALA-12533: Support row materialization outside of fetch lock ...................................................................... Patch Set 15: (3 comments) http://gerrit.cloudera.org:8080/#/c/20850/11//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20850/11//COMMIT_MSG@24 PS11, Line 24: It is expected that the default : threshold will never be reached with a single fetch stream. > Added counter for number of streams as suggested. Thanks! http://gerrit.cloudera.org:8080/#/c/20850/13/be/src/exec/buffered-plan-root-sink.cc File be/src/exec/buffered-plan-root-sink.cc: http://gerrit.cloudera.org:8080/#/c/20850/13/be/src/exec/buffered-plan-root-sink.cc@186 PS13, Line 186: Initialize > Added support to BlockingPlanRootSync. I am not sure if it is really useful to support this in BlockingPlanRootSink - it only holds a single row batch until the client fetches it, so parallel fetches are likely to just wait for BufferedPlanRootSink to get another batch from the plan tree. http://gerrit.cloudera.org:8080/#/c/20850/13/be/src/service/impala-hs2-server.cc File be/src/service/impala-hs2-server.cc: http://gerrit.cloudera.org:8080/#/c/20850/13/be/src/service/impala-hs2-server.cc@251 PS13, Line 251: fetch_results->__set_hasMoreRows(!query_handle->eos()); > The query_handle reference keeps the QueryDriver/RunTimeState/MemPool aroun Was able to cause a crash by forcing delayed materialization + adding a sleep here. I used the following query: select replace(string_col, "0", "1") from functional.alltypes; replace() was chosen because it uses FunctionContext::FRAGMENT_LOCAL scope in its prepare/close functions. Uploaded the patch here. the commit message also includes the calltack: https://github.com/csringhofer/Impala/commit/0ebc8fd36134614fe406253e1b1080901905f4ae When disabling result spooling I could crash it even with a query with functions with prepare/close: set SPOOL_QUERY_RESULTS=0; select concat(string_col, string_col) from functional.alltypes; This failed at calar-expr-evaluator.h:232] Check failed: i < fn_ctxs_.size() (0 vs. 0) impala::ScalarFnCall::OpenEvaluator() impala::ScalarExprEvaluator::Clone() impala::HS2ColumnarResultSet::InitDelayedMaterialization() -- To view, visit http://gerrit.cloudera.org:8080/20850 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If9512aa6022dbcf81e7eb5f559853b89fe80bd23 Gerrit-Change-Number: 20850 Gerrit-PatchSet: 15 Gerrit-Owner: Kurt Deschler <kdesc...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Kurt Deschler <kdesc...@cloudera.com> Gerrit-Reviewer: Michael Smith <michael.sm...@cloudera.com> Gerrit-Comment-Date: Tue, 19 Mar 2024 14:17:03 +0000 Gerrit-HasComments: Yes