starrocks icon indicating copy to clipboard operation
starrocks copied to clipboard

[BugFix] Fix mv union rewrite handling null partition bugs

Open LiShuMing opened this issue 6 months ago • 4 comments

Why I'm doing:

  • MV union rewrite may lose null partitions after rewrite which may cause result inconsistent with no rewrite;
  • MV union rewrite may cause bad plan with losing necessary columns.

What I'm doing:

This pull request introduces enhancements to the materialized view rewriting logic by replacing the use of CompoundPredicateOperator.not with a new utility class, NegateFilterShuttle, and improving the handling of enforced non-existent columns during query rewriting. These changes aim to make the codebase more modular, maintainable, and robust.

Refactoring for Predicate Negation:

  • Replaced the use of CompoundPredicateOperator.not with the NegateFilterShuttle utility class for negating predicates in AggregatedTimeSeriesRewriter and MaterializedViewRewriter. This improves modularity and reusability of predicate negation logic. ([[1]](diffhunk://#diff-58483d5aceb932787d60c442258da89f2f2cf7a812e2d626d4760eafc9700000L656-R658), [[2]](diffhunk://#diff-928c7b191892f7924196f34f738f512fb867cce2c78828d6f59fde45d69321cbL1996-R2007))

Enhanced Handling of Enforced Non-Existent Columns:

  • Improved the logic for pruning enforced non-existent columns by introducing a more robust mechanism to filter and handle these columns in the MaterializedViewRewriter. This includes modifying the pruneEnforcedColumns and doPruneEnforcedColumns methods to accept and process a set of enforced non-existent columns. ([fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rule/transformation/materialization/MaterializedViewRewriter.javaR2020-R2064](diffhunk://#diff-928c7b191892f7924196f34f738f512fb867cce2c78828d6f59fde45d69321cbR2020-R2064))

Minor Fixes and Adjustments:

  • Added a fallback to handle cases where a column is missing in mvProjection by defaulting to a NULL value in the doUnionRewrite method. ([fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rule/transformation/materialization/MaterializedViewRewriter.javaL2270-R2285](diffhunk://#diff-928c7b191892f7924196f34f738f512fb867cce2c78828d6f59fde45d69321cbL2270-R2285))

Fixes https://github.com/StarRocks/StarRocksTest/pull/9829

What type of PR is this:

  • [x] BugFix
  • [ ] Feature
  • [ ] Enhancement
  • [ ] Refactor
  • [ ] UT
  • [ ] Doc
  • [ ] Tool

Does this PR entail a change in behavior?

  • [ ] Yes, this PR will result in a change in behavior.
  • [x] No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

  • [ ] Interface/UI changes: syntax, type conversion, expression evaluation, display information
  • [ ] Parameter changes: default values, similar parameters but with different default values
  • [ ] Policy changes: use new policy to replace old one, functionality automatically enabled
  • [ ] Feature removed
  • [ ] Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

  • [x] I have added test cases for my bug fix or my new feature
  • [ ] This pr needs user documentation (for new or modified features or behaviors)
    • [ ] I have added documentation for my new feature or new function
  • [ ] This is a backport pr

Bugfix cherry-pick branch check:

  • [x] I have checked the version labels which the pr will be auto-backported to the target branch
    • [x] 3.5
    • [x] 3.4
    • [x] 3.3

LiShuMing avatar Jun 19 '25 11:06 LiShuMing

[Java-Extensions Incremental Coverage Report]

:white_check_mark: pass : 0 / 0 (0%)

github-actions[bot] avatar Jun 21 '25 02:06 github-actions[bot]

[FE Incremental Coverage Report]

:x: fail : 31 / 43 (72.09%)

file detail

path covered_line new_line coverage not_covered_line_detail
:large_blue_circle: com/starrocks/sql/optimizer/rule/transformation/materialization/MaterializedViewRewriter.java 21 33 63.64% [2032, 2046, 2049, 2054, 2056, 2057, 2058, 2059, 2061, 2062, 2063, 2064]
:large_blue_circle: com/starrocks/sql/optimizer/Utils.java 6 6 100.00% []
:large_blue_circle: com/starrocks/sql/optimizer/rule/transformation/materialization/AggregatedTimeSeriesRewriter.java 4 4 100.00% []

github-actions[bot] avatar Jun 21 '25 02:06 github-actions[bot]

[BE Incremental Coverage Report]

:white_check_mark: pass : 0 / 0 (0%)

github-actions[bot] avatar Jun 21 '25 02:06 github-actions[bot]

@Mergifyio backport branch-3.5

github-actions[bot] avatar Jun 23 '25 09:06 github-actions[bot]

@Mergifyio backport branch-3.4

github-actions[bot] avatar Jun 23 '25 09:06 github-actions[bot]

@Mergifyio backport branch-3.3

github-actions[bot] avatar Jun 23 '25 09:06 github-actions[bot]

backport branch-3.5

✅ Backports have been created

mergify[bot] avatar Jun 23 '25 09:06 mergify[bot]

backport branch-3.4

✅ Backports have been created

mergify[bot] avatar Jun 23 '25 09:06 mergify[bot]

backport branch-3.3

✅ Backports have been created

mergify[bot] avatar Jun 23 '25 09:06 mergify[bot]