yujun777 commented on code in PR #68787:
URL: https://github.com/apache/doris/pull/68787#discussion_r4219958246


##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/ivm/agg/IvmAggArrayProcessor.java:
##########
@@ -65,7 +65,7 @@ void mapApplyDeltaSlots(IvmAggTarget target, Map<String, 
Slot> outputByName,
     @Override
     public void appendApplyExpressions(IvmAggTarget target, IvmAggApplyContext 
applyContext) {
         IvmAggExpressionBuilder ctx = applyContext.expressions();
-        Slot oldArray = 
applyContext.rawMvSlot(target.getVisibleSlot().getName());
+        Slot oldArray = 
applyContext.rawMvSlot(target.getValueStateColumnName());

Review Comment:
   Added in 3a0ab7e03fa. `test_ivm_agg_expr_over_agg_1` now creates `SELECT k, 
ARRAY_SIZE(ARRAY_AGG(v)) AS n`, checks its layout 
(`__DORIS_IVM_AGG_0_ARRAY_AGG_COL__` is materialized), and refreshes it in 
every window — both incremental windows and a COMPLETE refresh — comparing it 
against the same query over the base table at each step.
   
   It also keeps the two array processors distinguishable at runtime: the group 
whose only row has a NULL `v` reports 1 for ARRAY_AGG, while the wrapped 
COLLECT_LIST view reports an empty array for the same group.
   



##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/ivm/agg/IvmAggExtremalProcessor.java:
##########
@@ -107,7 +107,7 @@ protected List<IvmAggFunctionKind> 
zeroDefaultSlotKinds(IvmAggTarget target, boo
     @Override
     public void appendApplyExpressions(IvmAggTarget target, IvmAggApplyContext 
applyContext) {
         IvmAggExpressionBuilder ctx = applyContext.expressions();
-        Slot oldExtreme = 
applyContext.rawMvSlot(target.getVisibleSlot().getName());
+        Slot oldExtreme = 
applyContext.rawMvSlot(target.getValueStateColumnName());

Review Comment:
   Added in 3a0ab7e03fa as part 11 of `test_ivm_agg_expr_over_agg_2`. `SELECT 
k, MIN(v) * 2` over three rows of one group, so the MIN state is the 
materialized carrier rather than a visible column, and then the row holding the 
current minimum is deleted.
   
   The task row is read back from `tasks('type'='mv')`, as 
`test_ivm_minmax_runtime_fallback` does, so the classification is asserted 
rather than assumed:
   
   - strict `REFRESH ... INCREMENTAL` fails with 
`IvmFallbackReason=MIN_MAX_BOUNDARY_HIT` and the MV keeps its previous value 
(20), i.e. nothing was written from the stale extreme;
   - `REFRESH ... INCREMENTAL FALLBACK` succeeds with `RefreshMode=COMPLETE` 
and the same reason, and the MV then matches the source query (40).
   



##########
regression-test/suites/mtmv_p0/ivm/test_ivm_agg_expr_over_agg_1.groovy:
##########
@@ -0,0 +1,667 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+suite("test_ivm_agg_expr_over_agg_1") {
+
+    // =========================================================
+    // A scalar expression wrapped around an aggregate result, as
+    // in SELECT k, SUM(v) * 100 FROM t GROUP BY k, must stay
+    // incrementally maintainable.
+    //
+    // Apply merges the old MV state in the state domain and then
+    // re-applies the outer expression:
+    //     new.s100 = f(apply(old_mv.sum_v, delta.sum_v))
+    // so the MV must persist a column carrying SUM(v) itself. That
+    // column is the visible aggregate output when the select list
+    // projects it (SELECT SUM(v) AS s, SUM(v) * 100) and a
+    // materialized hidden column when an upper expression consumes
+    // it without projecting it (SELECT SUM(v) * 100).
+    //
+    // These cases verify both halves of the invariant:
+    //   * the hidden layout, via DESC (the dropped state column is
+    //     materialized, once per aggregate state, reusing existing
+    //     columns when they already carry it);
+    //   * the merged values, through INSERT/UPDATE/DELETE and
+    //     incremental refreshes.
+    //
+    // NOTE: set show_hidden_columns=true right before a DESC only —
+    // enabling it earlier puts the session in debug mode and blocks
+    // CREATE MATERIALIZED VIEW.
+    // =========================================================
+
+    def refreshIncremental = { mv ->
+        sql """REFRESH MATERIALIZED VIEW ${mv} INCREMENTAL"""
+        waitingMTMVTaskFinishedByMvName(mv)
+    }
+
+    sql """drop materialized view if exists test_ivm_expr_over_agg_sum;"""
+    sql """drop materialized view if exists test_ivm_expr_over_agg_cnt;"""
+    sql """drop materialized view if exists test_ivm_expr_over_agg_min;"""
+    sql """drop materialized view if exists test_ivm_expr_over_agg_max;"""
+    sql """drop materialized view if exists test_ivm_expr_over_agg_list;"""
+    sql """drop materialized view if exists test_ivm_expr_over_agg_div;"""
+    sql """drop materialized view if exists test_ivm_expr_over_agg_cast;"""
+    sql """drop materialized view if exists test_ivm_expr_over_agg_scalar;"""
+    sql """drop materialized view if exists test_ivm_expr_over_agg_sum_avg;"""
+    sql """drop materialized view if exists 
test_ivm_expr_over_agg_sum_avg_mul;"""
+    sql """drop materialized view if exists test_ivm_expr_over_agg_plain;"""
+    sql """drop materialized view if exists test_ivm_expr_over_agg_cnt_star;"""
+    sql """drop materialized view if exists 
test_ivm_expr_over_agg_avg_round;"""
+    sql """drop materialized view if exists test_ivm_expr_over_agg_bitmap;"""
+    sql """drop materialized view if exists 
test_ivm_expr_over_agg_bitmap_union;"""
+    sql """drop materialized view if exists test_ivm_expr_over_agg_agg_arg;"""
+    sql """drop materialized view if exists test_ivm_expr_over_agg_key_expr;"""
+    sql """drop table if exists test_ivm_expr_over_agg_base;"""
+
+    sql """
+        CREATE TABLE test_ivm_expr_over_agg_base (
+            id INT,
+            k INT,
+            v INT
+        )
+        UNIQUE KEY(id)
+        DISTRIBUTED BY HASH(id) BUCKETS 2
+        PROPERTIES (
+            "replication_num" = "1",
+            "binlog.enable" = "true",
+            "binlog.format" = "ROW", "binlog.need_historical_value" = "true",
+            "enable_unique_key_merge_on_write" = "true"
+        );
+    """
+
+    // =========================================================
+    // Part 1: hidden layout of the wrapped-aggregate shapes
+    // =========================================================
+
+    // SUM(v) * 100: SUM's own value is its mergeable state and the visible 
column is
+    // consumed by the outer expression, so the state is materialized as 
_0_SUM_COL__
+    // next to the hidden non-NULL count.
+    sql """
+        CREATE MATERIALIZED VIEW test_ivm_expr_over_agg_sum
+        BUILD DEFERRED REFRESH INCREMENTAL ON MANUAL
+        DISTRIBUTED BY RANDOM BUCKETS 2
+        PROPERTIES ('replication_num' = '1')
+        AS SELECT k, SUM(v) * 100 AS s100 FROM test_ivm_expr_over_agg_base 
GROUP BY k;
+    """
+    sql """set show_hidden_columns=true"""
+    qt_sum_desc """DESC test_ivm_expr_over_agg_sum"""
+    sql """set show_hidden_columns=false"""
+
+    // COUNT(v) + 1: same for COUNT(expr), whose visible column is the count 
state.
+    sql """
+        CREATE MATERIALIZED VIEW test_ivm_expr_over_agg_cnt
+        BUILD DEFERRED REFRESH INCREMENTAL ON MANUAL
+        DISTRIBUTED BY RANDOM BUCKETS 2
+        PROPERTIES ('replication_num' = '1')
+        AS SELECT k, COUNT(v) + 1 AS c1 FROM test_ivm_expr_over_agg_base GROUP 
BY k;
+    """
+    sql """set show_hidden_columns=true"""
+    qt_cnt_desc """DESC test_ivm_expr_over_agg_cnt"""
+    sql """set show_hidden_columns=false"""
+
+    // MIN(v) * 2 and MAX(v) + 1.
+    sql """
+        CREATE MATERIALIZED VIEW test_ivm_expr_over_agg_min
+        BUILD DEFERRED REFRESH INCREMENTAL ON MANUAL
+        DISTRIBUTED BY RANDOM BUCKETS 2
+        PROPERTIES ('replication_num' = '1')
+        AS SELECT k, MIN(v) * 2 AS m2 FROM test_ivm_expr_over_agg_base GROUP 
BY k;
+    """
+    sql """set show_hidden_columns=true"""
+    qt_min_desc """DESC test_ivm_expr_over_agg_min"""
+    sql """set show_hidden_columns=false"""
+
+    sql """
+        CREATE MATERIALIZED VIEW test_ivm_expr_over_agg_max
+        BUILD DEFERRED REFRESH INCREMENTAL ON MANUAL
+        DISTRIBUTED BY RANDOM BUCKETS 2
+        PROPERTIES ('replication_num' = '1')
+        AS SELECT k, MAX(v) + 1 AS m1 FROM test_ivm_expr_over_agg_base GROUP 
BY k;
+    """
+    sql """set show_hidden_columns=true"""
+    qt_max_desc """DESC test_ivm_expr_over_agg_max"""
+    sql """set show_hidden_columns=false"""
+
+    // ARRAY_SIZE(COLLECT_LIST(v)): the visible array is the whole aggregate 
state.
+    sql """
+        CREATE MATERIALIZED VIEW test_ivm_expr_over_agg_list
+        BUILD DEFERRED REFRESH INCREMENTAL ON MANUAL
+        DISTRIBUTED BY RANDOM BUCKETS 2
+        PROPERTIES ('replication_num' = '1')
+        AS SELECT k, ARRAY_SIZE(COLLECT_LIST(v)) AS n FROM 
test_ivm_expr_over_agg_base GROUP BY k;

Review Comment:
   Added in 3a0ab7e03fa. `test_ivm_agg_expr_over_agg_1` additionally creates 
`SELECT k, ARRAY_SORT(COLLECT_LIST(v)) AS lst` and compares it element-wise 
against the source query in every window plus a COMPLETE refresh, so the 
carrier contents are asserted rather than the element count.
   
   The equal-count update from your example is now covered: after id=2 changes 
20 to 25 the recorded row is `1 [10, 25, 30]`, which a stale `[10, 20, 30]` 
carrier could not produce, while the count-only view could not tell them apart.
   



-- 
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]

Reply via email to