Skip to content

[fix](mtmv) Avoid mutating excluded trigger tables#62984

Merged
morrySnow merged 2 commits into
apache:masterfrom
foxtail463:fix-mtmv-partition-sync-immutable-exclude
May 11, 2026
Merged

[fix](mtmv) Avoid mutating excluded trigger tables#62984
morrySnow merged 2 commits into
apache:masterfrom
foxtail463:fix-mtmv-partition-sync-immutable-exclude

Conversation

@foxtail463

@foxtail463 foxtail463 commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

intro by #56745

Problem Summary:

isMTMVPartitionSync adds followed PCT tables into the excluded trigger table set while checking partition sync. Some callers pass immutable sets, such as the force-consistent rewrite path using ImmutableSet.of(), so partitioned MTMVs can fail with UnsupportedOperationException before the sync check finishes. Copy the input set into a local mutable set before adding derived PCT tables, avoiding mutation of caller-owned state and supporting immutable inputs.

@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@foxtail463

Copy link
Copy Markdown
Contributor Author

run buildall

@foxtail463

Copy link
Copy Markdown
Contributor Author

run buildall

@seawinde seawinde left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@github-actions github-actions Bot added the approved Indicates a PR has been approved by one committer. label May 8, 2026
@github-actions

github-actions Bot commented May 8, 2026

Copy link
Copy Markdown
Contributor

PR approved by at least one committer and no changes requested.

@github-actions

github-actions Bot commented May 8, 2026

Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

@morrySnow

Copy link
Copy Markdown
Contributor

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review result: no additional blocking issues found.

Critical checkpoint conclusions:

  • Goal/test coverage: The PR fixes isMTMVPartitionSync so it no longer mutates caller-owned excluded trigger table sets, and the added FE unit test covers the immutable ImmutableSet.of() caller path from force-consistent rewrite.
  • Scope/clarity: The production change is small and focused; it copies the input set once and uses the local mutable set for derived PCT exclusions.
  • Concurrency/lifecycle/config/compatibility: No new concurrency, lifecycle, configuration, persistence, protocol, or storage compatibility concerns found.
  • Parallel paths: The reviewed callers (MTMVRewriteUtil, refresh partition calculation, and task freshness check) continue to use the same helper, so the fix applies to the relevant paths without needing separate changes.
  • Error handling/data correctness: Existing AnalysisException behavior and partition/base-table sync semantics are preserved; the change avoids an UnsupportedOperationException without hiding sync failures.
  • Performance/observability: The extra set copy is bounded by the excluded table set size and is not a hot-path concern for this check; no additional logs or metrics are needed.
  • Tests: I reviewed the added unit test path but did not run the FE test suite in this Actions review session.

User focus: No additional user-provided review focus was specified.

@morrySnow morrySnow merged commit 464d89c into apache:master May 11, 2026
31 of 32 checks passed
zhaorongsheng pushed a commit to zhaorongsheng/doris that referenced this pull request Jun 4, 2026
Problem Summary:

isMTMVPartitionSync adds followed PCT tables into the excluded trigger
table set while checking partition sync. Some callers pass immutable
sets, such as the force-consistent rewrite path using ImmutableSet.of(),
so partitioned MTMVs can fail with UnsupportedOperationException before
the sync check finishes. Copy the input set into a local mutable set
before adding derived PCT tables, avoiding mutation of caller-owned
state and supporting immutable inputs.

---------

Co-authored-by: yangtao555 <yangtao555@jd.com>
yiguolei pushed a commit that referenced this pull request Jun 16, 2026
… (#64519)

pr: #62984
commitId: 464d89c

Backport #62984 to branch-4.1.

This fixes isMTMVPartitionSync mutating caller-owned excluded trigger
table sets. The backport adapts the upstream TableNameInfo change to
branch-4.1, which still uses TableName in this code path, by copying the
input set into a local mutable Set<TableName> before adding followed PCT
tables.

Conflict resolution:
- fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVPartitionUtil.java:
kept branch-4.1 TableName API and applied the local-copy behavior.
-
fe/fe-core/src/test/java/org/apache/doris/mtmv/MTMVPartitionUtilTest.java:
adapted the upstream immutable-set test to branch-4.1 JMockit style and
TableName type.

Tests:
- git diff --check upstream/branch-4.1..HEAD
- ./run-fe-ut.sh --run org.apache.doris.mtmv.MTMVPartitionUtilTest

Co-authored-by: foxtail463 <foxtail463@gmail.com>
Co-authored-by: yangtao555 <yangtao555@jd.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants