Aranya7 opened a new pull request, #69212:
URL: https://github.com/apache/doris/pull/69212
### What problem does this PR solve?
Issue Number: N/A - No tracking issue has been filed for this fix.
Related PR: N/A
Problem Summary:
#### Problem
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` SQL-mode flag.
For example:
- Rollup creation session: `ONLY_FULL_GROUP_BY`
- Query execution session: `ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES`
Although this difference does not change the semantics of the aggregation
covered by the regression test, the existing session-variable compatibility
check considers the two configurations different.
Consequently, an otherwise eligible synchronous rollup may fail
materialized-view rewriting, causing the optimizer to miss a potentially more
efficient execution plan.
#### Root Cause
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. When the maps differ only in the `STRICT_TRANS_TABLES` bit, the
comparison returns `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 a SQL-mode
difference that is not relevant to the tested aggregation.
#### Solution
This PR introduces a synchronous-MV-specific session-variable compatibility
check:
`SessionVarGuardRewriter.checkSessionVariablesMatchForSyncMv()`
The new comparison:
- Allows differences exclusively in the `STRICT_TRANS_TABLES` SQL-mode bit.
- Preserves strict comparison of every other SQL-mode bit.
- 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 the existing behavior for older materialized views without
persisted session-variable metadata.
The implementation compares SQL modes using their persisted numeric bitmask
representations and excludes only `SqlModeHelper.MODE_STRICT_TRANS_TABLES` from
the compatibility comparison.
The change is restricted to synchronous materialized-view context
initialization in `InitMaterializationContextHook`.
The existing general-purpose `checkSessionVariablesMatch()` method remains
unchanged.
#### Expected Behavior
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,STRICT_TRANS_TABLES | ONLY_FULL_GROUP_BY | Eligible |
| ONLY_FULL_GROUP_BY | ONLY_FULL_GROUP_BY,STRICT_ALL_TABLES | Existing
compatibility restrictions preserved |
| ONLY_FULL_GROUP_BY |
ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,PIPES_AS_CONCAT | Existing compatibility
restrictions preserved |
Other result-affecting session-variable differences, such as changes to
`enable_decimal256`, continue to require the existing compatibility protections.
#### Regression Test
A new regression test has been added:
`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. Executes an aggregation query under the original SQL mode.
5. Verifies successful materialized-view rewriting using `EXPLAIN VERBOSE`.
6. Enables `STRICT_TRANS_TABLES` without changing the query.
7. Verifies that the rollup remains eligible for rewriting.
8. Verifies that the aggregation returns identical results under both SQL
modes.
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.
#### Before/After Validation
Tested against the same Doris source revision.
**Before the fix**
- Matching SQL modes: rewrite successful.
- STRICT_TRANS_TABLES-only difference: rewrite unsuccessful.
**After the fix**
- Matching SQL modes: rewrite successful.
- STRICT_TRANS_TABLES-only difference: rewrite successful.
- Aggregation results: unchanged.
#### Unit Test Coverage
The existing `VariablePersistTest` class has been extended to cover:
- Exact session-variable matches.
- `STRICT_TRANS_TABLES` differences in both directions.
- Other SQL-mode differences remaining incompatible.
- Combinations of `STRICT_TRANS_TABLES` with unrelated SQL-mode changes.
- Differences in other result-affecting session variables.
- Missing session-variable entries.
- Missing current session-variable state.
- Malformed SQL-mode values.
- Compatibility with older materialized views without persisted session
variables.
- Preservation of the existing general-purpose session-variable comparison
behavior.
#### Scope
This is a narrowly scoped Nereids optimizer fix.
It does not modify:
- The behavior of the general-purpose session-variable comparator.
- Asynchronous materialized-view rewriting.
- Materialized-view storage or refresh behavior.
- The meaning of the `STRICT_TRANS_TABLES` SQL-mode flag.
- Other result-affecting session-variable compatibility rules.
### Release note
Fix synchronous materialized-view rollup rewriting when the query session
and materialized-view creation session differ only in the `STRICT_TRANS_TABLES`
SQL-mode flag.
### Check List (For Author)
- Test
- [x] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason
- Behavior changed:
- [ ] No.
- [x] Yes.
Synchronous materialized views can remain eligible for query rewriting
when the only relevant session-variable difference is the `STRICT_TRANS_TABLES`
SQL-mode bit. Other session-variable compatibility protections remain unchanged.
- Does this need documentation?
- [x] No.
- [ ] Yes.
This change corrects internal optimizer rewrite eligibility without
introducing new SQL syntax, configuration options, or user-facing APIs.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
--
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]
