Bugfix pullExpressionThroughVariables

prestodb · Sep 18, 2024 · 92315e0 · 92315e0
1 parent cefb7ab
commit 92315e0
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 0 deletions.
diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/EffectivePredicateExtractor.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/EffectivePredicateExtractor.java
@@ -60,6 +60,7 @@
 
 import static com.facebook.presto.common.function.OperatorType.EQUAL;
 import static com.facebook.presto.common.type.BooleanType.BOOLEAN;
+import static com.facebook.presto.expressions.LogicalRowExpressions.FALSE_CONSTANT;
 import static com.facebook.presto.expressions.LogicalRowExpressions.TRUE_CONSTANT;
 import static com.facebook.presto.expressions.LogicalRowExpressions.extractConjuncts;
 import static com.facebook.presto.spi.relation.SpecialFormExpression.Form.IS_NULL;
@@ -410,6 +411,7 @@ private static CallExpression buildEqualsExpression(FunctionAndTypeManager funct
 
         private RowExpression pullExpressionThroughVariables(RowExpression expression, Collection<VariableReferenceExpression> variables)
         {
+            // Would we better off just checking if the expression is true or false constant and returning early ?
             EqualityInference equalityInference = new EqualityInference.Builder(functionManger)
                     .addEqualityInference(expression)
                     .build();
@@ -419,6 +421,10 @@ private RowExpression pullExpressionThroughVariables(RowExpression expression, C
                 if (determinismEvaluator.isDeterministic(conjunct)) {
                     RowExpression rewritten = equalityInference.rewriteExpression(conjunct, in(variables));
                     if (rewritten != null) {
+                        if (VariablesExtractor.extractUnique(rewritten).isEmpty() && !rewritten.equals(FALSE_CONSTANT)) {
+                            // If we have reduced the rewritten expression to one that only refers to constants, we should not pull this up
+                            continue;
+                        }
                         effectiveConjuncts.add(rewritten);
                     }
                 }

diff --git a/...o-main/src/test/java/com/facebook/presto/sql/planner/TestEffectivePredicateExtractor.java b/...o-main/src/test/java/com/facebook/presto/sql/planner/TestEffectivePredicateExtractor.java
@@ -37,6 +37,7 @@
 import com.facebook.presto.spi.plan.TableScanNode;
 import com.facebook.presto.spi.plan.TopNNode;
 import com.facebook.presto.spi.plan.UnionNode;
+import com.facebook.presto.spi.relation.ConstantExpression;
 import com.facebook.presto.spi.relation.RowExpression;
 import com.facebook.presto.spi.relation.VariableReferenceExpression;
 import com.facebook.presto.sql.planner.plan.JoinNode;
@@ -238,6 +239,26 @@ public void testProject()
                         equals(DV, EV)));
     }
 
+    @Test
+    public void testProjectOverFilterWithNoReferencedAssignments()
+    {
+        PlanNode node = new ProjectNode(newId(),
+                filter(baseTableScan,
+                        and(
+                                call("mod",
+                                        metadata.getFunctionAndTypeManager().lookupFunction("mod", fromTypes(BIGINT, BIGINT)),
+                                        BIGINT,
+                                        ImmutableList.of(
+                                                CV,
+                                                bigintLiteral(5))),
+                                equals(CV, bigintLiteral(10)))),
+                assignment(DV, AV));
+
+        RowExpression effectivePredicate = effectivePredicateExtractor.extract(node);
+
+        assertEquals(effectivePredicate, TRUE_CONSTANT);
+    }
+
     @Test
     public void testTopN()
     {