From 30234cb0194d0a5156accf6ddfeeb8845f9b15d2 Mon Sep 17 00:00:00 2001 From: Tim Meehan Date: Wed, 31 Oct 2018 14:13:01 -0700 Subject: [PATCH] Add DDL support for JDBC connectors - create table, add column, and drop column for everything - rename column and rename table for MySQL and PostgreSQL Extracted-From: https://github.com/prestodb/presto --- .../prestosql/plugin/jdbc/BaseJdbcClient.java | 127 ++++++++++++++++-- .../io/prestosql/plugin/jdbc/JdbcClient.java | 11 ++ .../prestosql/plugin/jdbc/JdbcMetadata.java | 36 +++++ .../plugin/jdbc/TestJdbcMetadata.java | 39 ++++-- .../prestosql/plugin/mysql/MySqlClient.java | 52 +++++++ .../mysql/TestMySqlDistributedQueries.java | 23 +++- .../plugin/postgresql/PostgreSqlClient.java | 35 ++++- .../TestPostgreSqlDistributedQueries.java | 23 +++- .../plugin/redshift/RedshiftClient.java | 16 ++- .../plugin/sqlserver/SqlServerClient.java | 33 ++++- 10 files changed, 350 insertions(+), 45 deletions(-) diff --git a/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/BaseJdbcClient.java b/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/BaseJdbcClient.java index 4db5a7244e0de..1ae03a6976ef4 100644 --- a/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/BaseJdbcClient.java +++ b/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/BaseJdbcClient.java @@ -274,6 +274,17 @@ public PreparedStatement buildSql(ConnectorSession session, Connection connectio split.getAdditionalPredicate()); } + @Override + public void createTable(ConnectorSession session, ConnectorTableMetadata tableMetadata) + { + try { + createTable(session, tableMetadata, tableMetadata.getTable().getTableName()); + } + catch (SQLException e) { + throw new PrestoException(JDBC_ERROR, e); + } + } + @Override public JdbcOutputTableHandle beginCreateTable(ConnectorSession session, ConnectorTableMetadata tableMetadata) { @@ -287,6 +298,17 @@ public JdbcOutputTableHandle beginInsertTable(ConnectorSession session, Connecto } private JdbcOutputTableHandle beginWriteTable(ConnectorSession session, ConnectorTableMetadata tableMetadata) + { + try { + return createTable(session, tableMetadata, generateTemporaryTableName()); + } + catch (SQLException e) { + throw new PrestoException(JDBC_ERROR, e); + } + } + + protected JdbcOutputTableHandle createTable(ConnectorSession session, ConnectorTableMetadata tableMetadata, String tableName) + throws SQLException { SchemaTableName schemaTableName = tableMetadata.getTable(); String schema = schemaTableName.getSchemaName(); @@ -302,11 +324,10 @@ private JdbcOutputTableHandle beginWriteTable(ConnectorSession session, Connecto if (uppercase) { schema = schema.toUpperCase(ENGLISH); table = table.toUpperCase(ENGLISH); + tableName = tableName.toUpperCase(ENGLISH); } String catalog = connection.getCatalog(); - String temporaryName = generateTemporaryTableName(); - ImmutableList.Builder columnNames = ImmutableList.builder(); ImmutableList.Builder columnTypes = ImmutableList.builder(); ImmutableList.Builder columnList = ImmutableList.builder(); @@ -318,12 +339,12 @@ private JdbcOutputTableHandle beginWriteTable(ConnectorSession session, Connecto columnNames.add(columnName); columnTypes.add(column.getType()); // TODO in INSERT case, we should reuse original column type and, ideally, constraints (then JdbcPageSink must get writer from toPrestoType()) - columnList.add(format("%s %s", quoted(columnName), toWriteMapping(session, column.getType()).getDataType())); + columnList.add(getColumnSql(session, column, columnName)); } String sql = format( "CREATE TABLE %s (%s)", - quoted(catalog, schema, temporaryName), + quoted(catalog, schema, tableName), join(", ", columnList.build())); execute(connection, sql); @@ -333,13 +354,19 @@ private JdbcOutputTableHandle beginWriteTable(ConnectorSession session, Connecto table, columnNames.build(), columnTypes.build(), - temporaryName); - } - catch (SQLException e) { - throw new PrestoException(JDBC_ERROR, e); + tableName); } } + private String getColumnSql(ConnectorSession session, ColumnMetadata column, String columnName) + { + StringBuilder sb = new StringBuilder() + .append(quoted(columnName)) + .append(" ") + .append(toWriteMapping(session, column.getType()).getDataType()); + return sb.toString(); + } + protected String generateTemporaryTableName() { return "tmp_presto_" + UUID.randomUUID().toString().replace("-", ""); @@ -348,12 +375,33 @@ protected String generateTemporaryTableName() @Override public void commitCreateTable(JdbcIdentity identity, JdbcOutputTableHandle handle) { - String sql = format( - "ALTER TABLE %s RENAME TO %s", - quoted(handle.getCatalogName(), handle.getSchemaName(), handle.getTemporaryTableName()), - quoted(handle.getCatalogName(), handle.getSchemaName(), handle.getTableName())); + renameTable( + identity, + handle.getCatalogName(), + handle.getSchemaName(), + handle.getTemporaryTableName(), + new SchemaTableName(handle.getSchemaName(), handle.getTableName())); + } - try (Connection connection = getConnection(identity, handle)) { + @Override + public void renameTable(JdbcIdentity identity, JdbcTableHandle handle, SchemaTableName newTableName) + { + renameTable(identity, handle.getCatalogName(), handle.getSchemaName(), handle.getTableName(), newTableName); + } + + protected void renameTable(JdbcIdentity identity, String catalogName, String schemaName, String tableName, SchemaTableName newTable) + { + try (Connection connection = connectionFactory.openConnection(identity)) { + String newSchemaName = newTable.getSchemaName(); + String newTableName = newTable.getTableName(); + if (connection.getMetaData().storesUpperCaseIdentifiers()) { + newSchemaName = newSchemaName.toUpperCase(ENGLISH); + newTableName = newTableName.toUpperCase(ENGLISH); + } + String sql = format( + "ALTER TABLE %s RENAME TO %s", + quoted(catalogName, schemaName, tableName), + quoted(catalogName, newSchemaName, newTableName)); execute(connection, sql); } catch (SQLException e) { @@ -384,6 +432,59 @@ public void finishInsertTable(JdbcIdentity identity, JdbcOutputTableHandle handl } } + @Override + public void addColumn(ConnectorSession session, JdbcTableHandle handle, ColumnMetadata column) + { + try (Connection connection = connectionFactory.openConnection(JdbcIdentity.from(session))) { + String columnName = column.getName(); + if (connection.getMetaData().storesUpperCaseIdentifiers()) { + columnName = columnName.toUpperCase(ENGLISH); + } + String sql = format( + "ALTER TABLE %s ADD %s", + quoted(handle.getCatalogName(), handle.getSchemaName(), handle.getTableName()), + getColumnSql(session, column, columnName)); + execute(connection, sql); + } + catch (SQLException e) { + throw new PrestoException(JDBC_ERROR, e); + } + } + + @Override + public void renameColumn(JdbcIdentity identity, JdbcTableHandle handle, JdbcColumnHandle jdbcColumn, String newColumnName) + { + try (Connection connection = connectionFactory.openConnection(identity)) { + if (connection.getMetaData().storesUpperCaseIdentifiers()) { + newColumnName = newColumnName.toUpperCase(ENGLISH); + } + String sql = format( + "ALTER TABLE %s RENAME COLUMN %s TO %s", + quoted(handle.getCatalogName(), handle.getSchemaName(), handle.getTableName()), + jdbcColumn.getColumnName(), + newColumnName); + execute(connection, sql); + } + catch (SQLException e) { + throw new PrestoException(JDBC_ERROR, e); + } + } + + @Override + public void dropColumn(JdbcIdentity identity, JdbcTableHandle handle, JdbcColumnHandle column) + { + try (Connection connection = connectionFactory.openConnection(identity)) { + String sql = format( + "ALTER TABLE %s DROP COLUMN %s", + quoted(handle.getCatalogName(), handle.getSchemaName(), handle.getTableName()), + column.getColumnName()); + execute(connection, sql); + } + catch (SQLException e) { + throw new PrestoException(JDBC_ERROR, e); + } + } + @Override public void dropTable(JdbcIdentity identity, JdbcTableHandle handle) { diff --git a/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/JdbcClient.java b/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/JdbcClient.java index 5f605fa299de6..a9f7fc77e0aef 100644 --- a/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/JdbcClient.java +++ b/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/JdbcClient.java @@ -14,6 +14,7 @@ package io.prestosql.plugin.jdbc; import io.prestosql.spi.connector.ColumnHandle; +import io.prestosql.spi.connector.ColumnMetadata; import io.prestosql.spi.connector.ConnectorSession; import io.prestosql.spi.connector.ConnectorSplitSource; import io.prestosql.spi.connector.ConnectorTableMetadata; @@ -62,6 +63,16 @@ default void abortReadConnection(Connection connection) PreparedStatement buildSql(ConnectorSession session, Connection connection, JdbcSplit split, List columnHandles) throws SQLException; + void addColumn(ConnectorSession session, JdbcTableHandle handle, ColumnMetadata column); + + void dropColumn(JdbcIdentity identity, JdbcTableHandle handle, JdbcColumnHandle column); + + void renameColumn(JdbcIdentity identity, JdbcTableHandle handle, JdbcColumnHandle jdbcColumn, String newColumnName); + + void renameTable(JdbcIdentity identity, JdbcTableHandle handle, SchemaTableName newTableName); + + void createTable(ConnectorSession session, ConnectorTableMetadata tableMetadata); + JdbcOutputTableHandle beginCreateTable(ConnectorSession session, ConnectorTableMetadata tableMetadata); void commitCreateTable(JdbcIdentity identity, JdbcOutputTableHandle handle); diff --git a/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/JdbcMetadata.java b/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/JdbcMetadata.java index c06dda4750a7d..a6a545cd97eea 100644 --- a/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/JdbcMetadata.java +++ b/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/JdbcMetadata.java @@ -168,6 +168,12 @@ public ConnectorOutputTableHandle beginCreateTable(ConnectorSession session, Con return handle; } + @Override + public void createTable(ConnectorSession session, ConnectorTableMetadata tableMetadata, boolean ignoreExisting) + { + jdbcClient.createTable(session, tableMetadata); + } + @Override public Optional finishCreateTable(ConnectorSession session, ConnectorOutputTableHandle tableHandle, Collection fragments, Collection computedStatistics) { @@ -208,6 +214,36 @@ public Optional finishInsert(ConnectorSession session, return Optional.empty(); } + @Override + public void addColumn(ConnectorSession session, ConnectorTableHandle table, ColumnMetadata columnMetadata) + { + JdbcTableHandle tableHandle = (JdbcTableHandle) table; + jdbcClient.addColumn(session, tableHandle, columnMetadata); + } + + @Override + public void dropColumn(ConnectorSession session, ConnectorTableHandle table, ColumnHandle column) + { + JdbcTableHandle tableHandle = (JdbcTableHandle) table; + JdbcColumnHandle columnHandle = (JdbcColumnHandle) column; + jdbcClient.dropColumn(JdbcIdentity.from(session), tableHandle, columnHandle); + } + + @Override + public void renameColumn(ConnectorSession session, ConnectorTableHandle table, ColumnHandle column, String target) + { + JdbcTableHandle tableHandle = (JdbcTableHandle) table; + JdbcColumnHandle columnHandle = (JdbcColumnHandle) column; + jdbcClient.renameColumn(JdbcIdentity.from(session), tableHandle, columnHandle, target); + } + + @Override + public void renameTable(ConnectorSession session, ConnectorTableHandle table, SchemaTableName newTableName) + { + JdbcTableHandle tableHandle = (JdbcTableHandle) table; + jdbcClient.renameTable(JdbcIdentity.from(session), tableHandle, newTableName); + } + @Override public TableStatistics getTableStatistics(ConnectorSession session, ConnectorTableHandle tableHandle, Constraint constraint) { diff --git a/presto-base-jdbc/src/test/java/io/prestosql/plugin/jdbc/TestJdbcMetadata.java b/presto-base-jdbc/src/test/java/io/prestosql/plugin/jdbc/TestJdbcMetadata.java index 4693e129219a3..19d52c922e54f 100644 --- a/presto-base-jdbc/src/test/java/io/prestosql/plugin/jdbc/TestJdbcMetadata.java +++ b/presto-base-jdbc/src/test/java/io/prestosql/plugin/jdbc/TestJdbcMetadata.java @@ -175,15 +175,38 @@ public void getColumnMetadata() new ColumnMetadata("text", VARCHAR)); } - @Test(expectedExceptions = PrestoException.class) - public void testCreateTable() + @Test + public void testCreateAndAlterTable() { - metadata.createTable( - SESSION, - new ConnectorTableMetadata( - new SchemaTableName("example", "foo"), - ImmutableList.of(new ColumnMetadata("text", VARCHAR))), - false); + SchemaTableName table = new SchemaTableName("example", "foo"); + metadata.createTable(SESSION, new ConnectorTableMetadata(table, ImmutableList.of(new ColumnMetadata("text", VARCHAR))), false); + + JdbcTableHandle handle = metadata.getTableHandle(SESSION, table); + + ConnectorTableMetadata layout = metadata.getTableMetadata(SESSION, handle); + assertEquals(layout.getTable(), table); + assertEquals(layout.getColumns().size(), 1); + assertEquals(layout.getColumns().get(0), new ColumnMetadata("text", VARCHAR)); + + metadata.addColumn(SESSION, handle, new ColumnMetadata("x", VARCHAR)); + layout = metadata.getTableMetadata(SESSION, handle); + assertEquals(layout.getColumns().size(), 2); + assertEquals(layout.getColumns().get(0), new ColumnMetadata("text", VARCHAR)); + assertEquals(layout.getColumns().get(1), new ColumnMetadata("x", VARCHAR)); + + JdbcColumnHandle columnHandle = new JdbcColumnHandle("x", JDBC_VARCHAR, VARCHAR); + metadata.dropColumn(SESSION, handle, columnHandle); + layout = metadata.getTableMetadata(SESSION, handle); + assertEquals(layout.getColumns().size(), 1); + assertEquals(layout.getColumns().get(0), new ColumnMetadata("text", VARCHAR)); + + SchemaTableName newTableName = new SchemaTableName("example", "bar"); + metadata.renameTable(SESSION, handle, newTableName); + handle = metadata.getTableHandle(SESSION, newTableName); + layout = metadata.getTableMetadata(SESSION, handle); + assertEquals(layout.getTable(), newTableName); + assertEquals(layout.getColumns().size(), 1); + assertEquals(layout.getColumns().get(0), new ColumnMetadata("text", VARCHAR)); } @Test diff --git a/presto-mysql/src/main/java/io/prestosql/plugin/mysql/MySqlClient.java b/presto-mysql/src/main/java/io/prestosql/plugin/mysql/MySqlClient.java index 057b1277a9dff..c535a388fd784 100644 --- a/presto-mysql/src/main/java/io/prestosql/plugin/mysql/MySqlClient.java +++ b/presto-mysql/src/main/java/io/prestosql/plugin/mysql/MySqlClient.java @@ -20,10 +20,13 @@ import io.prestosql.plugin.jdbc.BaseJdbcConfig; import io.prestosql.plugin.jdbc.ConnectionFactory; import io.prestosql.plugin.jdbc.DriverConnectionFactory; +import io.prestosql.plugin.jdbc.JdbcColumnHandle; import io.prestosql.plugin.jdbc.JdbcIdentity; +import io.prestosql.plugin.jdbc.JdbcTableHandle; import io.prestosql.plugin.jdbc.WriteMapping; import io.prestosql.spi.PrestoException; import io.prestosql.spi.connector.ConnectorSession; +import io.prestosql.spi.connector.ConnectorTableMetadata; import io.prestosql.spi.connector.SchemaTableName; import io.prestosql.spi.type.Type; import io.prestosql.spi.type.VarcharType; @@ -40,11 +43,15 @@ import java.util.Set; import static com.google.common.util.concurrent.MoreExecutors.directExecutor; +import static com.mysql.jdbc.SQLError.SQL_STATE_ER_TABLE_EXISTS_ERROR; +import static com.mysql.jdbc.SQLError.SQL_STATE_SYNTAX_ERROR; import static io.prestosql.plugin.jdbc.DriverConnectionFactory.basicConnectionProperties; +import static io.prestosql.plugin.jdbc.JdbcErrorCode.JDBC_ERROR; import static io.prestosql.plugin.jdbc.StandardColumnMappings.realWriteFunction; import static io.prestosql.plugin.jdbc.StandardColumnMappings.timestampWriteFunction; import static io.prestosql.plugin.jdbc.StandardColumnMappings.varbinaryWriteFunction; import static io.prestosql.plugin.jdbc.StandardColumnMappings.varcharWriteFunction; +import static io.prestosql.spi.StandardErrorCode.ALREADY_EXISTS; import static io.prestosql.spi.StandardErrorCode.NOT_SUPPORTED; import static io.prestosql.spi.type.RealType.REAL; import static io.prestosql.spi.type.TimeWithTimeZoneType.TIME_WITH_TIME_ZONE; @@ -52,6 +59,7 @@ import static io.prestosql.spi.type.TimestampWithTimeZoneType.TIMESTAMP_WITH_TIME_ZONE; import static io.prestosql.spi.type.VarbinaryType.VARBINARY; import static io.prestosql.spi.type.Varchars.isVarcharType; +import static java.lang.String.format; import static java.util.Locale.ENGLISH; public class MySqlClient @@ -187,4 +195,48 @@ else if (varcharType.getBoundedLength() <= 16777215) { return super.toWriteMapping(session, type); } + + @Override + public void createTable(ConnectorSession session, ConnectorTableMetadata tableMetadata) + { + try { + createTable(session, tableMetadata, tableMetadata.getTable().getTableName()); + } + catch (SQLException e) { + boolean exists = SQL_STATE_ER_TABLE_EXISTS_ERROR.equals(e.getSQLState()); + throw new PrestoException(exists ? ALREADY_EXISTS : JDBC_ERROR, e); + } + } + + @Override + public void renameColumn(JdbcIdentity identity, JdbcTableHandle handle, JdbcColumnHandle jdbcColumn, String newColumnName) + { + try (Connection connection = connectionFactory.openConnection(identity)) { + DatabaseMetaData metadata = connection.getMetaData(); + if (metadata.storesUpperCaseIdentifiers()) { + newColumnName = newColumnName.toUpperCase(ENGLISH); + } + String sql = format( + "ALTER TABLE %s RENAME COLUMN %s TO %s", + quoted(handle.getCatalogName(), handle.getSchemaName(), handle.getTableName()), + quoted(jdbcColumn.getColumnName()), + quoted(newColumnName)); + execute(connection, sql); + } + catch (SQLException e) { + // MySQL versions earlier than 8 do not support the above RENAME COLUMN syntax + if (SQL_STATE_SYNTAX_ERROR.equals(e.getSQLState())) { + throw new PrestoException(NOT_SUPPORTED, format("Rename column not supported in catalog: '%s'", handle.getCatalogName()), e); + } + throw new PrestoException(JDBC_ERROR, e); + } + } + + @Override + public void renameTable(JdbcIdentity identity, JdbcTableHandle handle, SchemaTableName newTableName) + { + // MySQL doesn't support specifying the catalog name in a rename. By setting the + // catalogName parameter to null, it will be omitted in the ALTER TABLE statement. + renameTable(identity, null, handle.getSchemaName(), handle.getTableName(), newTableName); + } } diff --git a/presto-mysql/src/test/java/io/prestosql/plugin/mysql/TestMySqlDistributedQueries.java b/presto-mysql/src/test/java/io/prestosql/plugin/mysql/TestMySqlDistributedQueries.java index 571a6b20399cc..ba389834960d3 100644 --- a/presto-mysql/src/test/java/io/prestosql/plugin/mysql/TestMySqlDistributedQueries.java +++ b/presto-mysql/src/test/java/io/prestosql/plugin/mysql/TestMySqlDistributedQueries.java @@ -16,7 +16,7 @@ import io.airlift.testing.mysql.TestingMySqlServer; import io.airlift.tpch.TpchTable; import io.prestosql.testing.MaterializedResult; -import io.prestosql.tests.AbstractTestQueries; +import io.prestosql.tests.AbstractTestDistributedQueries; import org.testng.annotations.AfterClass; import org.testng.annotations.Test; @@ -27,7 +27,7 @@ @Test public class TestMySqlDistributedQueries - extends AbstractTestQueries + extends AbstractTestDistributedQueries { private final TestingMySqlServer mysqlServer; @@ -49,6 +49,12 @@ public final void destroy() mysqlServer.close(); } + @Override + protected boolean supportsViews() + { + return false; + } + @Override public void testShowColumns() { @@ -81,5 +87,18 @@ public void testDescribeOutputNamedAndUnnamed() // this connector uses a non-canonical type for varchar columns in tpch } + @Override + public void testInsert() + { + // Test not supported due to lack of support for array types. + // See TestMySqlIntegrationSmokeTest for insertion tests. + } + + @Override + public void testDelete() + { + // delete is not supported + } + // MySQL specific tests should normally go in TestMySqlIntegrationSmokeTest } diff --git a/presto-postgresql/src/main/java/io/prestosql/plugin/postgresql/PostgreSqlClient.java b/presto-postgresql/src/main/java/io/prestosql/plugin/postgresql/PostgreSqlClient.java index 02f664b2818b7..d54fae9e3bd21 100644 --- a/presto-postgresql/src/main/java/io/prestosql/plugin/postgresql/PostgreSqlClient.java +++ b/presto-postgresql/src/main/java/io/prestosql/plugin/postgresql/PostgreSqlClient.java @@ -25,12 +25,13 @@ import io.prestosql.plugin.jdbc.ColumnMapping; import io.prestosql.plugin.jdbc.DriverConnectionFactory; import io.prestosql.plugin.jdbc.JdbcIdentity; -import io.prestosql.plugin.jdbc.JdbcOutputTableHandle; import io.prestosql.plugin.jdbc.JdbcTypeHandle; import io.prestosql.plugin.jdbc.SliceWriteFunction; import io.prestosql.plugin.jdbc.WriteMapping; import io.prestosql.spi.PrestoException; import io.prestosql.spi.connector.ConnectorSession; +import io.prestosql.spi.connector.ConnectorTableMetadata; +import io.prestosql.spi.connector.SchemaTableName; import io.prestosql.spi.type.StandardTypes; import io.prestosql.spi.type.Type; import io.prestosql.spi.type.TypeManager; @@ -54,8 +55,11 @@ import static com.fasterxml.jackson.databind.SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS; import static io.airlift.slice.Slices.utf8Slice; import static io.prestosql.plugin.jdbc.ColumnMapping.DISABLE_PUSHDOWN; +import static io.prestosql.plugin.jdbc.JdbcErrorCode.JDBC_ERROR; import static io.prestosql.plugin.jdbc.StandardColumnMappings.varbinaryWriteFunction; +import static io.prestosql.spi.StandardErrorCode.ALREADY_EXISTS; import static io.prestosql.spi.StandardErrorCode.INVALID_FUNCTION_ARGUMENT; +import static io.prestosql.spi.StandardErrorCode.NOT_SUPPORTED; import static io.prestosql.spi.type.VarbinaryType.VARBINARY; import static java.lang.String.format; import static java.nio.charset.StandardCharsets.UTF_8; @@ -63,6 +67,8 @@ public class PostgreSqlClient extends BaseJdbcClient { + private static final String DUPLICATE_TABLE_SQLSTATE = "42P07"; + protected final Type jsonType; @Inject @@ -73,19 +79,34 @@ public PostgreSqlClient(BaseJdbcConfig config, TypeManager typeManager) } @Override - public void commitCreateTable(JdbcIdentity identity, JdbcOutputTableHandle handle) + public void createTable(ConnectorSession session, ConnectorTableMetadata tableMetadata) + { + try { + createTable(session, tableMetadata, tableMetadata.getTable().getTableName()); + } + catch (SQLException e) { + boolean exists = DUPLICATE_TABLE_SQLSTATE.equals(e.getSQLState()); + throw new PrestoException(exists ? ALREADY_EXISTS : JDBC_ERROR, e); + } + } + + @Override + protected void renameTable(JdbcIdentity identity, String catalogName, String schemaName, String tableName, SchemaTableName newTable) { - // PostgreSQL does not allow qualifying the target of a rename + if (!schemaName.equals(newTable.getSchemaName())) { + throw new PrestoException(NOT_SUPPORTED, "Table rename across schemas is not supported in PostgreSQL"); + } + String sql = format( "ALTER TABLE %s RENAME TO %s", - quoted(handle.getCatalogName(), handle.getSchemaName(), handle.getTemporaryTableName()), - quoted(handle.getTableName())); + quoted(catalogName, schemaName, tableName), + quoted(newTable.getTableName())); - try (Connection connection = getConnection(identity, handle)) { + try (Connection connection = connectionFactory.openConnection(identity)) { execute(connection, sql); } catch (SQLException e) { - throw new RuntimeException(e); + throw new PrestoException(JDBC_ERROR, e); } } diff --git a/presto-postgresql/src/test/java/io/prestosql/plugin/postgresql/TestPostgreSqlDistributedQueries.java b/presto-postgresql/src/test/java/io/prestosql/plugin/postgresql/TestPostgreSqlDistributedQueries.java index 87adf5fb308b9..0db2829df4dd7 100644 --- a/presto-postgresql/src/test/java/io/prestosql/plugin/postgresql/TestPostgreSqlDistributedQueries.java +++ b/presto-postgresql/src/test/java/io/prestosql/plugin/postgresql/TestPostgreSqlDistributedQueries.java @@ -15,7 +15,7 @@ import io.airlift.testing.postgresql.TestingPostgreSqlServer; import io.airlift.tpch.TpchTable; -import io.prestosql.tests.AbstractTestQueries; +import io.prestosql.tests.AbstractTestDistributedQueries; import org.testng.annotations.AfterClass; import org.testng.annotations.Test; @@ -25,7 +25,7 @@ @Test public class TestPostgreSqlDistributedQueries - extends AbstractTestQueries + extends AbstractTestDistributedQueries { private final TestingPostgreSqlServer postgreSqlServer; @@ -48,5 +48,24 @@ public final void destroy() postgreSqlServer.close(); } + @Override + protected boolean supportsViews() + { + return false; + } + + @Override + public void testInsert() + { + // Test not supported due to lack of support for array types. + // See TestPostgreSqlIntegrationSmokeTest for insertion tests. + } + + @Override + public void testDelete() + { + // delete is not supported + } + // PostgreSQL specific tests should normally go in TestPostgreSqlIntegrationSmokeTest } diff --git a/presto-redshift/src/main/java/io/prestosql/plugin/redshift/RedshiftClient.java b/presto-redshift/src/main/java/io/prestosql/plugin/redshift/RedshiftClient.java index 268fe314a5c27..1b912c0a19738 100644 --- a/presto-redshift/src/main/java/io/prestosql/plugin/redshift/RedshiftClient.java +++ b/presto-redshift/src/main/java/io/prestosql/plugin/redshift/RedshiftClient.java @@ -17,8 +17,8 @@ import io.prestosql.plugin.jdbc.BaseJdbcConfig; import io.prestosql.plugin.jdbc.DriverConnectionFactory; import io.prestosql.plugin.jdbc.JdbcIdentity; -import io.prestosql.plugin.jdbc.JdbcOutputTableHandle; import io.prestosql.spi.PrestoException; +import io.prestosql.spi.connector.SchemaTableName; import org.postgresql.Driver; import javax.inject.Inject; @@ -28,6 +28,7 @@ import java.sql.SQLException; import static io.prestosql.plugin.jdbc.JdbcErrorCode.JDBC_ERROR; +import static io.prestosql.spi.StandardErrorCode.NOT_SUPPORTED; import static java.lang.String.format; public class RedshiftClient @@ -40,15 +41,18 @@ public RedshiftClient(BaseJdbcConfig config) } @Override - public void commitCreateTable(JdbcIdentity identity, JdbcOutputTableHandle handle) + protected void renameTable(JdbcIdentity identity, String catalogName, String schemaName, String tableName, SchemaTableName newTable) { - // Redshift does not allow qualifying the target of a rename + if (!schemaName.equals(newTable.getSchemaName())) { + throw new PrestoException(NOT_SUPPORTED, "Table rename across schemas is not supported in Redshift"); + } + String sql = format( "ALTER TABLE %s RENAME TO %s", - quoted(handle.getCatalogName(), handle.getSchemaName(), handle.getTemporaryTableName()), - quoted(handle.getTableName())); + quoted(catalogName, schemaName, tableName), + quoted(newTable.getTableName())); - try (Connection connection = getConnection(identity, handle)) { + try (Connection connection = connectionFactory.openConnection(identity)) { execute(connection, sql); } catch (SQLException e) { diff --git a/presto-sqlserver/src/main/java/io/prestosql/plugin/sqlserver/SqlServerClient.java b/presto-sqlserver/src/main/java/io/prestosql/plugin/sqlserver/SqlServerClient.java index 1c5eb3d486a2f..7ca3822b338e5 100644 --- a/presto-sqlserver/src/main/java/io/prestosql/plugin/sqlserver/SqlServerClient.java +++ b/presto-sqlserver/src/main/java/io/prestosql/plugin/sqlserver/SqlServerClient.java @@ -13,13 +13,16 @@ */ package io.prestosql.plugin.sqlserver; +import com.google.common.base.Joiner; import com.microsoft.sqlserver.jdbc.SQLServerDriver; import io.prestosql.plugin.jdbc.BaseJdbcClient; import io.prestosql.plugin.jdbc.BaseJdbcConfig; import io.prestosql.plugin.jdbc.DriverConnectionFactory; +import io.prestosql.plugin.jdbc.JdbcColumnHandle; import io.prestosql.plugin.jdbc.JdbcIdentity; -import io.prestosql.plugin.jdbc.JdbcOutputTableHandle; +import io.prestosql.plugin.jdbc.JdbcTableHandle; import io.prestosql.spi.PrestoException; +import io.prestosql.spi.connector.SchemaTableName; import javax.inject.Inject; @@ -32,6 +35,8 @@ public class SqlServerClient extends BaseJdbcClient { + private static final Joiner DOT_JOINER = Joiner.on("."); + @Inject public SqlServerClient(BaseJdbcConfig config) { @@ -39,14 +44,28 @@ public SqlServerClient(BaseJdbcConfig config) } @Override - public void commitCreateTable(JdbcIdentity identity, JdbcOutputTableHandle handle) + protected void renameTable(JdbcIdentity identity, String catalogName, String schemaName, String tableName, SchemaTableName newTable) { String sql = format( "sp_rename %s, %s", - singleQuote(handle.getCatalogName(), handle.getSchemaName(), handle.getTemporaryTableName()), - singleQuote(handle.getTableName())); + singleQuote(catalogName, schemaName, tableName), + singleQuote(newTable.getTableName())); + try (Connection connection = connectionFactory.openConnection(identity)) { + execute(connection, sql); + } + catch (SQLException e) { + throw new PrestoException(JDBC_ERROR, e); + } + } - try (Connection connection = getConnection(identity, handle)) { + @Override + public void renameColumn(JdbcIdentity identity, JdbcTableHandle handle, JdbcColumnHandle jdbcColumn, String newColumnName) + { + try (Connection connection = connectionFactory.openConnection(identity)) { + String sql = format( + "sp_rename %s, %s, 'COLUMN'", + singleQuote(handle.getCatalogName(), handle.getSchemaName(), handle.getTableName(), jdbcColumn.getColumnName()), + singleQuote(newColumnName)); execute(connection, sql); } catch (SQLException e) { @@ -54,9 +73,9 @@ public void commitCreateTable(JdbcIdentity identity, JdbcOutputTableHandle handl } } - private static String singleQuote(String catalog, String schema, String table) + private static String singleQuote(String... objects) { - return singleQuote(catalog + "." + schema + "." + table); + return singleQuote(DOT_JOINER.join(objects)); } private static String singleQuote(String literal)