feat(analyzer): Add session property to gate CTAS IF NOT EXISTS query analysis (#27504)

Author: kevintang2022

presto-main-base/src/main/java/com/facebook/presto/SystemSessionProperties.java

@@ -393,6 +393,7 @@ public final class SystemSessionProperties
     public static final String NATIVE_ENFORCE_JOIN_BUILD_INPUT_PARTITION = "native_enforce_join_build_input_partition";
     public static final String NATIVE_EXECUTION_SCALE_WRITER_THREADS_ENABLED = "native_execution_scale_writer_threads_enabled";
     public static final String TRY_FUNCTION_CATCHABLE_ERRORS = "try_function_catchable_errors";
+    public static final String ALWAYS_ANALYZE_CREATE_TABLE_QUERY_ENABLED = "always_analyze_create_table_query_enabled";
 
     private final List<PropertyMetadata<?>> sessionProperties;
 
@@ -2213,6 +2214,11 @@ public SystemSessionProperties(
                         TRY_FUNCTION_CATCHABLE_ERRORS,
                         "Comma-separated list of error code names that TRY function should catch (such as 'GENERIC_INTERNAL_ERROR,INVALID_ARGUMENTS')",
                         featuresConfig.getTryFunctionCatchableErrors(),
+                        false),
+                booleanProperty(
+                        ALWAYS_ANALYZE_CREATE_TABLE_QUERY_ENABLED,
+                        "When enabled, analyze inner query on CTAS IF NOT EXISTS to populate view definitions for access control checks",
+                        false,
                         false));
     }
 
@@ -3772,4 +3778,9 @@ public static String getTryFunctionCatchableErrors(Session session)
     {
         return session.getSystemProperty(TRY_FUNCTION_CATCHABLE_ERRORS, String.class);
     }
+
+    public static boolean isAlwaysAnalyzeCreateTableQueryEnabled(Session session)
+    {
+        return session.getSystemProperty(ALWAYS_ANALYZE_CREATE_TABLE_QUERY_ENABLED, Boolean.class);
+    }
 }

presto-main-base/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java

@@ -243,6 +243,7 @@
 
 import static com.facebook.presto.SystemSessionProperties.getMaxGroupingSets;
 import static com.facebook.presto.SystemSessionProperties.isAllowWindowOrderByLiterals;
+import static com.facebook.presto.SystemSessionProperties.isAlwaysAnalyzeCreateTableQueryEnabled;
 import static com.facebook.presto.SystemSessionProperties.isLegacyMaterializedViews;
 import static com.facebook.presto.SystemSessionProperties.isMaterializedViewDataConsistencyEnabled;
 import static com.facebook.presto.SystemSessionProperties.isMaterializedViewPartitionFilteringEnabled;
@@ -774,6 +775,10 @@ protected Scope visitCreateTableAsSelect(CreateTableAsSelect node, Optional<Scop
                     warningCollector.add(new PrestoWarning(
                             StandardWarningCode.SEMANTIC_WARNING,
                             format("Table '%s' already exists, skipping table creation", targetTable)));
+                    // Analyze the inner query to populate view definitions for access control checks
+                    if (isAlwaysAnalyzeCreateTableQueryEnabled(session)) {
+                        process(node.getQuery(), scope);
+                    }
                     return createAndAssignScope(node, scope, Field.newUnqualified(node.getLocation(), "rows", BIGINT));
                 }
                 throw new SemanticException(TABLE_ALREADY_EXISTS, node, "Destination table '%s' already exists", targetTable);

presto-main-base/src/test/java/com/facebook/presto/sql/analyzer/TestViewDefinitionCollector.java

@@ -13,6 +13,7 @@
  */
 package com.facebook.presto.sql.analyzer;
 
+import com.facebook.presto.Session;
 import com.facebook.presto.spi.WarningCollector;
 import com.facebook.presto.spi.analyzer.AccessControlReferences;
 import com.facebook.presto.sql.tree.Statement;
@@ -24,6 +25,10 @@
 import java.util.Optional;
 import java.util.stream.Collectors;
 
+import static com.facebook.presto.SystemSessionProperties.ALWAYS_ANALYZE_CREATE_TABLE_QUERY_ENABLED;
+import static com.facebook.presto.SystemSessionProperties.CHECK_ACCESS_CONTROL_ON_UTILIZED_COLUMNS_ONLY;
+import static com.facebook.presto.SystemSessionProperties.CHECK_ACCESS_CONTROL_WITH_SUBFIELDS;
+import static com.facebook.presto.testing.TestingSession.testSessionBuilder;
 import static com.facebook.presto.transaction.TransactionBuilder.transaction;
 import static com.facebook.presto.util.AnalyzerUtil.checkAccessPermissions;
 import static org.testng.Assert.assertEquals;
@@ -62,6 +67,26 @@ public void testCreateTableAsSelectWithViews()
         ), ImmutableMap.of());
     }
 
+    public void testCreateTableAsSelectIfNotExistsWithViews()
+    {
+        // t1 already exists, so this hits the IF NOT EXISTS no-op path.
+        // With always_analyze_create_table_query_enabled, view definitions should still be populated.
+        Session sessionWithProperty = testSessionBuilder()
+                .setCatalog(TPCH_CATALOG)
+                .setSchema("s1")
+                .setSystemProperty(CHECK_ACCESS_CONTROL_ON_UTILIZED_COLUMNS_ONLY, "true")
+                .setSystemProperty(CHECK_ACCESS_CONTROL_WITH_SUBFIELDS, "true")
+                .setSystemProperty(ALWAYS_ANALYZE_CREATE_TABLE_QUERY_ENABLED, "true")
+                .build();
+
+        @Language("SQL") String query = "CREATE TABLE IF NOT EXISTS t1 AS SELECT view_definer1.a, view_definer1.c, view_invoker2.y FROM view_definer1 left join view_invoker2 on view_invoker2.y = view_definer1.c";
+
+        assertViewDefinitions(sessionWithProperty, query, ImmutableMap.of(
+                "tpch.s1.view_invoker2", "select x, y, z from t13",
+                "tpch.s1.view_definer1", "select a,b,c from t1"
+        ), ImmutableMap.of());
+    }
+
     public void testExplainWithViews()
     {
         @Language("SQL") String query = "EXPLAIN SELECT view_definer1.a, view_definer1.c, view_invoker2.y FROM view_definer1 left join view_invoker2 on view_invoker2.y = view_definer1.c";
@@ -193,12 +218,17 @@ public void testExplainTypeValidateExplainAnalyzeWithViews()
     }
 
     private void assertViewDefinitions(@Language("SQL") String query, Map<String, String> expectedViewDefinitions, Map<String, String> expectedMaterializedViewDefinitions)
+    {
+        assertViewDefinitions(CLIENT_SESSION, query, expectedViewDefinitions, expectedMaterializedViewDefinitions);
+    }
+
+    private void assertViewDefinitions(Session clientSession, @Language("SQL") String query, Map<String, String> expectedViewDefinitions, Map<String, String> expectedMaterializedViewDefinitions)
     {
         transaction(transactionManager, accessControl)
                 .singleStatement()
                 .readUncommitted()
                 .readOnly()
-                .execute(CLIENT_SESSION, session -> {
+                .execute(clientSession, session -> {
                     Analyzer analyzer = createAnalyzer(session, metadata, WarningCollector.NOOP, Optional.of(createTestingQueryExplainer(session, accessControl, metadata)), query);
                     Statement statement = SQL_PARSER.createStatement(query);
                     Analysis analysis = analyzer.analyzeSemantic(statement, false);