Use alias in case of predicate stitching

For materialized view processing, always use alias for the predicate stitched node, otherwise the scope of the table is not exposed outside the predicate stitched query.
obinnaokechukwu · May 17, 2021 · 18f7e17 · 18f7e17
1 parent 2e180ba
commit 18f7e17
Show file tree

Hide file tree

Showing 3 changed files with 80 additions and 11 deletions.
diff --git a/presto-hive/src/test/java/com/facebook/presto/hive/TestHiveLogicalPlanner.java b/presto-hive/src/test/java/com/facebook/presto/hive/TestHiveLogicalPlanner.java
@@ -122,7 +122,9 @@
 import static com.google.common.collect.ImmutableMap.toImmutableMap;
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static io.airlift.slice.Slices.utf8Slice;
+import static io.airlift.tpch.TpchTable.CUSTOMER;
 import static io.airlift.tpch.TpchTable.LINE_ITEM;
+import static io.airlift.tpch.TpchTable.NATION;
 import static io.airlift.tpch.TpchTable.ORDERS;
 import static java.lang.String.format;
 import static java.util.Objects.requireNonNull;
@@ -137,7 +139,10 @@ public class TestHiveLogicalPlanner
     protected QueryRunner createQueryRunner()
             throws Exception
     {
-        return HiveQueryRunner.createQueryRunner(ImmutableList.of(ORDERS, LINE_ITEM), ImmutableMap.of("experimental.pushdown-subfields-enabled", "true"), Optional.empty());
+        return HiveQueryRunner.createQueryRunner(
+                ImmutableList.of(ORDERS, LINE_ITEM, CUSTOMER, NATION),
+                ImmutableMap.of("experimental.pushdown-subfields-enabled", "true"),
+                Optional.empty());
     }
 
     @Test
@@ -1219,6 +1224,46 @@ public void testMaterializedViewWithDifferentPartitions()
         }
     }
 
+    @Test(enabled = false)
+    public void testMaterializedViewJoinsWithOneTableAlias()
+    {
+        QueryRunner queryRunner = getQueryRunner();
+        String view = "view_join_with_one_alias";
+        String table1 = "nation_partitioned_join_with_one_alias";
+        String table2 = "customer_partitioned_join_with_one_alias";
+        try {
+            queryRunner.execute(format("CREATE TABLE %s WITH (partitioned_by = ARRAY['nationkey', 'regionkey']) AS " +
+                    "SELECT name, nationkey, regionkey FROM nation", table1));
+
+            queryRunner.execute(format("CREATE TABLE %s WITH (partitioned_by = ARRAY['nationkey']) AS SELECT custkey," +
+                    " name, mktsegment, nationkey FROM customer", table2));
+
+            assertUpdate(format("CREATE MATERIALIZED VIEW %s WITH (partitioned_by = ARRAY['marketsegment', " +
+                            "'nationkey', 'regionkey']) AS SELECT %s.name AS nationname, " +
+                            "customer.custkey, customer.name AS customername, UPPER(customer.mktsegment) AS marketsegment, customer.nationkey, regionkey " +
+                            "FROM %s JOIN %s customer ON (%s.nationkey = customer.nationkey)",
+                    view, table1, table1, table2, table1));
+
+            assertUpdate(format("INSERT INTO %s(nationname, custkey, customername, marketsegment, nationkey, regionkey) " +
+                            "SELECT %s.name AS nationname, customer.custkey, customer.name AS customername, UPPER(customer.mktsegment) " +
+                            "AS marketsegment, customer.nationkey, regionkey FROM %s JOIN %s customer ON (%s.nationkey = customer.nationkey) " +
+                            "WHERE customer.nationkey != 24 and %s.regionkey != 1",
+                    view, table1, table1, table2, table1, table1), 1200);
+
+            String viewQuery = format("SELECT nationname, custkey from %s", view);
+            String baseQuery = format("SELECT %s.name AS nationname, customer.custkey FROM %s JOIN %s customer ON (%s.nationkey = customer.nationkey)",
+                    table1, table1, table2, table1);
+
+            // getExplainPlan(viewQuery, LOGICAL);
+            assertEquals(computeActual(viewQuery).getRowCount(), computeActual(baseQuery).getRowCount());
+        }
+        finally {
+            queryRunner.execute("DROP TABLE IF EXISTS " + view);
+            queryRunner.execute("DROP TABLE IF EXISTS " + table1);
+            queryRunner.execute("DROP TABLE IF EXISTS " + table2);
+        }
+    }
+
     @Test(enabled = false)
     public void testMaterializedViewOptimizationWithDerivedFields()
     {

diff --git a/presto-main/src/main/java/com/facebook/presto/sql/analyzer/PredicateStitcher.java b/presto-main/src/main/java/com/facebook/presto/sql/analyzer/PredicateStitcher.java
@@ -32,12 +32,13 @@
 
 import static com.facebook.presto.metadata.MetadataUtil.createQualifiedObjectName;
 import static com.facebook.presto.metadata.MetadataUtil.toSchemaTableName;
+import static com.facebook.presto.sql.QueryUtil.identifier;
 import static com.facebook.presto.sql.QueryUtil.selectList;
 import static com.facebook.presto.sql.QueryUtil.subquery;
 import static java.util.Objects.requireNonNull;
 
 public class PredicateStitcher
-        extends AstVisitor<Node, Void>
+        extends AstVisitor<Node, PredicateStitcher.PredicateStitcherContext>
 {
     private final Map<SchemaTableName, Expression> predicates;
     private final Session session;
@@ -49,13 +50,13 @@ public PredicateStitcher(Session session, Map<SchemaTableName, Expression> predi
     }
 
     @Override
-    public Node process(Node node, Void context)
+    public Node process(Node node, PredicateStitcherContext context)
     {
         return super.process(node, context);
     }
 
     @Override
-    protected Node visitQuery(Query node, Void context)
+    protected Node visitQuery(Query node, PredicateStitcherContext context)
     {
         return new Query(
                 node.getWith(),
@@ -65,7 +66,7 @@ protected Node visitQuery(Query node, Void context)
     }
 
     @Override
-    protected Node visitQuerySpecification(QuerySpecification node, Void context)
+    protected Node visitQuerySpecification(QuerySpecification node, PredicateStitcherContext context)
     {
         if (node.getFrom().isPresent()) {
             return new QuerySpecification(
@@ -81,21 +82,24 @@ protected Node visitQuerySpecification(QuerySpecification node, Void context)
     }
 
     @Override
-    protected Node visitJoin(Join node, Void context)
+    protected Node visitJoin(Join node, PredicateStitcherContext context)
     {
         Relation rewrittenLeft = (Relation) process(node.getLeft(), context);
         Relation rewrittenRight = (Relation) process(node.getRight(), context);
         return new Join(node.getType(), rewrittenLeft, rewrittenRight, node.getCriteria());
     }
 
     @Override
-    protected Node visitAliasedRelation(AliasedRelation node, Void context)
+    protected Node visitAliasedRelation(AliasedRelation node, PredicateStitcherContext context)
     {
-        return new AliasedRelation((Relation) process(node.getRelation(), context), node.getAlias(), node.getColumnNames());
+        context.setCreateAlias(false);
+        AliasedRelation aliasedRelation = new AliasedRelation((Relation) process(node.getRelation(), context), node.getAlias(), node.getColumnNames());
+        context.setCreateAlias(true);
+        return aliasedRelation;
     }
 
     @Override
-    protected Node visitTable(Table table, Void context)
+    protected Node visitTable(Table table, PredicateStitcherContext context)
     {
         SchemaTableName schemaTableName = toSchemaTableName(createQualifiedObjectName(session, table, table.getName()));
         if (!predicates.containsKey(schemaTableName)) {
@@ -111,6 +115,25 @@ protected Node visitTable(Table table, Void context)
                 Optional.empty(),
                 Optional.empty());
 
-        return subquery(new Query(Optional.empty(), queryWithPredicateStitching, Optional.empty(), Optional.empty()));
+        Relation subquery = subquery(new Query(Optional.empty(), queryWithPredicateStitching, Optional.empty(), Optional.empty()));
+        if (context.isCreateAlias()) {
+            return new AliasedRelation(subquery, identifier(schemaTableName.getTableName()), null);
+        }
+        return subquery;
+    }
+
+    protected static final class PredicateStitcherContext
+    {
+        private boolean createAlias = true;
+
+        public boolean isCreateAlias()
+        {
+            return createAlias;
+        }
+
+        public void setCreateAlias(boolean createAlias)
+        {
+            this.createAlias = createAlias;
+        }
     }
 }
diff --git a/presto-main/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java b/presto-main/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java
@@ -202,6 +202,7 @@
 import static com.facebook.presto.sql.analyzer.ExpressionTreeUtils.extractExpressions;
 import static com.facebook.presto.sql.analyzer.ExpressionTreeUtils.extractWindowFunctions;
 import static com.facebook.presto.sql.analyzer.MaterializedViewPlanValidator.MaterializedViewPlanValidatorContext;
+import static com.facebook.presto.sql.analyzer.PredicateStitcher.PredicateStitcherContext;
 import static com.facebook.presto.sql.analyzer.ScopeReferenceExtractor.hasReferencesToScope;
 import static com.facebook.presto.sql.analyzer.SemanticErrorCode.AMBIGUOUS_ATTRIBUTE;
 import static com.facebook.presto.sql.analyzer.SemanticErrorCode.COLUMN_NAME_NOT_SPECIFIED;
@@ -1209,7 +1210,7 @@ private String getMaterializedViewSQL(
             // TODO: consider materialized view predicates https://github.com/prestodb/presto/issues/16034
             Map<SchemaTableName, Expression> partitionPredicates = generatePartitionPredicate(materializedViewStatus.getPartitionsFromBaseTables());
 
-            Node predicateStitchedNode = new PredicateStitcher(session, partitionPredicates).process(createSqlQuerySpecification);
+            Node predicateStitchedNode = new PredicateStitcher(session, partitionPredicates).process(createSqlQuerySpecification, new PredicateStitcherContext());
             checkState(predicateStitchedNode instanceof QuerySpecification);
             QuerySpecification createSqlSpecificationWithPredicates = (QuerySpecification) predicateStitchedNode;