Skip to content

Commit

Permalink
[BugFix] fix invalid partition predicate bug (StarRocks#19373)
Browse files Browse the repository at this point in the history 
Signed-off-by: ABingHuang <[email protected]>
  • Loading branch information
@ABingHuang
ABingHuang committed Mar 20, 2023
1 parent 75622b7 commit a1a0bc7
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 20 deletions.
4 changes: 2 additions & 2 deletions .../java/com/starrocks/sql/optimizer/rule/transformation/materialization/ColumnRewriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,12 +186,12 @@ public ScalarOperator visitVariableReference(ColumnRefOperator columnRef, Void c
if (enableRelationRewrite && srcToDstRelationIdMapping != null) {
Integer srcRelationId = srcRefFactory.getRelationId(columnRef.getId());
if (srcRelationId < 0) {
return null;
return result;
}
Integer targetRelationId = srcToDstRelationIdMapping.get(srcRelationId);
Map<String, ColumnRefOperator> relationColumns = dstRelationIdToColumns.get(targetRelationId);
if (relationColumns == null) {
return null;
return result;
}
result = relationColumns.getOrDefault(columnRef.getName(), columnRef);
}
Expand Down
39 changes: 22 additions & 17 deletions ...starrocks/sql/optimizer/rule/transformation/materialization/MaterializedViewRewriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1155,7 +1155,13 @@ private PredicateSplit getCompensationPredicates(RewriteContext rewriteContext,
if (srcPu == null && targetPu != null) {
// query: empid < 5
// mv: empid < 5 or salary > 100
srcPu = Utils.compoundAnd(compensationEqualPredicate, compensationPr);
if (!isQueryAgainstView) {
// compensationEqualPredicate and compensationPr is based on query, need to change it to view based
srcPu = Utils.compoundAnd(columnRewriter.rewriteQueryToView(compensationEqualPredicate),
columnRewriter.rewriteQueryToView(compensationPr));
} else {
srcPu = Utils.compoundAnd(compensationEqualPredicate, compensationPr);
}
}
ScalarOperator compensationPu =
getCompensationResidualPredicate(srcPu, targetPu, columnRewriter, isQueryAgainstView);
Expand All @@ -1178,25 +1184,25 @@ private ScalarOperator getCompensationResidualPredicate(ScalarOperator srcPu,
} else if (targetPu == null) {
compensationPu = srcPu;
} else {
ScalarOperator canonizedSrcPu = MvUtils.canonizePredicateForRewrite(srcPu.clone());
ScalarOperator canonizedTargetPu = MvUtils.canonizePredicateForRewrite(targetPu.clone());
ScalarOperator swappedSrcPu;
ScalarOperator swappedTargetPu;
if (isQueryAgainstView) {
// src is query
swappedSrcPu = columnRewriter.rewriteByQueryEc(canonizedSrcPu);
swappedSrcPu = columnRewriter.rewriteByQueryEc(srcPu.clone());
// target is view
swappedTargetPu = columnRewriter.rewriteViewToQueryWithQueryEc(canonizedTargetPu);
swappedTargetPu = columnRewriter.rewriteViewToQueryWithQueryEc(targetPu.clone());
} else {
// src is view
swappedSrcPu = columnRewriter.rewriteViewToQueryWithViewEc(canonizedSrcPu);
swappedSrcPu = columnRewriter.rewriteViewToQueryWithViewEc(srcPu.clone());
// target is query
swappedTargetPu = columnRewriter.rewriteByViewEc(canonizedTargetPu);
swappedTargetPu = columnRewriter.rewriteByViewEc(targetPu.clone());
}
ScalarOperator canonizedSrcPu = MvUtils.canonizePredicateForRewrite(swappedSrcPu);
ScalarOperator canonizedTargetPu = MvUtils.canonizePredicateForRewrite(swappedTargetPu);

compensationPu = MvUtils.getCompensationPredicateForDisjunctive(swappedSrcPu, swappedTargetPu);
compensationPu = MvUtils.getCompensationPredicateForDisjunctive(canonizedSrcPu, canonizedTargetPu);
if (compensationPu == null) {
compensationPu = getCompensationResidualPredicate(swappedSrcPu, swappedTargetPu);
compensationPu = getCompensationResidualPredicate(canonizedSrcPu, canonizedTargetPu);
if (compensationPu == null) {
return null;
}
Expand Down Expand Up @@ -1234,24 +1240,23 @@ private ScalarOperator getCompensationRangePredicate(ScalarOperator srcPr,
} else if (srcPr == null) {
return null;
} else {
ScalarOperator canonizedSrcPr = MvUtils.canonizePredicateForRewrite(srcPr.clone());
ScalarOperator canonizedTargetPr = MvUtils.canonizePredicateForRewrite(targetPr.clone());
// swap column by query EC
ScalarOperator swappedSrcPr;
ScalarOperator swappedTargetPr;
if (isQueryAgainstView) {
// for query
swappedSrcPr = columnRewriter.rewriteByQueryEc(canonizedSrcPr);
swappedSrcPr = columnRewriter.rewriteByQueryEc(srcPr.clone());
// for view, swap column by relation mapping and query ec
swappedTargetPr = columnRewriter.rewriteViewToQueryWithQueryEc(canonizedTargetPr);
swappedTargetPr = columnRewriter.rewriteViewToQueryWithQueryEc(targetPr.clone());
} else {
// for view
swappedSrcPr = columnRewriter.rewriteViewToQueryWithViewEc(canonizedSrcPr);
swappedSrcPr = columnRewriter.rewriteViewToQueryWithViewEc(srcPr.clone());
// for query
swappedTargetPr = columnRewriter.rewriteByViewEc(canonizedTargetPr);
swappedTargetPr = columnRewriter.rewriteByViewEc(targetPr.clone());
}

compensationPr = getCompensationRangePredicate(swappedSrcPr, swappedTargetPr);
ScalarOperator canonizedSrcPr = MvUtils.canonizePredicateForRewrite(swappedSrcPr);
ScalarOperator canonizedTargetPr = MvUtils.canonizePredicateForRewrite(swappedTargetPr);
compensationPr = getCompensationRangePredicate(canonizedSrcPr, canonizedTargetPr);
}
return MvUtils.canonizePredicate(compensationPr);
}
Expand Down
38 changes: 38 additions & 0 deletions ...java/com/starrocks/sql/optimizer/rule/transformation/materialization/RangeSimplifier.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ public ScalarOperator simplify(List<ScalarOperator> targets) {
List<ScalarOperator> columnScalars = srcPredicates.stream().filter(
predicate -> isScalarForColumns(predicate, columnId)
).collect(Collectors.toList());
columnScalars = filterScalarOperators(columnScalars, columnRange);
columnScalars.forEach(
predicate -> result.set(Utils.compoundAnd(result.get(), predicate))
);
Expand All @@ -141,6 +142,43 @@ public ScalarOperator simplify(List<ScalarOperator> targets) {
}
}

private List<ScalarOperator> filterScalarOperators(
List<ScalarOperator> columnScalars, Range<ConstantOperator> validRange) {
List<ScalarOperator> results = Lists.newArrayList();;
for (ScalarOperator candidate : columnScalars) {
if (isRedundantPredicate(candidate, validRange)) {
continue;
}
results.add(candidate);
}
return results;
}

private boolean isRedundantPredicate(ScalarOperator scalarOperator, Range<ConstantOperator> validRange) {
Preconditions.checkState(scalarOperator instanceof BinaryPredicateOperator);
Preconditions.checkState(scalarOperator.getChild(0) instanceof ColumnRefOperator);
Preconditions.checkState(scalarOperator.getChild(1) instanceof ConstantOperator);
BinaryPredicateOperator binary = scalarOperator.cast();
ConstantOperator right = binary.getChild(1).cast();
Range predicateRange = range(binary.getBinaryType(), right);
// only range border's predicate is non redundant
if (predicateRange.hasLowerBound()
&& validRange.hasLowerBound()
&& predicateRange.lowerBoundType() == validRange.lowerBoundType()
&& predicateRange.lowerEndpoint().equals(validRange.lowerEndpoint())) {
// predicate is lower border
return false;
}
if (predicateRange.hasUpperBound()
&& validRange.hasUpperBound()
&& predicateRange.upperBoundType() == validRange.upperBoundType()
&& predicateRange.upperEndpoint().equals(validRange.upperEndpoint())) {
// predicate is upper border
return false;
}
return true;
}

private boolean isScalarForColumns(ScalarOperator predicate, int columnId) {
BinaryPredicateOperator binary = (BinaryPredicateOperator) predicate;
ColumnRefOperator targetColumn = (ColumnRefOperator) binary.getChild(0);
Expand Down
14 changes: 14 additions & 0 deletions fe/fe-core/src/test/java/com/starrocks/planner/MaterializedViewSSBTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import com.google.common.collect.Lists;
import com.starrocks.common.FeConstants;
import com.starrocks.sql.plan.PlanTestBase;
import org.junit.BeforeClass;
import org.junit.Test;

Expand Down Expand Up @@ -99,4 +100,17 @@ public void testQuery4_2() {
public void testQuery4_3() {
runFileUnitTest("materialized-view/ssb/q4-3");
}

@Test
public void testPartitionPredicate() throws Exception {
String query = "select sum(LO_EXTENDEDPRICE * LO_DISCOUNT) AS revenue\n" +
"from lineorder\n" +
"join dates on lo_orderdate = d_datekey\n" +
"where weekofyear(LO_ORDERDATE) = 6 AND LO_ORDERDATE >= 19940101 and LO_ORDERDATE <= 19941231\n" +
"and lo_discount between 5 and 7\n" +
"and lo_quantity between 26 and 35;";
String plan = getFragmentPlan(query);
PlanTestBase.assertContains(plan, "lineorder_flat_mv");
PlanTestBase.assertNotContains(plan, "LO_ORDERDATE <= 19950100");
}
}
2 changes: 1 addition & 1 deletion fe/fe-core/src/test/java/com/starrocks/planner/MaterializedViewWithPartitionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ public void testPartitionPrune_MultiTables2() throws Exception {
" where t1.c3 < 2000 and t2.c3 > 100;")
.contains("TABLE: partial_mv_6\n" +
" PREAGGREGATION: ON\n" +
" PREDICATES: 11: c3 <= 1999, 11: c3 >= 100, 11: c3 >= 101, 11: c3 < 2000\n" +
" PREDICATES: 11: c3 <= 1999, 11: c3 >= 101, 11: c3 < 2000\n" +
" partitions=1/1\n" +
" rollup: partial_mv_6\n" +
" tabletRatio=2/2");
Expand Down

0 comments on commit a1a0bc7

Please sign in to comment.