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

Change subject: IMPALA-14600: Support HBO for AggregationNode cardinality
......................................................................


Patch Set 8:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/24297/8/common/thrift/Frontend.thrift
File common/thrift/Frontend.thrift:

http://gerrit.cloudera.org:8080/#/c/24297/8/common/thrift/Frontend.thrift@789
PS8, Line 789:   28: optional list<i32> node_parent_ids
> > I'm also gonna work under the assumption that we can create runtime filte
I guess fair enough.

I still kinda prefer doing it in the Frontend. Heck, it's the type of 
information that we might even want to include in a verbose explain plan to 
show all the nodes affected by a runtime filter.

Another possibility is to have the TRuntimeFilterDesc structure contain all its 
affected NodeIds and just search for the nodeId through all the effective 
TRuntimeFilterDescs. Still extra information, I suppose though. Personally, I 
don't think it's too egregious in terms of memory, but I get the avoidance of 
unnecessary information, and if you like it the way you've already coded it, I 
will give it a +1 as/is.


http://gerrit.cloudera.org:8080/#/c/24297/10/fe/src/main/java/org/apache/impala/planner/PlanNode.java
File fe/src/main/java/org/apache/impala/planner/PlanNode.java:

http://gerrit.cloudera.org:8080/#/c/24297/10/fe/src/main/java/org/apache/impala/planner/PlanNode.java@592
PS10, Line 592:       Preconditions.checkState(nodeResourceProfile_.isValid());
Optional nit: Do we need to check query options here?  Logic in "if" should 
always apply and it's not really a (tiny) performance boost if we have to 
always do an "if" check anyway.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0fafaf9d827f3bf533b1af7e62fdb2303c126ce
Gerrit-Change-Number: 24297
Gerrit-PatchSet: 8
Gerrit-Owner: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Steve Carlin <[email protected]>
Gerrit-Comment-Date: Tue, 07 Jul 2026 20:49:25 +0000
Gerrit-HasComments: Yes

Reply via email to