Prioritize Facts over HBO stats

prestodb · Jun 27, 2024 · c3e4c8e · c3e4c8e
1 parent a150ea6
commit c3e4c8e
Show file tree

Hide file tree

Showing 8 changed files with 207 additions and 2 deletions.
diff --git a/presto-main/src/main/java/com/facebook/presto/cost/HistoryBasedPlanStatisticsCalculator.java b/presto-main/src/main/java/com/facebook/presto/cost/HistoryBasedPlanStatisticsCalculator.java
@@ -44,6 +44,7 @@
 import static com.facebook.presto.common.RuntimeUnit.NANO;
 import static com.facebook.presto.cost.HistoricalPlanStatisticsUtil.getSelectedHistoricalPlanStatisticsEntry;
 import static com.facebook.presto.cost.HistoryBasedPlanStatisticsManager.historyBasedPlanCanonicalizationStrategyList;
+import static com.facebook.presto.spi.statistics.PlanStatistics.toConfidenceLevel;
 import static com.facebook.presto.sql.planner.iterative.Plans.resolveGroupReferences;
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.graph.Traverser.forTree;
@@ -201,7 +202,7 @@ private PlanNodeStatsEstimate getStatistics(PlanNode planNode, Session session,
                         Optional<HistoricalPlanStatisticsEntry> historicalPlanStatisticsEntry = getSelectedHistoricalPlanStatisticsEntry(entry.getValue(), inputTableStatistics.get(), historyMatchingThreshold);
                         if (historicalPlanStatisticsEntry.isPresent()) {
                             PlanStatistics predictedPlanStatistics = historicalPlanStatisticsEntry.get().getPlanStatistics();
-                            if (predictedPlanStatistics.getConfidence() > 0) {
+                            if ((toConfidenceLevel(predictedPlanStatistics.getConfidence()).getConfidenceOrdinal() >= delegateStats.confidenceLevel().getConfidenceOrdinal())) {
                                 return delegateStats.combineStats(
                                         predictedPlanStatistics,
                                         new HistoryBasedSourceInfo(entry.getKey().getHash(), inputTableStatistics, Optional.ofNullable(historicalPlanStatisticsEntry.get().getHistoricalPlanStatisticsEntryInfo())));

diff --git a/presto-main/src/test/java/com/facebook/presto/cost/TestPlanStatistics.java b/presto-main/src/test/java/com/facebook/presto/cost/TestPlanStatistics.java
@@ -0,0 +1,47 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.facebook.presto.cost;
+
+import com.facebook.presto.spi.statistics.Estimate;
+import com.facebook.presto.spi.statistics.JoinNodeStatistics;
+import com.facebook.presto.spi.statistics.PartialAggregationStatistics;
+import com.facebook.presto.spi.statistics.PlanStatistics;
+import com.facebook.presto.spi.statistics.TableWriterNodeStatistics;
+import org.testng.annotations.Test;
+
+import static com.facebook.presto.spi.statistics.PlanStatistics.toConfidenceLevel;
+import static com.facebook.presto.spi.statistics.SourceInfo.ConfidenceLevel.HIGH;
+import static com.facebook.presto.spi.statistics.SourceInfo.ConfidenceLevel.LOW;
+import static com.facebook.presto.testing.assertions.Assert.assertEquals;
+
+public class TestPlanStatistics
+{
+    @Test
+    public void testGetEnumConfidenceHigh()
+    {
+        PlanStatistics planStatistics1 = new PlanStatistics(Estimate.of(100), Estimate.of(1000), 1, JoinNodeStatistics.empty(), TableWriterNodeStatistics.empty(), PartialAggregationStatistics.empty());
+        PlanStatistics planStatistics2 = new PlanStatistics(Estimate.of(100), Estimate.of(1000), .5, JoinNodeStatistics.empty(), TableWriterNodeStatistics.empty(), PartialAggregationStatistics.empty());
+
+        assertEquals(toConfidenceLevel(planStatistics1.getConfidence()), HIGH);
+        assertEquals(toConfidenceLevel(planStatistics2.getConfidence()), HIGH);
+    }
+
+    @Test
+    public void testGetEnumConfidenceLow()
+    {
+        PlanStatistics planStatistics1 = new PlanStatistics(Estimate.of(100), Estimate.of(1000), 0, JoinNodeStatistics.empty(), TableWriterNodeStatistics.empty(), PartialAggregationStatistics.empty());
+
+        assertEquals(toConfidenceLevel(planStatistics1.getConfidence()), LOW);
+    }
+}
diff --git a/presto-main/src/test/java/com/facebook/presto/sql/planner/assertions/PlanMatchPattern.java b/presto-main/src/test/java/com/facebook/presto/sql/planner/assertions/PlanMatchPattern.java
@@ -38,6 +38,8 @@
 import com.facebook.presto.spi.plan.TopNNode;
 import com.facebook.presto.spi.plan.UnionNode;
 import com.facebook.presto.spi.plan.ValuesNode;
+import com.facebook.presto.spi.statistics.SourceInfo;
+import com.facebook.presto.spi.statistics.SourceInfo.ConfidenceLevel;
 import com.facebook.presto.sql.parser.ParsingOptions;
 import com.facebook.presto.sql.parser.ParsingOptions.DecimalLiteralTreatment;
 import com.facebook.presto.sql.parser.SqlParser;
@@ -784,6 +786,18 @@ public PlanMatchPattern withOutputRowCount(double expectedOutputRowCount)
         return this;
     }
 
+    public PlanMatchPattern withSourceInfo(SourceInfo sourceInfo)
+    {
+        matchers.add(new StatsSourceInfoMatcher(sourceInfo));
+        return this;
+    }
+
+    public PlanMatchPattern withConfidenceLevel(ConfidenceLevel confidenceLevel)
+    {
+        matchers.add(new StatsConfidenceLevelMatcher(confidenceLevel));
+        return this;
+    }
+
     public PlanMatchPattern withOutputSize(double expectedOutputSize)
     {
         matchers.add(new StatsOutputSizeMatcher(expectedOutputSize));

diff --git a/...src/test/java/com/facebook/presto/sql/planner/assertions/StatsConfidenceLevelMatcher.java b/...src/test/java/com/facebook/presto/sql/planner/assertions/StatsConfidenceLevelMatcher.java
@@ -0,0 +1,48 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.facebook.presto.sql.planner.assertions;
+
+import com.facebook.presto.Session;
+import com.facebook.presto.cost.StatsProvider;
+import com.facebook.presto.metadata.Metadata;
+import com.facebook.presto.spi.plan.PlanNode;
+import com.facebook.presto.spi.statistics.SourceInfo.ConfidenceLevel;
+
+public class StatsConfidenceLevelMatcher
+        implements Matcher
+{
+    private final ConfidenceLevel expectedConfidenceLevel;
+
+    StatsConfidenceLevelMatcher(ConfidenceLevel expectedConfidenceLevel)
+    {
+        this.expectedConfidenceLevel = expectedConfidenceLevel;
+    }
+
+    @Override
+    public boolean shapeMatches(PlanNode node)
+    {
+        return true;
+    }
+
+    @Override
+    public MatchResult detailMatches(PlanNode node, StatsProvider stats, Session session, Metadata metadata, SymbolAliases symbolAliases)
+    {
+        return new MatchResult(stats.getStats(node).confidenceLevel() == expectedConfidenceLevel);
+    }
+
+    public String toString()
+    {
+        return "expectedConfidenceLevel(" + expectedConfidenceLevel + ")";
+    }
+}
diff --git a/...main/src/test/java/com/facebook/presto/sql/planner/assertions/StatsSourceInfoMatcher.java b/...main/src/test/java/com/facebook/presto/sql/planner/assertions/StatsSourceInfoMatcher.java
@@ -0,0 +1,48 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.facebook.presto.sql.planner.assertions;
+
+import com.facebook.presto.Session;
+import com.facebook.presto.cost.StatsProvider;
+import com.facebook.presto.metadata.Metadata;
+import com.facebook.presto.spi.plan.PlanNode;
+import com.facebook.presto.spi.statistics.SourceInfo;
+
+public class StatsSourceInfoMatcher
+        implements Matcher
+{
+    private final SourceInfo expectedSourceInfo;
+
+    StatsSourceInfoMatcher(SourceInfo expectedSourceInfo)
+    {
+        this.expectedSourceInfo = expectedSourceInfo;
+    }
+
+    @Override
+    public boolean shapeMatches(PlanNode node)
+    {
+        return true;
+    }
+
+    @Override
+    public MatchResult detailMatches(PlanNode node, StatsProvider stats, Session session, Metadata metadata, SymbolAliases symbolAliases)
+    {
+        return new MatchResult(stats.getStats(node).getSourceInfo().equals(expectedSourceInfo));
+    }
+
+    public String toString()
+    {
+        return "expectedSourceInfo(" + expectedSourceInfo + ")";
+    }
+}
diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/statistics/PlanStatistics.java b/presto-spi/src/main/java/com/facebook/presto/spi/statistics/PlanStatistics.java
@@ -22,6 +22,9 @@
 import java.util.Objects;
 
 import static com.facebook.drift.annotations.ThriftField.Requiredness.OPTIONAL;
+import static com.facebook.presto.spi.statistics.SourceInfo.ConfidenceLevel;
+import static com.facebook.presto.spi.statistics.SourceInfo.ConfidenceLevel.HIGH;
+import static com.facebook.presto.spi.statistics.SourceInfo.ConfidenceLevel.LOW;
 import static java.util.Objects.requireNonNull;
 
 @ThriftStruct
@@ -127,6 +130,14 @@ public PlanStatistics updateAggregationStatistics(PartialAggregationStatistics p
                 partialAggregationStatistics);
     }
 
+    public static ConfidenceLevel toConfidenceLevel(double confidence)
+    {
+        if (confidence > 0) {
+            return HIGH;
+        }
+        return LOW;
+    }
+
     private static void checkArgument(boolean condition, String message)
     {
         if (!condition) {

diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/statistics/SourceInfo.java b/presto-spi/src/main/java/com/facebook/presto/spi/statistics/SourceInfo.java
@@ -21,7 +21,21 @@ public abstract class SourceInfo
 {
     public enum ConfidenceLevel
     {
-        LOW, HIGH, FACT;
+        LOW(0),
+        HIGH(1),
+        FACT(2);
+
+        private final int confidenceOrdinal;
+
+        ConfidenceLevel(int confidenceOrdinal)
+        {
+            this.confidenceOrdinal = confidenceOrdinal;
+        }
+
+        public int getConfidenceOrdinal()
+        {
+            return confidenceOrdinal;
+        }
     }
 
     public abstract ConfidenceLevel confidenceLevel();

diff --git a/presto-tests/src/test/java/com/facebook/presto/execution/TestHistoryBasedStatsTracking.java b/presto-tests/src/test/java/com/facebook/presto/execution/TestHistoryBasedStatsTracking.java
@@ -27,10 +27,12 @@
 import com.facebook.presto.spi.plan.ProjectNode;
 import com.facebook.presto.spi.plan.SortNode;
 import com.facebook.presto.spi.plan.TopNNode;
+import com.facebook.presto.spi.statistics.CostBasedSourceInfo;
 import com.facebook.presto.spi.statistics.HistoryBasedPlanStatisticsProvider;
 import com.facebook.presto.sql.planner.assertions.MatchResult;
 import com.facebook.presto.sql.planner.assertions.Matcher;
 import com.facebook.presto.sql.planner.assertions.SymbolAliases;
+import com.facebook.presto.sql.planner.plan.EnforceSingleRowNode;
 import com.facebook.presto.sql.planner.plan.ExchangeNode;
 import com.facebook.presto.sql.planner.plan.JoinNode;
 import com.facebook.presto.sql.planner.plan.RowNumberNode;
@@ -55,6 +57,7 @@
 import static com.facebook.presto.SystemSessionProperties.TRACK_HISTORY_STATS_FROM_FAILED_QUERIES;
 import static com.facebook.presto.SystemSessionProperties.USE_HISTORY_BASED_PLAN_STATISTICS;
 import static com.facebook.presto.SystemSessionProperties.USE_PERFECTLY_CONSISTENT_HISTORIES;
+import static com.facebook.presto.spi.statistics.SourceInfo.ConfidenceLevel.FACT;
 import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.any;
 import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.anyTree;
 import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.node;
@@ -471,6 +474,25 @@ public void testBroadcastJoin()
                         anyTree(any())));
     }
 
+    @Test
+    public void testFactPrioritization()
+    {
+        String query1 = "SELECT (SELECT nationkey FROM nation WHERE name = 'UNITED STATES') AS us_nationkey";
+        executeAndTrackHistory(query1);
+        assertPlan(query1, anyTree(node(EnforceSingleRowNode.class, anyTree(any()))
+                .withOutputRowCount(1)
+                .withSourceInfo(new CostBasedSourceInfo(FACT)))
+                .withConfidenceLevel(FACT));
+
+        Session session = Session.builder(createSession()).setSystemProperty("prefer_partial_aggregation", "false").build();
+        String query2 = "SELECT COUNT(*) FROM orders";
+        executeAndTrackHistory(query2);
+        assertPlan(session, query2, anyTree(node(AggregationNode.class, anyTree(any())))
+                .withOutputRowCount(1)
+                .withSourceInfo(new CostBasedSourceInfo(FACT))
+                .withConfidenceLevel(FACT));
+    }
+
     private void executeAndTrackHistory(String sql)
     {
         getQueryRunner().execute(sql);