From 1d1f2fa27ee559b1af75d29abeb1bfad79465258 Mon Sep 17 00:00:00 2001 From: Sah Oss Date: Mon, 27 Feb 2023 21:33:20 +0530 Subject: [PATCH 1/2] Remove unreachable code from IgniteClient Exception was unreachable as the same check is also present from create table task --- .../src/main/java/io/trino/plugin/ignite/IgniteClient.java | 7 ------- 1 file changed, 7 deletions(-) diff --git a/plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteClient.java b/plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteClient.java index ba28654ac057..8b5d4564d886 100644 --- a/plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteClient.java +++ b/plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteClient.java @@ -404,13 +404,6 @@ private String buildCreateSql(SchemaTableName schemaTableName, List colu } columnDefinitions.add("PRIMARY KEY (" + join(", ", primaryKeys.stream().map(this::quoted).collect(joining(", "))) + ")"); - for (Map.Entry propertyEntry : tableProperties.entrySet()) { - String propertyKey = propertyEntry.getKey(); - if (!PRIMARY_KEY_PROPERTY.equalsIgnoreCase(propertyKey)) { - throw new UnsupportedOperationException("Not support table property " + propertyKey); - } - } - String remoteTableName = quoted(null, schemaTableName.getSchemaName(), schemaTableName.getTableName()); return format("CREATE TABLE %s (%s) ", remoteTableName, join(", ", columnDefinitions.build())); } From ab3f3642b9b1c584442e791dbb98bf3573eceb43 Mon Sep 17 00:00:00 2001 From: Sah Oss Date: Sat, 25 Feb 2023 10:26:45 +0530 Subject: [PATCH 2/2] Check primary_key exist during Ignite create table Adds a check for create table in IgniteClient. The check ensures that the column names mentioned in the table property 'primary_key' exists in the column defined in the create table statement. --- .../io/trino/plugin/ignite/IgniteClient.java | 25 +++++++++++++------ .../ignite/TestIgniteConnectorTest.java | 15 +++++++++++ 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteClient.java b/plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteClient.java index 8b5d4564d886..1449f6795b74 100644 --- a/plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteClient.java +++ b/plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteClient.java @@ -109,6 +109,7 @@ import static io.trino.plugin.jdbc.StandardColumnMappings.varcharReadFunction; import static io.trino.plugin.jdbc.StandardColumnMappings.varcharWriteFunction; import static io.trino.spi.StandardErrorCode.INVALID_ARGUMENTS; +import static io.trino.spi.StandardErrorCode.INVALID_TABLE_PROPERTY; import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED; import static io.trino.spi.type.BigintType.BIGINT; import static io.trino.spi.type.BooleanType.BOOLEAN; @@ -366,14 +367,25 @@ public JdbcOutputTableHandle beginCreateTable(ConnectorSession session, Connecto int expectedSize = tableMetadata.getColumns().size(); ImmutableList.Builder columns = ImmutableList.builderWithExpectedSize(expectedSize); - ImmutableList.Builder columnNames = ImmutableList.builderWithExpectedSize(expectedSize); + ImmutableList.Builder columnNamesBuilder = ImmutableList.builderWithExpectedSize(expectedSize); ImmutableList.Builder columnTypes = ImmutableList.builderWithExpectedSize(expectedSize); for (ColumnMetadata columnMetadata : tableMetadata.getColumns()) { columns.add(getColumnDefinitionSql(session, columnMetadata, columnMetadata.getName())); - columnNames.add(columnMetadata.getName()); + columnNamesBuilder.add(columnMetadata.getName()); columnTypes.add(columnMetadata.getType()); } - String sql = buildCreateSql(schemaTableName, columns.build(), tableMetadata.getProperties()); + + List columnNames = columnNamesBuilder.build(); + List primaryKeys = IgniteTableProperties.getPrimaryKey(tableMetadata.getProperties()); + + for (String primaryKey : primaryKeys) { + if (!columnNames.contains(primaryKey)) { + throw new TrinoException(INVALID_TABLE_PROPERTY, + format("Column '%s' specified in property '%s' doesn't exist in table", primaryKey, PRIMARY_KEY_PROPERTY)); + } + } + + String sql = buildCreateSql(schemaTableName, columns.build(), primaryKeys); try (Connection connection = connectionFactory.openConnection(session)) { execute(session, connection, sql); @@ -381,22 +393,21 @@ public JdbcOutputTableHandle beginCreateTable(ConnectorSession session, Connecto return new IgniteOutputTableHandle( schemaTableName.getSchemaName(), schemaTableName.getTableName(), - columnNames.build(), + columnNames, columnTypes.build(), Optional.empty(), - IgniteTableProperties.getPrimaryKey(tableMetadata.getProperties()).isEmpty() ? Optional.of(IGNITE_DUMMY_ID) : Optional.empty()); + primaryKeys.isEmpty() ? Optional.of(IGNITE_DUMMY_ID) : Optional.empty()); } catch (SQLException e) { throw new TrinoException(JDBC_ERROR, e); } } - private String buildCreateSql(SchemaTableName schemaTableName, List columns, Map tableProperties) + private String buildCreateSql(SchemaTableName schemaTableName, List columns, List primaryKeys) { ImmutableList.Builder columnDefinitions = ImmutableList.builder(); columnDefinitions.addAll(columns); - List primaryKeys = IgniteTableProperties.getPrimaryKey(tableProperties); checkArgument(primaryKeys.size() < columns.size(), "Ignite table must have at least one non PRIMARY KEY column."); if (primaryKeys.isEmpty()) { columnDefinitions.add(quoted(IGNITE_DUMMY_ID) + " VARCHAR NOT NULL"); diff --git a/plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteConnectorTest.java b/plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteConnectorTest.java index d867a1c156e7..e8780c594bb9 100644 --- a/plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteConnectorTest.java +++ b/plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteConnectorTest.java @@ -30,6 +30,7 @@ import static com.google.common.base.Strings.nullToEmpty; import static io.trino.plugin.ignite.IgniteQueryRunner.createIgniteQueryRunner; +import static io.trino.testing.TestingNames.randomNameSuffix; import static java.lang.String.format; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -147,6 +148,20 @@ public void testCreateTableWithCommaPropertyColumn() } } + @Test + public void testCreateTableWithNonExistingPrimaryKey() + { + String tableName = "test_invalid_primary_key" + randomNameSuffix(); + assertQueryFails("CREATE TABLE " + tableName + "(a bigint) WITH (primary_key = ARRAY['not_existing_column'])", + "Column 'not_existing_column' specified in property 'primary_key' doesn't exist in table"); + + assertQueryFails("CREATE TABLE " + tableName + "(a bigint) WITH (primary_key = ARRAY['dummy_id'])", + "Column 'dummy_id' specified in property 'primary_key' doesn't exist in table"); + + assertQueryFails("CREATE TABLE " + tableName + "(a bigint) WITH (primary_key = ARRAY['A'])", + "Column 'A' specified in property 'primary_key' doesn't exist in table"); + } + @Test public void testCreateTableWithAllProperties() {