branch-4.2: [fix](nereids) Allow sync MV rewrite across strict-table SQL modes

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<String, String> 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<String, String> currentSessionVars,
+            Map<String, String> 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<String, String> 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;
+    }
 }

fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/InitMaterializationContextHook.java

@@ -284,7 +284,7 @@ private List<MaterializationContext> 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(),

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<String, String> 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<String, String> current = syncMvSessionVars(mode, false);
+        Map<String, String> 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<String, String> withoutStrict = syncMvSessionVars(baseMode, false);
+        Map<String, String> 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<String, String> persistedSqlModeOnly =
+                ImmutableMap.of(SessionVariable.SQL_MODE, Long.toString(baseMode));
+        Map<String, String> 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<String, String> persisted = syncMvSessionVars(baseMode, false);
+        long[] otherModes = {
+                SqlModeHelper.MODE_PIPES_AS_CONCAT,
+                SqlModeHelper.MODE_NO_BACKSLASH_ESCAPES
+        };
+
+        for (long otherMode : otherModes) {
+            Map<String, String> 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<String, String> persistedSqlModeOnly =
+                ImmutableMap.of(SessionVariable.SQL_MODE, Long.toString(baseMode));
+        Map<String, String> 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<String, String> persisted = syncMvSessionVars(baseMode, false);
+
+        long[] incompatibleModes = {
+            SqlModeHelper.MODE_PIPES_AS_CONCAT,
+            SqlModeHelper.MODE_NO_BACKSLASH_ESCAPES
+        };
+
+        for (long incompatibleMode : incompatibleModes) {
+            Map<String, String> 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<String, String> persisted = syncMvSessionVars(baseMode, false);
+
+        // A strict-mode difference is allowed, but an unrelated variable mismatch is not.
+        Map<String, String> current = syncMvSessionVars(strictMode, true);
+        Assertions.assertFalse(
+                SessionVarGuardRewriter.checkSessionVariablesMatchForSyncMv(current, persisted));
+    }
+
+    @Test
+    public void testSyncMvMissingSessionVariablesRemainIncompatible() {
+        long baseMode = SqlModeHelper.MODE_ONLY_FULL_GROUP_BY;
+        Map<String, String> persisted = syncMvSessionVars(baseMode, false);
+        Map<String, String> missingSqlMode = ImmutableMap.of("enable_decimal256", "false");
+        Map<String, String> 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<String, String> persisted = syncMvSessionVars(baseMode, false);
+        Map<String, String> 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<String, String> 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<String, String> persisted = syncMvSessionVars(baseMode, false);
+        Map<String, String> 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<String, String> baseSqlModeOnly = ImmutableMap.of(
+                SessionVariable.SQL_MODE, Long.toString(baseMode));
+        Map<String, String> bothStrictSqlModeOnly = ImmutableMap.of(
+                SessionVariable.SQL_MODE, Long.toString(baseMode | strictTrans | strictAll));
+
+        Assertions.assertTrue(
+                SessionVarGuardRewriter.checkSessionVariablesMatchForSyncMv(
+                bothStrictSqlModeOnly, baseSqlModeOnly));
+        Assertions.assertTrue(
+                SessionVarGuardRewriter.checkSessionVariablesMatchForSyncMv(
+                baseSqlModeOnly, bothStrictSqlModeOnly));
+    }
 }

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