Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/24297 )
Change subject: IMPALA-14600: Support HBO for AggregationNode cardinality ...................................................................... Patch Set 12: (3 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: // HDFS URI for the jar > Another possibility is to have the TRuntimeFilterDesc structure contain all > its affected NodeIds and just search for the nodeId through all the effective > TRuntimeFilterDescs. We already have these info in TRuntimeFilterDesc: list<TRuntimeFilterTargetDesc> https://github.com/apache/impala/blob/21cd965f1670580f96d0ea846d9a2e83f66a2176/common/thrift/PlanNodes.thrift#L166 However, that's not enough. Nodes whose cardinalities are impacted are those from the target scan node through the path to the join node (exclusive) that produces the RF. E.g. in the following example, 06:JoinNode produces RF00 which is assigned to 00:ScanNode. 06:JoinNode ---> RF00 |-- 03:ScanNode 05:JoinNode |-- 02:ScanNode 04:JoinNode |-- 01:ScanNode 00:ScanNode <--- RF00 If RF00 filtered out some rows in 00:ScanNode, the cardinalities of 00:ScanNode, 04:JoinNode, 05:JoinNode are impacted. In BE, the effective RF info just have the pair of (RF00, 00:ScanNode). We need the singleNodePlan to walk through the tree till the source JoinNode. That's where the new parend id field helps. > 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. Reusing the above example, caculating the list (external RFs with target nodes) in FE will let TPlanNode of 00:ScanNode, 04:JoinNode, 05:JoinNode both have the item (RF00, node_00) in the list, which is redundant. I think if we want to show nodes affected by a runtime filter, we won't use this info since the explain output is generated in FE directly (don't need to pass these lists to BE). 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: if (queryOptions != null && queryOptions.store_hbo_stats) { > Optional nit: Do we need to check query options here? Logic in "if" should store_hbo_stats is false by default. When it's false, the optional node_parent_id field will be unset instead of -1. I don't know if this field will be helpful for other works. If so, we can always set them in Frontend.createPlanExecInfo() and remove this if-check. http://gerrit.cloudera.org:8080/#/c/24297/10/fe/src/main/java/org/apache/impala/planner/SortNode.java File fe/src/main/java/org/apache/impala/planner/SortNode.java: http://gerrit.cloudera.org:8080/#/c/24297/10/fe/src/main/java/org/apache/impala/planner/SortNode.java@214 PS10, Line 214: * Under special cases, the planner may decide to convert a total sort or SortNode changes the cardinality if it applies a limit, i.e. TopN. Removed this in the current patch for simplicity. We will add HBO support for SortNode in a future patch. -- 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: 12 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, 14 Jul 2026 23:51:41 +0000 Gerrit-HasComments: Yes
