Skip applyFilter when pushed down predicate is wider then enforced one

trinodb · Sep 5, 2019 · 25966d2 · 25966d2
1 parent 4c03384
commit 25966d2
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 1 deletion.
diff --git a/...ain/src/main/java/io/prestosql/sql/planner/iterative/rule/PushPredicateIntoTableScan.java b/...ain/src/main/java/io/prestosql/sql/planner/iterative/rule/PushPredicateIntoTableScan.java
@@ -186,6 +186,12 @@ public static Optional<PlanNode> pushFilterIntoTableScan(
         TableHandle newTable;
         TupleDomain<ColumnHandle> remainingFilter;
         if (!metadata.usesLegacyTableLayouts(session, node.getTable())) {
+            if (!constraint.predicate().isPresent() && newDomain.contains(node.getEnforcedConstraint())) {
+                // new domain is wider than domain already provided by table scan
+                // TODO: remove deterministic filter predicate
+                return Optional.empty();
+            }
+
             if (newDomain.isNone()) {
                 // TODO: DomainTranslator.fromPredicate can infer that the expression is "false" in some cases (TupleDomain.none()).
                 // This should move to another rule that simplifies the filter using that logic and then rely on RemoveTrivialFilters

diff --git a/...src/test/java/io/prestosql/sql/planner/iterative/rule/TestPushPredicateIntoTableScan.java b/...src/test/java/io/prestosql/sql/planner/iterative/rule/TestPushPredicateIntoTableScan.java
@@ -23,6 +23,7 @@
 import io.prestosql.plugin.tpch.TpchTransactionHandle;
 import io.prestosql.spi.connector.ColumnHandle;
 import io.prestosql.spi.predicate.Domain;
+import io.prestosql.spi.predicate.NullableValue;
 import io.prestosql.spi.predicate.TupleDomain;
 import io.prestosql.spi.type.Type;
 import io.prestosql.sql.parser.SqlParser;
@@ -103,10 +104,41 @@ public void replaceWithExistsWhenNoLayoutExist()
                                 nationTableHandle,
                                 ImmutableList.of(p.symbol("nationkey", BIGINT)),
                                 ImmutableMap.of(p.symbol("nationkey", BIGINT), columnHandle),
-                                TupleDomain.none())))
+                                TupleDomain.fromFixedValues(ImmutableMap.of(
+                                        columnHandle, NullableValue.of(BIGINT, (long) 45))))))
                 .matches(values("A"));
     }
 
+    @Test
+    public void doesNotFireIfNewDomainIsSame()
+    {
+        ColumnHandle columnHandle = new TpchColumnHandle("nationkey", BIGINT);
+        tester().assertThat(pushPredicateIntoTableScan)
+                .on(p -> p.filter(expression("nationkey = BIGINT '44'"),
+                        p.tableScan(
+                                nationTableHandle,
+                                ImmutableList.of(p.symbol("nationkey", BIGINT)),
+                                ImmutableMap.of(p.symbol("nationkey", BIGINT), columnHandle),
+                                TupleDomain.fromFixedValues(ImmutableMap.of(
+                                        columnHandle, NullableValue.of(BIGINT, (long) 44))))))
+                .doesNotFire();
+    }
+
+    @Test
+    public void doesNotFireIfNewDomainIsWider()
+    {
+        ColumnHandle columnHandle = new TpchColumnHandle("nationkey", BIGINT);
+        tester().assertThat(pushPredicateIntoTableScan)
+                .on(p -> p.filter(expression("nationkey = BIGINT '44' OR nationkey = BIGINT '45'"),
+                        p.tableScan(
+                                nationTableHandle,
+                                ImmutableList.of(p.symbol("nationkey", BIGINT)),
+                                ImmutableMap.of(p.symbol("nationkey", BIGINT), columnHandle),
+                                TupleDomain.fromFixedValues(ImmutableMap.of(
+                                        columnHandle, NullableValue.of(BIGINT, (long) 44))))))
+                .doesNotFire();
+    }
+
     @Test
     public void doesNotFireIfRuleNotChangePlan()
     {