Fix filter pushdown for DELETE queries

prestodb · Nov 16, 2019 · a0cf226 · a0cf226
1 parent dacfc45
commit a0cf226
Show file tree

Hide file tree

Showing 8 changed files with 51 additions and 14 deletions.
diff --git a/presto-hive/src/main/java/com/facebook/presto/hive/HiveMetadata.java b/presto-hive/src/main/java/com/facebook/presto/hive/HiveMetadata.java
@@ -1793,9 +1793,42 @@ static Predicate<Map<ColumnHandle, NullableValue>> convertToPredicate(TupleDomai
     }
 
     @Override
-    public boolean supportsMetadataDelete(ConnectorSession session, ConnectorTableHandle tableHandle)
+    public boolean supportsMetadataDelete(ConnectorSession session, ConnectorTableHandle tableHandle, Optional<ConnectorTableLayoutHandle> tableLayoutHandle)
     {
-        return true;
+        if (!tableLayoutHandle.isPresent()) {
+            return true;
+        }
+
+        // Allow metadata delete for range filters on partition columns.
+        // TODO Add support for metadata delete for any filter on partition columns.
+
+        HiveTableLayoutHandle layoutHandle = (HiveTableLayoutHandle) tableLayoutHandle.get();
+        if (!layoutHandle.isPushdownFilterEnabled()) {
+            return true;
+        }
+
+        if (layoutHandle.getPredicateColumns().isEmpty()) {
+            return true;
+        }
+
+        if (!TRUE_CONSTANT.equals(layoutHandle.getRemainingPredicate())) {
+            return false;
+        }
+
+        TupleDomain<Subfield> domainPredicate = layoutHandle.getDomainPredicate();
+        if (domainPredicate.isAll()) {
+            return true;
+        }
+
+        Set<String> predicateColumnNames = domainPredicate.getDomains().get().keySet().stream()
+                .map(Subfield::getRootName)
+                .collect(toImmutableSet());
+
+        Set<String> partitionColumnNames = layoutHandle.getPartitionColumns().stream()
+                .map(HiveColumnHandle::getName)
+                .collect(toImmutableSet());
+
+        return partitionColumnNames.containsAll(predicateColumnNames);
     }
 
     @Override

diff --git a/presto-hive/src/test/java/com/facebook/presto/hive/TestHiveIntegrationSmokeTest.java b/presto-hive/src/test/java/com/facebook/presto/hive/TestHiveIntegrationSmokeTest.java
@@ -1677,6 +1677,15 @@ public void testMetadataDelete()
 
         assertQuery("SELECT * from test_metadata_delete", "SELECT orderkey, linenumber, linestatus FROM lineitem WHERE linestatus<>'F' or linenumber<>3");
 
+        // TODO This use case can be supported
+        try {
+            getQueryRunner().execute("DELETE FROM test_metadata_delete WHERE lower(LINE_STATUS)='f' and LINE_NUMBER=CAST(4 AS INTEGER)");
+            fail("expected exception");
+        }
+        catch (RuntimeException e) {
+            assertEquals(e.getMessage(), "This connector only supports delete where one or more partitions are deleted entirely");
+        }
+
         assertUpdate("DELETE FROM test_metadata_delete WHERE LINE_STATUS='O'");
 
         assertQuery("SELECT * from test_metadata_delete", "SELECT orderkey, linenumber, linestatus FROM lineitem WHERE linestatus<>'O' and linenumber<>3");

diff --git a/presto-hive/src/test/java/com/facebook/presto/hive/TestHivePushdownIntegrationSmokeTest.java b/presto-hive/src/test/java/com/facebook/presto/hive/TestHivePushdownIntegrationSmokeTest.java
@@ -47,10 +47,4 @@ public TestHivePushdownIntegrationSmokeTest()
     public void testMaterializedPartitioning()
     {
     }
-
-    // TODO Enable this test after we fix the query plan to retain information to indicate that a column filter is pushed down and the "DELETE" operation is not on entire partition.
-    @Test
-    public void testMetadataDelete()
-    {
-    }
 }
diff --git a/presto-kudu/src/main/java/com/facebook/presto/kudu/KuduMetadata.java b/presto-kudu/src/main/java/com/facebook/presto/kudu/KuduMetadata.java
@@ -368,7 +368,7 @@ public void finishDelete(ConnectorSession session, ConnectorTableHandle tableHan
     }
 
     @Override
-    public boolean supportsMetadataDelete(ConnectorSession session, ConnectorTableHandle tableHandle)
+    public boolean supportsMetadataDelete(ConnectorSession session, ConnectorTableHandle tableHandle, Optional<ConnectorTableLayoutHandle> tableLayoutHandle)
     {
         return false;
     }

diff --git a/presto-main/src/main/java/com/facebook/presto/metadata/MetadataManager.java b/presto-main/src/main/java/com/facebook/presto/metadata/MetadataManager.java
@@ -871,7 +871,8 @@ public boolean supportsMetadataDelete(Session session, TableHandle tableHandle)
         ConnectorMetadata metadata = getMetadata(session, connectorId);
         return metadata.supportsMetadataDelete(
                 session.toConnectorSession(connectorId),
-                tableHandle.getConnectorHandle());
+                tableHandle.getConnectorHandle(),
+                tableHandle.getLayout());
     }
 
     @Override

diff --git a/presto-raptor/src/main/java/com/facebook/presto/raptor/RaptorMetadata.java b/presto-raptor/src/main/java/com/facebook/presto/raptor/RaptorMetadata.java
@@ -834,7 +834,7 @@ public void finishDelete(ConnectorSession session, ConnectorTableHandle tableHan
     }
 
     @Override
-    public boolean supportsMetadataDelete(ConnectorSession session, ConnectorTableHandle tableHandle)
+    public boolean supportsMetadataDelete(ConnectorSession session, ConnectorTableHandle tableHandle, Optional<ConnectorTableLayoutHandle> tableLayoutHandle)
     {
         return false;
     }

diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/connector/ConnectorMetadata.java b/presto-spi/src/main/java/com/facebook/presto/spi/connector/ConnectorMetadata.java
@@ -538,7 +538,7 @@ default Map<SchemaTableName, ConnectorViewDefinition> getViews(ConnectorSession
     /**
      * @return whether delete without table scan is supported
      */
-    default boolean supportsMetadataDelete(ConnectorSession session, ConnectorTableHandle tableHandle)
+    default boolean supportsMetadataDelete(ConnectorSession session, ConnectorTableHandle tableHandle, Optional<ConnectorTableLayoutHandle> tableLayoutHandle)
     {
         throw new PrestoException(NOT_SUPPORTED, "This connector does not support deletes");
     }

diff --git a/.../java/com/facebook/presto/spi/connector/classloader/ClassLoaderSafeConnectorMetadata.java b/.../java/com/facebook/presto/spi/connector/classloader/ClassLoaderSafeConnectorMetadata.java
@@ -482,10 +482,10 @@ public void finishDelete(ConnectorSession session, ConnectorTableHandle tableHan
     }
 
     @Override
-    public boolean supportsMetadataDelete(ConnectorSession session, ConnectorTableHandle tableHandle)
+    public boolean supportsMetadataDelete(ConnectorSession session, ConnectorTableHandle tableHandle, Optional<ConnectorTableLayoutHandle> tableLayoutHandle)
     {
         try (ThreadContextClassLoader ignored = new ThreadContextClassLoader(classLoader)) {
-            return delegate.supportsMetadataDelete(session, tableHandle);
+            return delegate.supportsMetadataDelete(session, tableHandle, tableLayoutHandle);
         }
     }