From a0cf226c5eb92a739ae1d5e6a213ecda4744eee9 Mon Sep 17 00:00:00 2001 From: Masha Basmanova Date: Thu, 14 Nov 2019 17:53:51 -0500 Subject: [PATCH] Fix filter pushdown for DELETE queries --- .../facebook/presto/hive/HiveMetadata.java | 37 ++++++++++++++++++- .../hive/TestHiveIntegrationSmokeTest.java | 9 +++++ .../TestHivePushdownIntegrationSmokeTest.java | 6 --- .../facebook/presto/kudu/KuduMetadata.java | 2 +- .../presto/metadata/MetadataManager.java | 3 +- .../presto/raptor/RaptorMetadata.java | 2 +- .../spi/connector/ConnectorMetadata.java | 2 +- .../ClassLoaderSafeConnectorMetadata.java | 4 +- 8 files changed, 51 insertions(+), 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 index 27b05762f87e..99d4be5cbc66 100644 --- 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> convertToPredicate(TupleDomai } @Override - public boolean supportsMetadataDelete(ConnectorSession session, ConnectorTableHandle tableHandle) + public boolean supportsMetadataDelete(ConnectorSession session, ConnectorTableHandle tableHandle, Optional 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 domainPredicate = layoutHandle.getDomainPredicate(); + if (domainPredicate.isAll()) { + return true; + } + + Set predicateColumnNames = domainPredicate.getDomains().get().keySet().stream() + .map(Subfield::getRootName) + .collect(toImmutableSet()); + + Set 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 index b0f7e0c40e54..9a1abe8eb251 100644 --- 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 index b1e169c1735d..f08eaf45b5d0 100644 --- 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 index e00155c18688..5aa63dc0b506 100755 --- 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 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 index 9b686dd76235..13ec06d06315 100644 --- 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 index 596ee4f2967a..5fc00339c72b 100644 --- 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 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 index 449461acea96..ed17045fe4ad 100644 --- 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 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 tableLayoutHandle) { throw new PrestoException(NOT_SUPPORTED, "This connector does not support deletes"); } diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/connector/classloader/ClassLoaderSafeConnectorMetadata.java b/presto-spi/src/main/java/com/facebook/presto/spi/connector/classloader/ClassLoaderSafeConnectorMetadata.java index 9aaa01342701..b7c628710f91 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/connector/classloader/ClassLoaderSafeConnectorMetadata.java +++ b/presto-spi/src/main/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 tableLayoutHandle) { try (ThreadContextClassLoader ignored = new ThreadContextClassLoader(classLoader)) { - return delegate.supportsMetadataDelete(session, tableHandle); + return delegate.supportsMetadataDelete(session, tableHandle, tableLayoutHandle); } }