diff --git a/presto-hive/src/test/java/com/facebook/presto/hive/TestHiveMaterializedViewLogicalPlanner.java b/presto-hive/src/test/java/com/facebook/presto/hive/TestHiveMaterializedViewLogicalPlanner.java index 6767fc4d96b9c..e872725458c38 100644 --- a/presto-hive/src/test/java/com/facebook/presto/hive/TestHiveMaterializedViewLogicalPlanner.java +++ b/presto-hive/src/test/java/com/facebook/presto/hive/TestHiveMaterializedViewLogicalPlanner.java @@ -2169,6 +2169,11 @@ public void testMaterializedViewOptimizationWithUnsupportedFunctionSubquery() assertPlan(queryOptimizationWithMaterializedView, baseQuery, anyTree( node(JoinNode.class, + exchange( + anyTree( + constrainedTableScan(table, + ImmutableMap.of(), + ImmutableMap.of()))), anyTree( exchange( anyTree( @@ -2178,12 +2183,7 @@ public void testMaterializedViewOptimizationWithUnsupportedFunctionSubquery() anyTree( constrainedTableScan(view2, ImmutableMap.of(), - ImmutableMap.of("ds_42", "ds", "orderkey_41", "orderkey"))))), - exchange( - anyTree( - constrainedTableScan(table, - ImmutableMap.of(), - ImmutableMap.of())))))); + ImmutableMap.of("ds_42", "ds", "orderkey_41", "orderkey")))))))); } finally { queryRunner.execute("DROP MATERIALIZED VIEW IF EXISTS " + view2); diff --git a/presto-main-base/src/main/java/com/facebook/presto/cost/ScalarStatsCalculator.java b/presto-main-base/src/main/java/com/facebook/presto/cost/ScalarStatsCalculator.java index 14ecc0aa325b0..f03cb55eab36c 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/cost/ScalarStatsCalculator.java +++ b/presto-main-base/src/main/java/com/facebook/presto/cost/ScalarStatsCalculator.java @@ -51,6 +51,7 @@ import com.facebook.presto.sql.tree.SymbolReference; import com.facebook.presto.type.TypeUtils; import com.google.common.collect.ImmutableMap; +import io.airlift.slice.Slice; import jakarta.inject.Inject; import java.util.Map; @@ -172,6 +173,9 @@ public VariableStatsEstimate visitConstant(ConstantExpression literal, Void cont estimate.setLowValue(doubleValue.getAsDouble()); estimate.setHighValue(doubleValue.getAsDouble()); } + + setConstantSizeEstimate(literal.getValue(), estimate); + return estimate.build(); } @@ -373,6 +377,9 @@ protected VariableStatsEstimate visitLiteral(Literal node, Void context) estimate.setLowValue(doubleValue.getAsDouble()); estimate.setHighValue(doubleValue.getAsDouble()); } + + setConstantSizeEstimate(value, estimate); + return estimate.build(); } @@ -393,10 +400,13 @@ protected VariableStatsEstimate visitFunctionCall(FunctionCall node, Void contex } // value is a constant - return VariableStatsEstimate.builder() + VariableStatsEstimate.Builder statsEstimate = VariableStatsEstimate.builder() .setNullsFraction(0) - .setDistinctValuesCount(1) - .build(); + .setDistinctValuesCount(1); + + setConstantSizeEstimate(value, statsEstimate); + + return statsEstimate.build(); } private Map, Type> getExpressionTypes(Session session, Expression expression, TypeProvider types) @@ -555,6 +565,13 @@ protected VariableStatsEstimate visitCoalesceExpression(CoalesceExpression node, } } + private static void setConstantSizeEstimate(Object value, VariableStatsEstimate.Builder statsEstimate) + { + if (value instanceof Slice) { + statsEstimate.setAverageRowSize(((Slice) value).length()); + } + } + private static VariableStatsEstimate estimateCoalesce(PlanNodeStatsEstimate input, VariableStatsEstimate left, VariableStatsEstimate right) { // Question to reviewer: do you have a method to check if fraction is empty or saturated? diff --git a/presto-main-base/src/test/java/com/facebook/presto/cost/TestScalarStatsCalculator.java b/presto-main-base/src/test/java/com/facebook/presto/cost/TestScalarStatsCalculator.java index afe69f74e8fe7..c5248a3d8ae3b 100644 --- a/presto-main-base/src/test/java/com/facebook/presto/cost/TestScalarStatsCalculator.java +++ b/presto-main-base/src/test/java/com/facebook/presto/cost/TestScalarStatsCalculator.java @@ -114,10 +114,12 @@ public void testLiteral() .highValue(75.5) .nullsFraction(0.0); - assertCalculate(new StringLiteral("blah")) + VariableStatsAssertion blah = assertCalculate(new StringLiteral("blah")); + blah .distinctValuesCount(1.0) .lowValueUnknown() .highValueUnknown() + .averageRowSize(4.0) .nullsFraction(0.0); assertCalculate(new NullLiteral()) @@ -162,6 +164,7 @@ public void testVarbinaryConstant() .distinctValuesCount(1.0) .lowValueUnknown() .highValueUnknown() + .averageRowSize(11.0) .nullsFraction(0.0); } diff --git a/presto-main-base/src/test/java/com/facebook/presto/sql/planner/optimizations/TestReplaceConstantVariableReferencesWithConstants.java b/presto-main-base/src/test/java/com/facebook/presto/sql/planner/optimizations/TestReplaceConstantVariableReferencesWithConstants.java index 6d0a3c59959c7..1f4f52246f77e 100644 --- a/presto-main-base/src/test/java/com/facebook/presto/sql/planner/optimizations/TestReplaceConstantVariableReferencesWithConstants.java +++ b/presto-main-base/src/test/java/com/facebook/presto/sql/planner/optimizations/TestReplaceConstantVariableReferencesWithConstants.java @@ -14,16 +14,27 @@ package com.facebook.presto.sql.planner.optimizations; import com.facebook.presto.Session; +import com.facebook.presto.cost.StatsProvider; +import com.facebook.presto.cost.VariableStatsEstimate; +import com.facebook.presto.metadata.Metadata; import com.facebook.presto.spi.plan.JoinType; +import com.facebook.presto.spi.plan.OutputNode; +import com.facebook.presto.spi.plan.PlanNode; import com.facebook.presto.spi.relation.VariableReferenceExpression; import com.facebook.presto.sql.planner.assertions.BasePlanTest; +import com.facebook.presto.sql.planner.assertions.MatchResult; +import com.facebook.presto.sql.planner.assertions.Matcher; import com.facebook.presto.sql.planner.assertions.PlanMatchPattern; +import com.facebook.presto.sql.planner.assertions.SymbolAliases; import com.facebook.presto.sql.planner.iterative.rule.test.RuleTester; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableListMultimap; import com.google.common.collect.ImmutableMap; +import org.testng.annotations.DataProvider; import org.testng.annotations.Test; +import java.util.Map; + import static com.facebook.presto.SystemSessionProperties.REWRITE_EXPRESSION_WITH_CONSTANT_EXPRESSION; import static com.facebook.presto.common.type.IntegerType.INTEGER; import static com.facebook.presto.metadata.FunctionAndTypeManager.createTestFunctionAndTypeManager; @@ -48,10 +59,21 @@ import static com.facebook.presto.sql.planner.iterative.rule.test.PlanBuilder.assignment; import static com.facebook.presto.sql.tree.SortItem.NullOrdering.LAST; import static com.facebook.presto.sql.tree.SortItem.Ordering.ASCENDING; +import static org.testng.Assert.assertEquals; public class TestReplaceConstantVariableReferencesWithConstants extends BasePlanTest { + @DataProvider + public static Object[][] filterProviders() + { + return new Object[][] { + {"3-MEDIUM"}, + {"2-LOOOOOOONG"}, + {"123456789012345"} // orderpriority is a VARCHAR(15), this is the max length after which a Constant replacement is not applied + }; + } + private Session enableOptimization() { return Session.builder(this.getQueryRunner().getDefaultSession()) @@ -179,18 +201,46 @@ public void testSemiJoin() tableScan("lineitem", ImmutableMap.of("orderkey_1", "orderkey")))))))))); } - @Test - public void testSimpleFilter() + @Test(dataProvider = "filterProviders") + public void testSimpleFilter(String filter) { - assertPlan("select orderkey, orderpriority from orders where orderpriority='3-MEDIUM'", + assertPlan("select orderkey, orderpriority from orders where orderpriority='" + filter + "'", enableOptimization(), output( ImmutableList.of("orderkey", "expr_6"), project( - ImmutableMap.of("expr_6", expression("'3-MEDIUM'")), + ImmutableMap.of("expr_6", expression("'" + filter + "'")), filter( - "orderpriority = '3-MEDIUM'", - tableScan("orders", ImmutableMap.of("orderkey", "orderkey", "orderpriority", "orderpriority")))))); + "orderpriority = '" + filter + "'", + tableScan("orders", ImmutableMap.of("orderkey", "orderkey", "orderpriority", "orderpriority"))))) + .with(new Matcher() + { + @Override + public boolean shapeMatches(PlanNode node) + { + return node instanceof OutputNode; + } + + @Override + public MatchResult detailMatches(PlanNode node, StatsProvider stats, Session session, Metadata metadata, SymbolAliases symbolAliases) + { + // Assert additionally ont size estimate of the replaced VARCHAR ConstantExpression + VariableStatsEstimate expectedStats = VariableStatsEstimate.builder() + .setAverageRowSize(filter.length()) + .setDistinctValuesCount(1) + .setNullsFraction(0.0) + .build(); + + VariableStatsEstimate actualStats = stats.getStats(node).getVariableStatistics().entrySet().stream() + .filter(es -> es.getKey().getName().equals("orderpriority")) + .map(Map.Entry::getValue) + .findFirst() + .orElseThrow(() -> new AssertionError("Variable 'orderpriority' not found in statistics")); + + assertEquals(actualStats, expectedStats); + return MatchResult.match(); + } + })); } @Test