diff --git a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/ColumnWithAliasFormatter.java b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/ColumnWithAliasFormatter.java new file mode 100644 index 000000000000..7a697587db20 --- /dev/null +++ b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/ColumnWithAliasFormatter.java @@ -0,0 +1,40 @@ +/* + * 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.jdbc; + +import io.trino.spi.connector.ConnectorSession; + +import static com.google.common.base.Splitter.fixedLength; +import static com.google.common.base.Strings.padStart; + +public class ColumnWithAliasFormatter +{ + public static final int DEFAULT_COLUMN_ALIAS_LENGTH = 30; + public static final int ORIGINAL_COLUMN_NAME_LENGTH = 24; + + public JdbcColumnHandle format(ConnectorSession session, JdbcColumnHandle column, int nextSyntheticColumnId) + { + int sequentialNumberLength = DEFAULT_COLUMN_ALIAS_LENGTH - ORIGINAL_COLUMN_NAME_LENGTH - 1; + + String originalColumnNameTruncated = fixedLength(ORIGINAL_COLUMN_NAME_LENGTH) + .split(column.getColumnName()) + .iterator() + .next(); + String formatString = "%s_%0" + sequentialNumberLength + "d"; + String columnName = originalColumnNameTruncated + "_" + padStart(Integer.toString(nextSyntheticColumnId), sequentialNumberLength, '0'); + return JdbcColumnHandle.builderFrom(column) + .setColumnName(columnName) + .build(); + } +} diff --git a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/ColumnWithAliasFormatterModule.java b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/ColumnWithAliasFormatterModule.java new file mode 100644 index 000000000000..4914d07694ca --- /dev/null +++ b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/ColumnWithAliasFormatterModule.java @@ -0,0 +1,27 @@ +/* + * 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.jdbc; + +import com.google.inject.AbstractModule; +import com.google.inject.Singleton; + +public class ColumnWithAliasFormatterModule + extends AbstractModule +{ + @Override + public void configure() + { + bind(ColumnWithAliasFormatter.class).in(Singleton.class); + } +} diff --git a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/DefaultJdbcMetadata.java b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/DefaultJdbcMetadata.java index 2103de28831f..135720dd1700 100644 --- a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/DefaultJdbcMetadata.java +++ b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/DefaultJdbcMetadata.java @@ -113,11 +113,17 @@ public class DefaultJdbcMetadata private final AtomicReference rollbackAction = new AtomicReference<>(); - public DefaultJdbcMetadata(JdbcClient jdbcClient, boolean precalculateStatisticsForPushdown, Set jdbcQueryEventListeners) + private final ColumnWithAliasFormatter aliasFormatter; + + public DefaultJdbcMetadata(JdbcClient jdbcClient, + boolean precalculateStatisticsForPushdown, + Set jdbcQueryEventListeners, + ColumnWithAliasFormatter aliasFormatter) { this.jdbcClient = requireNonNull(jdbcClient, "jdbcClient is null"); this.precalculateStatisticsForPushdown = precalculateStatisticsForPushdown; this.jdbcQueryEventListeners = ImmutableSet.copyOf(requireNonNull(jdbcQueryEventListeners, "queryEventListeners is null")); + this.aliasFormatter = requireNonNull(aliasFormatter, "aliasFormatter is null"); } @Override @@ -453,18 +459,14 @@ public Optional> applyJoin( ImmutableMap.Builder newLeftColumnsBuilder = ImmutableMap.builder(); for (JdbcColumnHandle column : jdbcClient.getColumns(session, leftHandle)) { - newLeftColumnsBuilder.put(column, JdbcColumnHandle.builderFrom(column) - .setColumnName(column.getColumnName() + "_" + nextSyntheticColumnId) - .build()); + newLeftColumnsBuilder.put(column, aliasFormatter.format(session, column, nextSyntheticColumnId)); nextSyntheticColumnId++; } Map newLeftColumns = newLeftColumnsBuilder.buildOrThrow(); ImmutableMap.Builder newRightColumnsBuilder = ImmutableMap.builder(); for (JdbcColumnHandle column : jdbcClient.getColumns(session, rightHandle)) { - newRightColumnsBuilder.put(column, JdbcColumnHandle.builderFrom(column) - .setColumnName(column.getColumnName() + "_" + nextSyntheticColumnId) - .build()); + newRightColumnsBuilder.put(column, aliasFormatter.format(session, column, nextSyntheticColumnId)); nextSyntheticColumnId++; } Map newRightColumns = newRightColumnsBuilder.buildOrThrow(); diff --git a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/DefaultJdbcMetadataFactory.java b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/DefaultJdbcMetadataFactory.java index 76b1a91dc9dc..bdd4186696e3 100644 --- a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/DefaultJdbcMetadataFactory.java +++ b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/DefaultJdbcMetadataFactory.java @@ -28,11 +28,14 @@ public class DefaultJdbcMetadataFactory private final JdbcClient jdbcClient; private final Set jdbcQueryEventListeners; + protected final ColumnWithAliasFormatter aliasFormatter; + @Inject - public DefaultJdbcMetadataFactory(JdbcClient jdbcClient, Set jdbcQueryEventListeners) + public DefaultJdbcMetadataFactory(JdbcClient jdbcClient, Set jdbcQueryEventListeners, ColumnWithAliasFormatter aliasFormatter) { this.jdbcClient = requireNonNull(jdbcClient, "jdbcClient is null"); this.jdbcQueryEventListeners = ImmutableSet.copyOf(requireNonNull(jdbcQueryEventListeners, "queryEventListeners is null")); + this.aliasFormatter = requireNonNull(aliasFormatter, "aliasFormatter is null"); } @Override @@ -51,6 +54,6 @@ public JdbcMetadata create(JdbcTransactionHandle transaction) protected JdbcMetadata create(JdbcClient transactionCachingJdbcClient) { - return new DefaultJdbcMetadata(transactionCachingJdbcClient, true, jdbcQueryEventListeners); + return new DefaultJdbcMetadata(transactionCachingJdbcClient, true, jdbcQueryEventListeners, aliasFormatter); } } diff --git a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/JdbcModule.java b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/JdbcModule.java index fbb3d98760aa..d025021eb532 100644 --- a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/JdbcModule.java +++ b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/JdbcModule.java @@ -50,6 +50,7 @@ public void setup(Binder binder) install(new JdbcDiagnosticModule()); install(new IdentifierMappingModule()); install(new RemoteQueryModifierModule()); + install(new ColumnWithAliasFormatterModule()); newOptionalBinder(binder, ConnectorAccessControl.class); newOptionalBinder(binder, QueryBuilder.class).setDefault().to(DefaultQueryBuilder.class).in(Scopes.SINGLETON); diff --git a/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestColumnWithAliasFormatter.java b/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestColumnWithAliasFormatter.java new file mode 100644 index 000000000000..050f89d6e25e --- /dev/null +++ b/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestColumnWithAliasFormatter.java @@ -0,0 +1,63 @@ +/* + * 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.jdbc; + +import com.google.inject.Inject; +import io.trino.testing.TestingConnectorSession; +import org.testng.annotations.Guice; +import org.testng.annotations.Test; + +import static io.trino.plugin.jdbc.TestingJdbcTypeHandle.JDBC_VARCHAR; +import static io.trino.spi.type.VarcharType.VARCHAR; +import static org.assertj.core.api.Assertions.assertThat; + +@Guice(modules = ColumnWithAliasFormatterModule.class) +public class TestColumnWithAliasFormatter +{ + @Inject + private ColumnWithAliasFormatter actor; + + private final TestingConnectorSession session = TestingConnectorSession.builder().build(); + + @Test + public void testTooLongName() + { + JdbcColumnHandle column = getDefaultColumnHandleBuilder() + .setColumnName("column_with_over_twenty_characters") + .build(); + + JdbcColumnHandle result = actor.format(session, column, 100); + + assertThat(result.getColumnName()).isEqualTo("column_with_over_twenty__00100"); + } + + @Test + public void testTooShortName() + { + JdbcColumnHandle column = getDefaultColumnHandleBuilder() + .setColumnName("column_0") + .build(); + + JdbcColumnHandle result = actor.format(session, column, 999); + + assertThat(result.getColumnName()).isEqualTo("column_0_00999"); + } + + private static JdbcColumnHandle.Builder getDefaultColumnHandleBuilder() + { + return JdbcColumnHandle.builder() + .setJdbcTypeHandle(JDBC_VARCHAR) + .setColumnType(VARCHAR); + } +} diff --git a/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestDefaultJdbcMetadata.java b/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestDefaultJdbcMetadata.java index 9a55e499eb91..ed9040730039 100644 --- a/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestDefaultJdbcMetadata.java +++ b/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestDefaultJdbcMetadata.java @@ -16,6 +16,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.inject.Inject; import io.trino.spi.connector.AggregateFunction; import io.trino.spi.connector.AggregationApplicationResult; import io.trino.spi.connector.ColumnHandle; @@ -34,6 +35,7 @@ import io.trino.testing.TestingConnectorSession; import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Guice; import org.testng.annotations.Test; import java.util.List; @@ -54,25 +56,37 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; @Test(singleThreaded = true) +@Guice(modules = ColumnWithAliasFormatterModule.class) public class TestDefaultJdbcMetadata { private TestingDatabase database; private DefaultJdbcMetadata metadata; private JdbcTableHandle tableHandle; + @Inject + private ColumnWithAliasFormatter aliasFormatter; + @BeforeMethod public void setUp() throws Exception { database = new TestingDatabase(); - metadata = new DefaultJdbcMetadata(new GroupingSetsEnabledJdbcClient(database.getJdbcClient(), Optional.empty()), false, ImmutableSet.of()); + metadata = new DefaultJdbcMetadata(new GroupingSetsEnabledJdbcClient(database.getJdbcClient(), + Optional.empty()), + false, + ImmutableSet.of(), + aliasFormatter); tableHandle = metadata.getTableHandle(SESSION, new SchemaTableName("example", "numbers")); } @Test public void testSupportsRetriesValidation() { - metadata = new DefaultJdbcMetadata(new GroupingSetsEnabledJdbcClient(database.getJdbcClient(), Optional.of(false)), false, ImmutableSet.of()); + metadata = new DefaultJdbcMetadata(new GroupingSetsEnabledJdbcClient(database.getJdbcClient(), + Optional.of(false)), + false, + ImmutableSet.of(), + aliasFormatter); ConnectorTableMetadata tableMetadata = new ConnectorTableMetadata(new SchemaTableName("example", "numbers"), ImmutableList.of()); assertThatThrownBy(() -> { @@ -87,7 +101,11 @@ public void testSupportsRetriesValidation() @Test public void testNonTransactionalInsertValidation() { - metadata = new DefaultJdbcMetadata(new GroupingSetsEnabledJdbcClient(database.getJdbcClient(), Optional.of(true)), false, ImmutableSet.of()); + metadata = new DefaultJdbcMetadata(new GroupingSetsEnabledJdbcClient(database.getJdbcClient(), + Optional.of(true)), + false, + ImmutableSet.of(), + aliasFormatter); ConnectorTableMetadata tableMetadata = new ConnectorTableMetadata(new SchemaTableName("example", "numbers"), ImmutableList.of()); ConnectorSession session = TestingConnectorSession.builder() diff --git a/plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteJdbcMetadataFactory.java b/plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteJdbcMetadataFactory.java index db1963f9d8b7..7bb353dcd09a 100644 --- a/plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteJdbcMetadataFactory.java +++ b/plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteJdbcMetadataFactory.java @@ -15,6 +15,7 @@ import com.google.common.collect.ImmutableSet; import com.google.inject.Inject; +import io.trino.plugin.jdbc.ColumnWithAliasFormatter; import io.trino.plugin.jdbc.DefaultJdbcMetadataFactory; import io.trino.plugin.jdbc.JdbcClient; import io.trino.plugin.jdbc.JdbcMetadata; @@ -30,15 +31,16 @@ public class IgniteJdbcMetadataFactory private final Set jdbcQueryEventListeners; @Inject - public IgniteJdbcMetadataFactory(JdbcClient jdbcClient, Set jdbcQueryEventListeners) + public IgniteJdbcMetadataFactory(JdbcClient jdbcClient, Set jdbcQueryEventListeners, + ColumnWithAliasFormatter aliasFormatter) { - super(jdbcClient, jdbcQueryEventListeners); + super(jdbcClient, jdbcQueryEventListeners, aliasFormatter); this.jdbcQueryEventListeners = ImmutableSet.copyOf(requireNonNull(jdbcQueryEventListeners, "jdbcQueryEventListeners is null")); } @Override protected JdbcMetadata create(JdbcClient transactionCachingJdbcClient) { - return new IgniteMetadata(transactionCachingJdbcClient, jdbcQueryEventListeners); + return new IgniteMetadata(transactionCachingJdbcClient, jdbcQueryEventListeners, aliasFormatter); } } diff --git a/plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteMetadata.java b/plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteMetadata.java index 3ce1a2b1f378..5de953fb53c2 100644 --- a/plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteMetadata.java +++ b/plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteMetadata.java @@ -16,6 +16,7 @@ import com.google.common.collect.ImmutableList; import com.google.inject.Inject; import io.airlift.slice.Slice; +import io.trino.plugin.jdbc.ColumnWithAliasFormatter; import io.trino.plugin.jdbc.DefaultJdbcMetadata; import io.trino.plugin.jdbc.JdbcClient; import io.trino.plugin.jdbc.JdbcColumnHandle; @@ -57,9 +58,10 @@ public class IgniteMetadata private final JdbcClient igniteClient; @Inject - public IgniteMetadata(JdbcClient igniteClient, Set jdbcQueryEventListeners) + public IgniteMetadata(JdbcClient igniteClient, Set jdbcQueryEventListeners, + ColumnWithAliasFormatter aliasFormatter) { - super(igniteClient, false, jdbcQueryEventListeners); + super(igniteClient, false, jdbcQueryEventListeners, aliasFormatter); this.igniteClient = requireNonNull(igniteClient, "igniteClient is null"); } diff --git a/plugin/trino-oracle/src/test/java/io/trino/plugin/oracle/TestOracleConnectorTest.java b/plugin/trino-oracle/src/test/java/io/trino/plugin/oracle/TestOracleConnectorTest.java index 4b582d787aa4..70a795c868ae 100644 --- a/plugin/trino-oracle/src/test/java/io/trino/plugin/oracle/TestOracleConnectorTest.java +++ b/plugin/trino-oracle/src/test/java/io/trino/plugin/oracle/TestOracleConnectorTest.java @@ -15,21 +15,28 @@ import com.google.common.collect.ImmutableMap; import io.airlift.testing.Closeables; +import io.trino.Session; import io.trino.testing.QueryRunner; import io.trino.testing.sql.SqlExecutor; import org.testng.annotations.AfterClass; import org.testng.annotations.Test; +import static io.trino.plugin.jdbc.ColumnWithAliasFormatter.DEFAULT_COLUMN_ALIAS_LENGTH; +import static io.trino.plugin.jdbc.JdbcMetadataSessionProperties.JOIN_PUSHDOWN_ENABLED; import static io.trino.plugin.oracle.TestingOracleServer.TEST_PASS; import static io.trino.plugin.oracle.TestingOracleServer.TEST_SCHEMA; import static io.trino.plugin.oracle.TestingOracleServer.TEST_USER; +import static io.trino.testing.TestingNames.randomNameSuffix; import static java.lang.String.format; import static java.util.stream.Collectors.joining; import static java.util.stream.IntStream.range; +@Test(singleThreaded = true) public class TestOracleConnectorTest extends BaseOracleConnectorTest { + private static final int MAX_CHARS_COLUMN_ALIAS = DEFAULT_COLUMN_ALIAS_LENGTH; + private TestingOracleServer oracleServer; @Override @@ -83,4 +90,56 @@ protected SqlExecutor onRemoteDatabase() { return oracleServer::execute; } + + @Test + public void testPushdownJoinWithLongNameSucceeds() + { + tryCleanupTemporaryTable(); + try { + String baseColumnName = "test_pushdown_" + randomNameSuffix(); + String validColumnName = baseColumnName + "z".repeat(MAX_CHARS_COLUMN_ALIAS - baseColumnName.length()); + + assertUpdate(format(""" + CREATE TABLE orders_1 as + SELECT orderkey as %s, + custkey, + orderstatus, + totalprice, + orderdate, + orderpriority, + clerk, + shippriority, + comment + FROM orders + """, validColumnName), "VALUES 15000"); + + Session session = Session.builder(getSession()) + .setCatalogSessionProperty("oracle", JOIN_PUSHDOWN_ENABLED, "true") + .build(); + assertQuery(session, + format(""" + SELECT c.custkey, o.%s, n.nationkey + FROM orders_1 o JOIN customer c ON c.custkey = o.custkey + JOIN nation n ON c.nationkey = n.nationkey + """, validColumnName), + """ + SELECT c.custkey, o.orderkey, n.nationkey + FROM orders o JOIN customer c ON c.custkey = o.custkey + JOIN nation n ON c.nationkey = n.nationkey + """); + } + finally { + tryCleanupTemporaryTable(); + } + } + + private void tryCleanupTemporaryTable() + { + try { + assertUpdate("DROP TABLE orders_1"); + } + catch (Exception ex) { + ex.printStackTrace(); + } + } } diff --git a/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixConnectorFactory.java b/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixConnectorFactory.java index da46fa3a2127..83c796a45957 100644 --- a/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixConnectorFactory.java +++ b/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixConnectorFactory.java @@ -18,6 +18,7 @@ import io.airlift.json.JsonModule; import io.opentelemetry.api.OpenTelemetry; import io.trino.plugin.base.CatalogName; +import io.trino.plugin.jdbc.ColumnWithAliasFormatterModule; import io.trino.spi.NodeManager; import io.trino.spi.classloader.ThreadContextClassLoader; import io.trino.spi.connector.Connector; @@ -56,6 +57,7 @@ public Connector create(String catalogName, Map requiredConfig, Bootstrap app = new Bootstrap( new JsonModule(), new PhoenixClientModule(catalogName), + new ColumnWithAliasFormatterModule(), binder -> { binder.bind(CatalogName.class).toInstance(new CatalogName(catalogName)); binder.bind(ClassLoader.class).toInstance(PhoenixConnectorFactory.class.getClassLoader()); diff --git a/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixMetadata.java b/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixMetadata.java index 1e1cca29b2da..ed954999e9c5 100644 --- a/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixMetadata.java +++ b/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixMetadata.java @@ -17,6 +17,7 @@ import com.google.inject.Inject; import io.airlift.slice.Slice; import io.trino.plugin.base.mapping.IdentifierMapping; +import io.trino.plugin.jdbc.ColumnWithAliasFormatter; import io.trino.plugin.jdbc.DefaultJdbcMetadata; import io.trino.plugin.jdbc.JdbcColumnHandle; import io.trino.plugin.jdbc.JdbcNamedRelationHandle; @@ -84,9 +85,10 @@ public class PhoenixMetadata private final IdentifierMapping identifierMapping; @Inject - public PhoenixMetadata(PhoenixClient phoenixClient, IdentifierMapping identifierMapping, Set jdbcQueryEventListeners) + public PhoenixMetadata(PhoenixClient phoenixClient, IdentifierMapping identifierMapping, Set jdbcQueryEventListeners, + ColumnWithAliasFormatter aliasFormatter) { - super(phoenixClient, false, jdbcQueryEventListeners); + super(phoenixClient, false, jdbcQueryEventListeners, aliasFormatter); this.phoenixClient = requireNonNull(phoenixClient, "phoenixClient is null"); this.identifierMapping = requireNonNull(identifierMapping, "identifierMapping is null"); }