From 561da944c38a9afbf017311adc7b7c08dbd3930d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Kokosi=C5=84ski?= Date: Tue, 26 Nov 2019 12:33:29 +0100 Subject: [PATCH 1/2] Fix formatting --- .../src/main/java/io/prestosql/testing/AbstractTestQueries.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/presto-testing/src/main/java/io/prestosql/testing/AbstractTestQueries.java b/presto-testing/src/main/java/io/prestosql/testing/AbstractTestQueries.java index beb0b8c263cc..e38e52e46157 100644 --- a/presto-testing/src/main/java/io/prestosql/testing/AbstractTestQueries.java +++ b/presto-testing/src/main/java/io/prestosql/testing/AbstractTestQueries.java @@ -2464,7 +2464,7 @@ public void testExplainSetSessionWithUsing() assertEquals( getOnlyElement(result.getOnlyColumnAsSet()), "SET SESSION foo = ?\n" + - "Parameters: [7]"); + "Parameters: [7]"); } @Test From 9e59703326199c47fae7d64e733b90b42df89c6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Kokosi=C5=84ski?= Date: Wed, 4 Dec 2019 10:56:34 +0100 Subject: [PATCH 2/2] Allow to map unsupported types to VARCHAR in JDBC connectors --- .../prestosql/plugin/jdbc/BaseJdbcClient.java | 43 ++++--- .../prestosql/plugin/jdbc/BaseJdbcConfig.java | 16 +++ .../jdbc/BaseJdbcPropertiesProvider.java | 55 +++++++++ .../io/prestosql/plugin/jdbc/JdbcModule.java | 2 +- .../prestosql/plugin/jdbc/JdbcPageSink.java | 10 +- .../plugin/jdbc/UnsupportedTypeHandling.java | 21 ++++ .../prestosql/plugin/jdbc/H2QueryRunner.java | 8 +- .../plugin/jdbc/TestBaseJdbcConfig.java | 7 +- .../jdbc/TestJdbcIntegrationSmokeTest.java | 111 +++++++++++++++++- .../plugin/postgresql/PostgreSqlClient.java | 4 + .../postgresql/TestPostgreSqlTypeMapping.java | 90 ++++++++++++-- .../io/prestosql/testing/sql/TestTable.java | 26 +++- 12 files changed, 349 insertions(+), 44 deletions(-) create mode 100644 presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/BaseJdbcPropertiesProvider.java create mode 100644 presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/UnsupportedTypeHandling.java 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 e57b5120c1cc..68617554d8f6 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 @@ -63,6 +63,7 @@ import static com.google.common.collect.ImmutableMap.toImmutableMap; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.collect.Iterables.getOnlyElement; +import static io.prestosql.plugin.jdbc.BaseJdbcPropertiesProvider.getUnsupportedTypeHandling; 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.bigintWriteFunction; @@ -80,6 +81,8 @@ import static io.prestosql.plugin.jdbc.StandardColumnMappings.varbinaryWriteFunction; import static io.prestosql.plugin.jdbc.StandardColumnMappings.varcharReadFunction; import static io.prestosql.plugin.jdbc.StandardColumnMappings.varcharWriteFunction; +import static io.prestosql.plugin.jdbc.UnsupportedTypeHandling.CONVERT_TO_VARCHAR; +import static io.prestosql.plugin.jdbc.UnsupportedTypeHandling.IGNORE; import static io.prestosql.spi.StandardErrorCode.NOT_FOUND; import static io.prestosql.spi.StandardErrorCode.NOT_SUPPORTED; import static io.prestosql.spi.type.BigintType.BIGINT; @@ -257,10 +260,10 @@ public List getColumns(ConnectorSession session, JdbcTableHand Optional columnMapping = toPrestoType(session, connection, typeHandle); log.debug("Mapping data type of '%s' column '%s': %s mapped to %s", tableHandle.getSchemaTableName(), columnName, typeHandle, columnMapping); // skip unsupported column types + boolean nullable = (resultSet.getInt("NULLABLE") != columnNoNulls); + // Note: some databases (e.g. SQL Server) do not return column remarks/comment here. + Optional comment = Optional.ofNullable(emptyToNull(resultSet.getString("REMARKS"))); if (columnMapping.isPresent()) { - boolean nullable = (resultSet.getInt("NULLABLE") != columnNoNulls); - // Note: some databases (e.g. SQL Server) do not return column remarks/comment here. - Optional comment = Optional.ofNullable(emptyToNull(resultSet.getString("REMARKS"))); columns.add(JdbcColumnHandle.builder() .setColumnName(columnName) .setJdbcTypeHandle(typeHandle) @@ -269,6 +272,7 @@ public List getColumns(ConnectorSession session, JdbcTableHand .setComment(comment) .build()); } + verify(columnMapping.isPresent() || getUnsupportedTypeHandling(session) == IGNORE, "Unsupported type handling is set to %s, but toPrestoType() returned empty"); } if (columns.isEmpty()) { // A table may have no supported columns. In rare cases (e.g. PostgreSQL) a table might have no columns at all. @@ -300,26 +304,37 @@ public Optional toPrestoType(ConnectorSession session, Connection if (mapping.isPresent()) { return mapping; } - return jdbcTypeToPrestoType(session, typeHandle); + Optional connectorMapping = jdbcTypeToPrestoType(session, typeHandle); + if (connectorMapping.isPresent()) { + return connectorMapping; + } + if (getUnsupportedTypeHandling(session) == CONVERT_TO_VARCHAR) { + return mapToUnboundedVarchar(typeHandle); + } + return Optional.empty(); } protected Optional getForcedMappingToVarchar(JdbcTypeHandle typeHandle) { if (typeHandle.getJdbcTypeName().isPresent() && jdbcTypesMappedToVarchar.contains(typeHandle.getJdbcTypeName().get())) { - return Optional.of(ColumnMapping.sliceMapping( - createUnboundedVarcharType(), - varcharReadFunction(), - (statement, index, value) -> { - // TODO this should be handled during planning phase - throw new PrestoException( - NOT_SUPPORTED, - "Underlying type that is force mapped to VARCHAR is not supported for INSERT: " + typeHandle.getJdbcTypeName().get()); - }, - DISABLE_PUSHDOWN)); + return mapToUnboundedVarchar(typeHandle); } return Optional.empty(); } + private static Optional mapToUnboundedVarchar(JdbcTypeHandle typeHandle) + { + return Optional.of(ColumnMapping.sliceMapping( + createUnboundedVarcharType(), + varcharReadFunction(), + (statement, index, value) -> { + throw new PrestoException( + NOT_SUPPORTED, + "Underlying type that is mapped to VARCHAR is not supported for INSERT: " + typeHandle.getJdbcTypeName().get()); + }, + DISABLE_PUSHDOWN)); + } + @Override public ConnectorSplitSource getSplits(JdbcIdentity identity, JdbcTableHandle tableHandle) { diff --git a/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/BaseJdbcConfig.java b/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/BaseJdbcConfig.java index 41247830f73e..cbf47dbde4a8 100644 --- a/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/BaseJdbcConfig.java +++ b/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/BaseJdbcConfig.java @@ -16,6 +16,7 @@ import com.google.common.base.Splitter; import com.google.common.collect.ImmutableSet; import io.airlift.configuration.Config; +import io.airlift.configuration.ConfigDescription; import io.airlift.units.Duration; import io.airlift.units.MinDuration; @@ -32,6 +33,7 @@ public class BaseJdbcConfig private boolean caseInsensitiveNameMatching; private Duration caseInsensitiveNameMatchingCacheTtl = new Duration(1, MINUTES); private Set jdbcTypesMappedToVarchar = ImmutableSet.of(); + private UnsupportedTypeHandling unsupportedTypeHandling = UnsupportedTypeHandling.IGNORE; @NotNull public String getConnectionUrl() @@ -83,4 +85,18 @@ public BaseJdbcConfig setJdbcTypesMappedToVarchar(String jdbcTypesMappedToVarcha this.jdbcTypesMappedToVarchar = ImmutableSet.copyOf(Splitter.on(",").omitEmptyStrings().trimResults().split(nullToEmpty(jdbcTypesMappedToVarchar))); return this; } + + @NotNull + public UnsupportedTypeHandling getUnsupportedTypeHandling() + { + return unsupportedTypeHandling; + } + + @Config("unsupported-type-handling") + @ConfigDescription("Unsupported type handling strategy") + public BaseJdbcConfig setUnsupportedTypeHandling(UnsupportedTypeHandling unsupportedTypeHandling) + { + this.unsupportedTypeHandling = unsupportedTypeHandling; + return this; + } } diff --git a/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/BaseJdbcPropertiesProvider.java b/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/BaseJdbcPropertiesProvider.java new file mode 100644 index 000000000000..c8efbb456bf9 --- /dev/null +++ b/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/BaseJdbcPropertiesProvider.java @@ -0,0 +1,55 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.prestosql.plugin.jdbc; + +import com.google.common.collect.ImmutableList; +import io.prestosql.spi.connector.ConnectorSession; +import io.prestosql.spi.session.PropertyMetadata; + +import javax.inject.Inject; + +import java.util.List; + +import static io.prestosql.spi.session.PropertyMetadata.enumProperty; + +public class BaseJdbcPropertiesProvider + implements SessionPropertiesProvider +{ + public static final String UNSUPPORTED_TYPE_HANDLING = "unsupported_type_handling"; + + private final List> properties; + + @Inject + public BaseJdbcPropertiesProvider(BaseJdbcConfig baseJdbcConfig) + { + properties = ImmutableList.of( + enumProperty( + UNSUPPORTED_TYPE_HANDLING, + "Unsupported type handling strategy", + UnsupportedTypeHandling.class, + baseJdbcConfig.getUnsupportedTypeHandling(), + false)); + } + + @Override + public List> getSessionProperties() + { + return properties; + } + + public static UnsupportedTypeHandling getUnsupportedTypeHandling(ConnectorSession session) + { + return session.getProperty(UNSUPPORTED_TYPE_HANDLING, UnsupportedTypeHandling.class); + } +} diff --git a/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/JdbcModule.java b/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/JdbcModule.java index 5a5835c45214..7241ca554f86 100644 --- a/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/JdbcModule.java +++ b/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/JdbcModule.java @@ -42,7 +42,7 @@ public void configure(Binder binder) newOptionalBinder(binder, ConnectorAccessControl.class); newSetBinder(binder, Procedure.class); - newSetBinder(binder, SessionPropertiesProvider.class); + newSetBinder(binder, SessionPropertiesProvider.class).addBinding().to(BaseJdbcPropertiesProvider.class); binder.bind(JdbcMetadataFactory.class).in(Scopes.SINGLETON); binder.bind(JdbcSplitManager.class).in(Scopes.SINGLETON); binder.bind(JdbcRecordSetProvider.class).in(Scopes.SINGLETON); diff --git a/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/JdbcPageSink.java b/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/JdbcPageSink.java index 965758fb01c0..1ceb03c24f66 100644 --- a/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/JdbcPageSink.java +++ b/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/JdbcPageSink.java @@ -29,14 +29,13 @@ import java.sql.SQLNonTransientException; import java.util.Collection; import java.util.List; -import java.util.Optional; import java.util.concurrent.CompletableFuture; -import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Verify.verify; import static com.google.common.collect.ImmutableList.toImmutableList; import static io.prestosql.plugin.jdbc.JdbcErrorCode.JDBC_ERROR; import static io.prestosql.plugin.jdbc.JdbcErrorCode.JDBC_NON_TRANSIENT_ERROR; +import static io.prestosql.spi.StandardErrorCode.NOT_SUPPORTED; import static java.lang.String.format; import static java.util.concurrent.CompletableFuture.completedFuture; @@ -96,11 +95,8 @@ public JdbcPageSink(ConnectorSession session, JdbcOutputTableHandle handle, Jdbc } else { List columnMappings = handle.getJdbcColumnTypes().get().stream() - .map(typeHandle -> { - Optional columnMapping = jdbcClient.toPrestoType(session, connection, typeHandle); - checkState(columnMapping.isPresent(), "missing column mapping"); - return columnMapping.get(); - }) + .map(typeHandle -> jdbcClient.toPrestoType(session, connection, typeHandle) + .orElseThrow(() -> new PrestoException(NOT_SUPPORTED, "Underlying type is not supported for INSERT: " + typeHandle))) .collect(toImmutableList()); columnWriters = columnMappings.stream() diff --git a/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/UnsupportedTypeHandling.java b/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/UnsupportedTypeHandling.java new file mode 100644 index 000000000000..c73d2cacb85d --- /dev/null +++ b/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/UnsupportedTypeHandling.java @@ -0,0 +1,21 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.prestosql.plugin.jdbc; + +public enum UnsupportedTypeHandling +{ + IGNORE, + CONVERT_TO_VARCHAR, + /**/; +} diff --git a/presto-base-jdbc/src/test/java/io/prestosql/plugin/jdbc/H2QueryRunner.java b/presto-base-jdbc/src/test/java/io/prestosql/plugin/jdbc/H2QueryRunner.java index e22dfc1cf16a..c49ea6257d02 100644 --- a/presto-base-jdbc/src/test/java/io/prestosql/plugin/jdbc/H2QueryRunner.java +++ b/presto-base-jdbc/src/test/java/io/prestosql/plugin/jdbc/H2QueryRunner.java @@ -44,6 +44,13 @@ public static DistributedQueryRunner createH2QueryRunner(TpchTable... tables) public static DistributedQueryRunner createH2QueryRunner(Iterable> tables) throws Exception + + { + return createH2QueryRunner(tables, TestingH2JdbcModule.createProperties()); + } + + public static DistributedQueryRunner createH2QueryRunner(Iterable> tables, Map properties) + throws Exception { DistributedQueryRunner queryRunner = null; try { @@ -52,7 +59,6 @@ public static DistributedQueryRunner createH2QueryRunner(Iterable> queryRunner.installPlugin(new TpchPlugin()); queryRunner.createCatalog("tpch", "tpch"); - Map properties = TestingH2JdbcModule.createProperties(); createSchema(properties, "tpch"); queryRunner.installPlugin(new JdbcPlugin("base-jdbc", new TestingH2JdbcModule())); diff --git a/presto-base-jdbc/src/test/java/io/prestosql/plugin/jdbc/TestBaseJdbcConfig.java b/presto-base-jdbc/src/test/java/io/prestosql/plugin/jdbc/TestBaseJdbcConfig.java index b07ed8926ca4..649aa7c1976f 100644 --- a/presto-base-jdbc/src/test/java/io/prestosql/plugin/jdbc/TestBaseJdbcConfig.java +++ b/presto-base-jdbc/src/test/java/io/prestosql/plugin/jdbc/TestBaseJdbcConfig.java @@ -36,7 +36,8 @@ public void testDefaults() .setConnectionUrl(null) .setCaseInsensitiveNameMatching(false) .setCaseInsensitiveNameMatchingCacheTtl(new Duration(1, MINUTES)) - .setJdbcTypesMappedToVarchar(null)); + .setJdbcTypesMappedToVarchar(null) + .setUnsupportedTypeHandling(UnsupportedTypeHandling.IGNORE)); } @Test @@ -47,13 +48,15 @@ public void testExplicitPropertyMappings() .put("case-insensitive-name-matching", "true") .put("case-insensitive-name-matching.cache-ttl", "1s") .put("jdbc-types-mapped-to-varchar", "mytype,struct_type1") + .put("unsupported-type-handling", "CONVERT_TO_VARCHAR") .build(); BaseJdbcConfig expected = new BaseJdbcConfig() .setConnectionUrl("jdbc:h2:mem:config") .setCaseInsensitiveNameMatching(true) .setCaseInsensitiveNameMatchingCacheTtl(new Duration(1, SECONDS)) - .setJdbcTypesMappedToVarchar("mytype, struct_type1"); + .setJdbcTypesMappedToVarchar("mytype, struct_type1") + .setUnsupportedTypeHandling(UnsupportedTypeHandling.CONVERT_TO_VARCHAR); assertFullMapping(properties, expected); diff --git a/presto-base-jdbc/src/test/java/io/prestosql/plugin/jdbc/TestJdbcIntegrationSmokeTest.java b/presto-base-jdbc/src/test/java/io/prestosql/plugin/jdbc/TestJdbcIntegrationSmokeTest.java index d4018749c7ea..a520ef2f4311 100644 --- a/presto-base-jdbc/src/test/java/io/prestosql/plugin/jdbc/TestJdbcIntegrationSmokeTest.java +++ b/presto-base-jdbc/src/test/java/io/prestosql/plugin/jdbc/TestJdbcIntegrationSmokeTest.java @@ -13,16 +13,121 @@ */ package io.prestosql.plugin.jdbc; +import com.google.common.collect.ImmutableList; +import io.airlift.tpch.TpchTable; +import io.prestosql.Session; import io.prestosql.testing.AbstractTestIntegrationSmokeTest; +import io.prestosql.testing.QueryRunner; +import io.prestosql.testing.sql.JdbcSqlExecutor; +import io.prestosql.testing.sql.TestTable; +import org.testng.annotations.Test; -import static io.airlift.tpch.TpchTable.ORDERS; +import java.util.Map; +import java.util.Properties; + +import static io.prestosql.plugin.jdbc.BaseJdbcPropertiesProvider.UNSUPPORTED_TYPE_HANDLING; import static io.prestosql.plugin.jdbc.H2QueryRunner.createH2QueryRunner; +import static io.prestosql.plugin.jdbc.UnsupportedTypeHandling.CONVERT_TO_VARCHAR; +import static io.prestosql.plugin.jdbc.UnsupportedTypeHandling.IGNORE; +import static java.lang.String.format; public class TestJdbcIntegrationSmokeTest extends AbstractTestIntegrationSmokeTest { - public TestJdbcIntegrationSmokeTest() + private final Map properties = TestingH2JdbcModule.createProperties(); + + @Override + protected QueryRunner createQueryRunner() + throws Exception + { + return createH2QueryRunner(ImmutableList.copyOf(TpchTable.getTables()), properties); + } + + @Test + public void testUnknownTypeAsIgnored() + { + try (TestTable table = new TestTable( + getSqlExecutor(), + "tpch.test_failure_on_unknown_type", + "(int_column int, geometry_column GEOMETRY)", + ImmutableList.of( + "1, NULL", + "2, 'POINT(7 52)'"))) { + Session ignoreUnsupportedType = unsupportedTypeHandling(IGNORE); + assertQuery(ignoreUnsupportedType, "SELECT int_column FROM " + table.getName(), "VALUES 1, 2"); + assertQuery(ignoreUnsupportedType, "SELECT * FROM " + table.getName(), "VALUES 1, 2"); + assertQuery( + ignoreUnsupportedType, + "SELECT column_name, data_type FROM information_schema.columns WHERE table_name LIKE '%_unknown_%'", + "VALUES ('int_column', 'integer')"); + assertQuery( + ignoreUnsupportedType, + "DESCRIBE " + table.getName(), + "VALUES ('int_column', 'integer', '', '')"); + + assertUpdate(ignoreUnsupportedType, format("INSERT INTO %s (int_column) VALUES (3)", table.getName()), 1); + assertQuery(ignoreUnsupportedType, "SELECT * FROM " + table.getName(), "VALUES 1, 2, 3"); + } + } + + @Test + public void testUnknownTypeAsVarchar() + { + try (TestTable table = new TestTable( + getSqlExecutor(), + "tpch.test_failure_on_unknown_type", + "(int_column int, geometry_column GEOMETRY)", + ImmutableList.of( + "1, NULL", + "2, 'POINT(7 52)'"))) { + Session convertToVarcharUnsupportedTypes = unsupportedTypeHandling(CONVERT_TO_VARCHAR); + assertQuery(convertToVarcharUnsupportedTypes, "SELECT int_column FROM " + table.getName(), "VALUES 1, 2"); + assertQuery(convertToVarcharUnsupportedTypes, "SELECT * FROM " + table.getName(), "VALUES (1, NULL), (2, 'POINT (7 52)')"); + + // predicate pushdown + assertQuery( + convertToVarcharUnsupportedTypes, + format("SELECT int_column FROM %s WHERE geometry_column = 'POINT (7 52)'", table.getName()), + "VALUES 2"); + assertQuery( + convertToVarcharUnsupportedTypes, + format("SELECT int_column FROM %s WHERE geometry_column = 'invalid data'", table.getName()), + "SELECT 1 WHERE false"); + + assertQuery( + convertToVarcharUnsupportedTypes, + "SELECT column_name, data_type FROM information_schema.columns WHERE table_name LIKE '%_unknown_%'", + "VALUES ('int_column', 'integer'), ('geometry_column', 'varchar')"); + assertQuery( + convertToVarcharUnsupportedTypes, + "DESCRIBE " + table.getName(), + "VALUES ('int_column', 'integer', '', ''), ('geometry_column', 'varchar', '','')"); + + assertUpdate( + convertToVarcharUnsupportedTypes, + format("INSERT INTO %s (int_column) VALUES (3)", table.getName()), + 1); + assertQueryFails( + convertToVarcharUnsupportedTypes, + format("INSERT INTO %s (int_column, geometry_column) VALUES (3, 'POINT (7 52)')", table.getName()), + "Underlying type that is mapped to VARCHAR is not supported for INSERT: GEOMETRY"); + + assertQuery( + convertToVarcharUnsupportedTypes, + "SELECT * FROM " + table.getName(), + "VALUES (1, NULL), (2, 'POINT (7 52)'), (3, NULL)"); + } + } + + private Session unsupportedTypeHandling(UnsupportedTypeHandling unsupportedTypeHandling) + { + return Session.builder(getSession()) + .setCatalogSessionProperty("jdbc", UNSUPPORTED_TYPE_HANDLING, unsupportedTypeHandling.name()) + .build(); + } + + private JdbcSqlExecutor getSqlExecutor() { - super(() -> createH2QueryRunner(ORDERS)); + return new JdbcSqlExecutor(properties.get("connection-url"), new Properties()); } } 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 3d0cb1ad76b8..ed5823bb797f 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 @@ -91,9 +91,11 @@ import static com.fasterxml.jackson.core.JsonFactory.Feature.CANONICALIZE_FIELD_NAMES; import static com.fasterxml.jackson.databind.SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS; import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Verify.verify; import static io.airlift.slice.SizeOf.SIZE_OF_LONG; import static io.airlift.slice.Slices.utf8Slice; import static io.airlift.slice.Slices.wrappedLongArray; +import static io.prestosql.plugin.jdbc.BaseJdbcPropertiesProvider.getUnsupportedTypeHandling; 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.fromPrestoLegacyTimestamp; @@ -101,6 +103,7 @@ import static io.prestosql.plugin.jdbc.StandardColumnMappings.timestampReadFunction; import static io.prestosql.plugin.jdbc.StandardColumnMappings.tinyintWriteFunction; import static io.prestosql.plugin.jdbc.StandardColumnMappings.varbinaryWriteFunction; +import static io.prestosql.plugin.jdbc.UnsupportedTypeHandling.IGNORE; import static io.prestosql.plugin.postgresql.PostgreSqlConfig.ArrayMapping.AS_ARRAY; import static io.prestosql.plugin.postgresql.PostgreSqlConfig.ArrayMapping.AS_JSON; import static io.prestosql.plugin.postgresql.PostgreSqlConfig.ArrayMapping.DISABLED; @@ -251,6 +254,7 @@ public List getColumns(ConnectorSession session, JdbcTableHand .setComment(comment) .build()); } + verify(columnMapping.isPresent() || getUnsupportedTypeHandling(session) == IGNORE, "Unsupported type handling is set to %s, but toPrestoType() returned empty"); } if (columns.isEmpty()) { // In rare cases a table might have no columns. diff --git a/presto-postgresql/src/test/java/io/prestosql/plugin/postgresql/TestPostgreSqlTypeMapping.java b/presto-postgresql/src/test/java/io/prestosql/plugin/postgresql/TestPostgreSqlTypeMapping.java index b84ee869f8e4..8269d25f1fe6 100644 --- a/presto-postgresql/src/test/java/io/prestosql/plugin/postgresql/TestPostgreSqlTypeMapping.java +++ b/presto-postgresql/src/test/java/io/prestosql/plugin/postgresql/TestPostgreSqlTypeMapping.java @@ -18,6 +18,7 @@ import com.google.common.collect.ImmutableMap; import io.airlift.json.JsonCodec; import io.prestosql.Session; +import io.prestosql.plugin.jdbc.UnsupportedTypeHandling; import io.prestosql.spi.type.ArrayType; import io.prestosql.spi.type.TimeZoneKey; import io.prestosql.testing.AbstractTestQueryFramework; @@ -31,6 +32,7 @@ import io.prestosql.testing.datatype.DataTypeTest; import io.prestosql.testing.sql.JdbcSqlExecutor; import io.prestosql.testing.sql.PrestoSqlExecutor; +import io.prestosql.testing.sql.TestTable; import org.testng.annotations.AfterClass; import org.testng.annotations.BeforeClass; import org.testng.annotations.DataProvider; @@ -55,6 +57,8 @@ import static com.google.common.io.BaseEncoding.base16; import static io.airlift.json.JsonCodec.listJsonCodec; import static io.airlift.json.JsonCodec.mapJsonCodec; +import static io.prestosql.plugin.jdbc.BaseJdbcPropertiesProvider.UNSUPPORTED_TYPE_HANDLING; +import static io.prestosql.plugin.jdbc.UnsupportedTypeHandling.CONVERT_TO_VARCHAR; import static io.prestosql.plugin.postgresql.PostgreSqlConfig.ArrayMapping.AS_ARRAY; import static io.prestosql.plugin.postgresql.PostgreSqlConfig.ArrayMapping.AS_JSON; import static io.prestosql.plugin.postgresql.PostgreSqlQueryRunner.createPostgreSqlQueryRunner; @@ -335,7 +339,7 @@ public void testForcedMappingToVarchar() // test insert into column that has forced varchar mapping assertQueryFails( "INSERT INTO tpch.test_forced_varchar_mapping (tsrange_col) VALUES ('some value')", - "Underlying type that is force mapped to VARCHAR is not supported for INSERT: tsrange"); + "Underlying type that is mapped to VARCHAR is not supported for INSERT: tsrange"); } finally { jdbcSqlExecutor.execute("DROP TABLE tpch.test_forced_varchar_mapping"); @@ -343,9 +347,18 @@ public void testForcedMappingToVarchar() } @Test - public void testDecimalExceedingPrecisionMax() + public void testDecimalExceedingPrecisionMaxIgnored() { - testUnsupportedDataType("decimal(50,0)"); + testUnsupportedDataTypeAsIgnored("decimal(50,0)", "12345678901234567890123456789012345678901234567890"); + } + + @Test + public void testDecimalExceedingPrecisionMaxConvertedToVarchar() + { + testUnsupportedDataTypeConvertedToVarchar( + "decimal(50,0)", + "12345678901234567890123456789012345678901234567890", + "'12345678901234567890123456789012345678901234567890'"); } @Test @@ -920,20 +933,77 @@ private DataTypeTest uuidTestCases(DataType uuidDataType) .addRoundTrip(uuidDataType, java.util.UUID.fromString("123e4567-e89b-12d3-a456-426655440000")); } - private void testUnsupportedDataType(String databaseDataType) + private void testUnsupportedDataTypeAsIgnored(String dataTypeName, String databaseValue) { JdbcSqlExecutor jdbcSqlExecutor = new JdbcSqlExecutor(postgreSqlServer.getJdbcUrl()); - jdbcSqlExecutor.execute(format("CREATE TABLE tpch.test_unsupported_data_type(key varchar(5), unsupported_column %s)", databaseDataType)); - try { + try (TestTable table = new TestTable( + jdbcSqlExecutor, + "tpch.unsupported_type", + format("(key varchar(5), unsupported_column %s)", dataTypeName), + ImmutableList.of( + "'1', NULL", + "'2', " + databaseValue))) { + assertQuery("SELECT * FROM " + table.getName(), "VALUES 1, 2"); assertQuery( - "SELECT column_name FROM information_schema.columns WHERE table_schema = 'tpch' AND table_name = 'test_unsupported_data_type'", - "VALUES 'key'"); // no 'unsupported_column' + "DESC " + table.getName(), + "VALUES ('key', 'varchar(5)','', '')"); // no 'unsupported_column' + + assertUpdate(format("INSERT INTO %s VALUES '3'", table.getName()), 1); + assertQuery("SELECT * FROM " + table.getName(), "VALUES '1', '2', '3'"); } - finally { - jdbcSqlExecutor.execute("DROP TABLE tpch.test_unsupported_data_type"); + } + + private void testUnsupportedDataTypeConvertedToVarchar(String dataTypeName, String databaseValue, String prestoValue) + { + JdbcSqlExecutor jdbcSqlExecutor = new JdbcSqlExecutor(postgreSqlServer.getJdbcUrl()); + try (TestTable table = new TestTable( + jdbcSqlExecutor, + "tpch.unsupported_type", + format("(key varchar(5), unsupported_column %s)", dataTypeName), + ImmutableList.of( + "1, NULL", + "2, " + databaseValue))) { + Session convertToVarchar = withUnsupportedType(CONVERT_TO_VARCHAR); + assertQuery( + convertToVarchar, + "SELECT * FROM " + table.getName(), + format("VALUES ('1', NULL), ('2', %s)", prestoValue)); + assertQuery( + convertToVarchar, + format("SELECT key FROM %s WHERE unsupported_column = %s", table.getName(), prestoValue), + "VALUES '2'"); + assertQuery( + convertToVarchar, + "DESC " + table.getName(), + "VALUES " + + "('key', 'varchar(5)', '', ''), " + + "('unsupported_column', 'varchar', '', '')"); + assertQueryFails( + convertToVarchar, + format("INSERT INTO %s (key, unsupported_column) VALUES (3, NULL)", table.getName()), + "Insert query has mismatched column types: Table: \\[varchar\\(5\\), varchar\\], Query: \\[integer, unknown\\]"); + assertQueryFails( + convertToVarchar, + format("INSERT INTO %s (key, unsupported_column) VALUES (4, %s)", table.getName(), prestoValue), + "Insert query has mismatched column types: Table: \\[varchar\\(5\\), varchar\\], Query: \\[integer, varchar\\(50\\)\\]"); + assertUpdate( + convertToVarchar, + format("INSERT INTO %s (key) VALUES '5'", table.getName()), + 1); + assertQuery( + convertToVarchar, + "SELECT * FROM " + table.getName(), + format("VALUES ('1', NULL), ('2', %s), ('5', NULL)", prestoValue)); } } + private Session withUnsupportedType(UnsupportedTypeHandling unsupportedTypeHandling) + { + return Session.builder(getSession()) + .setCatalogSessionProperty("postgresql", UNSUPPORTED_TYPE_HANDLING, unsupportedTypeHandling.name()) + .build(); + } + public static DataType prestoTimestampWithTimeZoneDataType() { return dataType( diff --git a/presto-testing/src/main/java/io/prestosql/testing/sql/TestTable.java b/presto-testing/src/main/java/io/prestosql/testing/sql/TestTable.java index ce604488965f..0579a2e9b310 100644 --- a/presto-testing/src/main/java/io/prestosql/testing/sql/TestTable.java +++ b/presto-testing/src/main/java/io/prestosql/testing/sql/TestTable.java @@ -13,14 +13,15 @@ */ package io.prestosql.testing.sql; +import com.google.common.collect.ImmutableList; + import java.security.SecureRandom; +import java.util.List; -import static com.google.common.base.Preconditions.checkArgument; import static java.lang.Character.MAX_RADIX; import static java.lang.Math.abs; import static java.lang.Math.min; import static java.lang.String.format; -import static java.util.Objects.requireNonNull; public class TestTable implements AutoCloseable @@ -33,12 +34,25 @@ public class TestTable public TestTable(SqlExecutor sqlExecutor, String namePrefix, String tableDefinition) { - checkArgument(!tableDefinition.contains("{TABLE_NAME}"), "tableDefinition should not contain '{TABLE_NAME}': %s", tableDefinition); - this.sqlExecutor = requireNonNull(sqlExecutor, "sqlExecutor is null"); - requireNonNull(namePrefix, "namePrefix is null"); - requireNonNull(tableDefinition, "tableDefinition is null"); + this(sqlExecutor, namePrefix, tableDefinition, ImmutableList.of()); + } + + public TestTable(SqlExecutor sqlExecutor, String namePrefix, String tableDefinition, List rowsToInsert) + { + this.sqlExecutor = sqlExecutor; this.name = namePrefix + "_" + randomTableSuffix(); sqlExecutor.execute(format("CREATE TABLE %s %s", name, tableDefinition)); + try { + for (String row : rowsToInsert) { + // some databases do not support multi value insert statement + sqlExecutor.execute(format("INSERT INTO %s VALUES (%s)", name, row)); + } + } + catch (Exception e) { + try (TestTable ignored = this) { + throw e; + } + } } public String getName()