Aleksandr Efimov has posted comments on this change. ( http://gerrit.cloudera.org:8080/24608 )
Change subject: IMPALA-15175: Support HBO for AnalyticEvalNode cardinality ...................................................................... Patch Set 4: (2 comments) Went through PS4. The operand-qualifier extension for analytic inputs (mapping the output tuple to the operand's own prefix) fits the JoinNode descent model, and the FNS/PART sorted-set vs ORDER sequence split matches how cardinality actually behaves. Two comments below; the first one is the same key-identity gap that IMPALA-15127's review already fixed in UnionNode/AggregationNode/SortNode. http://gerrit.cloudera.org:8080/#/c/24608/4/fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java File fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java: http://gerrit.cloudera.org:8080/#/c/24608/4/fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java@357 PS4, Line 357: .append(":AnalyticEvalNode:"); limit_ is missing from the key. An AnalyticEvalNode can carry a limit: for a statement with LIMIT and no ORDER BY the planner sets it on the root node (SingleNodePlanner.java:337) and immediately re-runs computeStats(), so `select rank() over (order by id) from t limit 10` and the same query without LIMIT share this key. The limited execution then writes num_rows=10 under the shared key, and the unlimited query's lookup matches it (same scan input stats) and adopts cardinality 10 with no cap to rescue it - capCardinalityAtLimit() is a no-op when limit_ is unset. UnionNode, AggregationNode and SortNode all append `limit:` for exactly this reason; this node needs it too, plus a regression test with the limited/unlimited pair. http://gerrit.cloudera.org:8080/#/c/24608/4/fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java@380 PS4, Line 380: Boolean nullsFirst = e.getNullsFirstParam(); getNullsFirstParam() is the raw parse-level value (null when unspecified), so an explicit `ASC NULLS LAST` and a plain `ASC` - semantically identical - render different keys and miss each other's stats. SortNode's key builds on SortInfo's normalized booleans, so the same ordering hashes differently depending on which node renders it. Normalizing with OrderByElement.nullsFirst(e.getNullsFirstParam(), e.isAsc()) (OrderByElement.java:121) and always printing the resolved direction would make equal orderings share a key and keep the two nodes consistent. -- To view, visit http://gerrit.cloudera.org:8080/24608 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8d87ab472db9273622f48fa62bb1d8c12e04e86b Gerrit-Change-Number: 24608 Gerrit-PatchSet: 4 Gerrit-Owner: Quanlong Huang <[email protected]> Gerrit-Reviewer: Aleksandr Efimov <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Steve Carlin <[email protected]> Gerrit-Comment-Date: Mon, 17 Aug 2026 08:55:01 +0000 Gerrit-HasComments: Yes
