From 11d1fc7de795a1e0c44f8faca265353dcad9a0dc Mon Sep 17 00:00:00 2001 From: Aranya Singh Chauhan <51438967+Aranya7@users.noreply.github.com> Date: Wed, 21 Oct 2026 08:37:58 +0530 Subject: [PATCH] [fix](nereids) Allow sync MV rewrite across strict-table SQL modes (#69212) Nereids can fail to rewrite an aggregation query using a compatible synchronous materialized-view rollup when the querying session and the rollup creation session differ only in the `STRICT_TRANS_TABLES` and/or `STRICT_ALL_TABLES` SQL-mode flags. For example: - Rollup creation session: `ONLY_FULL_GROUP_BY` - Query execution session: `ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,STRICT_ALL_TABLES` Neither strict-table flag currently affects the SQL expression semantics relevant to synchronous MV rewriting. However, the existing session-variable compatibility check treats these configurations as different. Consequently, an otherwise eligible synchronous rollup may fail materialized-view rewriting, causing the optimizer to miss a potentially more efficient execution plan. The issue originates from the interaction between persisted session variables and session-variable guards in the Nereids materialized-view rewrite pipeline. 1. `sql_mode` is marked as a result-affecting session variable and is persisted as part of the synchronous materialized view's creation metadata. 2. `SessionVarGuardRewriter.checkSessionVariablesMatch()` performs an exact comparison of the current and persisted session-variable maps. 3. Differences in either strict-table SQL-mode bit cause this comparison to return `false`. 4. `InitMaterializationContextHook.createSyncMvContexts()` uses this compatibility result when constructing the synchronous materialization context. 5. When the session variables are considered incompatible, expressions implementing `NeedSessionVarGuard`, including `SUM`, may be wrapped in `SessionVarGuardExpr`. 6. The additional expression wrapper can prevent the aggregate-rollup matcher from recognizing an otherwise compatible aggregation expression. This results in a materialized-view rewrite failure caused by SQL-mode differences that do not affect the relevant query semantics. This PR introduces a synchronous-MV-specific session-variable compatibility check: `SessionVarGuardRewriter.checkSessionVariablesMatchForSyncMv()` The new comparison: - Allows differences exclusively in `STRICT_TRANS_TABLES` and/or `STRICT_ALL_TABLES`. - Preserves strict comparison of all other SQL-mode bits. - Preserves strict comparison of other result-affecting session variables. - Rejects incompatible or missing session-variable entries. - Does not relax compatibility when differing SQL-mode values cannot be parsed. - Preserves existing behavior for older materialized views without persisted session-variable metadata. The implementation compares SQL modes using their persisted numeric bitmask representations, excluding only: - `SqlModeHelper.MODE_STRICT_TRANS_TABLES` - `SqlModeHelper.MODE_STRICT_ALL_TABLES` Both flags are treated symmetrically because neither currently affects the SQL expression semantics used for synchronous MV rewriting. The change is restricted to synchronous materialized-view context initialization in `InitMaterializationContextHook`. The existing general-purpose `checkSessionVariablesMatch()` method remains unchanged. For an otherwise compatible synchronous aggregate rollup: | Rollup creation SQL mode | Query SQL mode | Expected rewrite behavior | | --- | --- | --- | | `ONLY_FULL_GROUP_BY` | `ONLY_FULL_GROUP_BY` | Eligible | | `ONLY_FULL_GROUP_BY` | `ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES` | Eligible | | `ONLY_FULL_GROUP_BY` | `ONLY_FULL_GROUP_BY,STRICT_ALL_TABLES` | Eligible | | `ONLY_FULL_GROUP_BY` | `ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,STRICT_ALL_TABLES` | Eligible | | `ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,STRICT_ALL_TABLES` | `ONLY_FULL_GROUP_BY` | Eligible | | `ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES` | `ONLY_FULL_GROUP_BY,STRICT_ALL_TABLES` | Eligible | | `ONLY_FULL_GROUP_BY` | `ONLY_FULL_GROUP_BY,PIPES_AS_CONCAT` | Incompatible | | `ONLY_FULL_GROUP_BY` | `ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,STRICT_ALL_TABLES,PIPES_AS_CONCAT` | Incompatible | Other result-affecting session-variable differences, such as changes to `enable_decimal256`, continue to require the existing compatibility protections. Regression test: `regression-test/suites/query_p0/variables_persist/test_sync_rollup_strict_sql_mode.groovy` The test: 1. Creates an Aggregate Key table with `region_id`, `store_id`, and `sales`. 2. Creates a synchronous aggregate rollup under `ONLY_FULL_GROUP_BY`. 3. Waits for the rollup creation job to finish successfully. 4. Validates rollup rewrite eligibility using `EXPLAIN VERBOSE` under four SQL-mode configurations: - `ONLY_FULL_GROUP_BY` - `ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES` - `ONLY_FULL_GROUP_BY,STRICT_ALL_TABLES` - `ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,STRICT_ALL_TABLES` 5. Verifies identical aggregation results across all four configurations. The query used for validation is: ```sql SELECT region_id, SUM(sales) FROM test_sync_rollup_strict_sql_mode GROUP BY region_id ORDER BY region_id; ``` Expected results: | region_id | SUM(sales) | | --- | --- | | 1 | 300 | | 2 | 700 | The regression validates successful rewrite eligibility independently of whether the cost-based optimizer ultimately selects the rollup as the final physical execution plan. --------- Co-authored-by: Aranya Singh Chauhan (cherry picked from commit 3b8b5d8f54ce69076f83e343d3838bbbf5c613d2) --- .../analysis/SessionVarGuardRewriter.java | 72 ++++++ .../mv/InitMaterializationContextHook.java | 2 +- .../expressions/VariablePersistTest.java | 211 ++++++++++++++++- .../test_sync_rollup_strict_sql_mode.out | 16 ++ .../test_sync_rollup_strict_sql_mode.groovy | 224 ++++++++++++++++++ 5 files changed, 523 insertions(+), 2 deletions(-) create mode 100644 regression-test/data/nereids_p0/variables_persist/test_sync_rollup_strict_sql_mode.out create mode 100644 regression-test/suites/nereids_p0/variables_persist/test_sync_rollup_strict_sql_mode.groovy diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/SessionVarGuardRewriter.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/SessionVarGuardRewriter.java index a10cd23719ac6b..463f12702c9251 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/SessionVarGuardRewriter.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/SessionVarGuardRewriter.java @@ -32,11 +32,14 @@ import org.apache.doris.nereids.trees.expressions.visitor.DefaultExpressionRewriter; import org.apache.doris.nereids.trees.plans.Plan; import org.apache.doris.nereids.trees.plans.visitor.DefaultPlanRewriter; +import org.apache.doris.qe.SessionVariable; +import org.apache.doris.qe.SqlModeHelper; import com.google.common.collect.ImmutableList; import java.util.List; import java.util.Map; +import java.util.Objects; /** * The purpose of this class is to add session var guards to all expressions that require guarding @@ -146,4 +149,73 @@ public static boolean checkSessionVariablesMatch(Map currentSess } return currentSessionVars.equals(persistSessionVars); } + + /** + * Check session-variable compatibility for synchronous materialized views. + * + * Differences only in STRICT_TRANS_TABLES and STRICT_ALL_TABLES do not + * affect the SQL expressions used for synchronous MV rewriting. + * + * Other SQL-mode bits and result-affecting session variables + * must still match. + */ + public static boolean checkSessionVariablesMatchForSyncMv( + Map currentSessionVars, + Map persistSessionVars) { + + // Preserve compatibility with older MVs that have no saved session variables. + if (persistSessionVars == null || persistSessionVars.isEmpty()) { + return true; + } + + // A nonempty persisted configuration cannot match missing current state. + if (currentSessionVars == null) { + return false; + } + + // Retain the existing exact-match behavior whenever possible. + if (checkSessionVariablesMatch(currentSessionVars, persistSessionVars)) { + return true; + } + + // Every result-affecting session variable must still be present. + if (!currentSessionVars.keySet().equals(persistSessionVars.keySet())) { + return false; + } + + for (Map.Entry entry : currentSessionVars.entrySet()) { + String name = entry.getKey(); + String currentValue = entry.getValue(); + String persistedValue = persistSessionVars.get(name); + + if (Objects.equals(currentValue, persistedValue)) { + continue; + } + + // Only sql_mode is eligible for the narrowly scoped exception. + if (!SessionVariable.SQL_MODE.equals(name)) { + return false; + } + + try { + long currentMode = Long.parseLong(currentValue); + long persistedMode = Long.parseLong(persistedValue); + + // Neither strict-table SQL-mode bit affects synchronous MV rewrite semantics. + long ignoredModes = SqlModeHelper.MODE_STRICT_TRANS_TABLES + | SqlModeHelper.MODE_STRICT_ALL_TABLES; + long relevantModes = ~ignoredModes; + + if ((currentMode & relevantModes) + != (persistedMode & relevantModes)) { + return false; + } + } catch (NumberFormatException e) { + // Malformed, missing, or nonnumeric SQL modes cannot use the exception. + return false; + } + } + + return true; + } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/InitMaterializationContextHook.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/InitMaterializationContextHook.java index bc2c705a4f742b..08fc591a85cd96 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/InitMaterializationContextHook.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/InitMaterializationContextHook.java @@ -284,7 +284,7 @@ private List createSyncMvContexts(OlapTable olapTable, MTMVPlanUtil.DISABLE_RULES_WHEN_GENERATE_MTMV_CACHE, meta.getSessionVariables()); basicMvContext.setDatabase(meta.getDbName()); - boolean sessionVarMatch = SessionVarGuardRewriter.checkSessionVariablesMatch( + boolean sessionVarMatch = SessionVarGuardRewriter.checkSessionVariablesMatchForSyncMv( ConnectContextUtil.getAffectQueryResultInPlanVariables( cascadesContext.getConnectContext()), meta.getSessionVariables()); MTMVCache mtmvCache = MTMVCache.from(querySql.get(), diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/expressions/VariablePersistTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/expressions/VariablePersistTest.java index 31bf7e0a6ccfd2..2abef74979512c 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/expressions/VariablePersistTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/expressions/VariablePersistTest.java @@ -27,6 +27,8 @@ import org.apache.doris.nereids.trees.plans.RelationId; import org.apache.doris.nereids.types.BooleanType; import org.apache.doris.nereids.util.MemoTestUtils; +import org.apache.doris.qe.SessionVariable; +import org.apache.doris.qe.SqlModeHelper; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -179,7 +181,6 @@ public void testMergeGuardExprComplicated() { Assertions.assertEquals(rewritten, new And(slotA, slotB)); } - // guard1(guard2(child)) -> exception @Test public void testMergeGuardExprException() { @@ -212,4 +213,212 @@ public void testMergeGuardExprDeepNesting() { Assertions.assertEquals(child, resultGuard.child()); Assertions.assertEquals(sessionVars, resultGuard.getSessionVars()); } + + private static Map syncMvSessionVars(long sqlMode, boolean enableDecimal256) { + return ImmutableMap.of( + SessionVariable.SQL_MODE, Long.toString(sqlMode), + "enable_decimal256", Boolean.toString(enableDecimal256)); + } + + @Test + public void testSyncMvMatchingSqlModes() { + long mode = SqlModeHelper.MODE_ONLY_FULL_GROUP_BY; + Map current = syncMvSessionVars(mode, false); + Map persisted = syncMvSessionVars(mode, false); + + Assertions.assertTrue( + SessionVarGuardRewriter.checkSessionVariablesMatchForSyncMv(current, persisted)); + } + + @Test + public void testSyncMvStrictTransTablesDifferenceInBothDirections() { + long baseMode = SqlModeHelper.MODE_ONLY_FULL_GROUP_BY; + long strictMode = baseMode | SqlModeHelper.MODE_STRICT_TRANS_TABLES; + Map withoutStrict = syncMvSessionVars(baseMode, false); + Map withStrict = syncMvSessionVars(strictMode, false); + + // Rollup created without strict mode, queried with strict mode. + Assertions.assertTrue( + SessionVarGuardRewriter.checkSessionVariablesMatchForSyncMv(withStrict, withoutStrict)); + + // Rollup created with strict mode, queried without strict mode. + Assertions.assertTrue( + SessionVarGuardRewriter.checkSessionVariablesMatchForSyncMv(withoutStrict, withStrict)); + + // Identical strict-mode configurations must also match. + Assertions.assertTrue( + SessionVarGuardRewriter.checkSessionVariablesMatchForSyncMv(withStrict, withStrict)); + + // Compatibility also applies when SQL_MODE is the only persisted variable. + Map persistedSqlModeOnly = + ImmutableMap.of(SessionVariable.SQL_MODE, Long.toString(baseMode)); + Map currentSqlModeOnly = + ImmutableMap.of(SessionVariable.SQL_MODE, Long.toString(strictMode)); + Assertions.assertTrue( + SessionVarGuardRewriter.checkSessionVariablesMatchForSyncMv( + currentSqlModeOnly, persistedSqlModeOnly)); + } + + @Test + public void testSyncMvOtherSqlModeDifferencesRemainIncompatible() { + long baseMode = SqlModeHelper.MODE_ONLY_FULL_GROUP_BY; + Map persisted = syncMvSessionVars(baseMode, false); + long[] otherModes = { + SqlModeHelper.MODE_PIPES_AS_CONCAT, + SqlModeHelper.MODE_NO_BACKSLASH_ESCAPES + }; + + for (long otherMode : otherModes) { + Map current = syncMvSessionVars(baseMode | otherMode, false); + Assertions.assertFalse( + SessionVarGuardRewriter.checkSessionVariablesMatchForSyncMv(current, persisted), + "Unexpectedly accepted SQL-mode bit: " + otherMode); + } + + // The exception must not allow other mode changes with SQL_MODE-only state, either. + Map persistedSqlModeOnly = + ImmutableMap.of(SessionVariable.SQL_MODE, Long.toString(baseMode)); + Map currentSqlModeOnly = ImmutableMap.of( + SessionVariable.SQL_MODE, + Long.toString(baseMode | SqlModeHelper.MODE_PIPES_AS_CONCAT)); + Assertions.assertFalse( + SessionVarGuardRewriter.checkSessionVariablesMatchForSyncMv( + currentSqlModeOnly, persistedSqlModeOnly)); + } + + @Test + public void testSyncMvStrictTableModesDoNotHideOtherModeDifferences() { + long baseMode = SqlModeHelper.MODE_ONLY_FULL_GROUP_BY; + long strictModes = SqlModeHelper.MODE_STRICT_TRANS_TABLES + | SqlModeHelper.MODE_STRICT_ALL_TABLES; + + Map persisted = syncMvSessionVars(baseMode, false); + + long[] incompatibleModes = { + SqlModeHelper.MODE_PIPES_AS_CONCAT, + SqlModeHelper.MODE_NO_BACKSLASH_ESCAPES + }; + + for (long incompatibleMode : incompatibleModes) { + Map current = syncMvSessionVars( + baseMode | strictModes | incompatibleMode, false); + + Assertions.assertFalse( + SessionVarGuardRewriter.checkSessionVariablesMatchForSyncMv( + current, persisted), + "Strict-table exemption must not hide other SQL-mode differences: " + + incompatibleMode); + } + } + + @Test + public void testSyncMvOtherSessionVariablesStillRequireExactMatch() { + long baseMode = SqlModeHelper.MODE_ONLY_FULL_GROUP_BY; + long strictMode = baseMode + | SqlModeHelper.MODE_STRICT_TRANS_TABLES + | SqlModeHelper.MODE_STRICT_ALL_TABLES; + Map persisted = syncMvSessionVars(baseMode, false); + + // A strict-mode difference is allowed, but an unrelated variable mismatch is not. + Map current = syncMvSessionVars(strictMode, true); + Assertions.assertFalse( + SessionVarGuardRewriter.checkSessionVariablesMatchForSyncMv(current, persisted)); + } + + @Test + public void testSyncMvMissingSessionVariablesRemainIncompatible() { + long baseMode = SqlModeHelper.MODE_ONLY_FULL_GROUP_BY; + Map persisted = syncMvSessionVars(baseMode, false); + Map missingSqlMode = ImmutableMap.of("enable_decimal256", "false"); + Map missingDecimal256 = + ImmutableMap.of(SessionVariable.SQL_MODE, Long.toString(baseMode)); + + Assertions.assertFalse( + SessionVarGuardRewriter.checkSessionVariablesMatchForSyncMv(missingSqlMode, persisted)); + Assertions.assertFalse( + SessionVarGuardRewriter.checkSessionVariablesMatchForSyncMv(missingDecimal256, persisted)); + Assertions.assertFalse( + SessionVarGuardRewriter.checkSessionVariablesMatchForSyncMv(null, persisted)); + } + + @Test + public void testSyncMvMalformedSqlModeDoesNotBypassGuard() { + long baseMode = SqlModeHelper.MODE_ONLY_FULL_GROUP_BY; + Map persisted = syncMvSessionVars(baseMode, false); + Map malformed = ImmutableMap.of( + SessionVariable.SQL_MODE, "INVALID_SQL_MODE", + "enable_decimal256", "false"); + + Assertions.assertFalse( + SessionVarGuardRewriter.checkSessionVariablesMatchForSyncMv(malformed, persisted)); + Assertions.assertFalse( + SessionVarGuardRewriter.checkSessionVariablesMatchForSyncMv(persisted, malformed)); + } + + @Test + public void testSyncMvPreservesEmptyPersistedVariableBehavior() { + Map current = syncMvSessionVars( + SqlModeHelper.MODE_ONLY_FULL_GROUP_BY, false); + + // Preserve the existing behavior for older MVs without saved variables. + Assertions.assertTrue( + SessionVarGuardRewriter.checkSessionVariablesMatchForSyncMv(current, null)); + Assertions.assertTrue( + SessionVarGuardRewriter.checkSessionVariablesMatchForSyncMv(current, ImmutableMap.of())); + } + + @Test + public void testGeneralSessionVariableComparisonRemainsStrict() { + long baseMode = SqlModeHelper.MODE_ONLY_FULL_GROUP_BY; + long strictMode = baseMode + | SqlModeHelper.MODE_STRICT_TRANS_TABLES + | SqlModeHelper.MODE_STRICT_ALL_TABLES; + Map persisted = syncMvSessionVars(baseMode, false); + Map current = syncMvSessionVars(strictMode, false); + + // The Sync MV exception must not weaken the general-purpose comparator. + Assertions.assertFalse( + SessionVarGuardRewriter.checkSessionVariablesMatch(current, persisted)); + Assertions.assertTrue( + SessionVarGuardRewriter.checkSessionVariablesMatchForSyncMv(current, persisted)); + } + + @Test + public void testSyncMvStrictTableModeCombinations() { + long baseMode = SqlModeHelper.MODE_ONLY_FULL_GROUP_BY; + long strictTrans = SqlModeHelper.MODE_STRICT_TRANS_TABLES; + long strictAll = SqlModeHelper.MODE_STRICT_ALL_TABLES; + + long[] modes = { + baseMode, + baseMode | strictTrans, + baseMode | strictAll, + baseMode | strictTrans | strictAll + }; + + // All four configurations should be mutually compatible for sync MVs. + for (long persistedMode : modes) { + for (long currentMode : modes) { + Assertions.assertTrue( + SessionVarGuardRewriter.checkSessionVariablesMatchForSyncMv( + syncMvSessionVars(currentMode, false), + syncMvSessionVars(persistedMode, false)), + "Strict-table SQL-mode flags should not block sync MV rewriting: " + + "current=" + currentMode + ", persisted=" + persistedMode); + } + } + + // The same exemption applies when SQL_MODE is the only saved variable. + Map baseSqlModeOnly = ImmutableMap.of( + SessionVariable.SQL_MODE, Long.toString(baseMode)); + Map bothStrictSqlModeOnly = ImmutableMap.of( + SessionVariable.SQL_MODE, Long.toString(baseMode | strictTrans | strictAll)); + + Assertions.assertTrue( + SessionVarGuardRewriter.checkSessionVariablesMatchForSyncMv( + bothStrictSqlModeOnly, baseSqlModeOnly)); + Assertions.assertTrue( + SessionVarGuardRewriter.checkSessionVariablesMatchForSyncMv( + baseSqlModeOnly, bothStrictSqlModeOnly)); + } } diff --git a/regression-test/data/nereids_p0/variables_persist/test_sync_rollup_strict_sql_mode.out b/regression-test/data/nereids_p0/variables_persist/test_sync_rollup_strict_sql_mode.out new file mode 100644 index 00000000000000..7d557551cccd30 --- /dev/null +++ b/regression-test/data/nereids_p0/variables_persist/test_sync_rollup_strict_sql_mode.out @@ -0,0 +1,16 @@ +-- This file is automatically generated. You should know what you did if you want to edit this +-- !normal_sql_mode -- +1 300 +2 700 + +-- !strict_sql_mode -- +1 300 +2 700 + +-- !strict_all_sql_mode -- +1 300 +2 700 + +-- !both_strict_sql_modes -- +1 300 +2 700 diff --git a/regression-test/suites/nereids_p0/variables_persist/test_sync_rollup_strict_sql_mode.groovy b/regression-test/suites/nereids_p0/variables_persist/test_sync_rollup_strict_sql_mode.groovy new file mode 100644 index 00000000000000..0e2c25dc5cbeaf --- /dev/null +++ b/regression-test/suites/nereids_p0/variables_persist/test_sync_rollup_strict_sql_mode.groovy @@ -0,0 +1,224 @@ +// 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_sync_rollup_strict_sql_mode", "p0") { + + def rollupName = "rollup_by_region" + + def query = """ + SELECT region_id, SUM(sales) + FROM test_sync_rollup_strict_sql_mode + GROUP BY region_id + ORDER BY region_id + """ + + // Preserve session settings modified by this test. + def originalSqlMode = sql("SELECT @@sql_mode")[0][0].toString() + def originalNereids = sql("SELECT @@enable_nereids_planner")[0][0] + def originalFallback = sql("SELECT @@enable_fallback_to_original_planner")[0][0] + def originalSqlCache = sql("SELECT @@enable_sql_cache")[0][0] + + try { + sql "SET enable_nereids_planner = true" + sql "SET enable_fallback_to_original_planner = false" + sql "SET enable_sql_cache = false" + + // Create the synchronous rollup without either strict-table SQL-mode bit. + sql "SET sql_mode = 'ONLY_FULL_GROUP_BY'" + + sql "DROP TABLE IF EXISTS test_sync_rollup_strict_sql_mode" + + sql """ + CREATE TABLE test_sync_rollup_strict_sql_mode ( + region_id INT NOT NULL, + store_id INT NOT NULL, + sales BIGINT SUM NOT NULL + ) + AGGREGATE KEY(region_id, store_id) + DISTRIBUTED BY HASH(region_id) BUCKETS 1 + PROPERTIES ("replication_num" = "1") + """ + + sql """ + INSERT INTO test_sync_rollup_strict_sql_mode VALUES + (1, 101, 100), + (1, 102, 200), + (2, 201, 300), + (2, 202, 400) + """ + + sql """ + ALTER TABLE test_sync_rollup_strict_sql_mode + ADD ROLLUP ${rollupName} ( + region_id, + sales + ) + """ + + // Rollup creation is asynchronous. Wait until it is usable. + boolean rollupFinished = false + + for (int attempt = 0; attempt < 90; attempt++) { + def jobs = sql """ + SHOW ALTER TABLE ROLLUP + WHERE TableName = 'test_sync_rollup_strict_sql_mode' + ORDER BY CreateTime DESC + LIMIT 1 + """ + + if (!jobs.isEmpty()) { + String state = jobs[0][8].toString() + + if (state == "FINISHED") { + rollupFinished = true + break + } + + assertFalse( + state == "CANCELLED", + "Rollup creation was cancelled: ${jobs[0]}" + ) + } + + Thread.sleep(2000) + } + + assertTrue( + rollupFinished, + "Rollup ${rollupName} did not finish creating" + ) + + // Baseline: matching SQL modes must allow rollup rewriting. + explain { + sql query + verbose true + + check { plan -> + boolean rewriteSucceeded = + plan.contains("${rollupName} chose") + || plan.contains("${rollupName} not chose") + + assertTrue( + rewriteSucceeded, + "Rollup rewrite failed with matching SQL mode:\n${plan}" + ) + + assertFalse( + plan.contains("${rollupName} fail"), + "Unexpected rollup rewrite failure:\n${plan}" + ) + + return true + } + } + + qt_normal_sql_mode(query) + + // Regression: STRICT_TRANS_TABLES alone must not + // invalidate an otherwise compatible synchronous rollup. + sql "SET sql_mode = 'ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES'" + + explain { + sql query + verbose true + + check { plan -> + boolean rewriteSucceeded = + plan.contains("${rollupName} chose") + || plan.contains("${rollupName} not chose") + + assertTrue( + rewriteSucceeded, + "STRICT_TRANS_TABLES prevented rollup rewriting:\n${plan}" + ) + + assertFalse( + plan.contains("${rollupName} fail"), + "Unexpected rollup rewrite failure:\n${plan}" + ) + + return true + } + } + + qt_strict_sql_mode(query) + + // STRICT_ALL_TABLES alone must not prevent compatible rollup rewriting. + sql "SET sql_mode = 'ONLY_FULL_GROUP_BY,STRICT_ALL_TABLES'" + + explain { + sql query + verbose true + + check { plan -> + boolean rewriteSucceeded = + plan.contains("${rollupName} chose") + || plan.contains("${rollupName} not chose") + + assertTrue( + rewriteSucceeded, + "STRICT_ALL_TABLES prevented rollup rewriting:\n${plan}" + ) + + assertFalse( + plan.contains("${rollupName} fail"), + "Unexpected rollup rewrite failure:\n${plan}" + ) + + return true + } + } + + qt_strict_all_sql_mode(query) + + // Both strict-table SQL-mode bits together must also remain compatible. + sql "SET sql_mode = 'ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,STRICT_ALL_TABLES'" + + explain { + sql query + verbose true + + check { plan -> + boolean rewriteSucceeded = + plan.contains("${rollupName} chose") + || plan.contains("${rollupName} not chose") + + assertTrue( + rewriteSucceeded, + "Both strict-table bits prevented rollup rewriting:\n${plan}" + ) + + assertFalse( + plan.contains("${rollupName} fail"), + "Unexpected rollup rewrite failure:\n${plan}" + ) + + return true + } + } + + qt_both_strict_sql_modes(query) + + } finally { + + // Restore the original session settings. + sql "SET sql_mode = '${originalSqlMode}'" + sql "SET enable_sql_cache = ${originalSqlCache}" + sql "SET enable_fallback_to_original_planner = ${originalFallback}" + sql "SET enable_nereids_planner = ${originalNereids}" + } +}