diff --git a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/CachingJdbcClient.java b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/CachingJdbcClient.java index 140c5818d7d8..f8e920ad095b 100644 --- a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/CachingJdbcClient.java +++ b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/CachingJdbcClient.java @@ -603,6 +603,12 @@ public void truncateTable(ConnectorSession session, JdbcTableHandle handle) onDataChanged(handle.getRequiredNamedRelation().getSchemaTableName()); } + @Override + public OptionalInt getMaximumIdentifierLength() + { + return delegate.getMaximumIdentifierLength(); + } + @Managed public void flushCache() { 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..79713d3e7dc4 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,22 @@ public class DefaultJdbcMetadata private final AtomicReference rollbackAction = new AtomicReference<>(); + private final SyntheticColumnBuilder syntheticColumnBuilder; + public DefaultJdbcMetadata(JdbcClient jdbcClient, boolean precalculateStatisticsForPushdown, Set jdbcQueryEventListeners) + { + this(jdbcClient, precalculateStatisticsForPushdown, jdbcQueryEventListeners, new SyntheticColumnBuilder()); + } + + public DefaultJdbcMetadata(JdbcClient jdbcClient, + boolean precalculateStatisticsForPushdown, + Set jdbcQueryEventListeners, + SyntheticColumnBuilder syntheticColumnBuilder) { this.jdbcClient = requireNonNull(jdbcClient, "jdbcClient is null"); this.precalculateStatisticsForPushdown = precalculateStatisticsForPushdown; this.jdbcQueryEventListeners = ImmutableSet.copyOf(requireNonNull(jdbcQueryEventListeners, "queryEventListeners is null")); + this.syntheticColumnBuilder = requireNonNull(syntheticColumnBuilder, "syntheticColumnBuilder is null"); } @Override @@ -453,22 +464,17 @@ 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, syntheticColumnBuilder.aliasForColumn(column, jdbcClient.getMaximumIdentifierLength(), 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, syntheticColumnBuilder.aliasForColumn(column, jdbcClient.getMaximumIdentifierLength(), nextSyntheticColumnId)); nextSyntheticColumnId++; } Map newRightColumns = newRightColumnsBuilder.buildOrThrow(); - ImmutableList.Builder jdbcJoinConditions = ImmutableList.builder(); for (JoinCondition joinCondition : joinConditions) { Optional leftColumn = getVariableColumnHandle(leftAssignments, joinCondition.getLeftExpression()); diff --git a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/ForwardingJdbcClient.java b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/ForwardingJdbcClient.java index b20d479da859..b02492720244 100644 --- a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/ForwardingJdbcClient.java +++ b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/ForwardingJdbcClient.java @@ -438,4 +438,10 @@ public OptionalInt getMaxWriteParallelism(ConnectorSession session) { return delegate().getMaxWriteParallelism(session); } + + @Override + public OptionalInt getMaximumIdentifierLength() + { + return delegate().getMaximumIdentifierLength(); + } } diff --git a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/JdbcClient.java b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/JdbcClient.java index 3debdc48edbb..12ddd4981006 100644 --- a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/JdbcClient.java +++ b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/JdbcClient.java @@ -232,4 +232,9 @@ default Optional getTableScanRedirection(Con void truncateTable(ConnectorSession session, JdbcTableHandle handle); OptionalInt getMaxWriteParallelism(ConnectorSession session); + + default OptionalInt getMaximumIdentifierLength() + { + return OptionalInt.empty(); + } } diff --git a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/SyntheticColumnBuilder.java b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/SyntheticColumnBuilder.java new file mode 100644 index 000000000000..96c4067a6c45 --- /dev/null +++ b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/SyntheticColumnBuilder.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 java.util.OptionalInt; + +import static com.google.common.base.Verify.verify; + +public class SyntheticColumnBuilder +{ + private static final String SEPARATOR = "_"; + private static final int SEPARATOR_LENGTH = SEPARATOR.length(); + + public JdbcColumnHandle aliasForColumn(JdbcColumnHandle column, OptionalInt maximumIdentifierLength, int nextSyntheticColumnId) + { + if (maximumIdentifierLength.isEmpty()) { + // No identifier length limit + return syntheticColumnHandle(column, nextSyntheticColumnId); + } + + String nextColumnId = String.valueOf(nextSyntheticColumnId); + int nextColumnIdLength = nextColumnId.length(); + + int maximumLength = maximumIdentifierLength.getAsInt(); + int columnLength = column.getColumnName().length(); + + int totalLength = columnLength + nextColumnIdLength + SEPARATOR_LENGTH; + verify(nextColumnIdLength <= maximumLength, "Maximum allowed identifier length is %s but synthetic column has length %s", maximumLength, nextColumnIdLength); + + if (nextColumnIdLength == maximumLength) { + return JdbcColumnHandle.builderFrom(column) + .setColumnName(nextColumnId) + .build(); + } + + if (totalLength > maximumLength) { + String strippedColumnName = column.getColumnName().substring(0, columnLength - (totalLength - maximumLength)); + return JdbcColumnHandle.builderFrom(column) + .setColumnName(strippedColumnName + SEPARATOR + nextColumnId) + .build(); + } + + return syntheticColumnHandle(column, nextSyntheticColumnId); + } + + private static JdbcColumnHandle syntheticColumnHandle(JdbcColumnHandle column, int nextSyntheticColumnId) + { + return JdbcColumnHandle.builderFrom(column) + .setColumnName(column.getColumnName() + SEPARATOR + nextSyntheticColumnId) + .build(); + } +} diff --git a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/jmx/StatisticsAwareJdbcClient.java b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/jmx/StatisticsAwareJdbcClient.java index d9c0ed64a2d8..3c3c39e6ae01 100644 --- a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/jmx/StatisticsAwareJdbcClient.java +++ b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/jmx/StatisticsAwareJdbcClient.java @@ -458,4 +458,10 @@ public OptionalInt getMaxWriteParallelism(ConnectorSession session) { return delegate().getMaxWriteParallelism(session); } + + @Override + public OptionalInt getMaximumIdentifierLength() + { + return delegate().getMaximumIdentifierLength(); + } } diff --git a/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestSyntheticColumnBuilder.java b/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestSyntheticColumnBuilder.java new file mode 100644 index 000000000000..b745b82dae3d --- /dev/null +++ b/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestSyntheticColumnBuilder.java @@ -0,0 +1,110 @@ +/* + * 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.common.base.VerifyException; +import org.testng.annotations.Test; + +import java.util.OptionalInt; + +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; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +public class TestSyntheticColumnBuilder +{ + private static final SyntheticColumnBuilder FORMATTER = new SyntheticColumnBuilder(); + private static final int MAX_SYNTHETIC_ID_LENGTH = String.valueOf(Integer.MAX_VALUE).length(); + + @Test + public void columnNameShorterThanMaximum() + { + int maximumLength = 30; + String columnName = "a".repeat(maximumLength - 4); + + JdbcColumnHandle column = getDefaultColumnHandleBuilder() + .setColumnName(columnName) + .build(); + + JdbcColumnHandle result = FORMATTER.aliasForColumn(column, OptionalInt.of(maximumLength), 456); + assertThat(result.getColumnName()).isEqualTo(columnName + "_456"); + } + + @Test + public void columnNameLongerThanMaximum() + { + int maximumLength = 30; + String columnName = "a".repeat(maximumLength); + + JdbcColumnHandle column = getDefaultColumnHandleBuilder() + .setColumnName(columnName) + .build(); + + JdbcColumnHandle result = FORMATTER.aliasForColumn(column, OptionalInt.of(maximumLength), 123); + assertThat(result.getColumnName()).isEqualTo("a".repeat(maximumLength - 4) + "_123"); + } + + @Test + public void syntheticIdEqualsTheMaximum() + { + JdbcColumnHandle column = getDefaultColumnHandleBuilder() + .setColumnName("aaaa") + .build(); + + JdbcColumnHandle result = FORMATTER.aliasForColumn(column, OptionalInt.of(MAX_SYNTHETIC_ID_LENGTH), Integer.MAX_VALUE); + assertThat(result.getColumnName()).isEqualTo(String.valueOf(Integer.MAX_VALUE)); + } + + @Test + public void syntheticWithSeparatorIdEqualsTheMaximum() + { + JdbcColumnHandle column = getDefaultColumnHandleBuilder() + .setColumnName("aaaa") + .build(); + + JdbcColumnHandle result = FORMATTER.aliasForColumn(column, OptionalInt.of(MAX_SYNTHETIC_ID_LENGTH + 1), Integer.MAX_VALUE); + assertThat(result.getColumnName()).isEqualTo("_" + Integer.MAX_VALUE); + } + + @Test + public void syntheticWithSeparatorSmallerThanMaximum() + { + JdbcColumnHandle column = getDefaultColumnHandleBuilder() + .setColumnName("abcd") + .build(); + + JdbcColumnHandle result = FORMATTER.aliasForColumn(column, OptionalInt.of(MAX_SYNTHETIC_ID_LENGTH + 2), Integer.MAX_VALUE); + assertThat(result.getColumnName()).isEqualTo("a_" + Integer.MAX_VALUE); + } + + @Test + public void syntheticIdLongerThanMaximum() + { + JdbcColumnHandle column = getDefaultColumnHandleBuilder() + .setColumnName("a") + .build(); + + assertThatThrownBy(() -> FORMATTER.aliasForColumn(column, OptionalInt.of(MAX_SYNTHETIC_ID_LENGTH - 1), Integer.MAX_VALUE)) + .isInstanceOf(VerifyException.class) + .hasMessage("Maximum allowed identifier length is 9 but synthetic column has length 10"); + } + + private static JdbcColumnHandle.Builder getDefaultColumnHandleBuilder() + { + return JdbcColumnHandle.builder() + .setJdbcTypeHandle(JDBC_VARCHAR) + .setColumnType(VARCHAR); + } +} diff --git a/plugin/trino-oracle/src/main/java/io/trino/plugin/oracle/OracleClient.java b/plugin/trino-oracle/src/main/java/io/trino/plugin/oracle/OracleClient.java index eeeaccb62ac9..86066143b6c8 100644 --- a/plugin/trino-oracle/src/main/java/io/trino/plugin/oracle/OracleClient.java +++ b/plugin/trino-oracle/src/main/java/io/trino/plugin/oracle/OracleClient.java @@ -13,6 +13,7 @@ */ package io.trino.plugin.oracle; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -89,6 +90,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.OptionalInt; import java.util.Set; import java.util.function.BiFunction; @@ -157,6 +159,9 @@ public class OracleClient extends BaseJdbcClient { + @VisibleForTesting + public static final int ORACLE_MAXIMUM_IDENTIFIER_LENGTH = 30; + public static final int ORACLE_MAX_LIST_EXPRESSIONS = 1000; private static final int MAX_ORACLE_TIMESTAMP_PRECISION = 9; @@ -252,6 +257,12 @@ public OracleClient( .build()); } + @Override + public OptionalInt getMaximumIdentifierLength() + { + return OptionalInt.of(ORACLE_MAXIMUM_IDENTIFIER_LENGTH); + } + @Override protected Optional> getTableTypes() { 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..f9dbc0b5d50e 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 @@ -17,15 +17,19 @@ import io.airlift.testing.Closeables; import io.trino.testing.QueryRunner; import io.trino.testing.sql.SqlExecutor; +import io.trino.testing.sql.TestTable; import org.testng.annotations.AfterClass; import org.testng.annotations.Test; +import static io.trino.plugin.oracle.OracleClient.ORACLE_MAXIMUM_IDENTIFIER_LENGTH; 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 java.lang.String.format; import static java.util.stream.Collectors.joining; import static java.util.stream.IntStream.range; +import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy; +import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat; public class TestOracleConnectorTest extends BaseOracleConnectorTest @@ -83,4 +87,25 @@ protected SqlExecutor onRemoteDatabase() { return oracleServer::execute; } + + @Test + public void testJoinPushdownWithLongIdentifiers() + { + String maximumColumnIdentifier = "a".repeat(ORACLE_MAXIMUM_IDENTIFIER_LENGTH); + + // Verify that the ORACLE_MAXIMUM_IDENTIFIER_LENGTH is correct for the Oracle version + // used in the test. + oracleServer.execute("SELECT 1 AS " + maximumColumnIdentifier + " FROM DUAL"); + assertThatThrownBy(() -> { + oracleServer.execute("SELECT 1 AS " + maximumColumnIdentifier + "a FROM DUAL"); + }).hasMessageContaining("ORA-00972: identifier is too long"); + + try (TestTable table = new TestTable(onRemoteDatabase(), "join_long_", "(%s decimal(19, 0))".formatted(maximumColumnIdentifier))) { + assertThat(query(joinPushdownEnabled(getSession()), """ + SELECT c.custkey, o.%s, n.nationkey + FROM %s o JOIN customer c ON c.custkey = o.%s + JOIN nation n ON c.nationkey = n.nationkey""".formatted(maximumColumnIdentifier, table.getName(), maximumColumnIdentifier))) + .isFullyPushedDown(); + } + } }