Short-circuit applyFilter for catalog-wise information_schema tables

For tables that don't enumerate tables (schemata, roles, applicable_roles and enabled_roles), predicate pushdown can not be applied.
trinodb · Oct 11, 2019 · 35170fa · 35170fa
1 parent 5b08a90
commit 35170fa
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 19 deletions.
diff --git a/...ain/src/main/java/io/prestosql/connector/informationschema/InformationSchemaMetadata.java b/...ain/src/main/java/io/prestosql/connector/informationschema/InformationSchemaMetadata.java
@@ -189,7 +189,7 @@ public Optional<ConstraintApplicationResult<ConnectorTableHandle>> applyFilter(C
     {
         InformationSchemaTableHandle table = (InformationSchemaTableHandle) handle;
 
-        if (!table.getPrefixes().equals(defaultPrefixes())) {
+        if (!isTablesEnumeratingTable(table.getTable()) || !table.getPrefixes().equals(defaultPrefixes())) {
             return Optional.empty();
         }
 
@@ -217,13 +217,11 @@ private Set<QualifiedTablePrefix> getPrefixes(ConnectorSession session, Informat
         }
 
         InformationSchemaTable informationSchemaTable = table.getTable();
-        Set<QualifiedTablePrefix> prefixes = calculatePrefixesWithSchemaName(informationSchemaTable, session, constraint.getSummary(), constraint.predicate());
-        if (isTablesEnumeratingTable(informationSchemaTable)) {
-            Set<QualifiedTablePrefix> tablePrefixes = calculatePrefixesWithTableName(informationSchemaTable, session, prefixes, constraint.getSummary(), constraint.predicate());
-            // in case of high number of prefixes it is better to populate all data and then filter
-            if (tablePrefixes.size() <= MAX_PREFIXES_COUNT) {
-                prefixes = tablePrefixes;
-            }
+        Set<QualifiedTablePrefix> prefixes = calculatePrefixesWithSchemaName(session, constraint.getSummary(), constraint.predicate());
+        Set<QualifiedTablePrefix> tablePrefixes = calculatePrefixesWithTableName(informationSchemaTable, session, prefixes, constraint.getSummary(), constraint.predicate());
+        // in case of high number of prefixes it is better to populate all data and then filter
+        if (tablePrefixes.size() <= MAX_PREFIXES_COUNT) {
+            prefixes = tablePrefixes;
         }
         if (prefixes.size() > MAX_PREFIXES_COUNT) {
             // in case of high number of prefixes it is better to populate all data and then filter
@@ -239,7 +237,6 @@ private boolean isTablesEnumeratingTable(InformationSchemaTable table)
     }
 
     private Set<QualifiedTablePrefix> calculatePrefixesWithSchemaName(
-            InformationSchemaTable informationSchemaTable,
             ConnectorSession connectorSession,
             TupleDomain<ColumnHandle> constraint,
             Optional<Predicate<Map<ColumnHandle, NullableValue>>> predicate)
@@ -253,7 +250,7 @@ private Set<QualifiedTablePrefix> calculatePrefixesWithSchemaName(
                     .collect(toImmutableSet());
         }
 
-        if (!predicate.isPresent() || !isTablesEnumeratingTable(informationSchemaTable)) {
+        if (!predicate.isPresent()) {
             return ImmutableSet.of(new QualifiedTablePrefix(catalogName));
         }
 

diff --git a/presto-main/src/test/java/io/prestosql/metadata/TestInformationSchemaMetadata.java b/presto-main/src/test/java/io/prestosql/metadata/TestInformationSchemaMetadata.java
@@ -28,6 +28,7 @@
 import io.prestosql.spi.connector.Connector;
 import io.prestosql.spi.connector.ConnectorMetadata;
 import io.prestosql.spi.connector.ConnectorSession;
+import io.prestosql.spi.connector.ConnectorTableHandle;
 import io.prestosql.spi.connector.ConnectorViewDefinition;
 import io.prestosql.spi.connector.ConnectorViewDefinition.ViewColumn;
 import io.prestosql.spi.connector.Constraint;
@@ -55,6 +56,7 @@
 import static io.prestosql.transaction.InMemoryTransactionManager.createTestTransactionManager;
 import static java.util.Arrays.stream;
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
 
 public class TestInformationSchemaMetadata
 {
@@ -200,22 +202,19 @@ public void testInformationSchemaPredicatePushdownWithConstraintPredicateOnViews
     }
 
     @Test
-    public void testInformationSchemaPredicatePushdownWithConstraintPredicateOnSchemasTable()
+    public void testInformationSchemaPredicatePushdownOnCatalogWiseTables()
     {
         TransactionId transactionId = transactionManager.beginTransaction(false);
 
-        // predicate on non tables enumerating table should not cause schemas to be enumerated
-        Constraint constraint = new Constraint(TupleDomain.all(), TestInformationSchemaMetadata::testConstraint);
+        // Predicate pushdown shouldn't work for catalog-wise tables because the table prefixes for them are always
+        // ImmutableSet.of(new QualifiedTablePrefix(catalogName));
+        Constraint constraint = new Constraint(TupleDomain.all());
         ConnectorSession session = createNewSession(transactionId);
         ConnectorMetadata metadata = new InformationSchemaMetadata("test_catalog", this.metadata);
         InformationSchemaTableHandle tableHandle = (InformationSchemaTableHandle)
                 metadata.getTableHandle(session, new SchemaTableName("information_schema", "schemata"));
-        tableHandle = metadata.applyFilter(session, tableHandle, constraint)
-                .map(ConstraintApplicationResult::getHandle)
-                .map(InformationSchemaTableHandle.class::cast)
-                .orElseThrow(AssertionError::new);
-
-        assertEquals(tableHandle.getPrefixes(), ImmutableSet.of(new QualifiedTablePrefix("test_catalog")));
+        Optional<ConstraintApplicationResult<ConnectorTableHandle>> result = metadata.applyFilter(session, tableHandle, constraint);
+        assertFalse(result.isPresent());
     }
 
     private static boolean testConstraint(Map<ColumnHandle, NullableValue> bindings)