yujun777 commented on code in PR #69890:
URL: https://github.com/apache/doris/pull/69890#discussion_r4510903681
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/CommonSubExpressionCollector.java:
##########
@@ -80,19 +81,64 @@ private int collectChildrenDepth(List<Expression> children,
Boolean inLambda) {
private int collectCommonExpressionByDepth(int depth, Expression expr,
boolean inLambda) {
Set<Expression> expressions = getExpressionsFromDepthMap(depth,
expressionsByDepth);
- // ArrayItemSlot and ArrayItemReference could not be common expressions
- // TODO: could not extract common expression when expression contains
same lambda expression
- // because ArrayItemSlot in Lambda are not same.
+ // An expression that mentions a lambda argument bound outside it
cannot be evaluated
+ // outside that lambda: hoisting it into an earlier projection leaves
the argument without
+ // its binding. The check must not depend on the inLambda flag -- a
lambda nested in another
+ // expression can be visited again from a context that does not carry
the flag (for example
+ // when the same subtree appears in two projections), and the
extraction then breaks the
+ // lambda scope. An expression that *contains* the whole lambda, such
as an array_map with
+ // its lambda, binds its own arguments and stays extractable.
+ // A lambda and its parameter references are not standalone values:
only an expression that
+ // contains the whole binding can be hoisted, never the binding pieces
themselves.
Review Comment:
Fixed in
https://github.com/apache/doris/commit/b153c38b1f7116d79671f11e0363d1b98cafe877.
Each ArrayItemReference input array is now checked under the enclosing bound
set before the lambda parameters are introduced; the lambda body is checked
with its own parameters bound. Added testCapturedLambdaInputIsNotExtracted to
verify that the captured inner ArrayMap cannot be hoisted while the complete
outer expression remains extractable. Added a nested-lambda SQL regression in
topk_8. The CSE tests and the full IVM regression directory passed.
##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/ivm/IvmLinearDeltaHandler.java:
##########
@@ -119,6 +119,19 @@ Optional<IvmDeltaRewriteResult>
rewriteProject(LogicalProject<? extends Plan> pr
childOutputMapping.put(project.child().getOutput().get(i),
child.plan.getOutput().get(i));
}
for (NamedExpression output : project.getProjects()) {
+ // A hidden state the child computes itself wins over a
project-level derivation of it:
+ // above the aggregate, normalize derives the MIN/MAX bound from
the array for a full
+ // refresh, while the apply result is the bound the merge
computed. Only the states the
+ // child reports as its own are replaced; everything else (row_id,
other hidden
+ // expressions) keeps the normal rewrite.
+ if (child.ownedHiddenStateNames.contains(output.getName())) {
Review Comment:
Updated in
https://github.com/apache/doris/commit/b153c38b1f7116d79671f11e0363d1b98cafe877:
Project, Filter, and SubQueryAlias rewrites preserve ownedHiddenStateNames.
One clarification about the original failure chain: the immediate Project
consumes the child ownership marker before constructing its result, so dropping
the marker on return does not itself make that same Project re-derive the
bound. Preserving it still maintains the contract for subsequent wrappers.
Added topk_8 with multiple projections: merge exactly K values from complete
sides, retain NULL bounds, replace an extreme, then delete the other extreme.
Hidden bounds and COUNT/SUM state are checked against generated output, and the
final MV matches the source. All 124 IVM suites passed.
##########
regression-test/suites/mtmv_p0/ivm/test_ivm_agg_minmax_topk_1.groovy:
##########
@@ -0,0 +1,128 @@
+// 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.
+
+
+// Core top-K state transitions with an explicit capacity: truncation,
deleting duplicate and last
+// extremes, deleting above the bound, exhausting the known range, and
changing K with ALTER.
+// `SELECT *` with hidden columns prints the state struct of every step; the
sequence column differs
+// between cloud and shared-nothing deployments and is masked.
+suite("test_ivm_agg_minmax_topk_1") {
+ sql """set enable_materialized_view_rewrite = false;"""
+ sql """set enable_dml_materialized_view_rewrite = false;"""
+ sql """drop materialized view if exists ivm_topk_1_mv"""
+ sql """drop table if exists ivm_topk_1_t"""
+
+ sql """
+ CREATE TABLE ivm_topk_1_t (
+ id BIGINT NOT NULL,
+ category VARCHAR(16) NOT NULL,
+ score BIGINT
+ )
+ UNIQUE KEY(id)
+ DISTRIBUTED BY HASH(id) BUCKETS 1
+ PROPERTIES (
+ "replication_num" = "1",
+ "enable_unique_key_merge_on_write" = "true",
+ "changelog.enable" = "true",
+ "changelog.need_historical_value" = "true"
+ )
+ """
+ sql """INSERT INTO ivm_topk_1_t VALUES
+ (1, 'a', 10), (7, 'a', 10), (2, 'a', 20), (3, 'a', 30), (4, 'a', 40)"""
+ sql """
+ CREATE MATERIALIZED VIEW ivm_topk_1_mv
+ BUILD DEFERRED REFRESH INCREMENTAL FALLBACK ON MANUAL
+ KEY(category)
+ DISTRIBUTED BY HASH(category) BUCKETS 1
+ PROPERTIES (
+ "replication_num" = "1",
+ "ivm_minmax_top_k" = "2"
+ )
+ AS SELECT category, MIN(score) AS min_score, MAX(score) AS max_score
+ FROM ivm_topk_1_t
+ GROUP BY category
+ """
+
+ // The resolved capacity is persisted at CREATE. Assert the one property
instead of pinning the
+ // whole DDL, whose unrelated defaults would make the golden churn.
+ def createDdl = sql """SHOW CREATE MATERIALIZED VIEW ivm_topk_1_mv"""
+ assertTrue(createDdl.toString().contains('"ivm_minmax_top_k" = "2"'))
Review Comment:
Fixed in
https://github.com/apache/doris/commit/b153c38b1f7116d79671f11e0363d1b98cafe877.
The new MIN/MAX suites now record capacity checks, terminal task status,
refresh mode where applicable, and fallback reasons through qt output. SHOW
CREATE checks emit only a stable property-presence boolean. All affected .out
files were generated by run-regression-test.sh; existing golden sections remain
unchanged. Added topk_8 output for the exactly-K bound and nested-lambda cases.
The complete IVM regression directory passed (124 suites, zero failures).
--
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]
