From ebcd3c2dee8148023ebf036cb24e20324b9116a5 Mon Sep 17 00:00:00 2001 From: Yuya Ebihara <ebyhry@gmail.com> Date: Fri, 25 Oct 2024 09:23:15 +0900 Subject: [PATCH 01/10] Change some classes to record in Faker --- .../io/trino/plugin/faker/ColumnInfo.java | 43 +++---------- .../io/trino/plugin/faker/FakerMetadata.java | 58 ++++++++--------- .../io/trino/plugin/faker/SchemaInfo.java | 26 ++------ .../java/io/trino/plugin/faker/TableInfo.java | 62 ++++--------------- 4 files changed, 52 insertions(+), 137 deletions(-) diff --git a/plugin/trino-faker/src/main/java/io/trino/plugin/faker/ColumnInfo.java b/plugin/trino-faker/src/main/java/io/trino/plugin/faker/ColumnInfo.java index 68de0d63ae65..6768790a77b1 100644 --- a/plugin/trino-faker/src/main/java/io/trino/plugin/faker/ColumnInfo.java +++ b/plugin/trino-faker/src/main/java/io/trino/plugin/faker/ColumnInfo.java @@ -15,60 +15,31 @@ import io.trino.spi.connector.ColumnHandle; import io.trino.spi.connector.ColumnMetadata; -import io.trino.spi.type.Type; -import java.util.Map; import java.util.Optional; import static java.util.Objects.requireNonNull; -public class ColumnInfo +public record ColumnInfo(ColumnHandle handle, ColumnMetadata metadata) { public static final String NULL_PROBABILITY_PROPERTY = "null_probability"; public static final String GENERATOR_PROPERTY = "generator"; - private final ColumnHandle handle; - private final String name; - private final Type type; - private ColumnMetadata metadata; - - public ColumnInfo(ColumnHandle handle, String name, Type type, Map<String, Object> properties, Optional<String> comment) - { - this(handle, ColumnMetadata.builder() - .setName(name) - .setType(type) - .setProperties(properties) - .setComment(comment) - .build()); - } - - public ColumnInfo(ColumnHandle handle, ColumnMetadata metadata) - { - this.handle = requireNonNull(handle, "handle is null"); - this.metadata = requireNonNull(metadata, "metadata is null"); - this.name = metadata.getName(); - this.type = metadata.getType(); - } - - public ColumnHandle getHandle() - { - return handle; - } - - public String getName() + public ColumnInfo { - return name; + requireNonNull(handle, "handle is null"); + requireNonNull(metadata, "metadata is null"); } - public ColumnMetadata getMetadata() + public String name() { - return metadata; + return metadata.getName(); } @Override public String toString() { - return name + "::" + type; + return metadata.getName() + "::" + metadata.getType(); } public ColumnInfo withComment(Optional<String> comment) diff --git a/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerMetadata.java b/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerMetadata.java index ff4463541efa..fe159cc7e9bf 100644 --- a/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerMetadata.java +++ b/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerMetadata.java @@ -89,7 +89,7 @@ public class FakerMetadata @Inject public FakerMetadata(FakerConfig config) { - this.schemas.add(new SchemaInfo(SCHEMA_NAME)); + this.schemas.add(new SchemaInfo(SCHEMA_NAME, Map.of())); this.nullProbability = config.getNullProbability(); this.defaultLimit = config.getDefaultLimit(); } @@ -98,14 +98,14 @@ public FakerMetadata(FakerConfig config) public synchronized List<String> listSchemaNames(ConnectorSession connectorSession) { return schemas.stream() - .map(SchemaInfo::getName) + .map(SchemaInfo::name) .collect(toImmutableList()); } @Override public synchronized void createSchema(ConnectorSession session, String schemaName, Map<String, Object> properties, TrinoPrincipal owner) { - if (schemas.stream().anyMatch(schema -> schema.getName().equals(schemaName))) { + if (schemas.stream().anyMatch(schema -> schema.name().equals(schemaName))) { throw new TrinoException(ALREADY_EXISTS, format("Schema [%s] already exists", schemaName)); } schemas.add(new SchemaInfo(schemaName, properties)); @@ -120,7 +120,7 @@ public synchronized void dropSchema(ConnectorSession session, String schemaName, private synchronized SchemaInfo getSchema(String name) { Optional<SchemaInfo> schema = schemas.stream() - .filter(schemaInfo -> schemaInfo.getName().equals(name)) + .filter(schemaInfo -> schemaInfo.name().equals(name)) .findAny(); if (schema.isEmpty()) { throw new TrinoException(NOT_FOUND, format("Schema [%s] does not exist", name)); @@ -139,8 +139,8 @@ public synchronized ConnectorTableHandle getTableHandle(ConnectorSession session if (id == null) { return null; } - long schemaLimit = (long) schema.getProperties().getOrDefault(SchemaInfo.DEFAULT_LIMIT_PROPERTY, defaultLimit); - long tableLimit = (long) tables.get(id).getProperties().getOrDefault(TableInfo.DEFAULT_LIMIT_PROPERTY, schemaLimit); + long schemaLimit = (long) schema.properties().getOrDefault(SchemaInfo.DEFAULT_LIMIT_PROPERTY, defaultLimit); + long tableLimit = (long) tables.get(id).properties().getOrDefault(TableInfo.DEFAULT_LIMIT_PROPERTY, schemaLimit); return new FakerTableHandle(id, tableName, TupleDomain.all(), tableLimit); } @@ -153,15 +153,15 @@ public synchronized ConnectorTableMetadata getTableMetadata( if (tableHandle.id() == null) { throw new TrinoException(INVALID_TABLE_FUNCTION_INVOCATION, "Table functions are not supported"); } - return tables.get(tableHandle.id()).getMetadata(); + return tables.get(tableHandle.id()).metadata(); } @Override public synchronized List<SchemaTableName> listTables(ConnectorSession session, Optional<String> schemaName) { return tables.values().stream() - .filter(table -> schemaName.map(table.getSchemaName()::contentEquals).orElse(true)) - .map(TableInfo::getSchemaTableName) + .filter(table -> schemaName.map(table.schemaName()::contentEquals).orElse(true)) + .map(TableInfo::schemaTableName) .collect(toImmutableList()); } @@ -172,8 +172,8 @@ public synchronized Map<String, ColumnHandle> getColumnHandles( { FakerTableHandle tableHandle = (FakerTableHandle) connectorTableHandle; return tables.get(tableHandle.id()) - .getColumns().stream() - .collect(toImmutableMap(ColumnInfo::getName, ColumnInfo::getHandle)); + .columns().stream() + .collect(toImmutableMap(ColumnInfo::name, ColumnInfo::handle)); } @Override @@ -187,18 +187,18 @@ public synchronized ColumnMetadata getColumnMetadata( throw new TrinoException(INVALID_TABLE_FUNCTION_INVOCATION, "Table functions are not supported"); } return tables.get(tableHandle.id()) - .getColumn(columnHandle) - .getMetadata(); + .column(columnHandle) + .metadata(); } @Override public synchronized Iterator<TableColumnsMetadata> streamTableColumns(ConnectorSession session, SchemaTablePrefix prefix) { return tables.values().stream() - .filter(table -> prefix.matches(table.getSchemaTableName())) + .filter(table -> prefix.matches(table.schemaTableName())) .map(table -> TableColumnsMetadata.forTable( - table.getSchemaTableName(), - table.getMetadata().getColumns())) + table.schemaTableName(), + table.metadata().getColumns())) .iterator(); } @@ -208,7 +208,7 @@ public synchronized void dropTable(ConnectorSession session, ConnectorTableHandl FakerTableHandle handle = (FakerTableHandle) tableHandle; TableInfo info = tables.remove(handle.id()); if (info != null) { - tableIds.remove(info.getSchemaTableName()); + tableIds.remove(info.schemaTableName()); } } @@ -226,11 +226,11 @@ public synchronized void renameTable(ConnectorSession session, ConnectorTableHan tableId, newTableName.getSchemaName(), newTableName.getTableName(), - oldInfo.getColumns(), - oldInfo.getProperties(), - oldInfo.getComment())); + oldInfo.columns(), + oldInfo.properties(), + oldInfo.comment())); - tableIds.remove(oldInfo.getSchemaTableName()); + tableIds.remove(oldInfo.schemaTableName()); tableIds.put(newTableName, tableId); } @@ -242,7 +242,7 @@ public synchronized void setTableProperties(ConnectorSession session, ConnectorT TableInfo oldInfo = tables.get(tableId); Map<String, Object> newProperties = Stream.concat( - oldInfo.getProperties().entrySet().stream() + oldInfo.properties().entrySet().stream() .filter(entry -> !properties.containsKey(entry.getKey())), properties.entrySet().stream() .filter(entry -> entry.getValue().isPresent())) @@ -267,9 +267,9 @@ public synchronized void setColumnComment(ConnectorSession session, ConnectorTab long tableId = handle.id(); TableInfo oldInfo = tables.get(tableId); - List<ColumnInfo> columns = oldInfo.getColumns().stream() + List<ColumnInfo> columns = oldInfo.columns().stream() .map(columnInfo -> { - if (columnInfo.getHandle().equals(column)) { + if (columnInfo.handle().equals(column)) { return columnInfo.withComment(comment); } return columnInfo; @@ -298,7 +298,7 @@ public synchronized FakerOutputTableHandle beginCreateTable(ConnectorSession ses checkTableNotExists(tableMetadata.getTable()); long tableId = nextTableId.getAndIncrement(); - double schemaNullProbability = (double) schema.getProperties().getOrDefault(SchemaInfo.NULL_PROBABILITY_PROPERTY, nullProbability); + double schemaNullProbability = (double) schema.properties().getOrDefault(SchemaInfo.NULL_PROBABILITY_PROPERTY, nullProbability); double tableNullProbability = (double) tableMetadata.getProperties().getOrDefault(TableInfo.NULL_PROBABILITY_PROPERTY, schemaNullProbability); ImmutableList.Builder<ColumnInfo> columns = ImmutableList.builder(); @@ -341,7 +341,7 @@ private boolean isCharacterColumn(ColumnMetadata column) private synchronized void checkSchemaExists(String schemaName) { - if (schemas.stream().noneMatch(schema -> schema.getName().equals(schemaName))) { + if (schemas.stream().noneMatch(schema -> schema.name().equals(schemaName))) { throw new SchemaNotFoundException(schemaName); } } @@ -366,7 +366,7 @@ public synchronized Optional<ConnectorOutputMetadata> finishCreateTable(Connecto // TODO ensure fragments is empty? - tables.put(tableId, new TableInfo(tableId, info.getSchemaName(), info.getTableName(), info.getColumns(), info.getProperties(), info.getComment())); + tables.put(tableId, new TableInfo(tableId, info.schemaName(), info.tableName(), info.columns(), info.properties(), info.comment())); return Optional.empty(); } @@ -374,13 +374,13 @@ public synchronized Optional<ConnectorOutputMetadata> finishCreateTable(Connecto public synchronized Map<String, Object> getSchemaProperties(ConnectorSession session, String schemaName) { Optional<SchemaInfo> schema = schemas.stream() - .filter(s -> s.getName().equals(schemaName)) + .filter(s -> s.name().equals(schemaName)) .findAny(); if (schema.isEmpty()) { throw new TrinoException(NOT_FOUND, format("Schema [%s] does not exist", schemaName)); } - return schema.get().getProperties(); + return schema.get().properties(); } @Override diff --git a/plugin/trino-faker/src/main/java/io/trino/plugin/faker/SchemaInfo.java b/plugin/trino-faker/src/main/java/io/trino/plugin/faker/SchemaInfo.java index 001bc3b2edb6..451823a6f4c1 100644 --- a/plugin/trino-faker/src/main/java/io/trino/plugin/faker/SchemaInfo.java +++ b/plugin/trino-faker/src/main/java/io/trino/plugin/faker/SchemaInfo.java @@ -19,32 +19,14 @@ import static java.util.Objects.requireNonNull; -public class SchemaInfo +public record SchemaInfo(String name, Map<String, Object> properties) { - private final String name; - private final Map<String, Object> properties; - public static final String NULL_PROBABILITY_PROPERTY = "null_probability"; public static final String DEFAULT_LIMIT_PROPERTY = "default_limit"; - public SchemaInfo(String name) - { - this(name, Map.of()); - } - - public SchemaInfo(String name, Map<String, Object> properties) - { - this.name = requireNonNull(name, "name is null"); - this.properties = ImmutableMap.copyOf(requireNonNull(properties, "properties is null")); - } - - public String getName() - { - return name; - } - - public Map<String, Object> getProperties() + public SchemaInfo { - return properties; + requireNonNull(name, "name is null"); + properties = ImmutableMap.copyOf(requireNonNull(properties, "properties is null")); } } diff --git a/plugin/trino-faker/src/main/java/io/trino/plugin/faker/TableInfo.java b/plugin/trino-faker/src/main/java/io/trino/plugin/faker/TableInfo.java index 341a1426a4cf..0499f870e0a2 100644 --- a/plugin/trino-faker/src/main/java/io/trino/plugin/faker/TableInfo.java +++ b/plugin/trino-faker/src/main/java/io/trino/plugin/faker/TableInfo.java @@ -26,82 +26,44 @@ import static com.google.common.collect.ImmutableList.toImmutableList; import static java.util.Objects.requireNonNull; -public class TableInfo +public record TableInfo(long id, String schemaName, String tableName, List<ColumnInfo> columns, Map<String, Object> properties, Optional<String> comment) { - private final long id; - private final String schemaName; - private final String tableName; - private List<ColumnInfo> columns; - private Map<String, Object> properties; - private Optional<String> comment; - public static final String NULL_PROBABILITY_PROPERTY = "null_probability"; public static final String DEFAULT_LIMIT_PROPERTY = "default_limit"; - public TableInfo(long id, String schemaName, String tableName, List<ColumnInfo> columns, Map<String, Object> properties, Optional<String> comment) - { - this.id = id; - this.schemaName = requireNonNull(schemaName, "schemaName is null"); - this.tableName = requireNonNull(tableName, "tableName is null"); - this.columns = ImmutableList.copyOf(columns); - this.properties = ImmutableMap.copyOf(requireNonNull(properties, "properties is null")); - this.comment = requireNonNull(comment, "comment is null"); - } - - public long getId() - { - return id; - } - - public String getSchemaName() - { - return schemaName; - } - - public String getTableName() + public TableInfo { - return tableName; + requireNonNull(schemaName, "schemaName is null"); + requireNonNull(tableName, "tableName is null"); + columns = ImmutableList.copyOf(columns); + properties = ImmutableMap.copyOf(requireNonNull(properties, "properties is null")); + requireNonNull(comment, "comment is null"); } - public SchemaTableName getSchemaTableName() + public SchemaTableName schemaTableName() { return new SchemaTableName(schemaName, tableName); } - public ConnectorTableMetadata getMetadata() + public ConnectorTableMetadata metadata() { return new ConnectorTableMetadata( new SchemaTableName(schemaName, tableName), columns.stream() - .map(ColumnInfo::getMetadata) + .map(ColumnInfo::metadata) .collect(toImmutableList()), properties, comment); } - public List<ColumnInfo> getColumns() - { - return columns; - } - - public ColumnInfo getColumn(ColumnHandle handle) + public ColumnInfo column(ColumnHandle handle) { return columns.stream() - .filter(column -> column.getHandle().equals(handle)) + .filter(column -> column.handle().equals(handle)) .findFirst() .get(); } - public Map<String, Object> getProperties() - { - return properties; - } - - public Optional<String> getComment() - { - return comment; - } - public TableInfo withColumns(List<ColumnInfo> columns) { return new TableInfo(id, schemaName, tableName, columns, properties, comment); From 3a33e36d285c8a2f5b9df8f452d7ba39e865a1bb Mon Sep 17 00:00:00 2001 From: Yuya Ebihara <ebyhry@gmail.com> Date: Fri, 25 Oct 2024 09:24:10 +0900 Subject: [PATCH 02/10] Add missing ImmutableSet.copyOf to FakerOutputTableHandle --- .../java/io/trino/plugin/faker/FakerOutputTableHandle.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerOutputTableHandle.java b/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerOutputTableHandle.java index 9fc1b1acda81..64a5178158c2 100644 --- a/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerOutputTableHandle.java +++ b/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerOutputTableHandle.java @@ -13,6 +13,7 @@ */ package io.trino.plugin.faker; +import com.google.common.collect.ImmutableSet; import io.trino.spi.connector.ConnectorOutputTableHandle; import java.util.Set; @@ -24,6 +25,6 @@ public record FakerOutputTableHandle(long table, Set<Long> activeTableIds) { public FakerOutputTableHandle { - requireNonNull(activeTableIds, "activeTableIds is null"); + activeTableIds = ImmutableSet.copyOf(requireNonNull(activeTableIds, "activeTableIds is null")); } } From 4f114e256dcb176eb98ce8bfd7e2d224a6469973 Mon Sep 17 00:00:00 2001 From: Yuya Ebihara <ebyhry@gmail.com> Date: Fri, 25 Oct 2024 09:25:51 +0900 Subject: [PATCH 03/10] Follow test guideline in Faker --- .../io/trino/plugin/faker/TestFakerQueries.java | 16 ++++++++-------- .../plugin/faker/TestFakerSplitManager.java | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/plugin/trino-faker/src/test/java/io/trino/plugin/faker/TestFakerQueries.java b/plugin/trino-faker/src/test/java/io/trino/plugin/faker/TestFakerQueries.java index df4a061cc31e..3b1a7ea23595 100644 --- a/plugin/trino-faker/src/test/java/io/trino/plugin/faker/TestFakerQueries.java +++ b/plugin/trino-faker/src/test/java/io/trino/plugin/faker/TestFakerQueries.java @@ -18,7 +18,7 @@ import org.intellij.lang.annotations.Language; import org.junit.jupiter.api.Test; -public class TestFakerQueries +final class TestFakerQueries extends AbstractTestQueryFramework { @Override @@ -29,7 +29,7 @@ protected QueryRunner createQueryRunner() } @Test - public void showTables() + void testShowTables() { assertQuery("SHOW SCHEMAS FROM faker", "VALUES 'default', 'information_schema'"); assertUpdate("CREATE TABLE faker.default.test (id INTEGER, name VARCHAR)"); @@ -37,7 +37,7 @@ public void showTables() } @Test - public void selectFromTable() + void testSelectFromTable() { @Language("SQL") String tableQuery = """ @@ -180,7 +180,7 @@ rnd_nchar char(1000) NOT NULL, } @Test - public void selectLimit() + void testSelectLimit() { @Language("SQL") String tableQuery = "CREATE TABLE faker.default.single_column (rnd_bigint bigint NOT NULL)"; @@ -196,7 +196,7 @@ public void selectLimit() } @Test - public void selectDefaultTableLimit() + void testSelectDefaultTableLimit() { @Language("SQL") String tableQuery = "CREATE TABLE faker.default.default_table_limit (rnd_bigint bigint NOT NULL) WITH (default_limit = 100)"; @@ -229,7 +229,7 @@ public void selectOnlyNulls() } @Test - public void selectGenerator() + void testSelectGenerator() { @Language("SQL") String tableQuery = "CREATE TABLE faker.default.generators (" + @@ -246,7 +246,7 @@ public void selectGenerator() } @Test - public void selectFunctions() + void testSelectFunctions() { @Language("SQL") String testQuery = "SELECT random_string('#{options.option ''a'', ''b''}') IN ('a', 'b')"; @@ -254,7 +254,7 @@ public void selectFunctions() } @Test - public void selectRange() + void testSelectRange() { @Language("SQL") String tableQuery = """ diff --git a/plugin/trino-faker/src/test/java/io/trino/plugin/faker/TestFakerSplitManager.java b/plugin/trino-faker/src/test/java/io/trino/plugin/faker/TestFakerSplitManager.java index afbd90dde168..094e2c4265eb 100644 --- a/plugin/trino-faker/src/test/java/io/trino/plugin/faker/TestFakerSplitManager.java +++ b/plugin/trino-faker/src/test/java/io/trino/plugin/faker/TestFakerSplitManager.java @@ -34,10 +34,10 @@ import static io.trino.plugin.faker.FakerSplitManager.MAX_ROWS_PER_SPLIT; import static org.assertj.core.api.Assertions.assertThat; -public class TestFakerSplitManager +final class TestFakerSplitManager { @Test - public void testSplits() + void testSplits() throws ExecutionException, InterruptedException { URI uriA = URI.create("http://a"); From 614fb61b768d7a67b6c61fad39d725ac2f513e84 Mon Sep 17 00:00:00 2001 From: Yuya Ebihara <ebyhry@gmail.com> Date: Fri, 25 Oct 2024 09:41:23 +0900 Subject: [PATCH 04/10] Remove unused FakerInsertTableHandle --- .../plugin/faker/FakerInsertTableHandle.java | 28 ------------------- 1 file changed, 28 deletions(-) delete mode 100644 plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerInsertTableHandle.java diff --git a/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerInsertTableHandle.java b/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerInsertTableHandle.java deleted file mode 100644 index e4108932d5f7..000000000000 --- a/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerInsertTableHandle.java +++ /dev/null @@ -1,28 +0,0 @@ -/* - * 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.faker; - -import io.trino.spi.connector.ConnectorInsertTableHandle; - -import static java.util.Objects.requireNonNull; - -public record FakerInsertTableHandle(FakerTableHandle tableHandle) - implements ConnectorInsertTableHandle -{ - public FakerInsertTableHandle - { - requireNonNull(tableHandle, "tableHandle is null"); - } -} From 03aa818be85ea51c5352277becde8ffdc79c43fa Mon Sep 17 00:00:00 2001 From: Yuya Ebihara <ebyhry@gmail.com> Date: Fri, 25 Oct 2024 09:47:34 +0900 Subject: [PATCH 05/10] Test FakerConfig --- .../trino/plugin/faker/TestFakerConfig.java | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 plugin/trino-faker/src/test/java/io/trino/plugin/faker/TestFakerConfig.java diff --git a/plugin/trino-faker/src/test/java/io/trino/plugin/faker/TestFakerConfig.java b/plugin/trino-faker/src/test/java/io/trino/plugin/faker/TestFakerConfig.java new file mode 100644 index 000000000000..000cbc0c5e76 --- /dev/null +++ b/plugin/trino-faker/src/test/java/io/trino/plugin/faker/TestFakerConfig.java @@ -0,0 +1,49 @@ +/* + * 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.faker; + +import com.google.common.collect.ImmutableMap; +import org.junit.jupiter.api.Test; + +import java.util.Map; + +import static io.airlift.configuration.testing.ConfigAssertions.assertFullMapping; +import static io.airlift.configuration.testing.ConfigAssertions.assertRecordedDefaults; +import static io.airlift.configuration.testing.ConfigAssertions.recordDefaults; + +final class TestFakerConfig +{ + @Test + void testDefaults() + { + assertRecordedDefaults(recordDefaults(FakerConfig.class) + .setNullProbability(0.5) + .setDefaultLimit(1000L)); + } + + @Test + void testExplicitPropertyMappings() + { + Map<String, String> properties = ImmutableMap.<String, String>builder() + .put("faker.null-probability", "1.0") + .put("faker.default-limit", "10") + .buildOrThrow(); + + FakerConfig expected = new FakerConfig() + .setNullProbability(1.0) + .setDefaultLimit(10L); + + assertFullMapping(properties, expected); + } +} From ba000b79352c7ea2b511bdfc470113dfb7f2ac6c Mon Sep 17 00:00:00 2001 From: Yuya Ebihara <ebyhry@gmail.com> Date: Fri, 25 Oct 2024 09:49:08 +0900 Subject: [PATCH 06/10] Test FakerPlugin --- .../trino/plugin/faker/TestFakerPlugin.java | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 plugin/trino-faker/src/test/java/io/trino/plugin/faker/TestFakerPlugin.java diff --git a/plugin/trino-faker/src/test/java/io/trino/plugin/faker/TestFakerPlugin.java b/plugin/trino-faker/src/test/java/io/trino/plugin/faker/TestFakerPlugin.java new file mode 100644 index 000000000000..5dc7abfd9851 --- /dev/null +++ b/plugin/trino-faker/src/test/java/io/trino/plugin/faker/TestFakerPlugin.java @@ -0,0 +1,37 @@ +/* + * 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.faker; + +import com.google.common.collect.ImmutableMap; +import io.trino.spi.connector.ConnectorFactory; +import io.trino.testing.TestingConnectorContext; +import org.junit.jupiter.api.Test; + +import static com.google.common.collect.Iterables.getOnlyElement; + +final class TestFakerPlugin +{ + @Test + void testCreateConnector() + { + FakerPlugin plugin = new FakerPlugin(); + + ConnectorFactory factory = getOnlyElement(plugin.getConnectorFactories()); + factory.create( + "test", + ImmutableMap.of("bootstrap.quiet", "true"), + new TestingConnectorContext()) + .shutdown(); + } +} From deb62685e47afb10535925856c5f351bf0007d57 Mon Sep 17 00:00:00 2001 From: Yuya Ebihara <ebyhry@gmail.com> Date: Fri, 25 Oct 2024 10:08:06 +0900 Subject: [PATCH 07/10] Fix error code and message in Faker --- .../io/trino/plugin/faker/FakerMetadata.java | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerMetadata.java b/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerMetadata.java index fe159cc7e9bf..16d5525a5e12 100644 --- a/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerMetadata.java +++ b/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerMetadata.java @@ -18,7 +18,6 @@ import com.google.common.collect.ImmutableSet; import com.google.errorprone.annotations.concurrent.GuardedBy; import io.airlift.slice.Slice; -import io.trino.spi.StandardErrorCode; import io.trino.spi.TrinoException; import io.trino.spi.connector.ColumnHandle; import io.trino.spi.connector.ColumnMetadata; @@ -62,10 +61,12 @@ import static com.google.common.base.Verify.verify; import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableMap.toImmutableMap; -import static io.trino.spi.StandardErrorCode.ALREADY_EXISTS; import static io.trino.spi.StandardErrorCode.INVALID_COLUMN_PROPERTY; import static io.trino.spi.StandardErrorCode.INVALID_TABLE_FUNCTION_INVOCATION; -import static io.trino.spi.StandardErrorCode.NOT_FOUND; +import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED; +import static io.trino.spi.StandardErrorCode.SCHEMA_ALREADY_EXISTS; +import static io.trino.spi.StandardErrorCode.SCHEMA_NOT_FOUND; +import static io.trino.spi.StandardErrorCode.TABLE_ALREADY_EXISTS; import static io.trino.spi.connector.RetryMode.NO_RETRIES; import static java.lang.String.format; import static java.util.Objects.requireNonNull; @@ -106,7 +107,7 @@ public synchronized List<String> listSchemaNames(ConnectorSession connectorSessi public synchronized void createSchema(ConnectorSession session, String schemaName, Map<String, Object> properties, TrinoPrincipal owner) { if (schemas.stream().anyMatch(schema -> schema.name().equals(schemaName))) { - throw new TrinoException(ALREADY_EXISTS, format("Schema [%s] already exists", schemaName)); + throw new TrinoException(SCHEMA_ALREADY_EXISTS, format("Schema '%s' already exists", schemaName)); } schemas.add(new SchemaInfo(schemaName, properties)); } @@ -123,7 +124,7 @@ private synchronized SchemaInfo getSchema(String name) .filter(schemaInfo -> schemaInfo.name().equals(name)) .findAny(); if (schema.isEmpty()) { - throw new TrinoException(NOT_FOUND, format("Schema [%s] does not exist", name)); + throw new TrinoException(SCHEMA_NOT_FOUND, format("Schema '%s' does not exist", name)); } return schema.get(); } @@ -132,7 +133,7 @@ private synchronized SchemaInfo getSchema(String name) public synchronized ConnectorTableHandle getTableHandle(ConnectorSession session, SchemaTableName tableName, Optional<ConnectorTableVersion> startVersion, Optional<ConnectorTableVersion> endVersion) { if (startVersion.isPresent() || endVersion.isPresent()) { - throw new TrinoException(StandardErrorCode.NOT_SUPPORTED, "This connector does not support versioned tables"); + throw new TrinoException(NOT_SUPPORTED, "This connector does not support versioned tables"); } SchemaInfo schema = getSchema(tableName.getSchemaName()); Long id = tableIds.get(tableName); @@ -282,7 +283,7 @@ public synchronized void setColumnComment(ConnectorSession session, ConnectorTab public synchronized void createTable(ConnectorSession session, ConnectorTableMetadata tableMetadata, SaveMode saveMode) { if (saveMode == SaveMode.REPLACE) { - throw new TrinoException(StandardErrorCode.NOT_SUPPORTED, "This connector does not support replacing tables"); + throw new TrinoException(NOT_SUPPORTED, "This connector does not support replacing tables"); } ConnectorOutputTableHandle outputTableHandle = beginCreateTable(session, tableMetadata, Optional.empty(), NO_RETRIES, false); finishCreateTable(session, outputTableHandle, ImmutableList.of(), ImmutableList.of()); @@ -292,7 +293,7 @@ public synchronized void createTable(ConnectorSession session, ConnectorTableMet public synchronized FakerOutputTableHandle beginCreateTable(ConnectorSession session, ConnectorTableMetadata tableMetadata, Optional<ConnectorTableLayout> layout, RetryMode retryMode, boolean replace) { if (replace) { - throw new TrinoException(StandardErrorCode.NOT_SUPPORTED, "This connector does not support replacing tables"); + throw new TrinoException(NOT_SUPPORTED, "This connector does not support replacing tables"); } SchemaInfo schema = getSchema(tableMetadata.getTable().getSchemaName()); checkTableNotExists(tableMetadata.getTable()); @@ -349,7 +350,7 @@ private synchronized void checkSchemaExists(String schemaName) private synchronized void checkTableNotExists(SchemaTableName tableName) { if (tableIds.containsKey(tableName)) { - throw new TrinoException(ALREADY_EXISTS, format("Table [%s] already exists", tableName)); + throw new TrinoException(TABLE_ALREADY_EXISTS, format("Table '%s' already exists", tableName)); } } @@ -377,7 +378,7 @@ public synchronized Map<String, Object> getSchemaProperties(ConnectorSession ses .filter(s -> s.name().equals(schemaName)) .findAny(); if (schema.isEmpty()) { - throw new TrinoException(NOT_FOUND, format("Schema [%s] does not exist", schemaName)); + throw new TrinoException(SCHEMA_NOT_FOUND, format("Schema '%s' does not exist", schemaName)); } return schema.get().properties(); From 97cd67cdbf81d98c7db65549e97edbc5172dfd75 Mon Sep 17 00:00:00 2001 From: Yuya Ebihara <ebyhry@gmail.com> Date: Fri, 25 Oct 2024 10:09:51 +0900 Subject: [PATCH 08/10] Make id primitive in FakerTableHandle --- .../src/main/java/io/trino/plugin/faker/FakerMetadata.java | 7 ------- .../main/java/io/trino/plugin/faker/FakerTableHandle.java | 3 +-- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerMetadata.java b/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerMetadata.java index 16d5525a5e12..198335330d5a 100644 --- a/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerMetadata.java +++ b/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerMetadata.java @@ -62,7 +62,6 @@ import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableMap.toImmutableMap; import static io.trino.spi.StandardErrorCode.INVALID_COLUMN_PROPERTY; -import static io.trino.spi.StandardErrorCode.INVALID_TABLE_FUNCTION_INVOCATION; import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED; import static io.trino.spi.StandardErrorCode.SCHEMA_ALREADY_EXISTS; import static io.trino.spi.StandardErrorCode.SCHEMA_NOT_FOUND; @@ -151,9 +150,6 @@ public synchronized ConnectorTableMetadata getTableMetadata( ConnectorTableHandle connectorTableHandle) { FakerTableHandle tableHandle = (FakerTableHandle) connectorTableHandle; - if (tableHandle.id() == null) { - throw new TrinoException(INVALID_TABLE_FUNCTION_INVOCATION, "Table functions are not supported"); - } return tables.get(tableHandle.id()).metadata(); } @@ -184,9 +180,6 @@ public synchronized ColumnMetadata getColumnMetadata( ColumnHandle columnHandle) { FakerTableHandle tableHandle = (FakerTableHandle) connectorTableHandle; - if (tableHandle.id() == null) { - throw new TrinoException(INVALID_TABLE_FUNCTION_INVOCATION, "Table functions are not supported"); - } return tables.get(tableHandle.id()) .column(columnHandle) .metadata(); diff --git a/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerTableHandle.java b/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerTableHandle.java index 9ed8bd7bf091..ab702606e5e9 100644 --- a/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerTableHandle.java +++ b/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerTableHandle.java @@ -21,12 +21,11 @@ import static java.util.Objects.requireNonNull; -public record FakerTableHandle(Long id, SchemaTableName schemaTableName, TupleDomain<ColumnHandle> constraint, long limit) +public record FakerTableHandle(long id, SchemaTableName schemaTableName, TupleDomain<ColumnHandle> constraint, long limit) implements ConnectorTableHandle { public FakerTableHandle { - requireNonNull(id, "id is null"); requireNonNull(schemaTableName, "schemaTableName is null"); requireNonNull(constraint, "constraint is null"); } From 31278de714a1774fc5a40012a37eb064487db30a Mon Sep 17 00:00:00 2001 From: Yuya Ebihara <ebyhry@gmail.com> Date: Fri, 25 Oct 2024 10:37:45 +0900 Subject: [PATCH 09/10] Remove unused field from FakerOutputTableHandle --- .../java/io/trino/plugin/faker/FakerMetadata.java | 3 +-- .../io/trino/plugin/faker/FakerOutputTableHandle.java | 11 +---------- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerMetadata.java b/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerMetadata.java index 198335330d5a..cd8d662a6496 100644 --- a/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerMetadata.java +++ b/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerMetadata.java @@ -15,7 +15,6 @@ package io.trino.plugin.faker; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; import com.google.errorprone.annotations.concurrent.GuardedBy; import io.airlift.slice.Slice; import io.trino.spi.TrinoException; @@ -325,7 +324,7 @@ public synchronized FakerOutputTableHandle beginCreateTable(ConnectorSession ses tableMetadata.getProperties(), tableMetadata.getComment())); - return new FakerOutputTableHandle(tableId, ImmutableSet.copyOf(tableIds.values())); + return new FakerOutputTableHandle(tableId); } private boolean isCharacterColumn(ColumnMetadata column) diff --git a/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerOutputTableHandle.java b/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerOutputTableHandle.java index 64a5178158c2..406627dfa6be 100644 --- a/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerOutputTableHandle.java +++ b/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerOutputTableHandle.java @@ -13,18 +13,9 @@ */ package io.trino.plugin.faker; -import com.google.common.collect.ImmutableSet; import io.trino.spi.connector.ConnectorOutputTableHandle; -import java.util.Set; - -import static java.util.Objects.requireNonNull; - -public record FakerOutputTableHandle(long table, Set<Long> activeTableIds) +public record FakerOutputTableHandle(long table) implements ConnectorOutputTableHandle { - public FakerOutputTableHandle - { - activeTableIds = ImmutableSet.copyOf(requireNonNull(activeTableIds, "activeTableIds is null")); - } } From a066f3bca2364faadca6348ae9e47a84c5d21a00 Mon Sep 17 00:00:00 2001 From: Yuya Ebihara <ebyhry@gmail.com> Date: Fri, 25 Oct 2024 10:54:56 +0900 Subject: [PATCH 10/10] Remove table id from Faker --- .../io/trino/plugin/faker/FakerMetadata.java | 101 ++++++++---------- .../plugin/faker/FakerOutputTableHandle.java | 9 +- .../trino/plugin/faker/FakerTableHandle.java | 6 +- .../java/io/trino/plugin/faker/TableInfo.java | 29 +---- .../plugin/faker/TestFakerSplitManager.java | 2 +- 5 files changed, 59 insertions(+), 88 deletions(-) diff --git a/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerMetadata.java b/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerMetadata.java index cd8d662a6496..392d33069347 100644 --- a/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerMetadata.java +++ b/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerMetadata.java @@ -53,7 +53,6 @@ import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.concurrent.atomic.AtomicLong; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -79,11 +78,8 @@ public class FakerMetadata private final double nullProbability; private final long defaultLimit; - private final AtomicLong nextTableId = new AtomicLong(); @GuardedBy("this") - private final Map<SchemaTableName, Long> tableIds = new HashMap<>(); - @GuardedBy("this") - private final Map<Long, TableInfo> tables = new HashMap<>(); + private final Map<SchemaTableName, TableInfo> tables = new HashMap<>(); @Inject public FakerMetadata(FakerConfig config) @@ -134,13 +130,13 @@ public synchronized ConnectorTableHandle getTableHandle(ConnectorSession session throw new TrinoException(NOT_SUPPORTED, "This connector does not support versioned tables"); } SchemaInfo schema = getSchema(tableName.getSchemaName()); - Long id = tableIds.get(tableName); - if (id == null) { + TableInfo tableInfo = tables.get(tableName); + if (tableInfo == null) { return null; } long schemaLimit = (long) schema.properties().getOrDefault(SchemaInfo.DEFAULT_LIMIT_PROPERTY, defaultLimit); - long tableLimit = (long) tables.get(id).properties().getOrDefault(TableInfo.DEFAULT_LIMIT_PROPERTY, schemaLimit); - return new FakerTableHandle(id, tableName, TupleDomain.all(), tableLimit); + long tableLimit = (long) tables.get(tableName).properties().getOrDefault(TableInfo.DEFAULT_LIMIT_PROPERTY, schemaLimit); + return new FakerTableHandle(tableName, TupleDomain.all(), tableLimit); } @Override @@ -149,15 +145,20 @@ public synchronized ConnectorTableMetadata getTableMetadata( ConnectorTableHandle connectorTableHandle) { FakerTableHandle tableHandle = (FakerTableHandle) connectorTableHandle; - return tables.get(tableHandle.id()).metadata(); + SchemaTableName name = tableHandle.schemaTableName(); + TableInfo tableInfo = tables.get(tableHandle.schemaTableName()); + return new ConnectorTableMetadata( + name, + tableInfo.columns().stream().map(ColumnInfo::metadata).collect(toImmutableList()), + tableInfo.properties(), + tableInfo.comment()); } @Override public synchronized List<SchemaTableName> listTables(ConnectorSession session, Optional<String> schemaName) { - return tables.values().stream() - .filter(table -> schemaName.map(table.schemaName()::contentEquals).orElse(true)) - .map(TableInfo::schemaTableName) + return tables.keySet().stream() + .filter(table -> schemaName.map(table.getSchemaName()::contentEquals).orElse(true)) .collect(toImmutableList()); } @@ -167,7 +168,7 @@ public synchronized Map<String, ColumnHandle> getColumnHandles( ConnectorTableHandle connectorTableHandle) { FakerTableHandle tableHandle = (FakerTableHandle) connectorTableHandle; - return tables.get(tableHandle.id()) + return tables.get(tableHandle.schemaTableName()) .columns().stream() .collect(toImmutableMap(ColumnInfo::name, ColumnInfo::handle)); } @@ -179,7 +180,7 @@ public synchronized ColumnMetadata getColumnMetadata( ColumnHandle columnHandle) { FakerTableHandle tableHandle = (FakerTableHandle) connectorTableHandle; - return tables.get(tableHandle.id()) + return tables.get(tableHandle.schemaTableName()) .column(columnHandle) .metadata(); } @@ -187,11 +188,11 @@ public synchronized ColumnMetadata getColumnMetadata( @Override public synchronized Iterator<TableColumnsMetadata> streamTableColumns(ConnectorSession session, SchemaTablePrefix prefix) { - return tables.values().stream() - .filter(table -> prefix.matches(table.schemaTableName())) - .map(table -> TableColumnsMetadata.forTable( - table.schemaTableName(), - table.metadata().getColumns())) + return tables.entrySet().stream() + .filter(entry -> prefix.matches(entry.getKey())) + .map(entry -> TableColumnsMetadata.forTable( + entry.getKey(), + entry.getValue().columns().stream().map(ColumnInfo::metadata).collect(toImmutableList()))) .iterator(); } @@ -199,10 +200,7 @@ public synchronized Iterator<TableColumnsMetadata> streamTableColumns(ConnectorS public synchronized void dropTable(ConnectorSession session, ConnectorTableHandle tableHandle) { FakerTableHandle handle = (FakerTableHandle) tableHandle; - TableInfo info = tables.remove(handle.id()); - if (info != null) { - tableIds.remove(info.schemaTableName()); - } + tables.remove(handle.schemaTableName()); } @Override @@ -212,54 +210,45 @@ public synchronized void renameTable(ConnectorSession session, ConnectorTableHan checkTableNotExists(newTableName); FakerTableHandle handle = (FakerTableHandle) tableHandle; - long tableId = handle.id(); - - TableInfo oldInfo = tables.get(tableId); - tables.put(tableId, new TableInfo( - tableId, - newTableName.getSchemaName(), - newTableName.getTableName(), - oldInfo.columns(), - oldInfo.properties(), - oldInfo.comment())); - - tableIds.remove(oldInfo.schemaTableName()); - tableIds.put(newTableName, tableId); + SchemaTableName oldTableName = handle.schemaTableName(); + + tables.remove(oldTableName); + tables.put(newTableName, tables.get(oldTableName)); } @Override public synchronized void setTableProperties(ConnectorSession session, ConnectorTableHandle tableHandle, Map<String, Optional<Object>> properties) { FakerTableHandle handle = (FakerTableHandle) tableHandle; - long tableId = handle.id(); + SchemaTableName tableName = handle.schemaTableName(); - TableInfo oldInfo = tables.get(tableId); + TableInfo oldInfo = tables.get(tableName); Map<String, Object> newProperties = Stream.concat( oldInfo.properties().entrySet().stream() .filter(entry -> !properties.containsKey(entry.getKey())), properties.entrySet().stream() .filter(entry -> entry.getValue().isPresent())) .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); - tables.put(tableId, oldInfo.withProperties(newProperties)); + tables.put(tableName, oldInfo.withProperties(newProperties)); } @Override public synchronized void setTableComment(ConnectorSession session, ConnectorTableHandle tableHandle, Optional<String> comment) { FakerTableHandle handle = (FakerTableHandle) tableHandle; - long tableId = handle.id(); + SchemaTableName tableName = handle.schemaTableName(); - TableInfo oldInfo = requireNonNull(tables.get(tableId), "tableInfo is null"); - tables.put(tableId, oldInfo.withComment(comment)); + TableInfo oldInfo = requireNonNull(tables.get(tableName), "tableInfo is null"); + tables.put(tableName, oldInfo.withComment(comment)); } @Override public synchronized void setColumnComment(ConnectorSession session, ConnectorTableHandle tableHandle, ColumnHandle column, Optional<String> comment) { FakerTableHandle handle = (FakerTableHandle) tableHandle; - long tableId = handle.id(); + SchemaTableName tableName = handle.schemaTableName(); - TableInfo oldInfo = tables.get(tableId); + TableInfo oldInfo = tables.get(tableName); List<ColumnInfo> columns = oldInfo.columns().stream() .map(columnInfo -> { if (columnInfo.handle().equals(column)) { @@ -268,7 +257,7 @@ public synchronized void setColumnComment(ConnectorSession session, ConnectorTab return columnInfo; }) .collect(toImmutableList()); - tables.put(tableId, oldInfo.withColumns(columns)); + tables.put(tableName, oldInfo.withColumns(columns)); } @Override @@ -287,9 +276,9 @@ public synchronized FakerOutputTableHandle beginCreateTable(ConnectorSession ses if (replace) { throw new TrinoException(NOT_SUPPORTED, "This connector does not support replacing tables"); } - SchemaInfo schema = getSchema(tableMetadata.getTable().getSchemaName()); + SchemaTableName tableName = tableMetadata.getTable(); + SchemaInfo schema = getSchema(tableName.getSchemaName()); checkTableNotExists(tableMetadata.getTable()); - long tableId = nextTableId.getAndIncrement(); double schemaNullProbability = (double) schema.properties().getOrDefault(SchemaInfo.NULL_PROBABILITY_PROPERTY, nullProbability); double tableNullProbability = (double) tableMetadata.getProperties().getOrDefault(TableInfo.NULL_PROBABILITY_PROPERTY, schemaNullProbability); @@ -315,16 +304,12 @@ public synchronized FakerOutputTableHandle beginCreateTable(ConnectorSession ses column)); } - tableIds.put(tableMetadata.getTable(), tableId); - tables.put(tableId, new TableInfo( - tableId, - tableMetadata.getTable().getSchemaName(), - tableMetadata.getTable().getTableName(), + tables.put(tableName, new TableInfo( columns.build(), tableMetadata.getProperties(), tableMetadata.getComment())); - return new FakerOutputTableHandle(tableId); + return new FakerOutputTableHandle(tableName); } private boolean isCharacterColumn(ColumnMetadata column) @@ -341,7 +326,7 @@ private synchronized void checkSchemaExists(String schemaName) private synchronized void checkTableNotExists(SchemaTableName tableName) { - if (tableIds.containsKey(tableName)) { + if (tables.containsKey(tableName)) { throw new TrinoException(TABLE_ALREADY_EXISTS, format("Table '%s' already exists", tableName)); } } @@ -352,14 +337,14 @@ public synchronized Optional<ConnectorOutputMetadata> finishCreateTable(Connecto requireNonNull(tableHandle, "tableHandle is null"); FakerOutputTableHandle fakerOutputHandle = (FakerOutputTableHandle) tableHandle; - long tableId = fakerOutputHandle.table(); + SchemaTableName tableName = fakerOutputHandle.schemaTableName(); - TableInfo info = tables.get(tableId); + TableInfo info = tables.get(tableName); requireNonNull(info, "info is null"); // TODO ensure fragments is empty? - tables.put(tableId, new TableInfo(tableId, info.schemaName(), info.tableName(), info.columns(), info.properties(), info.comment())); + tables.put(tableName, new TableInfo(info.columns(), info.properties(), info.comment())); return Optional.empty(); } diff --git a/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerOutputTableHandle.java b/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerOutputTableHandle.java index 406627dfa6be..186fb4708562 100644 --- a/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerOutputTableHandle.java +++ b/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerOutputTableHandle.java @@ -14,8 +14,15 @@ package io.trino.plugin.faker; import io.trino.spi.connector.ConnectorOutputTableHandle; +import io.trino.spi.connector.SchemaTableName; -public record FakerOutputTableHandle(long table) +import static java.util.Objects.requireNonNull; + +public record FakerOutputTableHandle(SchemaTableName schemaTableName) implements ConnectorOutputTableHandle { + public FakerOutputTableHandle + { + requireNonNull(schemaTableName, "schemaTableName is null"); + } } diff --git a/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerTableHandle.java b/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerTableHandle.java index ab702606e5e9..e9535e7126ce 100644 --- a/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerTableHandle.java +++ b/plugin/trino-faker/src/main/java/io/trino/plugin/faker/FakerTableHandle.java @@ -21,7 +21,7 @@ import static java.util.Objects.requireNonNull; -public record FakerTableHandle(long id, SchemaTableName schemaTableName, TupleDomain<ColumnHandle> constraint, long limit) +public record FakerTableHandle(SchemaTableName schemaTableName, TupleDomain<ColumnHandle> constraint, long limit) implements ConnectorTableHandle { public FakerTableHandle @@ -32,11 +32,11 @@ public record FakerTableHandle(long id, SchemaTableName schemaTableName, TupleDo public FakerTableHandle withConstraint(TupleDomain<ColumnHandle> constraint) { - return new FakerTableHandle(id, schemaTableName, constraint, limit); + return new FakerTableHandle(schemaTableName, constraint, limit); } public FakerTableHandle withLimit(long limit) { - return new FakerTableHandle(id, schemaTableName, constraint, limit); + return new FakerTableHandle(schemaTableName, constraint, limit); } } diff --git a/plugin/trino-faker/src/main/java/io/trino/plugin/faker/TableInfo.java b/plugin/trino-faker/src/main/java/io/trino/plugin/faker/TableInfo.java index 0499f870e0a2..3f1efcd9eba1 100644 --- a/plugin/trino-faker/src/main/java/io/trino/plugin/faker/TableInfo.java +++ b/plugin/trino-faker/src/main/java/io/trino/plugin/faker/TableInfo.java @@ -16,46 +16,25 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import io.trino.spi.connector.ColumnHandle; -import io.trino.spi.connector.ConnectorTableMetadata; -import io.trino.spi.connector.SchemaTableName; import java.util.List; import java.util.Map; import java.util.Optional; -import static com.google.common.collect.ImmutableList.toImmutableList; import static java.util.Objects.requireNonNull; -public record TableInfo(long id, String schemaName, String tableName, List<ColumnInfo> columns, Map<String, Object> properties, Optional<String> comment) +public record TableInfo(List<ColumnInfo> columns, Map<String, Object> properties, Optional<String> comment) { public static final String NULL_PROBABILITY_PROPERTY = "null_probability"; public static final String DEFAULT_LIMIT_PROPERTY = "default_limit"; public TableInfo { - requireNonNull(schemaName, "schemaName is null"); - requireNonNull(tableName, "tableName is null"); columns = ImmutableList.copyOf(columns); properties = ImmutableMap.copyOf(requireNonNull(properties, "properties is null")); requireNonNull(comment, "comment is null"); } - public SchemaTableName schemaTableName() - { - return new SchemaTableName(schemaName, tableName); - } - - public ConnectorTableMetadata metadata() - { - return new ConnectorTableMetadata( - new SchemaTableName(schemaName, tableName), - columns.stream() - .map(ColumnInfo::metadata) - .collect(toImmutableList()), - properties, - comment); - } - public ColumnInfo column(ColumnHandle handle) { return columns.stream() @@ -66,16 +45,16 @@ public ColumnInfo column(ColumnHandle handle) public TableInfo withColumns(List<ColumnInfo> columns) { - return new TableInfo(id, schemaName, tableName, columns, properties, comment); + return new TableInfo(columns, properties, comment); } public TableInfo withProperties(Map<String, Object> properties) { - return new TableInfo(id, schemaName, tableName, columns, properties, comment); + return new TableInfo(columns, properties, comment); } public TableInfo withComment(Optional<String> comment) { - return new TableInfo(id, schemaName, tableName, columns, properties, comment); + return new TableInfo(columns, properties, comment); } } diff --git a/plugin/trino-faker/src/test/java/io/trino/plugin/faker/TestFakerSplitManager.java b/plugin/trino-faker/src/test/java/io/trino/plugin/faker/TestFakerSplitManager.java index 094e2c4265eb..d0503283814a 100644 --- a/plugin/trino-faker/src/test/java/io/trino/plugin/faker/TestFakerSplitManager.java +++ b/plugin/trino-faker/src/test/java/io/trino/plugin/faker/TestFakerSplitManager.java @@ -51,7 +51,7 @@ void testSplits() ConnectorSplitSource splitSource = new FakerSplitManager(nodeManager).getSplits( TestingTransactionHandle.create(), TestingConnectorSession.SESSION, - new FakerTableHandle(1L, new SchemaTableName("schema", "table"), TupleDomain.all(), expectedRows), + new FakerTableHandle(new SchemaTableName("schema", "table"), TupleDomain.all(), expectedRows), DynamicFilter.EMPTY, Constraint.alwaysTrue()); List<ConnectorSplit> splits = splitSource.getNextBatch(1_000_000).get().getSplits();