github-actions[bot] commented on code in PR #65846:
URL: https://github.com/apache/doris/pull/65846#discussion_r3711830063
##########
regression-test/suites/nereids_rules_p0/set_preagg/set_preagg.groovy:
##########
@@ -349,4 +585,471 @@ suite("set_preagg") {
select count(*) from (select * from numbers("number"="10")) t;
""")
}
+ order_qt_q13 """
+ select count(*) from (select * from numbers("number"="10")) t;
+ """
+
+ explain {
+ sql("""select count(distinct k6, v7) from preagg_t1;""")
+ notContains "(preagg_t1), PREAGGREGATION: ON"
+ }
+ order_qt_q14 """
+ select count(distinct k6, v7) from preagg_t1;
+ """
+
+ explain {
+ sql("""select count(distinct k6, k5) from preagg_t1;""")
+ contains "(preagg_t1), PREAGGREGATION: ON"
+ }
+ order_qt_q15 """
+ select count(distinct k6, k5) from preagg_t1;
+ """
+
+ // Negative: count(DISTINCT IF(...), value_col) —
checkAggWithKeyAndValueSlots
+ // only inspects child(0) (the IF). Without a multi-arg guard, it would
+ // miss v7 in child(1) and incorrectly return ON.
+ explain {
+ sql("""
+ select count(distinct if(k6 > 0, k5, 0), v7) from preagg_t1;
+ """)
+ notContains "(preagg_t1), PREAGGREGATION: ON"
+ }
+ order_qt_q16 """
+ select count(distinct if(k6 > 0, k5, 0), v7) from preagg_t1;
+ """
+
+ explain {
+ sql("""
+ select count(distinct case when k6 > 0 then k5 else 0 end, v7)
from preagg_t1;
+ """)
+ notContains "(preagg_t1), PREAGGREGATION: ON"
+ }
+ order_qt_q17 """
+ select count(distinct case when k6 > 0 then k5 else 0 end, v7) from
preagg_t1;
+ """
+
+ // Negative: count(DISTINCT key + random()) — volatile in the expression
+ // argument. With pre-agg ON, random() would be evaluated per partial row
+ // instead of per merged logical row, changing the distinct count.
+ explain {
+ sql("""select count(distinct k6 + random()) from preagg_t1;""")
+ notContains "(preagg_t1), PREAGGREGATION: ON"
+ }
+
+ // max/min(key + random()) have the same volatile concern.
+ explain {
+ sql("""select max(k6 + random()) from preagg_t1;""")
+ notContains "(preagg_t1), PREAGGREGATION: ON"
+ }
+
+ explain {
+ sql("""select min(k6 + random()) from preagg_t1;""")
+ notContains "(preagg_t1), PREAGGREGATION: ON"
+ }
+
+ // Volatile in an IF condition inside a mixed-key-value aggregate.
+ // The condition k6 + random() > 0 uses only key input slots but is
+ // volatile, so pre-agg must be OFF.
+ explain {
+ sql("""
+ select sum(if(k6 + random() > 0, v7, 0)) from preagg_t1;
+ """)
+ notContains "(preagg_t1), PREAGGREGATION: ON"
+ }
+
+ // Positive: two-project join where both aliases resolve to key-only
+ // expressions. With merge+resolve, x = k5 + 1 resolves fully to base
+ // key columns, so pre-agg can be ON for both scans.
+ explain {
+ sql("""
+ select count(distinct a)
+ from (
+ select l.k1 + l.x as a
+ from (
+ select t1.k1, t1.k5 + 1 as x from preagg_t1 t1
+ ) l
+ inner join (
+ select abs(t2.k1) as rk from preagg_t2 t2
+ ) r on l.k1 = r.rk
+ ) t;
+ """)
+ contains "(preagg_t1), PREAGGREGATION: ON"
+ contains "(preagg_t2), PREAGGREGATION: ON"
+ }
+ order_qt_q18 """
+ select count(distinct a)
+ from (
+ select l.k1 + l.x as a
+ from (
+ select t1.k1, t1.k5 + 1 as x from preagg_t1 t1
+ ) l
+ inner join (
+ select abs(t2.k1) as rk from preagg_t2 t2
+ ) r on l.k1 = r.rk
+ ) t;
+ """
+
+ // Negative: two-project join; x carries v7 + 1 (a value column). Even
+ // with merge+resolve, the fully resolved expression contains v7 so
+ // pre-agg is correctly OFF.
+ explain {
+ sql("""
+ select count(distinct a)
+ from (
+ select l.k1 + l.x as a
+ from (
+ select t1.k1, t1.v7 + 1 as x from preagg_t1 t1
+ ) l
+ inner join (
+ select abs(t2.k1) as rk from preagg_t2 t2
+ ) r on l.k1 = r.rk
+ ) t;
+ """)
+ notContains "(preagg_t1), PREAGGREGATION: ON"
+ }
+ order_qt_q19 """
+ select count(distinct a)
+ from (
+ select l.k1 + l.x as a
+ from (
+ select t1.k1, t1.v7 + 1 as x from preagg_t1 t1
+ ) l
+ inner join (
+ select abs(t2.k1) as rk from preagg_t2 t2
+ ) r on l.k1 = r.rk
+ ) t;
+ """
+
+ // Bypass 1: volatile in an other-table aggregate function. max(r.k1 +
random())
+ // is whitelisted as a duplicate-insensitive MAX for scan l; the candidate
set
+ // for l is empty, so without a central volatile check it returns ON before
+ // reaching the per-function guard. Both scans must be OFF.
+ explain {
+ sql("""
+ select max(r.k1 + random())
+ from preagg_t1 l
+ inner join preagg_t2 r on l.k1 = r.k1;
+ """)
+ notContains "(preagg_t1), PREAGGREGATION: ON"
+ notContains "(preagg_t2), PREAGGREGATION: ON"
+ }
+
+ // Bypass 2: volatile filter with no input slots. random() < 0.5 has an
+ // empty input-slot set, so the slot-based value-column check bypasses it.
+ // The central volatile guard must reject pre-agg on this scan.
+ explain {
+ sql("""
+ select sum(v7) from preagg_t1 where random() < 0.5;
+ """)
+ notContains "(preagg_t1), PREAGGREGATION: ON"
+ }
+
+ // Foreign value column in mixed aggregate: sum(if(abs(l.k1) > 0, r.v7, 0))
+ // references l.k1 (local key) and r.v7 (foreign value). The mixed helper
+ // must not use r.v7's SUM type to justify pre-agg on l — r.v7 is not
+ // a column of l. l must be OFF; r can be ON.
+ explain {
+ sql("""
+ select sum(if(t.a > 0, r.v7, 0))
+ from (select abs(k1) as a from preagg_t1) t
+ inner join preagg_t2 r on t.a = r.k1;
+ """)
+ notContains "(preagg_t1), PREAGGREGATION: ON"
+ // r's local slots are only {r.v7} with IF; the value-only IF/CaseWhen
+ // handler validates conditions (foreign key t.a → safe) and returns
ON.
+ contains "(preagg_t2), PREAGGREGATION: ON"
+ }
+
+ // CASE WHEN symmetry of the foreign-key-condition / local-value-return
+ // pattern on r: conditions reference only foreign keys, return references
+ // r.v7, so r should be ON.
+ explain {
+ sql("""
+ select sum(case when t.a > 0 then r.v7 else 0 end)
+ from (select abs(k1) as a from preagg_t1) t
+ inner join preagg_t2 r on t.a = r.k1;
+ """)
+ notContains "(preagg_t1), PREAGGREGATION: ON"
+ contains "(preagg_t2), PREAGGREGATION: ON"
+ }
+ order_qt_q20 """
+ select sum(case when t.a > 0 then r.v7 else 0 end)
+ from (select abs(k1) as a from preagg_t1) t
+ inner join preagg_t2 r on t.a = r.k1;
+ """
+
+ // Negative: max(cast(v9 as double)) — no cast is peeled for MAX/MIN.
+ // DOUBLE/DECIMAL→FLOAT can underflow to -0.0 and change the observable
+ // tie representative (signed zero) under MAX/MIN, so even nondecreasing
+ // casts are not MAX/MIN homomorphisms. The checker sees a Cast and
+ // returns OFF.
+ explain {
+ sql("""select max(cast(v9 as double)) from preagg_t1;""")
+ notContains "(preagg_t1), PREAGGREGATION: ON"
+ }
+ order_qt_q21 """select max(cast(v9 as double)) from preagg_t1;"""
+
+ // Negative: sum(cast(v7 as double)) — sum(cast(x)) and cast(sum(x)) are
+ // not interchangeable due to precision/overflow, so cast must NOT be
+ // unwrapped. OneValueSlotAggChecker sees a Cast, not a SlotReference,
+ // and correctly returns OFF.
+ explain {
+ sql("""select sum(cast(v7 as double)) from preagg_t1;""")
+ notContains "(preagg_t1), PREAGGREGATION: ON"
+ }
+ order_qt_q22 """select sum(cast(v7 as double)) from preagg_t1;"""
+
+ // Negative: max(cast(v9 as string)) — non-numeric cast is never safe
+ // for MAX/MIN because string comparison differs from numeric comparison.
+ explain {
+ sql("""select max(cast(v9 as string)) from preagg_t1;""")
+ notContains "(preagg_t1), PREAGGREGATION: ON"
+ }
+ order_qt_q23 """select max(cast(v9 as string)) from preagg_t1;"""
+
+ // Negative mixed-path IF with cast in return: max(if(k6 > 0, cast(v9 as
double), 0))
+ // — no cast is peeled for MAX/MIN, so the IF return stays wrapped in Cast
+ // and the checker returns OFF.
+ explain {
+ sql("""
+ select max(if(k6 > 0, cast(v9 as double), 0)) from preagg_t1;
+ """)
+ notContains "(preagg_t1), PREAGGREGATION: ON"
+ }
+ order_qt_q24 """
+ select max(if(k6 > 0, cast(v9 as double), 0)) from preagg_t1;
+ """
+
+ // Negative mixed-path IF with cast in return:
+ // sum(if(k6 > 0, cast(v7 as double), 0)) — sum(cast(x)) is not
+ // interchangeable with cast(sum(x)), so the guard keeps the Cast
+ // wrapper and the checker returns OFF.
+ explain {
+ sql("""
+ select sum(if(k6 > 0, cast(v7 as double), 0)) from preagg_t1;
+ """)
+ notContains "(preagg_t1), PREAGGREGATION: ON"
+ }
+ order_qt_q25 """
+ select sum(if(k6 > 0, cast(v7 as double), 0)) from preagg_t1;
+ """
+
+ // --- Cast regression tests ---
+ // No cast is peeled for MAX/MIN: DOUBLE/DECIMAL→FLOAT can underflow to
+ // -0.0 and change the observable tie representative (signed zero) under
+ // MAX/MIN, so even nondecreasing casts are not MAX/MIN homomorphisms.
+ // Any cast-wrapped aggregate is therefore conservatively OFF.
+
+ // Negative: BIGINT→DECIMAL(20,0) widening cast → OFF (no peeling).
+ explain {
+ sql("""select max(cast(v9 as decimal(20,0))) from preagg_t1;""")
+ notContains "(preagg_t1), PREAGGREGATION: ON"
+ }
+ order_qt_q26 """select max(cast(v9 as decimal(20,0))) from preagg_t1;"""
+
+ // Negative: BIGINT→LARGEINT widening cast → OFF (no peeling).
+ explain {
+ sql("""select max(cast(v9 as largeint)) from preagg_t1;""")
+ notContains "(preagg_t1), PREAGGREGATION: ON"
+ }
+ order_qt_q27 """select max(cast(v9 as largeint)) from preagg_t1;"""
+
+ // Negative: BIGINT→INT is narrowing (not injective, not float) → OFF.
+ explain {
+ sql("""select max(cast(v9 as int)) from preagg_t1;""")
+ notContains "(preagg_t1), PREAGGREGATION: ON"
+ }
+ order_qt_q28 """select max(cast(v9 as int)) from preagg_t1;"""
+
+ // Negative: BIGINT→TINYINT is narrowing (not injective, not float) → OFF.
+ explain {
+ sql("""select max(cast(v9 as tinyint)) from preagg_t1;""")
+ notContains "(preagg_t1), PREAGGREGATION: ON"
+ }
+ order_qt_q29 """select max(cast(v9 as tinyint)) from preagg_t1;"""
+
+ // Negative mixed-path IF with widening cast: no peeling → OFF.
+ explain {
+ sql("""
+ select max(if(k6 > 0, cast(v9 as decimal(20,0)), 0)) from
preagg_t1;
+ """)
+ notContains "(preagg_t1), PREAGGREGATION: ON"
+ }
+ order_qt_q30 """
+ select max(if(k6 > 0, cast(v9 as decimal(20,0)), 0)) from preagg_t1;
+ """
+
+ // Mixed-path IF with unsafe cast: max(if(..., cast(v9 as tinyint), 0))
+ // return cast is narrowing non-injective → not peeled → checker OFF.
+ explain {
+ sql("""
+ select max(if(k6 > 0, cast(v9 as tinyint), 0)) from preagg_t1;
+ """)
+ notContains "(preagg_t1), PREAGGREGATION: ON"
+ }
+ order_qt_q31 """
+ select max(if(k6 > 0, cast(v9 as tinyint), 0)) from preagg_t1;
+ """
+
+ // Negative: CASE WHEN with widening cast → OFF (no peeling).
+ explain {
+ sql("""
+ select max(case when k6 > 0 then cast(v9 as decimal(20,0)) else 0
end) from preagg_t1;
+ """)
+ notContains "(preagg_t1), PREAGGREGATION: ON"
+ }
+ order_qt_q32 """
+ select max(case when k6 > 0 then cast(v9 as decimal(20,0)) else 0 end)
from preagg_t1;
+ """
+
+ // CASE WHEN with unsafe narrowing cast → OFF.
+ explain {
+ sql("""
+ select max(case when k6 > 0 then cast(v9 as tinyint) else 0 end)
from preagg_t1;
+ """)
+ notContains "(preagg_t1), PREAGGREGATION: ON"
+ }
+ order_qt_q33 """
+ select max(case when k6 > 0 then cast(v9 as tinyint) else 0 end) from
preagg_t1;
+ """
+
+ //
-------------------------------------------------------------------------
+ // Two-table derived-key ownership tests (with repeated aggregate-key data)
+ //
+ // The mixed helper must validate returns relative to the current scan: a
+ // foreign value column must never justify exposing this scan's partial
+ // (unmerged) rows. Under join fan-out, a return that references a foreign
+ // value column would be evaluated once per partial row and double-counted.
+ //
-------------------------------------------------------------------------
+
+ // Test A: derived-key fan-out. t has two different keys k1=1 and k1=-1
that
+ // both map to the same derived key a=abs(k1)=1; r.v7 is a foreign value
+ // used in the IF return. t must stay OFF (aggSlots intersection keeps only
+ // local key k1 → key-only path → sum is not distinct), so each logical row
+ // of t joins every r row with r.k1 = a and counts r.v7 once. With the
+ // one-time dataset: a=1 (3 t-rows) × r.k1=1 (2 r-rows, v7=50,60) plus
+ // a=2 (1 t-row) × r.k1=2 (1 r-row, v7=70) → 3*(50+60)+70 = 400. If t were
+ // wrongly ON, its rows would fan out under join.
+ // r has local v7 and foreign key condition → ON is safe.
+ explain {
+ sql("""
+ select sum(if(t.a > 0, r.v7, 0))
+ from (select abs(k1) as a from preagg_t1) t
+ inner join preagg_t2 r on t.a = r.k1;
+ """)
+ notContains "(preagg_t1), PREAGGREGATION: ON"
+ contains "(preagg_t2), PREAGGREGATION: ON"
+ }
+ order_qt_test_a """
+ select sum(if(t.a > 0, r.v7, 0)) as res
+ from (select abs(k1) as a from preagg_t1) t
+ inner join preagg_t2 r on t.a = r.k1;
+ """
+
+ // Test B: foreign value in IF return on BOTH sides — ownership check must
+ // turn both scans OFF:
+ // sum(if(t.a > 0, t.v7, r.v7))
+ // - for scan t: return r.v7 is foreign → t OFF
+ // - for scan r: return t.v7 is foreign → r OFF
+ // With both OFF, storage merges by key and the logical result is exact.
+ // With the one-time dataset: a=1 (3 t-rows, v7=10,20,30) × r.k1=1 (2 rows)
+ // → 2*(10+20+30)=120, plus a=2 (t.v7=40) × r.k1=2 → 40, total 160.
+ explain {
+ sql("""
+ select sum(if(t.a > 0, t.v7, r.v7))
+ from (select abs(k1) as a, v7 from preagg_t1) t
+ inner join preagg_t2 r on t.a = r.k1;
+ """)
+ notContains "(preagg_t1), PREAGGREGATION: ON"
+ notContains "(preagg_t2), PREAGGREGATION: ON"
+ }
+ order_qt_test_b """
+ select sum(if(t.a > 0, t.v7, r.v7)) as res
+ from (select abs(k1) as a, v7 from preagg_t1) t
+ inner join preagg_t2 r on t.a = r.k1;
+ """
+
+ // Positive MAX join case: MAX is idempotent (max(x, x) = x), so a foreign
+ // value branch in the IF return cannot change the result even under join
+ // fan-out. The ownership fence is skipped for MAX/MIN (it stays for
+ // SUM/COUNT), so both scans may be ON. With the one-time dataset:
+ // l.k1=1 (v9 1000,900) and l.k1=2 (v9 700) all satisfy l.k1 > 0, so
+ // max(if(l.k1 > 0, l.v9, r.v9)) = 1000.
+ explain {
+ sql("""
+ select max(if(l.k1 > 0, l.v9, r.v9))
+ from preagg_t1 l
+ inner join preagg_t2 r on l.k1 = r.k1;
+ """)
+ contains "(preagg_t1), PREAGGREGATION: ON"
+ contains "(preagg_t2), PREAGGREGATION: ON"
+ }
+ order_qt_q34 """
+ select max(if(l.k1 > 0, l.v9, r.v9))
+ from preagg_t1 l
+ inner join preagg_t2 r on l.k1 = r.k1;
+ """
+
+ // Negative: sum(DISTINCT ...) with a nested-aggregate condition slot.
+ // sum(distinct if(t.c > 0, r.v7, 0))
+ // t.c = sum(t.v7) has no OriginalColumn, so splitKeyValueSlots drops it
from
+ // the condition check (it is unclassified). Previously this let the route
+ // reach visitSum, which did NOT reject DISTINCT, so preagg_t4 was wrongly
+ // turned ON. Storage SUM would then merge the duplicate full key
(v7=1+1=2)
+ // and break DISTINCT semantics: ON sees {1,1} → 1 while OFF sees merged 2.
+ // KeyAndValueSlotsAggChecker.visitSum must reject sum.isDistinct() exactly
+ // like OneValueSlotAggChecker.visitSum does.
+ explain {
+ sql("""
+ select sum(distinct if(t.c > 0, r.v7, 0))
+ from (select k1, sum(v7) as c from preagg_t1 group by k1) t
+ inner join preagg_t4 r on t.k1 = r.k1;
+ """)
+ contains "(preagg_t1), PREAGGREGATION: ON"
+ notContains "(preagg_t4), PREAGGREGATION: ON"
+ }
+ order_qt_q35 """
+ select sum(distinct if(t.c > 0, r.v7, 0))
+ from (select k1, sum(v7) as c from preagg_t1 group by k1) t
+ inner join preagg_t4 r on t.k1 = r.k1;
+ """
+
+ // Negative signed-zero: signbit(max(if(k1 > 0, cast(v as float), cast(0
as float))))
+ // with v a DOUBLE MAX column. No cast is peeled for MAX/MIN: DOUBLE→FLOAT
can
+ // underflow (-1e-300 → FLOAT -0.0) and change the observable tie
representative
+ // under signbit, so preagg_t5 must stay OFF. preagg_t5 holds the same
full key
+ // k1=1 in two loads (v = -1e-300 and +0.0); storage MAX merges to +0.0,
so the
+ // merged result is signbit(+0.0) = 0.
+ explain {
+ sql("""
+ select signbit(max(if(k1 > 0, cast(v as float), cast(0 as
float)))) from preagg_t5;
+ """)
+ notContains "(preagg_t5), PREAGGREGATION: ON"
+ }
+ order_qt_q36 """
+ select signbit(max(if(k1 > 0, cast(v as float), cast(0 as float))))
from preagg_t5;
+ """
+
+ // Negative ASOF join selected-side: r.v9 is a direct correctly typed MAX
+ // column, but ASOF's one-row selection does not commute with pre-agg ON.
+ // preagg_asof_r holds the same full key (grp=1, ts) in two loads with
+ // v9=100 and v9=200; storage MAX merges to 200. If r were ON, ASOF would
+ // see two equal-time rows and could pick the 100 partial before the upper
+ // MAX runs, returning 100 instead of 200. The selected side must stay OFF.
+ // l(1)↔r(1, v9=200), l(2)↔r(2, v9=300) → max(200, 300) = 300.
+ explain {
+ sql("""
+ select max(if(l.grp > 0, r.v9, 0))
+ from preagg_asof_l l asof left join preagg_asof_r r
+ MATCH_CONDITION(l.ts >= r.ts) on l.grp = r.grp;
+ """)
+ contains "(preagg_asof_l), PREAGGREGATION: ON"
+ notContains "(preagg_asof_r), PREAGGREGATION: ON"
+ }
+ order_qt_q37 """
Review Comment:
Restricting q37 to `grp = 1` removes the unrelated 300 row, but MAX still
does not guarantee that the faulty selected-side ON plan fails. The correctly
merged MAX is 200 and the exposed partials are 100 and 200, so an allowed
equal-time ASOF selection can pick the 200 partial and still match the expected
result. Please use an aggregate whose merged value differs from every
partial—for example, SUM partials 100 and 200, where OFF produces 300 and
faulty ON can select only 100 or 200—while retaining the `grp` restriction and
OFF EXPLAIN assertion.
--
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]
