Steve Carlin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23562 )

Change subject: IMPALA-13902: Calcite planner: Implement is_spool_query_results
......................................................................


Patch Set 13:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/23562/13/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java:

http://gerrit.cloudera.org:8080/#/c/23562/13/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@2380
PS13, Line 2380:     QueryStmt queryStmt = 
ctx_.getAnalysisResult().getQueryStmt();
> I'm guessing canSpoolResult() return value is only valid (may turn from Tru
I don't think it matters, actually, now that I looked at the code.  I just 
wanted lines 2380 and 2383 to be consistent.

The PlannerContext constructor also grabs from AnalysisResult.  The only 
difference is non query statements may have the querystmt buried, e.g. CTAS.  
So the AnalysisResult does a Preconditions check to ensure it is a query 
statement.

But this method is only called if it's a query statement.  Not sure which one 
is better.  Prolly calling it the other way is better, from ctx_.getQueryStmt() 
in both places.



--
To view, visit http://gerrit.cloudera.org:8080/23562
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5b9bf49e2874ee12de212b892bd898c296774c6f
Gerrit-Change-Number: 23562
Gerrit-PatchSet: 13
Gerrit-Owner: Steve Carlin <[email protected]>
Gerrit-Reviewer: Aman Sinha <[email protected]>
Gerrit-Reviewer: Fang-Yu Rao <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Reviewer: Steve Carlin <[email protected]>
Gerrit-Comment-Date: Fri, 14 Nov 2025 23:48:46 +0000
Gerrit-HasComments: Yes

Reply via email to