Aleksandr Efimov has posted comments on this change. ( http://gerrit.cloudera.org:8080/24556 )
Change subject: IMPALA-15127: Support HBO for UnionNode cardinality ...................................................................... Patch Set 8: (4 comments) http://gerrit.cloudera.org:8080/#/c/24556/8/fe/src/main/java/org/apache/impala/planner/UnionNode.java File fe/src/main/java/org/apache/impala/planner/UnionNode.java: http://gerrit.cloudera.org:8080/#/c/24556/8/fe/src/main/java/org/apache/impala/planner/UnionNode.java@383 PS8, Line 383: return sb.toString(); Should we include `limit_` in this key, as `HdfsScanNode` does? A top-level `UNION ALL ... LIMIT 1` keeps the limit on the UnionNode, and the runtime `num_rows` stored in HBO is 1. Since the current key is identical to the unlimited union, a later unlimited query can reuse cardinality 1. `capCardinalityAtLimit()` only protects the opposite direction. Could we add the UnionNode limit to the key and cover limited-to-unlimited reuse in a test? http://gerrit.cloudera.org:8080/#/c/24556/8/fe/src/main/java/org/apache/impala/planner/UnionNode.java@391 PS8, Line 391: List<PlanNode.HboKeyedNode> sortedChildren = PlanNode.buildSortedHboNodes( I don't think sorting this list by `EXPR_REWRITE` keeps it aligned with every strategy-specific operand list. For example: `Q1: (year=2009 AND int_col=1) UNION ALL (year=2019 AND int_col=-999)` `Q2: (year=2009 AND int_col=-999) UNION ALL (year=2019 AND int_col=1)` The `IGNORE_PARTITION_CONSTANTS` Union keys are equal. However, the scan stats for both runs are ordered by year, so positional matching compares 2009 with 2009 and 2019 with 2019. The corresponding `int_col` operands actually use different partitions and should fail the input-row check. This can produce a false HBO hit. Could we associate scan stats with the strategy-specific child identity, rather than using one positional list for all strategies? http://gerrit.cloudera.org:8080/#/c/24556/8/fe/src/main/java/org/apache/impala/planner/UnionNode.java@415 PS8, Line 415: populateHboThriftFields(msg, serialCtx); `populateHboThriftFields()` also runs for a constants-only Union. Its key is non-null (`constOps:N|OPERANDS:[]`), but `appendScanInputStats()` leaves the scan list empty. `tryUpdateCardinalityFromHbo()` always skips such a run; the first execution still stores it, and the second execution with the same key reaches `getSimilarRunIndex()` and fails its non-empty scan-stats precondition. Could we either skip HBO fields when the subtree has no scans or explicitly support key-only runs? A test that executes `select 1 union all select 2` twice with HBO storage enabled should cover this. http://gerrit.cloudera.org:8080/#/c/24556/8/fe/src/main/java/org/apache/impala/service/HistoricalStats.java File fe/src/main/java/org/apache/impala/service/HistoricalStats.java: http://gerrit.cloudera.org:8080/#/c/24556/8/fe/src/main/java/org/apache/impala/service/HistoricalStats.java@118 PS8, Line 118: for (int i = 0; i < runs.size(); i++) { Similarity ranges can overlap, so returning the first match does not always return the newest run. With a 10% threshold, write input rows 100, then 120, then 110. The first two are retained; writing 110 removes 100 and leaves `[120, 110]`. A lookup for 110 then matches the old 120 run before the exact new 110 run. Could we search from newest to oldest, or remove all matching runs on write? It would also be useful to extend `testWriteReadDedupBySize()` with this three-run case. -- To view, visit http://gerrit.cloudera.org:8080/24556 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie228f530bdcb171d3b717966673164bf9a4c45c8 Gerrit-Change-Number: 24556 Gerrit-PatchSet: 8 Gerrit-Owner: Quanlong Huang <[email protected]> Gerrit-Reviewer: Aleksandr Efimov <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Steve Carlin <[email protected]> Gerrit-Comment-Date: Sun, 02 Aug 2026 12:13:28 +0000 Gerrit-HasComments: Yes
