morrySnow commented on code in PR #63974:
URL: https://github.com/apache/doris/pull/63974#discussion_r3370631767
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/query/QueryStatsRecorder.java:
##########
@@ -260,6 +302,27 @@ private static void walkPlan(Plan plan,
for (NamedExpression expr : agg.getOutputExpressions()) {
Review Comment:
**Code duplication with `PhysicalSetOperation` handler (lines 366–385).**
The `PhysicalRecursiveUnion` handler is nearly identical to the
`PhysicalSetOperation` handler above. Both call `getRegularChildrenOutputs()`
and iterate `SlotReference` lists to record queryHit. Consider extracting a
shared private helper method to avoid ~15 lines of duplicated code and prevent
future drift between the two handlers.
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/query/QueryStatsRecorder.java:
##########
@@ -125,19 +128,42 @@ static Map<String, StatsDelta> collectDeltas(PhysicalPlan
plan) {
: plan.getOutput();
for (NamedExpression ne : rootExprs) {
SlotReference sr = unwrapAlias(ne);
- if (sr == null) {
- continue;
+ if (sr != null) {
+ PhysicalOlapScan sourceScan = exprIdToScan.get(sr.getExprId());
+ if (sourceScan != null) {
+ StatsDelta delta = getOrCreateDelta(deltas, sourceScan);
+ if (delta != null) {
+ String colName = sr.getOriginalColumn().map(col ->
col.getName())
+ .orElseGet(() ->
exprIdToColName.get(sr.getExprId()));
+ if (colName != null) {
+ delta.addQueryStats(colName);
+ }
+ }
+ continue;
+ }
}
- PhysicalOlapScan sourceScan = exprIdToScan.get(sr.getExprId());
- if (sourceScan == null) {
+ // Slot from a computed alias, or a complex root alias
(Alias(a+b)): expand via map.
+ ExprId lookupId = (sr != null) ? sr.getExprId() : ne.getExprId();
+ Set<Slot> inputSlots = aggOutputToInputSlots.get(lookupId);
+ if (inputSlots == null) {
continue;
}
Review Comment:
**Minor consistency issue:** `PhysicalCTEProducer` handler uses `.child()`
to walk the single child, while all other plan nodes (including
`PhysicalStorageLayerAggregate`) rely on `plan.children()` iteration (line
243). Since `PhysicalCTEProducer` extends `PhysicalUnary`, `.child()` and
`.children().get(0)` return the same instance. Using `.children()` would be
more consistent and more robust if the class hierarchy changes.
##########
regression-test/suites/query_p0/stats/query_stats_test.groovy:
##########
@@ -198,6 +198,41 @@ suite("query_stats_test") {
def joinResult = sql "show query stats from ${tbName} all"
assertTrue((joinResult[0][1] as int) >= 1)
+ // UNION ALL: both branches contribute queryHit for k1; WHERE on right
branch adds k2.filterHit.
+ sql "clean all query stats"
+ sql "select k1 from ${tbName} union all select k1 from ${tbName} where k2
= 100"
+ def unionStats = sql "show query stats from ${tbName}"
+ def uK1 = unionStats.find { it[0] == "k1" }
+ def uK2 = unionStats.find { it[0] == "k2" }
+ assertNotNull(uK1)
+ assertNotNull(uK2)
+ assertTrue((uK1[1] as int) >= 1, "k1: UNION ALL queryHit")
+ assertTrue((uK2[2] as int) >= 1, "k2: WHERE on right UNION branch
filterHit")
+
+ // HAVING: k1 queryHit from GROUP BY + SELECT, k2 queryHit from SUM input,
+ // k2 filterHit from HAVING SUM(k2) > -1000.
+ sql "clean all query stats"
+ sql "select k1, sum(k2) from ${tbName} group by k1 having sum(k2) > -1000"
+ def havingStats = sql "show query stats from ${tbName}"
+ def hvK1 = havingStats.find { it[0] == "k1" }
+ def hvK2 = havingStats.find { it[0] == "k2" }
+ assertNotNull(hvK1)
+ assertNotNull(hvK2)
+ assertTrue((hvK1[1] as int) >= 1, "k1: GROUP BY / SELECT queryHit")
+ assertTrue((hvK2[1] as int) >= 1, "k2: SUM aggregate input queryHit")
+ assertTrue((hvK2[2] as int) >= 1, "k2: HAVING SUM(k2) filterHit")
+
+ // CTE: consumer query records queryHit on k2 and filterHit on k1.
+ sql "clean all query stats"
+ sql """with cte as (select k2 from ${tbName} where k1 = 1) select k2 from
cte"""
+ def cteStats = sql "show query stats from ${tbName}"
+ def cteK2 = cteStats.find { it[0] == "k2" }
+ def cteK1 = cteStats.find { it[0] == "k1" }
+ assertNotNull(cteK2)
+ assertNotNull(cteK1)
Review Comment:
**Test coverage gap:** The regression tests cover UNION ALL, HAVING
(single-input), and non-recursive CTE. However, the following new code paths
lack end-to-end regression validation:
- **LATERAL VIEW / EXPLODE** (`PhysicalGenerate`) — very common in production
- **Computed SELECT expressions** (e.g., `SELECT k1+k2 FROM t`) — extremely
common
- **INTERSECT / EXCEPT**, **Recursive CTE**, **IN/EXISTS mark join**,
**Multi-input HAVING** (e.g., `HAVING SUM(k2+k3) > 0`)
Consider adding at least LATERAL VIEW and computed SELECT regression tests,
which are the most common production patterns.
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/query/QueryStatsRecorder.java:
##########
@@ -125,19 +128,42 @@ static Map<String, StatsDelta> collectDeltas(PhysicalPlan
plan) {
: plan.getOutput();
for (NamedExpression ne : rootExprs) {
SlotReference sr = unwrapAlias(ne);
- if (sr == null) {
- continue;
+ if (sr != null) {
+ PhysicalOlapScan sourceScan = exprIdToScan.get(sr.getExprId());
+ if (sourceScan != null) {
+ StatsDelta delta = getOrCreateDelta(deltas, sourceScan);
+ if (delta != null) {
+ String colName = sr.getOriginalColumn().map(col ->
col.getName())
+ .orElseGet(() ->
exprIdToColName.get(sr.getExprId()));
+ if (colName != null) {
+ delta.addQueryStats(colName);
+ }
+ }
+ continue;
+ }
}
- PhysicalOlapScan sourceScan = exprIdToScan.get(sr.getExprId());
- if (sourceScan == null) {
+ // Slot from a computed alias, or a complex root alias
(Alias(a+b)): expand via map.
+ ExprId lookupId = (sr != null) ? sr.getExprId() : ne.getExprId();
+ Set<Slot> inputSlots = aggOutputToInputSlots.get(lookupId);
+ if (inputSlots == null) {
continue;
}
- StatsDelta delta = getOrCreateDelta(deltas, sourceScan);
- if (delta != null) {
- String colName = sr.getOriginalColumn().map(col ->
col.getName())
- .orElseGet(() -> exprIdToColName.get(sr.getExprId()));
- if (colName != null) {
- delta.addQueryStats(colName);
+ for (Slot slot : inputSlots) {
+ if (!(slot instanceof SlotReference)) {
+ continue;
+ }
+ SlotReference inputSr = (SlotReference) slot;
Review Comment:
**Defensive design concern:** The comment says "leaf node: no children to
walk" and returns early. While true today, `PhysicalOlapScan` and
`PhysicalLazyMaterializeOlapScan` also have no plan children but don't
explicitly return — they fall through to the empty `children()` iteration
naturally. If `PhysicalCTEConsumer` ever gains children in the future, this
early return would silently skip them. Consider removing the explicit `return`
and moving the consumer slot mapping logic before the `children()` loop (above
line 243), following the same pattern as scan handlers above.
##########
fe/fe-core/src/test/java/org/apache/doris/statistics/query/QueryStatsRecorderTest.java:
##########
@@ -923,6 +930,433 @@ public void
testPhysicalPartitionTopNRegistersPartitionAndOrderKeysAsQueryHit()
// ── helpers
──────────────────────────────────────────────────────────────
+ /**
+ * Plan: CTEConsumer (consumerSlot#3 → producerSlot#1=k1) → Scan[k1(#1)]
+ * Expected: k1.queryHit=true via consumer slot.
+ */
+ @Test
+ public void testCteConsumerMapsToProducerScan() {
+ ExprId prodId = new ExprId(1);
+ ExprId consId = new ExprId(3);
+ SlotReference prodSlot = mockSlot(prodId, "k1");
+ SlotReference consSlot = mockSlot(consId, "k1");
+
+ PhysicalOlapScan scan = mockScan(1L, 1L, 1L, 1L,
ImmutableList.of(prodSlot));
+
+ PhysicalCTEConsumer consumer = Mockito.mock(PhysicalCTEConsumer.class);
+
Mockito.when(consumer.getOutput()).thenReturn(ImmutableList.of(consSlot));
+ Mockito.when(consumer.getProducerSlot(consSlot)).thenReturn(prodSlot);
+ Mockito.when(consumer.children()).thenReturn(ImmutableList.of());
+
+ // Root plan: scan (producer) visited first, consumer slots then get
registered.
+ PhysicalFilter<?> root = Mockito.mock(PhysicalFilter.class);
+ Mockito.when(root.children()).thenReturn(ImmutableList.of(scan,
consumer));
+ Mockito.when(root.getConjuncts()).thenReturn(ImmutableSet.of());
+ Mockito.when(root.getOutput()).thenReturn(ImmutableList.of(consSlot));
+
+ Map<String, StatsDelta> deltas =
QueryStatsRecorder.collectDeltas((PhysicalPlan) root);
+
+ Assertions.assertEquals(1, deltas.size());
+ StatsDelta delta = deltas.values().iterator().next();
+ Assertions.assertNotNull(delta.getColumnStats().get("k1"));
+ Assertions.assertTrue(delta.getColumnStats().get("k1").queryHit);
+ }
+
+ /**
+ * Plan: UNION of Scan1[k1(#1)] and Scan2[k1(#3)]
+ * Expected: k1.queryHit=true on both scan tables.
+ */
+ @Test
+ public void testUnionRecordsQueryHitOnAllBranches() {
+ ExprId id1 = new ExprId(1);
+ ExprId id3 = new ExprId(3);
+ SlotReference k1Left = mockSlot(id1, "k1");
+ SlotReference k1Right = mockSlot(id3, "k1");
+
+ PhysicalOlapScan scan1 = mockScan(1L, 1L, 1L, 1L,
ImmutableList.of(k1Left));
+ PhysicalOlapScan scan2 = mockScan(1L, 2L, 2L, 1L,
ImmutableList.of(k1Right));
+
+ PhysicalSetOperation union = Mockito.mock(PhysicalSetOperation.class);
+ Mockito.when(union.children()).thenReturn(ImmutableList.of(scan1,
scan2));
+ Mockito.when(union.getRegularChildrenOutputs())
+ .thenReturn(ImmutableList.of(ImmutableList.of(k1Left),
ImmutableList.of(k1Right)));
+ Mockito.when(union.getOutput()).thenReturn(ImmutableList.of());
+
+ Map<String, StatsDelta> deltas =
QueryStatsRecorder.collectDeltas((PhysicalPlan) union);
+
+ Assertions.assertEquals(2, deltas.size());
+ for (StatsDelta delta : deltas.values()) {
+ Assertions.assertNotNull(delta.getColumnStats().get("k1"));
+ Assertions.assertTrue(delta.getColumnStats().get("k1").queryHit);
+ }
+ }
+
+ /**
+ * Plan: Filter(SUM(k2)#5 > 0) → Agg[SUM(k2)] → Scan[k2(#2)]
+ * Expected: k2.filterHit=true from HAVING SUM(k2) > 0.
+ */
+ @Test
+ @SuppressWarnings("unchecked")
+ public void testHavingAggregateFilterHitRecorded() {
+ ExprId k2Id = new ExprId(2);
+ ExprId sumId = new ExprId(5);
+ SlotReference k2Slot = mockSlot(k2Id, "k2");
+ // Aggregate output slots have no originalColumn in real Doris; use
Optional.empty()
+ // so recordInputSlotsAsFilterHit falls back to
exprIdToColName.get(sumId) = "k2".
+ SlotReference sumSlot = Mockito.mock(SlotReference.class);
+ Mockito.when(sumSlot.getExprId()).thenReturn(sumId);
+ Mockito.when(sumSlot.getOriginalColumn()).thenReturn(Optional.empty());
+
+ PhysicalOlapScan scan = mockScan(1L, 1L, 1L, 1L,
ImmutableList.of(k2Slot));
+
+ NamedExpression sumExpr = Mockito.mock(NamedExpression.class);
+ Mockito.when(sumExpr.getExprId()).thenReturn(sumId);
+
Mockito.when(sumExpr.getInputSlots()).thenReturn(ImmutableSet.of(k2Slot));
+
+ Aggregate<?> agg = Mockito.mock(Aggregate.class);
+ Mockito.when(agg.children()).thenReturn(ImmutableList.of(scan));
+
Mockito.when(agg.getGroupByExpressions()).thenReturn(ImmutableList.of());
+ Mockito.when(agg.getOutputExpressions())
+ .thenReturn((List<NamedExpression>) (List<?>)
ImmutableList.of(sumExpr));
+ Mockito.when(agg.getOutput()).thenReturn(ImmutableList.of(sumSlot));
+
+ Expression havingConjunct = Mockito.mock(Expression.class);
+
Mockito.when(havingConjunct.getInputSlots()).thenReturn(ImmutableSet.of(sumSlot));
+
+ PhysicalFilter<?> havingFilter = Mockito.mock(PhysicalFilter.class);
+
Mockito.when(havingFilter.children()).thenReturn(ImmutableList.of(agg));
+
Mockito.when(havingFilter.getConjuncts()).thenReturn(ImmutableSet.of(havingConjunct));
+ Mockito.when(havingFilter.getOutput()).thenReturn(ImmutableList.of());
+
+ Map<String, StatsDelta> deltas =
+ QueryStatsRecorder.collectDeltas((PhysicalPlan) havingFilter);
+
+ Assertions.assertEquals(1, deltas.size());
+ StatsDelta delta = deltas.values().iterator().next();
+ Assertions.assertNotNull(delta.getColumnStats().get("k2"));
+ Assertions.assertTrue(delta.getColumnStats().get("k2").filterHit);
+ }
+
+ /**
+ * Plan: Generate(EXPLODE(tags#2)) → Scan[tags(#2)]
+ * Expected: tags.queryHit=true from the generator input column.
+ */
+ @Test
+ public void testLateralViewExplodeRecordsGeneratorInputAsQueryHit() {
+ ExprId tagsId = new ExprId(2);
+ SlotReference tagsSlot = mockSlot(tagsId, "tags");
+
+ PhysicalOlapScan scan = mockScan(1L, 1L, 1L, 1L,
ImmutableList.of(tagsSlot));
+
+ Function explodeFn = Mockito.mock(Function.class);
+
Mockito.when(explodeFn.getInputSlots()).thenReturn(ImmutableSet.of(tagsSlot));
+
+ PhysicalGenerate<?> generate = Mockito.mock(PhysicalGenerate.class);
+ Mockito.when(generate.children()).thenReturn(ImmutableList.of(scan));
+
Mockito.when(generate.getGenerators()).thenReturn(ImmutableList.of(explodeFn));
+ Mockito.when(generate.getOutput()).thenReturn(ImmutableList.of());
+
+ Map<String, StatsDelta> deltas =
+ QueryStatsRecorder.collectDeltas((PhysicalPlan) generate);
+
+ Assertions.assertEquals(1, deltas.size());
+ StatsDelta delta = deltas.values().iterator().next();
+ Assertions.assertNotNull(delta.getColumnStats().get("tags"));
+ Assertions.assertTrue(delta.getColumnStats().get("tags").queryHit);
+ Assertions.assertFalse(delta.getColumnStats().get("tags").filterHit);
+ }
+
+ /**
+ * Plan: PhysicalCTEProducer(Scan[k1(#1)]) + CTEConsumer(consSlot#3 →
prodSlot#1)
+ * PhysicalCTEProducer must walk its child so the scan is registered
before the
+ * consumer is processed by the sibling walk.
+ * Expected: k1.queryHit=true via consumer slot resolved through producer
scan.
+ */
+ @Test
+ @SuppressWarnings("unchecked")
+ public void testCTEProducerRegistersScansForConsumer() {
+ ExprId prodId = new ExprId(1);
+ ExprId consId = new ExprId(3);
+ SlotReference prodSlot = mockSlot(prodId, "k1");
+ SlotReference consSlot = mockSlot(consId, "k1");
+
+ PhysicalOlapScan scan = mockScan(1L, 1L, 1L, 1L,
ImmutableList.of(prodSlot));
+
+ PhysicalCTEProducer<?> producer =
Mockito.mock(PhysicalCTEProducer.class);
+ Mockito.when(producer.child()).thenReturn(scan);
+
+ PhysicalCTEConsumer consumer = Mockito.mock(PhysicalCTEConsumer.class);
+
Mockito.when(consumer.getOutput()).thenReturn(ImmutableList.of(consSlot));
+ Mockito.when(consumer.getProducerSlot(consSlot)).thenReturn(prodSlot);
+ Mockito.when(consumer.children()).thenReturn(ImmutableList.of());
+
+ PhysicalPlan root = Mockito.mock(PhysicalPlan.class);
+ Mockito.when(root.children()).thenReturn(ImmutableList.of(producer,
consumer));
+ Mockito.when(root.getOutput()).thenReturn(ImmutableList.of(consSlot));
+
+ Map<String, StatsDelta> deltas =
QueryStatsRecorder.collectDeltas(root);
+
+ Assertions.assertEquals(1, deltas.size());
+ StatsDelta delta = deltas.values().iterator().next();
+ Assertions.assertNotNull(delta.getColumnStats().get("k1"),
+ "k1 must be registered via PhysicalCTEProducer→scan path");
+ Assertions.assertTrue(delta.getColumnStats().get("k1").queryHit,
+ "k1.queryHit must be true via CTE producer→scan→consumer
chain");
+ }
+
+ /**
+ * Plan: Filter(SUM(k2+k3)#5 > 0) → Agg[SUM(k2+k3)] → Scan[k2(#2), k3(#3)]
+ * Multi-input aggregate output is stored in aggOutputToInputSlots so that
a HAVING
+ * filter on SUM(k2+k3) records filterHit on both k2 and k3.
+ * Expected: k2.filterHit=true AND k3.filterHit=true.
+ */
+ @Test
+ @SuppressWarnings("unchecked")
+ public void testHavingMultiInputAggregateFilterHitRecorded() {
+ ExprId k2Id = new ExprId(2);
+ ExprId k3Id = new ExprId(3);
+ ExprId sumId = new ExprId(5);
+ SlotReference k2Slot = mockSlot(k2Id, "k2");
+ SlotReference k3Slot = mockSlot(k3Id, "k3");
+ SlotReference sumSlot = mockSlot(sumId, "sum_k2_k3");
+
+ PhysicalOlapScan scan = mockScan(1L, 1L, 1L, 1L,
ImmutableList.of(k2Slot, k3Slot));
+
+ // SUM(k2+k3): two input slots → must go into aggOutputToInputSlots
+ NamedExpression sumExpr = Mockito.mock(NamedExpression.class);
+ Mockito.when(sumExpr.getExprId()).thenReturn(sumId);
+
Mockito.when(sumExpr.getInputSlots()).thenReturn(ImmutableSet.of(k2Slot,
k3Slot));
+
+ Aggregate<?> agg = Mockito.mock(Aggregate.class);
+ Mockito.when(agg.children()).thenReturn(ImmutableList.of(scan));
+
Mockito.when(agg.getGroupByExpressions()).thenReturn(ImmutableList.of());
+ Mockito.when(agg.getOutputExpressions())
+ .thenReturn((List<NamedExpression>) (List<?>)
ImmutableList.of(sumExpr));
+ Mockito.when(agg.getOutput()).thenReturn(ImmutableList.of(sumSlot));
+
+ // HAVING SUM(k2+k3) > 0: conjunct references the aggregate output slot
+ Expression havingConjunct = Mockito.mock(Expression.class);
+
Mockito.when(havingConjunct.getInputSlots()).thenReturn(ImmutableSet.of(sumSlot));
+
+ PhysicalFilter<?> havingFilter = Mockito.mock(PhysicalFilter.class);
+
Mockito.when(havingFilter.children()).thenReturn(ImmutableList.of(agg));
+
Mockito.when(havingFilter.getConjuncts()).thenReturn(ImmutableSet.of(havingConjunct));
+ Mockito.when(havingFilter.getOutput()).thenReturn(ImmutableList.of());
+
+ Map<String, StatsDelta> deltas =
+ QueryStatsRecorder.collectDeltas((PhysicalPlan) havingFilter);
+
+ Assertions.assertEquals(1, deltas.size());
+ StatsDelta delta = deltas.values().iterator().next();
+ Assertions.assertNotNull(delta.getColumnStats().get("k2"),
+ "k2 must be recorded via multi-input HAVING expansion");
+ Assertions.assertTrue(delta.getColumnStats().get("k2").filterHit,
+ "k2.filterHit must be true from HAVING SUM(k2+k3)");
+ Assertions.assertNotNull(delta.getColumnStats().get("k3"),
+ "k3 must be recorded via multi-input HAVING expansion");
+ Assertions.assertTrue(delta.getColumnStats().get("k3").filterHit,
+ "k3.filterHit must be true from HAVING SUM(k2+k3)");
+ }
+
+ /**
+ * Mark join conjuncts (from IN/EXISTS subquery rewriting) are stored
separately
+ * in AbstractPhysicalJoin and must be processed independently of
hash/other conjuncts.
+ * Plan: HashJoin with k1=k2 in hashJoinConjuncts and k3>0 in
markJoinConjuncts.
+ * Expected: k1.filterHit, k2.filterHit (hash), k3.filterHit (mark).
+ */
+ @Test
+ @SuppressWarnings("unchecked")
+ public void testMarkJoinConjunctsRecordFilterHit() {
+ ExprId id1 = new ExprId(1);
+ ExprId id2 = new ExprId(2);
+ ExprId id3 = new ExprId(3);
+ SlotReference k1Slot = mockSlot(id1, "k1");
+ SlotReference k2Slot = mockSlot(id2, "k2");
+ SlotReference k3Slot = mockSlot(id3, "k3");
+
+ PhysicalOlapScan left = mockScan(1L, 1L, 1L, 1L,
ImmutableList.of(k1Slot, k3Slot));
+ PhysicalOlapScan right = mockScan(2L, 2L, 2L, 2L,
ImmutableList.of(k2Slot));
+
+ Expression hashConjunct = Mockito.mock(Expression.class);
+
Mockito.when(hashConjunct.getInputSlots()).thenReturn(ImmutableSet.of(k1Slot,
k2Slot));
+
+ // Mark conjunct: k3 > 0 — only reachable via getMarkJoinConjuncts()
+ Expression markConjunct = Mockito.mock(Expression.class);
+
Mockito.when(markConjunct.getInputSlots()).thenReturn(ImmutableSet.of(k3Slot));
+
+ org.apache.doris.nereids.trees.plans.physical.AbstractPhysicalJoin<?,
?> join =
+
Mockito.mock(org.apache.doris.nereids.trees.plans.physical.PhysicalHashJoin.class);
+ Mockito.when(join.children()).thenReturn(ImmutableList.of(left,
right));
+
Mockito.when(join.getHashJoinConjuncts()).thenReturn(ImmutableList.of(hashConjunct));
+
Mockito.when(join.getOtherJoinConjuncts()).thenReturn(ImmutableList.of());
+
Mockito.when(join.getMarkJoinConjuncts()).thenReturn(ImmutableList.of(markConjunct));
+ Mockito.when(join.getOutput()).thenReturn(ImmutableList.of());
+
+ Map<String, StatsDelta> deltas =
QueryStatsRecorder.collectDeltas((PhysicalPlan) join);
+
+ Assertions.assertTrue(deltas.containsKey("1_1_1_1"), "left scan delta
missing");
+ Assertions.assertTrue(deltas.containsKey("2_2_2_2"), "right scan delta
missing");
+
Assertions.assertTrue(deltas.get("1_1_1_1").getColumnStats().get("k1").filterHit,
+ "k1.filterHit: hash join conjunct");
+
Assertions.assertTrue(deltas.get("2_2_2_2").getColumnStats().get("k2").filterHit,
+ "k2.filterHit: hash join conjunct");
+
Assertions.assertTrue(deltas.get("1_1_1_1").getColumnStats().get("k3").filterHit,
+ "k3.filterHit: mark join conjunct — previously missed");
+ }
+
+ /**
+ * PhysicalRecursiveUnion extends PhysicalBinary, not
PhysicalSetOperation, so its
+ * getRegularChildrenOutputs() was never called. The base-case slots (from
an OlapScan)
+ * must get queryHit; recursive-case slots (from WorkTableReference) are
silently skipped.
+ * Plan: RecursiveUnion with base-case output [k1(#1)] from scan,
recursive-case [k1(#5)] skipped.
+ * Expected: k1.queryHit=true (base case only).
+ */
+ @Test
+ @SuppressWarnings("unchecked")
+ public void testRecursiveUnionBaseColumnGetsQueryHit() {
+ ExprId baseScanId = new ExprId(1);
+ ExprId recursiveId = new ExprId(5); // WorkTableReference slot — not
in exprIdToScan
+ SlotReference baseScanSlot = mockSlot(baseScanId, "k1");
+ SlotReference recursiveSlot = mockSlot(recursiveId, "k1");
+
+ PhysicalOlapScan scan = mockScan(1L, 1L, 1L, 1L,
ImmutableList.of(baseScanSlot));
+
Review Comment:
**Minor: Mock accuracy.** `PhysicalRecursiveUnion` extends `PhysicalBinary`
and has two children (base case + recursive case), but the mock only returns
one
child:`Mockito.when(recUnion.children()).thenReturn(ImmutableList.of(scan));`.
Consider adding a second non-scan child (e.g., WorkTableReference mock) to
verify that recursive-case slot references are correctly skipped by the
null-check in the handler.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
