You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "foxtail463 (via GitHub)" <gi...@apache.org> on 2026/03/10 08:37:02 UTC
[PR] [fix](nereids) refine mv range/residual compensation for OR predicates #57279 [doris]
foxtail463 opened a new pull request, #61178:
URL: https://github.com/apache/doris/pull/61178
Example: view predicate = (id = 5 OR id > 10), query predicate = (id = 5) AND score = 1; Compensation = (id = 5) + (score = 1).
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
OR Condition Support Missing:
The residual predicate compensation did not support OR branch matching. If an MV had (A OR B) and the query had A, the system would reject the rewrite opportunity even though the MV could be used with appropriate compensation.
MV: select id, score from T1 where id = 5 or score = 1
query: select id, score from T1 where id = 5
The MV cannot match the query
Solution:
Implemented support for OR branch matching in residual predicate compensation:
Uncovered Predicate Collection: Modified compensateEquivalence and compensateRangePredicate to collect uncovered predicates (instead of failing immediately) into output parameters uncoveredEquals and uncoveredRanges.
Residual Compensation with Extra Predicates: Enhanced compensateResidualPredicate to accept extraQueryResiduals parameter, which includes uncovered predicates from earlier compensation stages.
OR Branch Matching: Implemented branch-level matching logic that:
Extracts disjunction branches from both MV and query residual predicates
Allows partial intersection: if MV has (A OR B) and query has A, considers it a match
Generates compensation: NOT(B) to exclude MV's extra branches, and includes query's extra branches directly
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [ ] 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 <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [ ] No.
- [ ] Yes. <!-- Add document PR link here. eg: https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into -->
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) refine mv range/residual compensation for OR predicates #57279 [doris]
Posted by "Thearas (via GitHub)" <gi...@apache.org>.
Thearas commented on PR #61178:
URL: https://github.com/apache/doris/pull/61178#issuecomment-4029616986
Thank you for your contribution to Apache Doris.
Don't know what should be done next? See [How to process your PR](https://cwiki.apache.org/confluence/display/DORIS/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?
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) refine mv range/residual compensation for OR predicates #57279 [doris]
Posted by "foxtail463 (via GitHub)" <gi...@apache.org>.
foxtail463 closed pull request #61178: [fix](nereids) refine mv range/residual compensation for OR predicates #57279
URL: https://github.com/apache/doris/pull/61178
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) refine mv range/residual compensation for OR predicates #57279 [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #61178:
URL: https://github.com/apache/doris/pull/61178#discussion_r2910274158
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/Predicates.java:
##########
@@ -128,121 +128,105 @@ public static Map<Expression, ExpressionInfo> compensateCouldNotPullUpPredicates
}
/**
- * compensate equivalence predicates
+ * Compensate equivalence predicates based on equivalence classes.
+ * Collects uncovered equivalence predicates into uncoveredEquals for residual compensation.
*/
public static Map<Expression, ExpressionInfo> compensateEquivalence(StructInfo queryStructInfo,
StructInfo viewStructInfo,
SlotMapping viewToQuerySlotMapping,
- ComparisonResult comparisonResult) {
+ ComparisonResult comparisonResult,
+ Set<Expression> uncoveredEquals) {
EquivalenceClass queryEquivalenceClass = queryStructInfo.getEquivalenceClass();
EquivalenceClass viewEquivalenceClass = viewStructInfo.getEquivalenceClass();
Map<SlotReference, SlotReference> viewToQuerySlotMap = viewToQuerySlotMapping.toSlotReferenceMap();
EquivalenceClass viewEquivalenceClassQueryBased = viewEquivalenceClass.permute(viewToQuerySlotMap);
if (viewEquivalenceClassQueryBased == null) {
return null;
}
- final Map<Expression, ExpressionInfo> equalCompensateConjunctions = new HashMap<>();
if (queryEquivalenceClass.isEmpty() && viewEquivalenceClass.isEmpty()) {
return ImmutableMap.of();
}
- if (queryEquivalenceClass.isEmpty() && !viewEquivalenceClass.isEmpty()) {
- return null;
- }
EquivalenceClassMapping queryToViewEquivalenceMapping =
EquivalenceClassMapping.generate(queryEquivalenceClass, viewEquivalenceClassQueryBased);
- // can not map all target equivalence class, can not compensate
if (queryToViewEquivalenceMapping.getEquivalenceClassSetMap().size()
< viewEquivalenceClass.getEquivalenceSetList().size()) {
return null;
}
- // do equal compensate
+ Map<Expression, ExpressionInfo> compensations = new HashMap<>();
Set<List<SlotReference>> mappedQueryEquivalenceSet =
queryToViewEquivalenceMapping.getEquivalenceClassSetMap().keySet();
+ // Process each query equivalence class:
+ // 1) If it is not mapped, view cannot cover this equal-set, so defer it to residual compensation.
+ // 2) If it is mapped, only compensate the extra equal constraints introduced by query.
for (List<SlotReference> queryEquivalenceSet : queryEquivalenceClass.getEquivalenceSetList()) {
- // compensate the equivalence in query but not in view
if (!mappedQueryEquivalenceSet.contains(queryEquivalenceSet)) {
- Iterator<SlotReference> iterator = queryEquivalenceSet.iterator();
- SlotReference first = iterator.next();
- while (iterator.hasNext()) {
- Expression equals = new EqualTo(first, iterator.next());
- if (equals.anyMatch(AggregateFunction.class::isInstance)) {
- return null;
- }
- equalCompensateConjunctions.put(equals, ExpressionInfo.EMPTY);
- }
+ // Uncovered class: use the first slot as anchor and generate a minimal equality set.
+ SlotReference first = queryEquivalenceSet.get(0);
+ queryEquivalenceSet.stream()
+ .skip(1)
+ .map(slot -> new EqualTo(first, slot))
+ .forEach(uncoveredEquals::add);
} else {
Review Comment:
**Dropped `AggregateFunction` check for uncovered equals.**
The old code checked `if (equals.anyMatch(AggregateFunction.class::isInstance)) return null;` for each generated equality before adding to `equalCompensateConjunctions`. The new code generates `EqualTo` expressions and adds them directly to `uncoveredEquals` without this check.
This is safe because: (a) equivalence class slots are `SlotReference` objects, not aggregate functions, so the check would never trigger, and (b) these expressions will be checked later in `compensateResidualPredicate` (line 303). A brief comment noting this would be helpful for future readers.
##########
fe/fe-core/src/test/java/org/apache/doris/nereids/mv/PredicatesTest.java:
##########
@@ -177,8 +182,68 @@ public void testCompensateCouldNotPullUpPredicatesSuccess() {
Map<Expression, ExpressionInfo> compensateRangePredicates = Predicates.compensateRangePredicate(
queryStructInfo, mvStructInfo, mvToQuerySlotMapping, comparisonResult,
- queryContext);
+ queryContext, new HashSet<>());
Assertions.assertNotNull(compensateRangePredicates);
Assertions.assertEquals(1, compensateRangePredicates.size());
+ String rangeSql = compensateRangePredicates.keySet().iterator().next().toSql();
+ Assertions.assertEquals("(score > 15)", rangeSql);
+ }
+
+ @Test
+ public void testResidualCompensateWithUncoveredRangeAsExtraResidual() {
Review Comment:
**Test coverage gaps:**
1. **No negative test cases for `computeResidualCompensation`**: Missing tests for scenarios where MV rewrite should fail, e.g.:
- Query OR is a superset of MV OR (MV: `id=5 OR id=2`, Query: `id=5 OR id=2 OR id>10`)
- MV has residual predicates but query has none
2. **No test for `compensateEquivalence` with `uncoveredEquals`**: The `uncoveredEquals` output parameter is introduced but has zero test coverage.
3. **No end-to-end regression test**: Per the project's testing standards, all kernel features must have corresponding regression tests under `regression-test/`. This OR-subset compensation feature has no regression test.
4. **No test for exact-match OR case**: MV and query have identical OR predicates — compensation should remove the query residual (empty compensation).
5. **No test for multiple OR clauses**: e.g., MV has `(a=1 OR a=2) AND (b=3 OR b=4)` and query has a subset of one or both.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/Predicates.java:
##########
@@ -128,121 +128,105 @@ public static Map<Expression, ExpressionInfo> compensateCouldNotPullUpPredicates
}
/**
- * compensate equivalence predicates
+ * Compensate equivalence predicates based on equivalence classes.
+ * Collects uncovered equivalence predicates into uncoveredEquals for residual compensation.
*/
public static Map<Expression, ExpressionInfo> compensateEquivalence(StructInfo queryStructInfo,
StructInfo viewStructInfo,
SlotMapping viewToQuerySlotMapping,
- ComparisonResult comparisonResult) {
+ ComparisonResult comparisonResult,
+ Set<Expression> uncoveredEquals) {
EquivalenceClass queryEquivalenceClass = queryStructInfo.getEquivalenceClass();
EquivalenceClass viewEquivalenceClass = viewStructInfo.getEquivalenceClass();
Map<SlotReference, SlotReference> viewToQuerySlotMap = viewToQuerySlotMapping.toSlotReferenceMap();
EquivalenceClass viewEquivalenceClassQueryBased = viewEquivalenceClass.permute(viewToQuerySlotMap);
if (viewEquivalenceClassQueryBased == null) {
return null;
}
- final Map<Expression, ExpressionInfo> equalCompensateConjunctions = new HashMap<>();
if (queryEquivalenceClass.isEmpty() && viewEquivalenceClass.isEmpty()) {
return ImmutableMap.of();
}
- if (queryEquivalenceClass.isEmpty() && !viewEquivalenceClass.isEmpty()) {
- return null;
- }
EquivalenceClassMapping queryToViewEquivalenceMapping =
EquivalenceClassMapping.generate(queryEquivalenceClass, viewEquivalenceClassQueryBased);
- // can not map all target equivalence class, can not compensate
if (queryToViewEquivalenceMapping.getEquivalenceClassSetMap().size()
< viewEquivalenceClass.getEquivalenceSetList().size()) {
return null;
}
- // do equal compensate
+ Map<Expression, ExpressionInfo> compensations = new HashMap<>();
Set<List<SlotReference>> mappedQueryEquivalenceSet =
queryToViewEquivalenceMapping.getEquivalenceClassSetMap().keySet();
+ // Process each query equivalence class:
+ // 1) If it is not mapped, view cannot cover this equal-set, so defer it to residual compensation.
+ // 2) If it is mapped, only compensate the extra equal constraints introduced by query.
for (List<SlotReference> queryEquivalenceSet : queryEquivalenceClass.getEquivalenceSetList()) {
- // compensate the equivalence in query but not in view
if (!mappedQueryEquivalenceSet.contains(queryEquivalenceSet)) {
- Iterator<SlotReference> iterator = queryEquivalenceSet.iterator();
- SlotReference first = iterator.next();
- while (iterator.hasNext()) {
- Expression equals = new EqualTo(first, iterator.next());
- if (equals.anyMatch(AggregateFunction.class::isInstance)) {
- return null;
- }
- equalCompensateConjunctions.put(equals, ExpressionInfo.EMPTY);
- }
+ // Uncovered class: use the first slot as anchor and generate a minimal equality set.
+ SlotReference first = queryEquivalenceSet.get(0);
+ queryEquivalenceSet.stream()
+ .skip(1)
+ .map(slot -> new EqualTo(first, slot))
+ .forEach(uncoveredEquals::add);
} else {
- // compensate the equivalence both in query and view, but query has more equivalence
+ // Partially covered: avoid duplicating view equalities, only add query extras.
List<SlotReference> viewEquivalenceSet =
queryToViewEquivalenceMapping.getEquivalenceClassSetMap().get(queryEquivalenceSet);
- List<SlotReference> copiedQueryEquivalenceSet = new ArrayList<>(queryEquivalenceSet);
- copiedQueryEquivalenceSet.removeAll(viewEquivalenceSet);
- SlotReference first = viewEquivalenceSet.iterator().next();
- for (SlotReference slotReference : copiedQueryEquivalenceSet) {
- Expression equals = new EqualTo(first, slotReference);
+ List<SlotReference> queryExtraSlots = new ArrayList<>(queryEquivalenceSet);
+ queryExtraSlots.removeAll(viewEquivalenceSet);
+
+ // Build compensation predicates by pairing a view base slot with each extra query slot.
+ SlotReference firstViewSlot = viewEquivalenceSet.get(0);
+ for (SlotReference extraSlot : queryExtraSlots) {
+ Expression equals = new EqualTo(firstViewSlot, extraSlot);
if (equals.anyMatch(AggregateFunction.class::isInstance)) {
return null;
}
- equalCompensateConjunctions.put(equals, ExpressionInfo.EMPTY);
+ compensations.put(equals, ExpressionInfo.EMPTY);
}
}
}
- return equalCompensateConjunctions;
+ return ImmutableMap.copyOf(compensations);
}
/**
- * compensate range predicates
+ * Compensate range predicates.
+ * Collects uncovered range predicates into uncoveredRanges for residual compensation.
*/
public static Map<Expression, ExpressionInfo> compensateRangePredicate(StructInfo queryStructInfo,
StructInfo viewStructInfo,
SlotMapping viewToQuerySlotMapping,
ComparisonResult comparisonResult,
- CascadesContext cascadesContext) {
+ CascadesContext cascadesContext,
+ Set<Expression> uncoveredRanges) {
SplitPredicate querySplitPredicate = queryStructInfo.getSplitPredicate();
SplitPredicate viewSplitPredicate = viewStructInfo.getSplitPredicate();
- Set<Expression> viewRangeQueryBasedSet = new HashSet<>();
- for (Expression viewExpression : viewSplitPredicate.getRangePredicateMap().keySet()) {
- viewRangeQueryBasedSet.add(
- ExpressionUtils.replace(viewExpression, viewToQuerySlotMapping.toSlotReferenceMap()));
- }
- viewRangeQueryBasedSet.remove(BooleanLiteral.TRUE);
+ Map<SlotReference, SlotReference> slotMap = viewToQuerySlotMapping.toSlotReferenceMap();
+ Set<Expression> viewRangeSetQueryBased = viewSplitPredicate.getRangePredicateMap().keySet().stream()
+ .filter(expr -> !ExpressionUtils.isInferred(expr))
+ .map(expr -> ExpressionUtils.replace(expr, slotMap))
+ .filter(expr -> expr != BooleanLiteral.TRUE)
+ .collect(ImmutableSet.toImmutableSet());
- Set<Expression> queryRangeSet = querySplitPredicate.getRangePredicateMap().keySet();
- queryRangeSet.remove(BooleanLiteral.TRUE);
+ Set<Expression> queryRangeSet = querySplitPredicate.getRangePredicateMap().keySet().stream()
+ .filter(expr -> !ExpressionUtils.isInferred(expr))
+ .collect(ImmutableSet.toImmutableSet());
Set<Expression> differentExpressions = new HashSet<>();
- Sets.difference(queryRangeSet, viewRangeQueryBasedSet).copyInto(differentExpressions);
- Sets.difference(viewRangeQueryBasedSet, queryRangeSet).copyInto(differentExpressions);
- // the range predicate in query and view is same, don't need to compensate
+ Sets.difference(queryRangeSet, viewRangeSetQueryBased).copyInto(differentExpressions);
+ Sets.difference(viewRangeSetQueryBased, queryRangeSet).copyInto(differentExpressions);
if (differentExpressions.isEmpty()) {
return ImmutableMap.of();
}
- // try to normalize the different expressions
- Set<Expression> normalizedExpressions =
- normalizeExpression(ExpressionUtils.and(differentExpressions), cascadesContext);
+ Set<Expression> normalizedExpressions = new HashSet<>(
+ normalizeExpression(ExpressionUtils.and(differentExpressions), cascadesContext));
+ // Avoid entering subsequent containsAll/uncoveredRanges; otherwise residual compensation may fail.
+ normalizedExpressions.remove(BooleanLiteral.TRUE);
if (!queryRangeSet.containsAll(normalizedExpressions)) {
- // normalized expressions is not in query, can not compensate
return null;
}
- Map<Expression, ExpressionInfo> normalizedExpressionsWithLiteral = new HashMap<>();
- for (Expression expression : normalizedExpressions) {
- Set<Literal> literalSet = expression.collect(expressionTreeNode -> expressionTreeNode instanceof Literal);
- if (!(expression instanceof ComparisonPredicate)
- || (expression instanceof GreaterThan || expression instanceof LessThanEqual)
- || literalSet.size() != 1) {
- if (expression.anyMatch(AggregateFunction.class::isInstance)) {
- return null;
- }
- normalizedExpressionsWithLiteral.put(expression, ExpressionInfo.EMPTY);
- continue;
- }
- if (expression.anyMatch(AggregateFunction.class::isInstance)) {
- return null;
- }
- normalizedExpressionsWithLiteral.put(expression, new ExpressionInfo(literalSet.iterator().next()));
- }
- return normalizedExpressionsWithLiteral;
+ uncoveredRanges.addAll(normalizedExpressions);
+ return buildRangeExpressionInfo(normalizedExpressions);
Review Comment:
**Design flaw: double-counting of range compensation predicates.**
`normalizedExpressions` is both returned as the range compensation map (via `buildRangeExpressionInfo`) AND added to `uncoveredRanges`. These uncovered ranges flow into `compensateResidualPredicate` as `extraQueryResiduals`, where they can survive into the residual compensation map. When `SplitPredicate.toList()` concatenates all three maps, the same expression can appear twice.
This is not a correctness bug because `Sets.newLinkedHashSet(...)` in `AbstractMaterializedViewRule` (line 289) deduplicates before creating `LogicalFilter`. However, it causes wasted work in `rewriteExpression` and is fragile — it relies on `Expression.equals()` being consistent across different code paths.
Consider either:
1. Only adding to `uncoveredRanges` and returning an empty map when the expressions are fully deferred to residual compensation, or
2. Not adding range expressions that are already in the range compensation to `uncoveredRanges`, or
3. Adding a comment explaining the intentional duplication and reliance on downstream dedup.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) refine mv range/residual compensation for OR predicates #57279 [doris]
Posted by "morrySnow (via GitHub)" <gi...@apache.org>.
morrySnow commented on PR #61178:
URL: https://github.com/apache/doris/pull/61178#issuecomment-4029637985
/review
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org