From 8e793be8b827942a3a654926ceefe972e116e512 Mon Sep 17 00:00:00 2001 From: Dain Sundstrom Date: Thu, 22 Dec 2022 15:57:59 -0800 Subject: [PATCH] Add config property to disable new Redshift type mappings --- .../trino/plugin/redshift/RedshiftClient.java | 15 +++++- .../plugin/redshift/RedshiftClientModule.java | 1 + .../trino/plugin/redshift/RedshiftConfig.java | 33 +++++++++++++ .../plugin/redshift/TestRedshiftConfig.java | 46 +++++++++++++++++++ 4 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 plugin/trino-redshift/src/main/java/io/trino/plugin/redshift/RedshiftConfig.java create mode 100644 plugin/trino-redshift/src/test/java/io/trino/plugin/redshift/TestRedshiftConfig.java diff --git a/plugin/trino-redshift/src/main/java/io/trino/plugin/redshift/RedshiftClient.java b/plugin/trino-redshift/src/main/java/io/trino/plugin/redshift/RedshiftClient.java index 21ee2eabcf02..54a8744cd4ab 100644 --- a/plugin/trino-redshift/src/main/java/io/trino/plugin/redshift/RedshiftClient.java +++ b/plugin/trino-redshift/src/main/java/io/trino/plugin/redshift/RedshiftClient.java @@ -229,6 +229,7 @@ public class RedshiftClient private final AggregateFunctionRewriter aggregateFunctionRewriter; private final boolean statisticsEnabled; private final RedshiftTableStatisticsReader statisticsReader; + private final boolean legacyTypeMapping; @Inject public RedshiftClient( @@ -237,9 +238,11 @@ public RedshiftClient( JdbcStatisticsConfig statisticsConfig, QueryBuilder queryBuilder, IdentifierMapping identifierMapping, - RemoteQueryModifier queryModifier) + RemoteQueryModifier queryModifier, + RedshiftConfig redshiftConfig) { super(config, "\"", connectionFactory, queryBuilder, identifierMapping, queryModifier); + this.legacyTypeMapping = redshiftConfig.isLegacyTypeMapping(); ConnectorExpressionRewriter connectorExpressionRewriter = JdbcConnectorExpressionRewriterBuilder.newBuilder() .addStandardRules(this::quoted) .build(); @@ -467,6 +470,11 @@ protected void verifyColumnName(DatabaseMetaData databaseMetadata, String column @Override public Optional toColumnMapping(ConnectorSession session, Connection connection, JdbcTypeHandle type) { + // todo remove this when legacy type mapping is no longer supported + if (legacyTypeMapping) { + return legacyToColumnMapping(session, type); + } + Optional mapping = getForcedMappingToVarchar(type); if (mapping.isPresent()) { return mapping; @@ -573,6 +581,11 @@ private Optional legacyToColumnMapping(ConnectorSession session, @Override public WriteMapping toWriteMapping(ConnectorSession session, Type type) { + // todo remove this when legacy type mapping is no longer supported + if (legacyTypeMapping) { + return legacyToWriteMapping(type); + } + if (BOOLEAN.equals(type)) { return WriteMapping.booleanMapping("boolean", booleanWriteFunction()); } diff --git a/plugin/trino-redshift/src/main/java/io/trino/plugin/redshift/RedshiftClientModule.java b/plugin/trino-redshift/src/main/java/io/trino/plugin/redshift/RedshiftClientModule.java index aeffaac16ff7..9ffb75c51be7 100644 --- a/plugin/trino-redshift/src/main/java/io/trino/plugin/redshift/RedshiftClientModule.java +++ b/plugin/trino-redshift/src/main/java/io/trino/plugin/redshift/RedshiftClientModule.java @@ -42,6 +42,7 @@ public class RedshiftClientModule @Override public void setup(Binder binder) { + configBinder(binder).bindConfig(RedshiftConfig.class); binder.bind(JdbcClient.class).annotatedWith(ForBaseJdbc.class).to(RedshiftClient.class).in(SINGLETON); newSetBinder(binder, ConnectorTableFunction.class).addBinding().toProvider(Query.class).in(SINGLETON); configBinder(binder).bindConfig(JdbcStatisticsConfig.class); diff --git a/plugin/trino-redshift/src/main/java/io/trino/plugin/redshift/RedshiftConfig.java b/plugin/trino-redshift/src/main/java/io/trino/plugin/redshift/RedshiftConfig.java new file mode 100644 index 000000000000..267b7f587b42 --- /dev/null +++ b/plugin/trino-redshift/src/main/java/io/trino/plugin/redshift/RedshiftConfig.java @@ -0,0 +1,33 @@ +/* + * 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.trino.plugin.redshift; + +import io.airlift.configuration.Config; + +public class RedshiftConfig +{ + private boolean legacyTypeMapping; + + public boolean isLegacyTypeMapping() + { + return legacyTypeMapping; + } + + @Config("redshift.use-legacy-type-mapping") + public RedshiftConfig setLegacyTypeMapping(boolean legacyTypeMapping) + { + this.legacyTypeMapping = legacyTypeMapping; + return this; + } +} diff --git a/plugin/trino-redshift/src/test/java/io/trino/plugin/redshift/TestRedshiftConfig.java b/plugin/trino-redshift/src/test/java/io/trino/plugin/redshift/TestRedshiftConfig.java new file mode 100644 index 000000000000..2b516055d33d --- /dev/null +++ b/plugin/trino-redshift/src/test/java/io/trino/plugin/redshift/TestRedshiftConfig.java @@ -0,0 +1,46 @@ +/* + * 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.trino.plugin.redshift; + +import com.google.common.collect.ImmutableMap; +import org.testng.annotations.Test; + +import java.util.Map; + +import static io.airlift.configuration.testing.ConfigAssertions.assertFullMapping; +import static io.airlift.configuration.testing.ConfigAssertions.assertRecordedDefaults; +import static io.airlift.configuration.testing.ConfigAssertions.recordDefaults; + +public class TestRedshiftConfig +{ + @Test + public void testDefaults() + { + assertRecordedDefaults(recordDefaults(RedshiftConfig.class) + .setLegacyTypeMapping(false)); + } + + @Test + public void testExplicitPropertyMappings() + { + Map properties = ImmutableMap.builder() + .put("redshift.use-legacy-type-mapping", "true") + .buildOrThrow(); + + RedshiftConfig expected = new RedshiftConfig() + .setLegacyTypeMapping(true); + + assertFullMapping(properties, expected); + } +}