From 2ee0b52dbdac3f0c3d78056c0ce36442a1af00da Mon Sep 17 00:00:00 2001 From: Dominik Zalewski Date: Tue, 5 Sep 2023 12:54:24 +0200 Subject: [PATCH] Follow-up code review --- .../plugin/jdbc/DefaultJdbcMetadata.java | 10 +-- .../jdbc/DefaultJdbcMetadataFactory.java | 8 +-- .../java/io/trino/plugin/jdbc/JdbcModule.java | 2 +- ...java => SyntheticColumnHandleBuilder.java} | 19 +++--- ...> SyntheticColumnHandleBuilderModule.java} | 4 +- .../plugin/jdbc/TestDefaultJdbcMetadata.java | 12 ++-- ... => TestSyntheticColumnHandleBuilder.java} | 39 +++++++----- .../ignite/IgniteJdbcMetadataFactory.java | 11 ++-- .../trino/plugin/ignite/IgniteMetadata.java | 9 +-- .../oracle/TestOracleConnectorTest.java | 62 +++---------------- .../plugin/phoenix5/PhoenixClientModule.java | 2 + .../phoenix5/PhoenixConnectorFactory.java | 2 - .../plugin/phoenix5/PhoenixMetadata.java | 10 +-- 13 files changed, 76 insertions(+), 114 deletions(-) rename plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/{ColumnWithAliasFormatter.java => SyntheticColumnHandleBuilder.java} (57%) rename plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/{ColumnWithAliasFormatterModule.java => SyntheticColumnHandleBuilderModule.java} (86%) rename plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/{TestColumnWithAliasFormatter.java => TestSyntheticColumnHandleBuilder.java} (50%) 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 135720dd1700..c08fa104ce51 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,17 +113,17 @@ public class DefaultJdbcMetadata private final AtomicReference rollbackAction = new AtomicReference<>(); - private final ColumnWithAliasFormatter aliasFormatter; + private final SyntheticColumnHandleBuilder syntheticColumnBuilder; public DefaultJdbcMetadata(JdbcClient jdbcClient, boolean precalculateStatisticsForPushdown, Set jdbcQueryEventListeners, - ColumnWithAliasFormatter aliasFormatter) + SyntheticColumnHandleBuilder syntheticColumnBuilder) { 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"); + this.syntheticColumnBuilder = requireNonNull(syntheticColumnBuilder, "syntheticColumnBuilder is null"); } @Override @@ -459,14 +459,14 @@ public Optional> applyJoin( ImmutableMap.Builder newLeftColumnsBuilder = ImmutableMap.builder(); for (JdbcColumnHandle column : jdbcClient.getColumns(session, leftHandle)) { - newLeftColumnsBuilder.put(column, aliasFormatter.format(session, column, nextSyntheticColumnId)); + newLeftColumnsBuilder.put(column, syntheticColumnBuilder.get(column, nextSyntheticColumnId)); nextSyntheticColumnId++; } Map newLeftColumns = newLeftColumnsBuilder.buildOrThrow(); ImmutableMap.Builder newRightColumnsBuilder = ImmutableMap.builder(); for (JdbcColumnHandle column : jdbcClient.getColumns(session, rightHandle)) { - newRightColumnsBuilder.put(column, aliasFormatter.format(session, column, nextSyntheticColumnId)); + newRightColumnsBuilder.put(column, syntheticColumnBuilder.get(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 bdd4186696e3..f1fe099e3ae2 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,14 +28,14 @@ public class DefaultJdbcMetadataFactory private final JdbcClient jdbcClient; private final Set jdbcQueryEventListeners; - protected final ColumnWithAliasFormatter aliasFormatter; + protected final SyntheticColumnHandleBuilder syntheticColumnBuilder; @Inject - public DefaultJdbcMetadataFactory(JdbcClient jdbcClient, Set jdbcQueryEventListeners, ColumnWithAliasFormatter aliasFormatter) + public DefaultJdbcMetadataFactory(JdbcClient jdbcClient, Set jdbcQueryEventListeners, SyntheticColumnHandleBuilder syntheticColumnBuilder) { this.jdbcClient = requireNonNull(jdbcClient, "jdbcClient is null"); this.jdbcQueryEventListeners = ImmutableSet.copyOf(requireNonNull(jdbcQueryEventListeners, "queryEventListeners is null")); - this.aliasFormatter = requireNonNull(aliasFormatter, "aliasFormatter is null"); + this.syntheticColumnBuilder = requireNonNull(syntheticColumnBuilder, "syntheticColumnBuilder is null"); } @Override @@ -54,6 +54,6 @@ public JdbcMetadata create(JdbcTransactionHandle transaction) protected JdbcMetadata create(JdbcClient transactionCachingJdbcClient) { - return new DefaultJdbcMetadata(transactionCachingJdbcClient, true, jdbcQueryEventListeners, aliasFormatter); + return new DefaultJdbcMetadata(transactionCachingJdbcClient, true, jdbcQueryEventListeners, syntheticColumnBuilder); } } 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 d025021eb532..b22f11cb66e1 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,7 +50,7 @@ public void setup(Binder binder) install(new JdbcDiagnosticModule()); install(new IdentifierMappingModule()); install(new RemoteQueryModifierModule()); - install(new ColumnWithAliasFormatterModule()); + install(new SyntheticColumnHandleBuilderModule()); newOptionalBinder(binder, ConnectorAccessControl.class); newOptionalBinder(binder, QueryBuilder.class).setDefault().to(DefaultQueryBuilder.class).in(Scopes.SINGLETON); 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/SyntheticColumnHandleBuilder.java similarity index 57% rename from plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/ColumnWithAliasFormatter.java rename to plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/SyntheticColumnHandleBuilder.java index 7a697587db20..e1ef62e9d7be 100644 --- 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/SyntheticColumnHandleBuilder.java @@ -13,26 +13,25 @@ */ 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; +import static com.google.common.base.Verify.verify; -public class ColumnWithAliasFormatter +public class SyntheticColumnHandleBuilder { 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) + public JdbcColumnHandle get(JdbcColumnHandle column, int nextSyntheticColumnId) { - int sequentialNumberLength = DEFAULT_COLUMN_ALIAS_LENGTH - ORIGINAL_COLUMN_NAME_LENGTH - 1; + verify(nextSyntheticColumnId >= 0, "nextSyntheticColumnId rolled over and is not monotonically increasing any more"); + + int sequentialNumberLength = String.valueOf(nextSyntheticColumnId).length(); + int originalColumnNameLength = DEFAULT_COLUMN_ALIAS_LENGTH - sequentialNumberLength - "_".length(); - String originalColumnNameTruncated = fixedLength(ORIGINAL_COLUMN_NAME_LENGTH) + String columnNameTruncated = fixedLength(originalColumnNameLength) .split(column.getColumnName()) .iterator() .next(); - String formatString = "%s_%0" + sequentialNumberLength + "d"; - String columnName = originalColumnNameTruncated + "_" + padStart(Integer.toString(nextSyntheticColumnId), sequentialNumberLength, '0'); + String columnName = columnNameTruncated + "_" + nextSyntheticColumnId; 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/SyntheticColumnHandleBuilderModule.java similarity index 86% rename from plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/ColumnWithAliasFormatterModule.java rename to plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/SyntheticColumnHandleBuilderModule.java index 4914d07694ca..b694a1dfac86 100644 --- 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/SyntheticColumnHandleBuilderModule.java @@ -16,12 +16,12 @@ import com.google.inject.AbstractModule; import com.google.inject.Singleton; -public class ColumnWithAliasFormatterModule +public class SyntheticColumnHandleBuilderModule extends AbstractModule { @Override public void configure() { - bind(ColumnWithAliasFormatter.class).in(Singleton.class); + bind(SyntheticColumnHandleBuilder.class).in(Singleton.class); } } 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 ed9040730039..066b64cd7b36 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,7 +16,6 @@ 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; @@ -35,7 +34,6 @@ 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; @@ -56,15 +54,13 @@ 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; + private final SyntheticColumnHandleBuilder syntheticColumnHandleBuilder = new SyntheticColumnHandleBuilder(); @BeforeMethod public void setUp() @@ -75,7 +71,7 @@ public void setUp() Optional.empty()), false, ImmutableSet.of(), - aliasFormatter); + syntheticColumnHandleBuilder); tableHandle = metadata.getTableHandle(SESSION, new SchemaTableName("example", "numbers")); } @@ -86,7 +82,7 @@ public void testSupportsRetriesValidation() Optional.of(false)), false, ImmutableSet.of(), - aliasFormatter); + syntheticColumnHandleBuilder); ConnectorTableMetadata tableMetadata = new ConnectorTableMetadata(new SchemaTableName("example", "numbers"), ImmutableList.of()); assertThatThrownBy(() -> { @@ -105,7 +101,7 @@ public void testNonTransactionalInsertValidation() Optional.of(true)), false, ImmutableSet.of(), - aliasFormatter); + syntheticColumnHandleBuilder); ConnectorTableMetadata tableMetadata = new ConnectorTableMetadata(new SchemaTableName("example", "numbers"), ImmutableList.of()); ConnectorSession session = TestingConnectorSession.builder() 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/TestSyntheticColumnHandleBuilder.java similarity index 50% rename from plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestColumnWithAliasFormatter.java rename to plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestSyntheticColumnHandleBuilder.java index 050f89d6e25e..4f9c367aa0cd 100644 --- 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/TestSyntheticColumnHandleBuilder.java @@ -13,45 +13,50 @@ */ package io.trino.plugin.jdbc; -import com.google.inject.Inject; -import io.trino.testing.TestingConnectorSession; -import org.testng.annotations.Guice; +import com.google.common.base.VerifyException; +import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import static io.trino.plugin.jdbc.TestingJdbcTypeHandle.JDBC_VARCHAR; import static io.trino.spi.type.VarcharType.VARCHAR; +import static java.lang.Integer.MAX_VALUE; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; -@Guice(modules = ColumnWithAliasFormatterModule.class) -public class TestColumnWithAliasFormatter +public class TestSyntheticColumnHandleBuilder { - @Inject - private ColumnWithAliasFormatter actor; + private final SyntheticColumnHandleBuilder syntheticColumnHandleBuilder = new SyntheticColumnHandleBuilder(); - private final TestingConnectorSession session = TestingConnectorSession.builder().build(); + @DataProvider(name = "columns") + public static Object[][] testData() + { + return new Object[][] { + {"column_0", 999, "column_0_999"}, + {"column_with_over_twenty_characters", 100, "column_with_over_twenty_ch_100"}, + {"column_with_over_twenty_characters", MAX_VALUE, "column_with_over_tw_2147483647"} + }; + } - @Test - public void testTooLongName() + @Test(dataProvider = "columns") + public void testColumnAliasTruncation(String columnName, int nextSynthenticId, String expectedSyntheticColumnName) { JdbcColumnHandle column = getDefaultColumnHandleBuilder() - .setColumnName("column_with_over_twenty_characters") + .setColumnName(columnName) .build(); - JdbcColumnHandle result = actor.format(session, column, 100); + JdbcColumnHandle result = syntheticColumnHandleBuilder.get(column, nextSynthenticId); - assertThat(result.getColumnName()).isEqualTo("column_with_over_twenty__00100"); + assertThat(result.getColumnName()).isEqualTo(expectedSyntheticColumnName); } @Test - public void testTooShortName() + public void testNegativeSyntheticId() { JdbcColumnHandle column = getDefaultColumnHandleBuilder() .setColumnName("column_0") .build(); - JdbcColumnHandle result = actor.format(session, column, 999); - - assertThat(result.getColumnName()).isEqualTo("column_0_00999"); + assertThatThrownBy(() -> syntheticColumnHandleBuilder.get(column, -2147483648)).isInstanceOf(VerifyException.class); } private static JdbcColumnHandle.Builder getDefaultColumnHandleBuilder() 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 7bb353dcd09a..09833b262a70 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,11 +15,11 @@ 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; import io.trino.plugin.jdbc.JdbcQueryEventListener; +import io.trino.plugin.jdbc.SyntheticColumnHandleBuilder; import java.util.Set; @@ -31,16 +31,17 @@ public class IgniteJdbcMetadataFactory private final Set jdbcQueryEventListeners; @Inject - public IgniteJdbcMetadataFactory(JdbcClient jdbcClient, Set jdbcQueryEventListeners, - ColumnWithAliasFormatter aliasFormatter) + public IgniteJdbcMetadataFactory(JdbcClient jdbcClient, + Set jdbcQueryEventListeners, + SyntheticColumnHandleBuilder syntheticColumnHandleBuilder) { - super(jdbcClient, jdbcQueryEventListeners, aliasFormatter); + super(jdbcClient, jdbcQueryEventListeners, syntheticColumnHandleBuilder); this.jdbcQueryEventListeners = ImmutableSet.copyOf(requireNonNull(jdbcQueryEventListeners, "jdbcQueryEventListeners is null")); } @Override protected JdbcMetadata create(JdbcClient transactionCachingJdbcClient) { - return new IgniteMetadata(transactionCachingJdbcClient, jdbcQueryEventListeners, aliasFormatter); + return new IgniteMetadata(transactionCachingJdbcClient, jdbcQueryEventListeners, syntheticColumnBuilder); } } 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 5de953fb53c2..3540b39aa08a 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,7 +16,6 @@ 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; @@ -25,6 +24,7 @@ import io.trino.plugin.jdbc.JdbcTableHandle; import io.trino.plugin.jdbc.JdbcTypeHandle; import io.trino.plugin.jdbc.RemoteTableName; +import io.trino.plugin.jdbc.SyntheticColumnHandleBuilder; import io.trino.spi.TrinoException; import io.trino.spi.connector.ColumnHandle; import io.trino.spi.connector.ColumnMetadata; @@ -58,10 +58,11 @@ public class IgniteMetadata private final JdbcClient igniteClient; @Inject - public IgniteMetadata(JdbcClient igniteClient, Set jdbcQueryEventListeners, - ColumnWithAliasFormatter aliasFormatter) + public IgniteMetadata(JdbcClient igniteClient, + Set jdbcQueryEventListeners, + SyntheticColumnHandleBuilder syntheticColumnHandleBuilder) { - super(igniteClient, false, jdbcQueryEventListeners, aliasFormatter); + super(igniteClient, false, jdbcQueryEventListeners, syntheticColumnHandleBuilder); 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 70a795c868ae..9d6d698fc2b5 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,27 +15,25 @@ 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 io.trino.testing.sql.TestTable; 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.jdbc.SyntheticColumnHandleBuilder.DEFAULT_COLUMN_ALIAS_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 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; +import static org.assertj.core.api.Assertions.assertThat; -@Test(singleThreaded = true) public class TestOracleConnectorTest extends BaseOracleConnectorTest { - private static final int MAX_CHARS_COLUMN_ALIAS = DEFAULT_COLUMN_ALIAS_LENGTH; + private static final String MAXIMUM_LENGTH_COLUMN_IDENTIFIER = "z".repeat(DEFAULT_COLUMN_ALIAS_LENGTH); private TestingOracleServer oracleServer; @@ -94,52 +92,12 @@ protected SqlExecutor onRemoteDatabase() @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(); + try (TestTable table = new TestTable(getQueryRunner()::execute, "long_identifier", "(%s bigint)".formatted(MAXIMUM_LENGTH_COLUMN_IDENTIFIER))) { + assertThat(query(joinPushdownEnabled(getSession()), """ + SELECT r.name, t.%s, n.name + FROM %s t JOIN region r ON r.regionkey = t.%s + JOIN nation n ON r.regionkey = n.regionkey""".formatted(MAXIMUM_LENGTH_COLUMN_IDENTIFIER, table.getName(), MAXIMUM_LENGTH_COLUMN_IDENTIFIER))) + .isFullyPushedDown(); } } } diff --git a/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixClientModule.java b/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixClientModule.java index 852d76723aa4..d7f653d09773 100644 --- a/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixClientModule.java +++ b/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixClientModule.java @@ -50,6 +50,7 @@ import io.trino.plugin.jdbc.QueryBuilder; import io.trino.plugin.jdbc.ReusableConnectionFactoryModule; import io.trino.plugin.jdbc.StatsCollecting; +import io.trino.plugin.jdbc.SyntheticColumnHandleBuilderModule; import io.trino.plugin.jdbc.TypeHandlingJdbcConfig; import io.trino.plugin.jdbc.TypeHandlingJdbcSessionProperties; import io.trino.plugin.jdbc.credential.EmptyCredentialProvider; @@ -150,6 +151,7 @@ protected void setup(Binder binder) install(new JdbcDiagnosticModule()); install(new IdentifierMappingModule()); install(new DecimalModule()); + install(new SyntheticColumnHandleBuilderModule()); } private void checkConfiguration(String connectionUrl) 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 83c796a45957..da46fa3a2127 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,7 +18,6 @@ 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; @@ -57,7 +56,6 @@ 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 ed954999e9c5..03d9d2334318 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,7 +17,6 @@ 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; @@ -25,6 +24,7 @@ import io.trino.plugin.jdbc.JdbcTableHandle; import io.trino.plugin.jdbc.JdbcTypeHandle; import io.trino.plugin.jdbc.RemoteTableName; +import io.trino.plugin.jdbc.SyntheticColumnHandleBuilder; import io.trino.spi.TrinoException; import io.trino.spi.connector.AggregateFunction; import io.trino.spi.connector.AggregationApplicationResult; @@ -85,10 +85,12 @@ public class PhoenixMetadata private final IdentifierMapping identifierMapping; @Inject - public PhoenixMetadata(PhoenixClient phoenixClient, IdentifierMapping identifierMapping, Set jdbcQueryEventListeners, - ColumnWithAliasFormatter aliasFormatter) + public PhoenixMetadata(PhoenixClient phoenixClient, + IdentifierMapping identifierMapping, + Set jdbcQueryEventListeners, + SyntheticColumnHandleBuilder syntheticColumnHandleBuilder) { - super(phoenixClient, false, jdbcQueryEventListeners, aliasFormatter); + super(phoenixClient, false, jdbcQueryEventListeners, syntheticColumnHandleBuilder); this.phoenixClient = requireNonNull(phoenixClient, "phoenixClient is null"); this.identifierMapping = requireNonNull(identifierMapping, "identifierMapping is null"); }