trinodb · wendigo · Aug 28, 2023
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
@@ -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
@@ -113,11 +113,22 @@ public class DefaultJdbcMetadata
 
     private final AtomicReference<Runnable> rollbackAction = new AtomicReference<>();
 
+    private final SyntheticColumnBuilder syntheticColumnBuilder;
+
     public DefaultJdbcMetadata(JdbcClient jdbcClient, boolean precalculateStatisticsForPushdown, Set<JdbcQueryEventListener> jdbcQueryEventListeners)
+    {
+        this(jdbcClient, precalculateStatisticsForPushdown, jdbcQueryEventListeners, new SyntheticColumnBuilder());
+    }
+
+    public DefaultJdbcMetadata(JdbcClient jdbcClient,
+            boolean precalculateStatisticsForPushdown,
+            Set<JdbcQueryEventListener> 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<JoinApplicationResult<ConnectorTableHandle>> applyJoin(
 
         ImmutableMap.Builder<JdbcColumnHandle, JdbcColumnHandle> 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<JdbcColumnHandle, JdbcColumnHandle> newLeftColumns = newLeftColumnsBuilder.buildOrThrow();
 
         ImmutableMap.Builder<JdbcColumnHandle, JdbcColumnHandle> 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<JdbcColumnHandle, JdbcColumnHandle> newRightColumns = newRightColumnsBuilder.buildOrThrow();
-
         ImmutableList.Builder<JdbcJoinCondition> jdbcJoinConditions = ImmutableList.builder();
         for (JoinCondition joinCondition : joinConditions) {
             Optional<JdbcColumnHandle> 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
@@ -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
@@ -232,4 +232,9 @@ default Optional<TableScanRedirectApplicationResult> 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
@@ -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
@@ -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
@@ -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
@@ -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<List<String>> 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
@@ -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();
+        }
+    }
 }