Remove JDBC connector allow-drop-table flag

lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedAccessControl.java

@@ -26,7 +26,7 @@
 import io.trino.spi.security.ViewExpression;
 import io.trino.spi.type.Type;
 
-import java.nio.file.Paths;
+import java.io.File;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -101,12 +101,12 @@ public class FileBasedAccessControl
     private final List<SessionPropertyAccessControlRule> sessionPropertyRules;
     private final Set<AnySchemaPermissionsRule> anySchemaPermissionsRules;
 
-    public FileBasedAccessControl(CatalogName catalogName, FileBasedAccessControlConfig config)
+    public FileBasedAccessControl(CatalogName catalogName, File configFile)
     {
         this.catalogName = requireNonNull(catalogName, "catalogName is null").toString();
 
-        AccessControlRules rules = parseJson(Paths.get(config.getConfigFile()), AccessControlRules.class);
-        checkArgument(!rules.hasRoleRules(), "File connector access control does not support role rules: %s", config.getConfigFile());
+        AccessControlRules rules = parseJson(configFile.toPath(), AccessControlRules.class);
+        checkArgument(!rules.hasRoleRules(), "File connector access control does not support role rules: %s", configFile);
 
         this.schemaRules = rules.getSchemaRules();
         this.tableRules = rules.getTableRules();

lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedAccessControlConfig.java

@@ -20,23 +20,25 @@
 
 import javax.validation.constraints.NotNull;
 
+import java.io.File;
+
 public class FileBasedAccessControlConfig
 {
     public static final String SECURITY_CONFIG_FILE = "security.config-file";
     public static final String SECURITY_REFRESH_PERIOD = "security.refresh-period";
 
-    private String configFile;
+    private File configFile;
     private Duration refreshPeriod;
 
     @NotNull
     @FileExists
-    public String getConfigFile()
+    public File getConfigFile()
     {
         return configFile;
     }
 
     @Config(SECURITY_CONFIG_FILE)
-    public FileBasedAccessControlConfig setConfigFile(String configFile)
+    public FileBasedAccessControlConfig setConfigFile(File configFile)
     {
         this.configFile = configFile;
         return this;

lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedAccessControlModule.java

@@ -21,6 +21,8 @@
 import io.trino.plugin.base.CatalogName;
 import io.trino.spi.connector.ConnectorAccessControl;
 
+import java.io.File;
+
 import static com.google.common.base.Suppliers.memoizeWithExpiration;
 import static io.airlift.configuration.ConfigBinder.configBinder;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
@@ -40,15 +42,16 @@ public void configure(Binder binder)
     @Provides
     public ConnectorAccessControl getConnectorAccessControl(CatalogName catalogName, FileBasedAccessControlConfig config)
     {
+        File configFile = config.getConfigFile();
         if (config.getRefreshPeriod() != null) {
             return ForwardingConnectorAccessControl.of(memoizeWithExpiration(
                     () -> {
-                        log.info("Refreshing access control for catalog '%s' from: %s", catalogName, config.getConfigFile());
-                        return new FileBasedAccessControl(catalogName, config);
+                        log.info("Refreshing access control for catalog '%s' from: %s", catalogName, configFile);
+                        return new FileBasedAccessControl(catalogName, configFile);
                     },
                     config.getRefreshPeriod().toMillis(),
                     MILLISECONDS));
         }
-        return new FileBasedAccessControl(catalogName, config);
+        return new FileBasedAccessControl(catalogName, configFile);
     }
 }

lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedSystemAccessControl.java

@@ -206,7 +206,7 @@ public SystemAccessControl create(Map<String, String> config)
                     .setRequiredConfigurationProperties(config)
                     .initialize();
             FileBasedAccessControlConfig fileBasedAccessControlConfig = injector.getInstance(FileBasedAccessControlConfig.class);
-            String configFileName = fileBasedAccessControlConfig.getConfigFile();
+            String configFileName = fileBasedAccessControlConfig.getConfigFile().getPath();
 
             if (config.containsKey(SECURITY_REFRESH_PERIOD)) {
                 Duration refreshPeriod;

lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedAccessControl.java

@@ -31,7 +31,6 @@
 import org.testng.annotations.Test;
 
 import java.io.File;
-import java.net.URISyntaxException;
 import java.util.EnumSet;
 import java.util.Optional;
 import java.util.Set;
@@ -475,17 +474,10 @@ private static ConnectorSecurityContext user(String name, Set<String> groups)
                 new QueryId("query_id"));
     }
 
-    private ConnectorAccessControl createAccessControl(String fileName)
+    private static ConnectorAccessControl createAccessControl(String fileName)
     {
-        try {
-            String path = new File(getResource(fileName).toURI()).getPath();
-            FileBasedAccessControlConfig config = new FileBasedAccessControlConfig();
-            config.setConfigFile(path);
-            return new FileBasedAccessControl(new CatalogName("test_catalog"), config);
-        }
-        catch (URISyntaxException e) {
-            throw new RuntimeException(e);
-        }
+        File configFile = new File(getResource(fileName).getPath());
+        return new FileBasedAccessControl(new CatalogName("test_catalog"), configFile);
     }
 
     private static void assertDenied(ThrowingRunnable runnable)

lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedAccessControlConfig.java

@@ -54,7 +54,7 @@ public void testExplicitPropertyMappings()
                 .buildOrThrow();
 
         FileBasedAccessControlConfig expected = new FileBasedAccessControlConfig()
-                .setConfigFile(securityConfigFile.toString())
+                .setConfigFile(securityConfigFile.toFile())
                 .setRefreshPeriod(new Duration(1, TimeUnit.SECONDS));
 
         assertFullMapping(properties, expected);

plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/DefaultJdbcMetadata.java

@@ -79,7 +79,6 @@
 import static io.trino.plugin.jdbc.JdbcMetadataSessionProperties.isJoinPushdownEnabled;
 import static io.trino.plugin.jdbc.JdbcMetadataSessionProperties.isTopNPushdownEnabled;
 import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED;
-import static io.trino.spi.StandardErrorCode.PERMISSION_DENIED;
 import static io.trino.spi.type.BigintType.BIGINT;
 import static java.lang.Math.max;
 import static java.util.Objects.requireNonNull;
@@ -90,14 +89,12 @@ public class DefaultJdbcMetadata
     private static final String SYNTHETIC_COLUMN_NAME_PREFIX = "_pfgnrtd_";
 
     private final JdbcClient jdbcClient;
-    private final boolean allowDropTable;
 
     private final AtomicReference<Runnable> rollbackAction = new AtomicReference<>();
 
-    public DefaultJdbcMetadata(JdbcClient jdbcClient, boolean allowDropTable)
+    public DefaultJdbcMetadata(JdbcClient jdbcClient)
     {
         this.jdbcClient = requireNonNull(jdbcClient, "jdbcClient is null");
-        this.allowDropTable = allowDropTable;
     }
 
     @Override
@@ -606,9 +603,6 @@ public ColumnMetadata getColumnMetadata(ConnectorSession session, ConnectorTable
     @Override
     public void dropTable(ConnectorSession session, ConnectorTableHandle tableHandle)
     {
-        if (!allowDropTable) {
-            throw new TrinoException(PERMISSION_DENIED, "DROP TABLE is disabled in this catalog");
-        }
         JdbcTableHandle handle = (JdbcTableHandle) tableHandle;
         verify(!handle.isSynthetic(), "Not a table reference: %s", handle);
         jdbcClient.dropTable(session, handle);

plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/DefaultJdbcMetadataFactory.java

@@ -26,14 +26,11 @@ public class DefaultJdbcMetadataFactory
         implements JdbcMetadataFactory
 {
     private final JdbcClient jdbcClient;
-    private final boolean allowDropTable;
 
     @Inject
-    public DefaultJdbcMetadataFactory(JdbcClient jdbcClient, JdbcMetadataConfig config)
+    public DefaultJdbcMetadataFactory(JdbcClient jdbcClient)
     {
         this.jdbcClient = requireNonNull(jdbcClient, "jdbcClient is null");
-        requireNonNull(config, "config is null");
-        this.allowDropTable = config.isAllowDropTable();
     }
 
     @Override
@@ -47,12 +44,11 @@ public JdbcMetadata create(JdbcTransactionHandle transaction)
                         new SingletonIdentityCacheMapping(),
                         new Duration(1, DAYS),
                         true,
-                        Integer.MAX_VALUE),
-                allowDropTable);
+                        Integer.MAX_VALUE));
     }
 
-    protected JdbcMetadata create(JdbcClient transactionCachingJdbcClient, boolean allowDropTable)
+    protected JdbcMetadata create(JdbcClient transactionCachingJdbcClient)
     {
-        return new DefaultJdbcMetadata(transactionCachingJdbcClient, allowDropTable);
+        return new DefaultJdbcMetadata(transactionCachingJdbcClient);
     }
 }

plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/JdbcMetadataConfig.java

@@ -15,13 +15,14 @@
 
 import io.airlift.configuration.Config;
 import io.airlift.configuration.ConfigDescription;
+import io.airlift.configuration.DefunctConfig;
 import io.airlift.configuration.LegacyConfig;
 
 import javax.validation.constraints.Min;
 
+@DefunctConfig("allow-drop-table")
 public class JdbcMetadataConfig
 {
-    private boolean allowDropTable;
     /*
      * Join pushdown is disabled by default as this is the safer option.
      * Pushing down a join which substantially increases the row count vs
@@ -40,19 +41,6 @@ public class JdbcMetadataConfig
     // between performance and pushdown capabilities
     private int domainCompactionThreshold = 32;
 
-    public boolean isAllowDropTable()
-    {
-        return allowDropTable;
-    }
-
-    @Config("allow-drop-table")
-    @ConfigDescription("Allow connector to drop tables")
-    public JdbcMetadataConfig setAllowDropTable(boolean allowDropTable)
-    {
-        this.allowDropTable = allowDropTable;
-        return this;
-    }
-
     public boolean isJoinPushdownEnabled()
     {
         return joinPushdownEnabled;

plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestDefaultJdbcMetadata.java

@@ -43,7 +43,6 @@
 import static io.trino.plugin.jdbc.TestingJdbcTypeHandle.JDBC_BIGINT;
 import static io.trino.plugin.jdbc.TestingJdbcTypeHandle.JDBC_VARCHAR;
 import static io.trino.spi.StandardErrorCode.NOT_FOUND;
-import static io.trino.spi.StandardErrorCode.PERMISSION_DENIED;
 import static io.trino.spi.type.BigintType.BIGINT;
 import static io.trino.spi.type.VarcharType.VARCHAR;
 import static io.trino.spi.type.VarcharType.createVarcharType;
@@ -68,7 +67,7 @@ public void setUp()
             throws Exception
     {
         database = new TestingDatabase();
-        metadata = new DefaultJdbcMetadata(new GroupingSetsEnabledJdbcClient(database.getJdbcClient()), false);
+        metadata = new DefaultJdbcMetadata(new GroupingSetsEnabledJdbcClient(database.getJdbcClient()));
         tableHandle = metadata.getTableHandle(SESSION, new SchemaTableName("example", "numbers"));
     }
 
@@ -224,16 +223,10 @@ public void testCreateAndAlterTable()
     @Test
     public void testDropTableTable()
     {
-        assertTrinoExceptionThrownBy(() -> metadata.dropTable(SESSION, tableHandle))
-                .hasErrorCode(PERMISSION_DENIED)
-                .hasMessage("DROP TABLE is disabled in this catalog");
-
-        metadata = new DefaultJdbcMetadata(database.getJdbcClient(), true);
         metadata.dropTable(SESSION, tableHandle);
 
         assertTrinoExceptionThrownBy(() -> metadata.getTableMetadata(SESSION, tableHandle))
-                .hasErrorCode(NOT_FOUND)
-                .hasMessageMatching("Table '.*' has no supported columns \\(all \\d+ columns are not supported\\)");
+                .hasErrorCode(NOT_FOUND);
     }
 
     @Test

plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestJdbcCachingConnectorSmokeTest.java

@@ -39,7 +39,6 @@ protected QueryRunner createQueryRunner()
                 .put("metadata.cache-ttl", "10m")
                 .put("metadata.cache-missing", "true")
                 .put("case-insensitive-name-matching", "true")
-                .put("allow-drop-table", "true")
                 .buildOrThrow();
         this.h2SqlExecutor = new JdbcSqlExecutor(properties.get("connection-url"), new Properties());
         return createH2QueryRunner(REQUIRED_TPCH_TABLES, properties);

plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestJdbcConnectorTest.java

@@ -43,7 +43,6 @@ protected QueryRunner createQueryRunner()
     {
         properties = ImmutableMap.<String, String>builder()
                 .putAll(TestingH2JdbcModule.createProperties())
-                .put("allow-drop-table", "true")
                 .buildOrThrow();
         return createH2QueryRunner(REQUIRED_TPCH_TABLES, properties);
     }

plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestJdbcMetadataConfig.java

@@ -28,7 +28,6 @@ public class TestJdbcMetadataConfig
     public void testDefaults()
     {
         assertRecordedDefaults(recordDefaults(JdbcMetadataConfig.class)
-                .setAllowDropTable(false)
                 .setJoinPushdownEnabled(false)
                 .setAggregationPushdownEnabled(true)
                 .setTopNPushdownEnabled(true)
@@ -39,15 +38,13 @@ public void testDefaults()
     public void testExplicitPropertyMappings()
     {
         Map<String, String> properties = new ImmutableMap.Builder<String, String>()
-                .put("allow-drop-table", "true")
                 .put("join-pushdown.enabled", "true")
                 .put("aggregation-pushdown.enabled", "false")
                 .put("domain-compaction-threshold", "42")
                 .put("topn-pushdown.enabled", "false")
                 .buildOrThrow();
 
         JdbcMetadataConfig expected = new JdbcMetadataConfig()
-                .setAllowDropTable(true)
                 .setJoinPushdownEnabled(true)
                 .setAggregationPushdownEnabled(false)
                 .setTopNPushdownEnabled(false)

plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/ClickHouseQueryRunner.java

@@ -61,7 +61,6 @@ public static DistributedQueryRunner createClickHouseQueryRunner(
 
             connectorProperties = new HashMap<>(ImmutableMap.copyOf(connectorProperties));
             connectorProperties.putIfAbsent("connection-url", server.getJdbcUrl());
-            connectorProperties.putIfAbsent("allow-drop-table", "true");
 
             queryRunner.installPlugin(new ClickHousePlugin());
             queryRunner.createCatalog("clickhouse", "clickhouse", connectorProperties);

plugin/trino-memsql/src/test/java/io/trino/plugin/memsql/MemSqlQueryRunner.java

@@ -51,7 +51,6 @@ public static DistributedQueryRunner createMemSqlQueryRunner(
             connectorProperties.putIfAbsent("connection-url", server.getJdbcUrl());
             connectorProperties.putIfAbsent("connection-user", server.getUsername());
             connectorProperties.putIfAbsent("connection-password", server.getPassword());
-            connectorProperties.putIfAbsent("allow-drop-table", "true");
 
             server.execute("CREATE SCHEMA tpch");
 

plugin/trino-mysql/src/test/java/io/trino/plugin/mysql/MySqlQueryRunner.java

@@ -61,7 +61,6 @@ public static DistributedQueryRunner createMySqlQueryRunner(
             connectorProperties.putIfAbsent("connection-url", server.getJdbcUrl());
             connectorProperties.putIfAbsent("connection-user", server.getUsername());
             connectorProperties.putIfAbsent("connection-password", server.getPassword());
-            connectorProperties.putIfAbsent("allow-drop-table", "true");
 
             queryRunner.installPlugin(new MySqlPlugin());
             queryRunner.createCatalog("mysql", "mysql", connectorProperties);

plugin/trino-oracle/src/test/java/io/trino/plugin/oracle/OracleQueryRunner.java

@@ -78,7 +78,6 @@ public static Map<String, String> connectionProperties(TestingOracleServer serve
                 .put("connection-url", server.getJdbcUrl())
                 .put("connection-user", TEST_USER)
                 .put("connection-password", TEST_PASS)
-                .put("allow-drop-table", "true")
                 .buildOrThrow();
     }
 
@@ -95,7 +94,6 @@ public static void main(String[] args)
                         .put("connection-url", server.getJdbcUrl())
                         .put("connection-user", TEST_USER)
                         .put("connection-password", TEST_PASS)
-                        .put("allow-drop-table", "true")
                         .put("oracle.connection-pool.enabled", "false")
                         .put("oracle.remarks-reporting.enabled", "false")
                         .buildOrThrow(),

plugin/trino-oracle/src/test/java/io/trino/plugin/oracle/TestOracleConnectorTest.java

@@ -46,7 +46,6 @@ protected QueryRunner createQueryRunner()
                         .put("connection-url", oracleServer.getJdbcUrl())
                         .put("connection-user", TEST_USER)
                         .put("connection-password", TEST_PASS)
-                        .put("allow-drop-table", "true")
                         .put("oracle.connection-pool.enabled", "false")
                         .put("oracle.remarks-reporting.enabled", "false")
                         .buildOrThrow(),

plugin/trino-oracle/src/test/java/io/trino/plugin/oracle/TestOraclePoolConnectorSmokeTest.java

@@ -40,7 +40,6 @@ protected QueryRunner createQueryRunner()
                         .put("connection-url", oracleServer.getJdbcUrl())
                         .put("connection-user", TEST_USER)
                         .put("connection-password", TEST_PASS)
-                        .put("allow-drop-table", "true")
                         .put("oracle.connection-pool.enabled", "true")
                         .put("oracle.remarks-reporting.enabled", "false")
                         .buildOrThrow(),

plugin/trino-oracle/src/test/java/io/trino/plugin/oracle/TestOraclePoolRemarksReportingConnectorSmokeTest.java

@@ -43,7 +43,6 @@ protected QueryRunner createQueryRunner()
                         .put("connection-url", oracleServer.getJdbcUrl())
                         .put("connection-user", TEST_USER)
                         .put("connection-password", TEST_PASS)
-                        .put("allow-drop-table", "true")
                         .put("oracle.connection-pool.enabled", "true")
                         .put("oracle.remarks-reporting.enabled", "true")
                         .buildOrThrow(),

plugin/trino-oracle/src/test/java/io/trino/plugin/oracle/TestOracleTypeMapping.java

@@ -39,7 +39,6 @@ protected QueryRunner createQueryRunner()
                         .put("connection-url", oracleServer.getJdbcUrl())
                         .put("connection-user", TEST_USER)
                         .put("connection-password", TEST_PASS)
-                        .put("allow-drop-table", "true")
                         .put("oracle.connection-pool.enabled", "false")
                         .put("oracle.remarks-reporting.enabled", "false")
                         .buildOrThrow(),

plugin/trino-phoenix/src/main/java/io/trino/plugin/phoenix/PhoenixClientModule.java

@@ -93,7 +93,6 @@ protected void setup(Binder binder)
 
         configBinder(binder).bindConfig(JdbcMetadataConfig.class);
         configBinder(binder).bindConfig(JdbcWriteConfig.class);
-        configBinder(binder).bindConfigDefaults(JdbcMetadataConfig.class, config -> config.setAllowDropTable(true));
 
         binder.bind(PhoenixClient.class).in(Scopes.SINGLETON);
         binder.bind(JdbcClient.class).annotatedWith(ForBaseJdbc.class).to(Key.get(PhoenixClient.class)).in(Scopes.SINGLETON);