From 17b55a46502e31975b9f61be3f17663a9596e848 Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Thu, 30 May 2024 21:34:04 +0900 Subject: [PATCH 01/19] Convert RedisQueryRunner to builder --- plugin/trino-redis/pom.xml | 6 + .../trino/plugin/redis/RedisQueryRunner.java | 127 ++++++++++-------- .../plugin/redis/TestRedisConnectorTest.java | 8 +- .../redis/TestRedisDistributedHash.java | 7 +- .../redis/TestRedisLatestConnectorTest.java | 11 +- 5 files changed, 92 insertions(+), 67 deletions(-) diff --git a/plugin/trino-redis/pom.xml b/plugin/trino-redis/pom.xml index 38fd8be8b330..44df9ae51288 100644 --- a/plugin/trino-redis/pom.xml +++ b/plugin/trino-redis/pom.xml @@ -139,6 +139,12 @@ runtime + + com.google.errorprone + error_prone_annotations + runtime + + io.airlift log-manager diff --git a/plugin/trino-redis/src/test/java/io/trino/plugin/redis/RedisQueryRunner.java b/plugin/trino-redis/src/test/java/io/trino/plugin/redis/RedisQueryRunner.java index 1b03e0bb58a1..2552e847e9c5 100644 --- a/plugin/trino-redis/src/test/java/io/trino/plugin/redis/RedisQueryRunner.java +++ b/plugin/trino-redis/src/test/java/io/trino/plugin/redis/RedisQueryRunner.java @@ -13,11 +13,13 @@ */ package io.trino.plugin.redis; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.errorprone.annotations.CanIgnoreReturnValue; import io.airlift.json.JsonCodec; import io.airlift.log.Logger; -import io.trino.Session; import io.trino.metadata.QualifiedObjectName; +import io.trino.plugin.base.util.Closables; import io.trino.plugin.redis.util.CodecSupplier; import io.trino.plugin.redis.util.RedisServer; import io.trino.plugin.redis.util.RedisTestUtils; @@ -29,15 +31,17 @@ import io.trino.testing.TestingTrinoClient; import io.trino.tpch.TpchTable; +import java.util.HashMap; +import java.util.List; import java.util.Map; -import static io.airlift.testing.Closeables.closeAllSuppress; import static io.airlift.units.Duration.nanosSince; import static io.trino.plugin.redis.util.RedisTestUtils.installRedisPlugin; import static io.trino.plugin.redis.util.RedisTestUtils.loadTpchTableDescription; import static io.trino.plugin.tpch.TpchMetadata.TINY_SCHEMA_NAME; import static io.trino.testing.TestingSession.testSessionBuilder; import static java.util.Locale.ENGLISH; +import static java.util.Objects.requireNonNull; import static java.util.concurrent.TimeUnit.SECONDS; public final class RedisQueryRunner @@ -47,53 +51,77 @@ private RedisQueryRunner() {} private static final Logger log = Logger.get(RedisQueryRunner.class); private static final String TPCH_SCHEMA = "tpch"; - // TODO convert to builder - public static QueryRunner createRedisQueryRunner( - RedisServer redisServer, - Map connectorProperties, - String dataFormat, - Iterable> tables) - throws Exception + public static Builder builder(RedisServer redisServer) { - return createRedisQueryRunner(redisServer, ImmutableMap.of(), connectorProperties, dataFormat, tables); + return new Builder(redisServer); } - // TODO convert to builder - private static QueryRunner createRedisQueryRunner( - RedisServer redisServer, - Map coordinatorProperties, - Map connectorProperties, - String dataFormat, - Iterable> tables) - throws Exception + public static class Builder + extends DistributedQueryRunner.Builder { - DistributedQueryRunner queryRunner = null; - try { - queryRunner = DistributedQueryRunner.builder(createSession()) - .setCoordinatorProperties(coordinatorProperties) - .build(); - - queryRunner.installPlugin(new TpchPlugin()); - queryRunner.createCatalog("tpch", "tpch"); + private final RedisServer redisServer; + private final Map connectorProperties = new HashMap<>(); + private String dataFormat; + private List> initialTables = ImmutableList.of(); + + private Builder(RedisServer redisServer) + { + super(testSessionBuilder() + .setCatalog("redis") + .setSchema(TPCH_SCHEMA) + .build()); + this.redisServer = requireNonNull(redisServer, "redisServer is null"); + } - Map tableDescriptions = createTpchTableDescriptions(queryRunner.getPlannerContext().getTypeManager(), tables, dataFormat); + @CanIgnoreReturnValue + public Builder setDataFormat(String dataFormat) + { + this.dataFormat = requireNonNull(dataFormat, "dataFormat is null"); + return this; + } - installRedisPlugin(redisServer, queryRunner, tableDescriptions, connectorProperties); + @CanIgnoreReturnValue + public Builder addConnectorProperties(Map connectorProperties) + { + this.connectorProperties.putAll(connectorProperties); + return this; + } - TestingTrinoClient trinoClient = queryRunner.getClient(); + @CanIgnoreReturnValue + public Builder setInitialTables(List> initialTables) + { + this.initialTables = ImmutableList.copyOf(initialTables); + return this; + } - log.info("Loading data..."); - long startTime = System.nanoTime(); - for (TpchTable table : tables) { - loadTpchTable(redisServer, trinoClient, table, dataFormat); + @Override + public DistributedQueryRunner build() + throws Exception + { + DistributedQueryRunner queryRunner = super.build(); + try { + queryRunner.installPlugin(new TpchPlugin()); + queryRunner.createCatalog("tpch", "tpch"); + + Map tableDescriptions = createTpchTableDescriptions(queryRunner.getPlannerContext().getTypeManager(), initialTables, dataFormat); + + installRedisPlugin(redisServer, queryRunner, tableDescriptions, connectorProperties); + + TestingTrinoClient trinoClient = queryRunner.getClient(); + + log.info("Loading data..."); + long startTime = System.nanoTime(); + for (TpchTable table : initialTables) { + loadTpchTable(redisServer, trinoClient, table, dataFormat); + } + log.info("Loading complete in %s", nanosSince(startTime).toString(SECONDS)); + redisServer.destroyJedisPool(); + return queryRunner; + } + catch (Throwable e) { + Closables.closeAllSuppress(e, queryRunner); + throw e; } - log.info("Loading complete in %s", nanosSince(startTime).toString(SECONDS)); - redisServer.destroyJedisPool(); - return queryRunner; - } - catch (Throwable e) { - closeAllSuppress(e, queryRunner, redisServer); - throw e; } } @@ -130,23 +158,14 @@ private static Map createTpchTableDescri return tableDescriptions.buildOrThrow(); } - public static Session createSession() - { - return testSessionBuilder() - .setCatalog("redis") - .setSchema(TPCH_SCHEMA) - .build(); - } - public static void main(String[] args) throws Exception { - QueryRunner queryRunner = createRedisQueryRunner( - new RedisServer(), - ImmutableMap.of("http-server.http.port", "8080"), - ImmutableMap.of(), - "string", - TpchTable.getTables()); + QueryRunner queryRunner = RedisQueryRunner.builder(new RedisServer()) + .addCoordinatorProperty("http-server.http.port", "8080") + .setDataFormat("string") + .setInitialTables(TpchTable.getTables()) + .build(); Logger log = Logger.get(RedisQueryRunner.class); log.info("======== SERVER STARTED ========"); diff --git a/plugin/trino-redis/src/test/java/io/trino/plugin/redis/TestRedisConnectorTest.java b/plugin/trino-redis/src/test/java/io/trino/plugin/redis/TestRedisConnectorTest.java index 7eb7ff1c133d..243ec2f28502 100644 --- a/plugin/trino-redis/src/test/java/io/trino/plugin/redis/TestRedisConnectorTest.java +++ b/plugin/trino-redis/src/test/java/io/trino/plugin/redis/TestRedisConnectorTest.java @@ -13,12 +13,9 @@ */ package io.trino.plugin.redis; -import com.google.common.collect.ImmutableMap; import io.trino.plugin.redis.util.RedisServer; import io.trino.testing.QueryRunner; -import static io.trino.plugin.redis.RedisQueryRunner.createRedisQueryRunner; - public class TestRedisConnectorTest extends BaseRedisConnectorTest { @@ -27,6 +24,9 @@ protected QueryRunner createQueryRunner() throws Exception { RedisServer redisServer = closeAfterClass(new RedisServer()); - return createRedisQueryRunner(redisServer, ImmutableMap.of(), "string", REQUIRED_TPCH_TABLES); + return RedisQueryRunner.builder(redisServer) + .setDataFormat("string") + .setInitialTables(REQUIRED_TPCH_TABLES) + .build(); } } diff --git a/plugin/trino-redis/src/test/java/io/trino/plugin/redis/TestRedisDistributedHash.java b/plugin/trino-redis/src/test/java/io/trino/plugin/redis/TestRedisDistributedHash.java index cb696367cc34..caee0af3d350 100644 --- a/plugin/trino-redis/src/test/java/io/trino/plugin/redis/TestRedisDistributedHash.java +++ b/plugin/trino-redis/src/test/java/io/trino/plugin/redis/TestRedisDistributedHash.java @@ -13,14 +13,12 @@ */ package io.trino.plugin.redis; -import com.google.common.collect.ImmutableMap; import io.trino.plugin.redis.util.RedisServer; import io.trino.sql.planner.plan.FilterNode; import io.trino.testing.AbstractTestQueries; import io.trino.testing.QueryRunner; import org.junit.jupiter.api.Test; -import static io.trino.plugin.redis.RedisQueryRunner.createRedisQueryRunner; import static org.assertj.core.api.Assertions.assertThat; public class TestRedisDistributedHash @@ -31,7 +29,10 @@ protected QueryRunner createQueryRunner() throws Exception { RedisServer redisServer = closeAfterClass(new RedisServer()); - return createRedisQueryRunner(redisServer, ImmutableMap.of(), "hash", REQUIRED_TPCH_TABLES); + return RedisQueryRunner.builder(redisServer) + .setDataFormat("hash") + .setInitialTables(REQUIRED_TPCH_TABLES) + .build(); } @Test diff --git a/plugin/trino-redis/src/test/java/io/trino/plugin/redis/TestRedisLatestConnectorTest.java b/plugin/trino-redis/src/test/java/io/trino/plugin/redis/TestRedisLatestConnectorTest.java index a168cd61f21b..02849c6ce16a 100644 --- a/plugin/trino-redis/src/test/java/io/trino/plugin/redis/TestRedisLatestConnectorTest.java +++ b/plugin/trino-redis/src/test/java/io/trino/plugin/redis/TestRedisLatestConnectorTest.java @@ -17,7 +17,6 @@ import io.trino.plugin.redis.util.RedisServer; import io.trino.testing.QueryRunner; -import static io.trino.plugin.redis.RedisQueryRunner.createRedisQueryRunner; import static io.trino.plugin.redis.util.RedisServer.LATEST_VERSION; import static io.trino.plugin.redis.util.RedisServer.PASSWORD; import static io.trino.plugin.redis.util.RedisServer.USER; @@ -30,10 +29,10 @@ protected QueryRunner createQueryRunner() throws Exception { RedisServer redisServer = closeAfterClass(new RedisServer(LATEST_VERSION, true)); - return createRedisQueryRunner( - redisServer, - ImmutableMap.of("redis.user", USER, "redis.password", PASSWORD), - "string", - REQUIRED_TPCH_TABLES); + return RedisQueryRunner.builder(redisServer) + .addConnectorProperties(ImmutableMap.of("redis.user", USER, "redis.password", PASSWORD)) + .setDataFormat("string") + .setInitialTables(REQUIRED_TPCH_TABLES) + .build(); } } From e9db520f4bb76bab7c85eb6a25f56006cb76c90e Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Thu, 30 May 2024 21:38:09 +0900 Subject: [PATCH 02/19] Convert BlackHoleQueryRunner to builder --- .../blackhole/BlackHoleQueryRunner.java | 59 ++++++++++--------- .../plugin/blackhole/TestBlackHoleSmoke.java | 2 +- 2 files changed, 31 insertions(+), 30 deletions(-) diff --git a/plugin/trino-blackhole/src/test/java/io/trino/plugin/blackhole/BlackHoleQueryRunner.java b/plugin/trino-blackhole/src/test/java/io/trino/plugin/blackhole/BlackHoleQueryRunner.java index ef231495ba95..0834d16c2aaf 100644 --- a/plugin/trino-blackhole/src/test/java/io/trino/plugin/blackhole/BlackHoleQueryRunner.java +++ b/plugin/trino-blackhole/src/test/java/io/trino/plugin/blackhole/BlackHoleQueryRunner.java @@ -15,58 +15,59 @@ import com.google.common.collect.ImmutableMap; import io.airlift.log.Logger; -import io.trino.Session; +import io.trino.plugin.base.util.Closables; import io.trino.plugin.tpch.TpchPlugin; import io.trino.testing.DistributedQueryRunner; import io.trino.testing.QueryRunner; -import java.util.Map; - -import static io.airlift.testing.Closeables.closeAllSuppress; import static io.trino.testing.TestingSession.testSessionBuilder; public final class BlackHoleQueryRunner { private BlackHoleQueryRunner() {} - public static QueryRunner createQueryRunner() - throws Exception + public static Builder builder() { - return createQueryRunner(ImmutableMap.of()); + return new Builder(); } - // TODO convert to builder - private static QueryRunner createQueryRunner(Map coordinatorProperties) - throws Exception + public static class Builder + extends DistributedQueryRunner.Builder { - Session session = testSessionBuilder() - .setCatalog("blackhole") - .setSchema("default") - .build(); - - QueryRunner queryRunner = DistributedQueryRunner.builder(session) - .setCoordinatorProperties(coordinatorProperties) - .build(); - - try { - queryRunner.installPlugin(new BlackHolePlugin()); - queryRunner.createCatalog("blackhole", "blackhole", ImmutableMap.of()); + protected Builder() + { + super(testSessionBuilder() + .setCatalog("blackhole") + .setSchema("default") + .build()); + } - queryRunner.installPlugin(new TpchPlugin()); - queryRunner.createCatalog("tpch", "tpch", ImmutableMap.of()); + @Override + public DistributedQueryRunner build() + throws Exception + { + DistributedQueryRunner queryRunner = super.build(); + try { + queryRunner.installPlugin(new BlackHolePlugin()); + queryRunner.createCatalog("blackhole", "blackhole", ImmutableMap.of()); + queryRunner.installPlugin(new TpchPlugin()); + queryRunner.createCatalog("tpch", "tpch", ImmutableMap.of()); + } + catch (Throwable e) { + Closables.closeAllSuppress(e, queryRunner); + throw e; + } return queryRunner; } - catch (Exception e) { - closeAllSuppress(e, queryRunner); - throw e; - } } public static void main(String[] args) throws Exception { - QueryRunner queryRunner = createQueryRunner(ImmutableMap.of("http-server.http.port", "8080")); + QueryRunner queryRunner = builder() + .addCoordinatorProperty("http-server.http.port", "8080") + .build(); Logger log = Logger.get(BlackHoleQueryRunner.class); log.info("======== SERVER STARTED ========"); log.info("\n====\n%s\n====", queryRunner.getCoordinator().getBaseUrl()); diff --git a/plugin/trino-blackhole/src/test/java/io/trino/plugin/blackhole/TestBlackHoleSmoke.java b/plugin/trino-blackhole/src/test/java/io/trino/plugin/blackhole/TestBlackHoleSmoke.java index 31cb70ac7475..4b8f152fe575 100644 --- a/plugin/trino-blackhole/src/test/java/io/trino/plugin/blackhole/TestBlackHoleSmoke.java +++ b/plugin/trino-blackhole/src/test/java/io/trino/plugin/blackhole/TestBlackHoleSmoke.java @@ -59,7 +59,7 @@ public class TestBlackHoleSmoke protected QueryRunner createQueryRunner() throws Exception { - return BlackHoleQueryRunner.createQueryRunner(); + return BlackHoleQueryRunner.builder().build(); } @AfterAll From 9b4c3388fff3832409aec47e23ef2f691edd423a Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Thu, 30 May 2024 22:04:05 +0900 Subject: [PATCH 03/19] Convert AccumuloQueryRunner to builder --- .../plugin/accumulo/AccumuloQueryRunner.java | 84 +++++++++++-------- .../accumulo/TestAccumuloConnectorTest.java | 3 +- 2 files changed, 51 insertions(+), 36 deletions(-) diff --git a/plugin/trino-accumulo/src/test/java/io/trino/plugin/accumulo/AccumuloQueryRunner.java b/plugin/trino-accumulo/src/test/java/io/trino/plugin/accumulo/AccumuloQueryRunner.java index 4152540e52e9..3787e46a6c11 100644 --- a/plugin/trino-accumulo/src/test/java/io/trino/plugin/accumulo/AccumuloQueryRunner.java +++ b/plugin/trino-accumulo/src/test/java/io/trino/plugin/accumulo/AccumuloQueryRunner.java @@ -22,6 +22,7 @@ import io.trino.metadata.QualifiedObjectName; import io.trino.plugin.accumulo.conf.AccumuloConfig; import io.trino.plugin.accumulo.serializers.LexicoderRowSerializer; +import io.trino.plugin.base.util.Closables; import io.trino.plugin.tpch.TpchPlugin; import io.trino.testing.DistributedQueryRunner; import io.trino.testing.QueryRunner; @@ -54,46 +55,59 @@ public final class AccumuloQueryRunner private AccumuloQueryRunner() {} - public static synchronized QueryRunner createAccumuloQueryRunner() - throws Exception + public static Builder builder() { - return createAccumuloQueryRunner(ImmutableMap.of()); + return new Builder(); } - // TODO convert to builder - private static synchronized QueryRunner createAccumuloQueryRunner(Map coordinatorProperties) - throws Exception + public static class Builder + extends DistributedQueryRunner.Builder { - QueryRunner queryRunner = DistributedQueryRunner.builder(createSession()) - .setCoordinatorProperties(coordinatorProperties) - .build(); + protected Builder() + { + super(testSessionBuilder() + .setCatalog("accumulo") + .setSchema("tpch") + .build()); + } - queryRunner.installPlugin(new TpchPlugin()); - queryRunner.createCatalog("tpch", "tpch"); - - TestingAccumuloServer server = TestingAccumuloServer.getInstance(); - queryRunner.installPlugin(new AccumuloPlugin()); - Map accumuloProperties = - ImmutableMap.builder() - .put(AccumuloConfig.INSTANCE, server.getInstanceName()) - .put(AccumuloConfig.ZOOKEEPERS, server.getZooKeepers()) - .put(AccumuloConfig.USERNAME, server.getUser()) - .put(AccumuloConfig.PASSWORD, server.getPassword()) - .put(AccumuloConfig.ZOOKEEPER_METADATA_ROOT, "/trino-accumulo-test") - .buildOrThrow(); - - queryRunner.createCatalog("accumulo", "accumulo", accumuloProperties); - - if (!tpchLoaded) { - queryRunner.execute("CREATE SCHEMA accumulo.tpch"); - copyTpchTables(queryRunner, "tpch", TINY_SCHEMA_NAME, createSession(), TpchTable.getTables()); - try (AccumuloClient client = server.createClient()) { - client.tableOperations().addSplits("tpch.orders", ImmutableSortedSet.of(new Text(new LexicoderRowSerializer().encode(BIGINT, 7500L)))); + @Override + public DistributedQueryRunner build() + throws Exception + { + DistributedQueryRunner queryRunner = super.build(); + try { + queryRunner.installPlugin(new TpchPlugin()); + queryRunner.createCatalog("tpch", "tpch"); + + TestingAccumuloServer server = TestingAccumuloServer.getInstance(); + queryRunner.installPlugin(new AccumuloPlugin()); + Map accumuloProperties = + ImmutableMap.builder() + .put(AccumuloConfig.INSTANCE, server.getInstanceName()) + .put(AccumuloConfig.ZOOKEEPERS, server.getZooKeepers()) + .put(AccumuloConfig.USERNAME, server.getUser()) + .put(AccumuloConfig.PASSWORD, server.getPassword()) + .put(AccumuloConfig.ZOOKEEPER_METADATA_ROOT, "/trino-accumulo-test") + .buildOrThrow(); + + queryRunner.createCatalog("accumulo", "accumulo", accumuloProperties); + + if (!tpchLoaded) { + queryRunner.execute("CREATE SCHEMA accumulo.tpch"); + copyTpchTables(queryRunner, "tpch", TINY_SCHEMA_NAME, createSession(), TpchTable.getTables()); + try (AccumuloClient client = server.createClient()) { + client.tableOperations().addSplits("tpch.orders", ImmutableSortedSet.of(new Text(new LexicoderRowSerializer().encode(BIGINT, 7500L)))); + } + tpchLoaded = true; + } } - tpchLoaded = true; + catch (Throwable e) { + Closables.closeAllSuppress(e, queryRunner); + throw e; + } + return queryRunner; } - - return queryRunner; } private static void copyTpchTables( @@ -161,7 +175,9 @@ public static Session createSession() public static void main(String[] args) throws Exception { - QueryRunner queryRunner = createAccumuloQueryRunner(ImmutableMap.of("http-server.http.port", "8080")); + QueryRunner queryRunner = builder() + .addCoordinatorProperty("http-server.http.port", "8080") + .build(); Logger log = Logger.get(AccumuloQueryRunner.class); log.info("======== SERVER STARTED ========"); log.info("\n====\n%s\n====", queryRunner.getCoordinator().getBaseUrl()); diff --git a/plugin/trino-accumulo/src/test/java/io/trino/plugin/accumulo/TestAccumuloConnectorTest.java b/plugin/trino-accumulo/src/test/java/io/trino/plugin/accumulo/TestAccumuloConnectorTest.java index 9548bcf33224..d6abec4898df 100644 --- a/plugin/trino-accumulo/src/test/java/io/trino/plugin/accumulo/TestAccumuloConnectorTest.java +++ b/plugin/trino-accumulo/src/test/java/io/trino/plugin/accumulo/TestAccumuloConnectorTest.java @@ -25,7 +25,6 @@ import java.util.OptionalInt; import java.util.regex.Pattern; -import static io.trino.plugin.accumulo.AccumuloQueryRunner.createAccumuloQueryRunner; import static io.trino.spi.type.VarcharType.VARCHAR; import static io.trino.testing.MaterializedResult.resultBuilder; import static java.util.regex.Pattern.DOTALL; @@ -50,7 +49,7 @@ public class TestAccumuloConnectorTest protected QueryRunner createQueryRunner() throws Exception { - return createAccumuloQueryRunner(); + return AccumuloQueryRunner.builder().build(); } @Override From a7955ce9ab04dbd73229b8e1f907ad2db9e09a32 Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Thu, 30 May 2024 22:07:08 +0900 Subject: [PATCH 04/19] Convert ExampleQueryRunner to builder --- plugin/trino-example-jdbc/pom.xml | 6 ++ .../plugin/example/ExampleQueryRunner.java | 72 ++++++++++++------- .../plugin/example/TestExampleQueries.java | 2 +- 3 files changed, 52 insertions(+), 28 deletions(-) diff --git a/plugin/trino-example-jdbc/pom.xml b/plugin/trino-example-jdbc/pom.xml index fff56c4c9d37..9c4b6e640f1b 100644 --- a/plugin/trino-example-jdbc/pom.xml +++ b/plugin/trino-example-jdbc/pom.xml @@ -79,6 +79,12 @@ runtime + + com.google.errorprone + error_prone_annotations + runtime + + com.google.guava guava diff --git a/plugin/trino-example-jdbc/src/test/java/io/trino/plugin/example/ExampleQueryRunner.java b/plugin/trino-example-jdbc/src/test/java/io/trino/plugin/example/ExampleQueryRunner.java index f3c8c1ff1faf..c0b667ba4c41 100644 --- a/plugin/trino-example-jdbc/src/test/java/io/trino/plugin/example/ExampleQueryRunner.java +++ b/plugin/trino-example-jdbc/src/test/java/io/trino/plugin/example/ExampleQueryRunner.java @@ -14,14 +14,15 @@ package io.trino.plugin.example; -import com.google.common.collect.ImmutableMap; +import com.google.errorprone.annotations.CanIgnoreReturnValue; import io.airlift.log.Level; import io.airlift.log.Logger; import io.airlift.log.Logging; -import io.trino.Session; +import io.trino.plugin.base.util.Closables; import io.trino.testing.DistributedQueryRunner; import io.trino.testing.QueryRunner; +import java.util.HashMap; import java.util.Map; import static io.trino.testing.TestingSession.testSessionBuilder; @@ -30,34 +31,49 @@ public final class ExampleQueryRunner { private ExampleQueryRunner() {} - // TODO convert to builder - public static QueryRunner createQueryRunner() - throws Exception + public static Builder builder() { - Session defaultSession = testSessionBuilder() - .setCatalog("example") - .setSchema("default") - .build(); + return new Builder() + .addConnectorProperty("connection-url", "jdbc:h2:mem:test;init=CREATE TABLE IF NOT EXISTS TEST AS SELECT * FROM (VALUES (1, 'one'), (2, 'two')) AS t(id, name)") + .addConnectorProperty("connection-user", "test") + .addConnectorProperty("connection-password", ""); + } - // TODO static port binding should be in main() only - Map coordinatorProperties = ImmutableMap.builder() - .put("http-server.http.port", "8080") - .buildOrThrow(); - QueryRunner queryRunner = DistributedQueryRunner.builder(defaultSession) - .setCoordinatorProperties(coordinatorProperties) - .build(); - queryRunner.installPlugin(new ExamplePlugin()); + public static class Builder + extends DistributedQueryRunner.Builder + { + private final Map connectorProperties = new HashMap<>(); + + protected Builder() + { + super(testSessionBuilder() + .setCatalog("example") + .setSchema("default") + .build()); + } - Map connectorProperties = Map.of( - "connection-url", "jdbc:h2:mem:test;init=CREATE TABLE IF NOT EXISTS TEST AS SELECT * FROM (VALUES (1, 'one'), (2, 'two')) AS t(id, name)", - "connection-user", "test", - "connection-password", ""); - queryRunner.createCatalog( - "example", - "example_jdbc", - connectorProperties); + @CanIgnoreReturnValue + public Builder addConnectorProperty(String key, String value) + { + this.connectorProperties.put(key, value); + return this; + } - return queryRunner; + @Override + public DistributedQueryRunner build() + throws Exception + { + DistributedQueryRunner queryRunner = super.build(); + try { + queryRunner.installPlugin(new ExamplePlugin()); + queryRunner.createCatalog("example", "example_jdbc", connectorProperties); + return queryRunner; + } + catch (Throwable e) { + Closables.closeAllSuppress(e, queryRunner); + throw e; + } + } } public static void main(String[] args) @@ -67,7 +83,9 @@ public static void main(String[] args) logger.setLevel("io.trino.plugin.example_jdbc", Level.DEBUG); logger.setLevel("io.trino", Level.INFO); - QueryRunner queryRunner = createQueryRunner(); + QueryRunner queryRunner = builder() + .addCoordinatorProperty("http-server.http.port", "8080") + .build(); Logger log = Logger.get(ExampleQueryRunner.class); log.info("======== SERVER STARTED ========"); diff --git a/plugin/trino-example-jdbc/src/test/java/io/trino/plugin/example/TestExampleQueries.java b/plugin/trino-example-jdbc/src/test/java/io/trino/plugin/example/TestExampleQueries.java index c3b45a92e478..bd30a1996ae9 100644 --- a/plugin/trino-example-jdbc/src/test/java/io/trino/plugin/example/TestExampleQueries.java +++ b/plugin/trino-example-jdbc/src/test/java/io/trino/plugin/example/TestExampleQueries.java @@ -25,7 +25,7 @@ public class TestExampleQueries protected QueryRunner createQueryRunner() throws Exception { - return ExampleQueryRunner.createQueryRunner(); + return ExampleQueryRunner.builder().build(); } @Test From 9ae534a2e4ddf3b832494361b7117b82efb62a87 Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Thu, 30 May 2024 22:15:06 +0900 Subject: [PATCH 05/19] Convert HudiQueryRunner to builder --- .../io/trino/plugin/hudi/HudiQueryRunner.java | 105 ++++++++++-------- ...stHudiConnectorParquetColumnNamesTest.java | 8 +- .../plugin/hudi/TestHudiConnectorTest.java | 9 +- .../trino/plugin/hudi/TestHudiSmokeTest.java | 6 +- .../plugin/hudi/TestHudiSystemTables.java | 7 +- 5 files changed, 75 insertions(+), 60 deletions(-) diff --git a/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/HudiQueryRunner.java b/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/HudiQueryRunner.java index de754a547080..8a416ed0a825 100644 --- a/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/HudiQueryRunner.java +++ b/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/HudiQueryRunner.java @@ -13,12 +13,12 @@ */ package io.trino.plugin.hudi; -import com.google.common.collect.ImmutableMap; +import com.google.errorprone.annotations.CanIgnoreReturnValue; import io.airlift.log.Level; import io.airlift.log.Logger; import io.airlift.log.Logging; -import io.trino.Session; import io.trino.filesystem.Location; +import io.trino.plugin.base.util.Closables; import io.trino.plugin.hive.metastore.Database; import io.trino.plugin.hive.metastore.HiveMetastoreFactory; import io.trino.plugin.hudi.testing.HudiTablesInitializer; @@ -27,6 +27,7 @@ import io.trino.testing.DistributedQueryRunner; import io.trino.testing.QueryRunner; +import java.util.HashMap; import java.util.Map; import java.util.Optional; @@ -43,50 +44,66 @@ private HudiQueryRunner() {} private static final String SCHEMA_NAME = "tests"; - // TODO convert to builder - public static QueryRunner createHudiQueryRunner( - Map connectorProperties, - HudiTablesInitializer dataLoader) - throws Exception + public static Builder builder() { - return createHudiQueryRunner(ImmutableMap.of(), connectorProperties, dataLoader); + return new Builder(); } - // TODO convert to builder - private static QueryRunner createHudiQueryRunner( - Map coordinatorProperties, - Map connectorProperties, - HudiTablesInitializer dataLoader) - throws Exception + public static class Builder + extends DistributedQueryRunner.Builder { - QueryRunner queryRunner = DistributedQueryRunner - .builder(createSession()) - .setCoordinatorProperties(coordinatorProperties) - .build(); + private HudiTablesInitializer dataLoader; + private final Map connectorProperties = new HashMap<>(); - queryRunner.installPlugin(new TestingHudiPlugin(queryRunner.getCoordinator().getBaseDataDir().resolve("hudi_data"))); - queryRunner.createCatalog("hudi", "hudi", connectorProperties); - - // Hudi connector does not support creating schema or any other write operations - ((HudiConnector) queryRunner.getCoordinator().getConnector("hudi")).getInjector() - .getInstance(HiveMetastoreFactory.class) - .createMetastore(Optional.empty()) - .createDatabase(Database.builder() - .setDatabaseName(SCHEMA_NAME) - .setOwnerName(Optional.of("public")) - .setOwnerType(Optional.of(PrincipalType.ROLE)) - .build()); - - dataLoader.initializeTables(queryRunner, Location.of("local:///"), SCHEMA_NAME); - return queryRunner; - } + protected Builder() + { + super(testSessionBuilder() + .setCatalog("hudi") + .setSchema(SCHEMA_NAME) + .build()); + } - private static Session createSession() - { - return testSessionBuilder() - .setCatalog("hudi") - .setSchema(SCHEMA_NAME) - .build(); + @CanIgnoreReturnValue + public Builder setDataLoader(HudiTablesInitializer dataLoader) + { + this.dataLoader = dataLoader; + return this; + } + + @CanIgnoreReturnValue + public Builder addConnectorProperty(String key, String value) + { + this.connectorProperties.put(key, value); + return this; + } + + @Override + public DistributedQueryRunner build() + throws Exception + { + DistributedQueryRunner queryRunner = super.build(); + try { + queryRunner.installPlugin(new TestingHudiPlugin(queryRunner.getCoordinator().getBaseDataDir().resolve("hudi_data"))); + queryRunner.createCatalog("hudi", "hudi", connectorProperties); + + // Hudi connector does not support creating schema or any other write operations + ((HudiConnector) queryRunner.getCoordinator().getConnector("hudi")).getInjector() + .getInstance(HiveMetastoreFactory.class) + .createMetastore(Optional.empty()) + .createDatabase(Database.builder() + .setDatabaseName(SCHEMA_NAME) + .setOwnerName(Optional.of("public")) + .setOwnerType(Optional.of(PrincipalType.ROLE)) + .build()); + + dataLoader.initializeTables(queryRunner, Location.of("local:///"), SCHEMA_NAME); + return queryRunner; + } + catch (Throwable e) { + Closables.closeAllSuppress(e, queryRunner); + throw e; + } + } } public static void main(String[] args) @@ -95,10 +112,10 @@ public static void main(String[] args) Logging.initialize(); Logger log = Logger.get(HudiQueryRunner.class); - QueryRunner queryRunner = createHudiQueryRunner( - ImmutableMap.of("http-server.http.port", "8080"), - ImmutableMap.of(), - new ResourceHudiTablesInitializer()); + QueryRunner queryRunner = builder() + .addCoordinatorProperty("http-server.http.port", "8080") + .setDataLoader(new ResourceHudiTablesInitializer()) + .build(); log.info("======== SERVER STARTED ========"); log.info("\n====\n%s\n====", queryRunner.getCoordinator().getBaseUrl()); diff --git a/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/TestHudiConnectorParquetColumnNamesTest.java b/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/TestHudiConnectorParquetColumnNamesTest.java index 3b38a04d975b..7e938ba3aed1 100644 --- a/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/TestHudiConnectorParquetColumnNamesTest.java +++ b/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/TestHudiConnectorParquetColumnNamesTest.java @@ -13,12 +13,9 @@ */ package io.trino.plugin.hudi; -import com.google.common.collect.ImmutableMap; import io.trino.plugin.hudi.testing.ResourceHudiTablesInitializer; import io.trino.testing.QueryRunner; -import static io.trino.plugin.hudi.HudiQueryRunner.createHudiQueryRunner; - public class TestHudiConnectorParquetColumnNamesTest extends TestHudiSmokeTest { @@ -26,6 +23,9 @@ public class TestHudiConnectorParquetColumnNamesTest protected QueryRunner createQueryRunner() throws Exception { - return createHudiQueryRunner(ImmutableMap.of("hudi.parquet.use-column-names", "false"), new ResourceHudiTablesInitializer()); + return HudiQueryRunner.builder() + .addConnectorProperty("hudi.parquet.use-column-names", "false") + .setDataLoader(new ResourceHudiTablesInitializer()) + .build(); } } diff --git a/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/TestHudiConnectorTest.java b/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/TestHudiConnectorTest.java index 7f97ab933110..84a4cbd60440 100644 --- a/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/TestHudiConnectorTest.java +++ b/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/TestHudiConnectorTest.java @@ -13,14 +13,12 @@ */ package io.trino.plugin.hudi; -import com.google.common.collect.ImmutableMap; import io.trino.plugin.hudi.testing.TpchHudiTablesInitializer; import io.trino.testing.BaseConnectorTest; import io.trino.testing.QueryRunner; import io.trino.testing.TestingConnectorBehavior; import org.junit.jupiter.api.Test; -import static io.trino.plugin.hudi.HudiQueryRunner.createHudiQueryRunner; import static io.trino.plugin.hudi.testing.HudiTestUtils.COLUMNS_TO_HIDE; import static org.assertj.core.api.Assertions.assertThat; @@ -31,9 +29,10 @@ public class TestHudiConnectorTest protected QueryRunner createQueryRunner() throws Exception { - return createHudiQueryRunner( - ImmutableMap.of("hudi.columns-to-hide", COLUMNS_TO_HIDE), - new TpchHudiTablesInitializer(REQUIRED_TPCH_TABLES)); + return HudiQueryRunner.builder() + .addConnectorProperty("hudi.columns-to-hide", COLUMNS_TO_HIDE) + .setDataLoader(new TpchHudiTablesInitializer(REQUIRED_TPCH_TABLES)) + .build(); } @Override diff --git a/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/TestHudiSmokeTest.java b/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/TestHudiSmokeTest.java index 443120331c07..24abd8b5509c 100644 --- a/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/TestHudiSmokeTest.java +++ b/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/TestHudiSmokeTest.java @@ -13,7 +13,6 @@ */ package io.trino.plugin.hudi; -import com.google.common.collect.ImmutableMap; import io.trino.Session; import io.trino.filesystem.Location; import io.trino.filesystem.TrinoFileSystemFactory; @@ -27,7 +26,6 @@ import java.time.ZonedDateTime; -import static io.trino.plugin.hudi.HudiQueryRunner.createHudiQueryRunner; import static io.trino.plugin.hudi.testing.ResourceHudiTablesInitializer.TestingTable.HUDI_COW_PT_TBL; import static io.trino.plugin.hudi.testing.ResourceHudiTablesInitializer.TestingTable.HUDI_NON_PART_COW; import static io.trino.plugin.hudi.testing.ResourceHudiTablesInitializer.TestingTable.STOCK_TICKS_COW; @@ -41,7 +39,9 @@ public class TestHudiSmokeTest protected QueryRunner createQueryRunner() throws Exception { - return createHudiQueryRunner(ImmutableMap.of(), new ResourceHudiTablesInitializer()); + return HudiQueryRunner.builder() + .setDataLoader(new ResourceHudiTablesInitializer()) + .build(); } @Test diff --git a/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/TestHudiSystemTables.java b/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/TestHudiSystemTables.java index a667b0c7439d..ad11418aa556 100644 --- a/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/TestHudiSystemTables.java +++ b/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/TestHudiSystemTables.java @@ -13,14 +13,11 @@ */ package io.trino.plugin.hudi; -import com.google.common.collect.ImmutableMap; import io.trino.plugin.hudi.testing.ResourceHudiTablesInitializer; import io.trino.testing.AbstractTestQueryFramework; import io.trino.testing.QueryRunner; import org.junit.jupiter.api.Test; -import static io.trino.plugin.hudi.HudiQueryRunner.createHudiQueryRunner; - public class TestHudiSystemTables extends AbstractTestQueryFramework { @@ -28,7 +25,9 @@ public class TestHudiSystemTables protected QueryRunner createQueryRunner() throws Exception { - return createHudiQueryRunner(ImmutableMap.of(), new ResourceHudiTablesInitializer()); + return HudiQueryRunner.builder() + .setDataLoader(new ResourceHudiTablesInitializer()) + .build(); } @Test From c6f0ebbb14ebc36a0f4d1258feeaa46c45935e36 Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Thu, 30 May 2024 22:21:49 +0900 Subject: [PATCH 06/19] Convert PrometheusQueryRunner to builder --- plugin/trino-prometheus/pom.xml | 6 ++ .../prometheus/PrometheusQueryRunner.java | 70 ++++++++++--------- .../prometheus/TestPrometheusBasicAuth.java | 7 +- .../prometheus/TestPrometheusIntegration.java | 4 +- 4 files changed, 49 insertions(+), 38 deletions(-) diff --git a/plugin/trino-prometheus/pom.xml b/plugin/trino-prometheus/pom.xml index b4a8df1c8e55..a3416f5d5b7c 100644 --- a/plugin/trino-prometheus/pom.xml +++ b/plugin/trino-prometheus/pom.xml @@ -129,6 +129,12 @@ provided + + com.google.errorprone + error_prone_annotations + runtime + + io.airlift log-manager diff --git a/plugin/trino-prometheus/src/test/java/io/trino/plugin/prometheus/PrometheusQueryRunner.java b/plugin/trino-prometheus/src/test/java/io/trino/plugin/prometheus/PrometheusQueryRunner.java index dd8dafd1e1c1..a3cf7473b6c1 100644 --- a/plugin/trino-prometheus/src/test/java/io/trino/plugin/prometheus/PrometheusQueryRunner.java +++ b/plugin/trino-prometheus/src/test/java/io/trino/plugin/prometheus/PrometheusQueryRunner.java @@ -13,17 +13,16 @@ */ package io.trino.plugin.prometheus; -import com.google.common.collect.ImmutableMap; +import com.google.errorprone.annotations.CanIgnoreReturnValue; import io.airlift.log.Logger; import io.airlift.units.Duration; -import io.trino.Session; +import io.trino.plugin.base.util.Closables; import io.trino.testing.DistributedQueryRunner; import io.trino.testing.QueryRunner; import java.util.HashMap; import java.util.Map; -import static io.airlift.testing.Closeables.closeAllSuppress; import static io.trino.plugin.prometheus.MetadataUtil.METRIC_CODEC; import static io.trino.testing.TestingSession.testSessionBuilder; import static io.trino.type.InternalTypeManager.TESTING_TYPE_MANAGER; @@ -34,42 +33,47 @@ public final class PrometheusQueryRunner { private PrometheusQueryRunner() {} - // TODO convert to builder - public static QueryRunner createPrometheusQueryRunner(PrometheusServer server, Map connectorProperties) - throws Exception + public static Builder builder(PrometheusServer prometheusServer) { - return createPrometheusQueryRunner(server, ImmutableMap.of(), connectorProperties); + return new Builder() + .addConnectorProperty("prometheus.uri", prometheusServer.getUri().toString()); } - // TODO convert to builder - private static QueryRunner createPrometheusQueryRunner(PrometheusServer server, Map coordinatorProperties, Map connectorProperties) - throws Exception + public static class Builder + extends DistributedQueryRunner.Builder { - QueryRunner queryRunner = null; - try { - queryRunner = DistributedQueryRunner.builder(createSession()) - .setCoordinatorProperties(coordinatorProperties) - .build(); + private final Map connectorProperties = new HashMap<>(); - queryRunner.installPlugin(new PrometheusPlugin()); - // note: additional copy via ImmutableList so that if fails on nulls - connectorProperties = new HashMap<>(ImmutableMap.copyOf(connectorProperties)); - connectorProperties.putIfAbsent("prometheus.uri", server.getUri().toString()); - queryRunner.createCatalog("prometheus", "prometheus", connectorProperties); - return queryRunner; + protected Builder() + { + super(testSessionBuilder() + .setCatalog("prometheus") + .setSchema("default") + .build()); } - catch (Throwable e) { - closeAllSuppress(e, queryRunner); - throw e; + + @CanIgnoreReturnValue + public Builder addConnectorProperty(String key, String value) + { + this.connectorProperties.put(key, value); + return this; } - } - private static Session createSession() - { - return testSessionBuilder() - .setCatalog("prometheus") - .setSchema("default") - .build(); + @Override + public DistributedQueryRunner build() + throws Exception + { + DistributedQueryRunner queryRunner = super.build(); + try { + queryRunner.installPlugin(new PrometheusPlugin()); + queryRunner.createCatalog("prometheus", "prometheus", connectorProperties); + } + catch (Throwable e) { + Closables.closeAllSuppress(e, queryRunner); + throw e; + } + return queryRunner; + } } public static PrometheusClient createPrometheusClient(PrometheusServer server) @@ -86,7 +90,9 @@ public static PrometheusClient createPrometheusClient(PrometheusServer server) public static void main(String[] args) throws Exception { - QueryRunner queryRunner = createPrometheusQueryRunner(new PrometheusServer(), ImmutableMap.of("http-server.http.port", "8080"), ImmutableMap.of()); + QueryRunner queryRunner = builder(new PrometheusServer()) + .addCoordinatorProperty("http-server.http.port", "8080") + .build(); Logger log = Logger.get(PrometheusQueryRunner.class); log.info("======== SERVER STARTED ========"); log.info("\n====\n%s\n====", queryRunner.getCoordinator().getBaseUrl()); diff --git a/plugin/trino-prometheus/src/test/java/io/trino/plugin/prometheus/TestPrometheusBasicAuth.java b/plugin/trino-prometheus/src/test/java/io/trino/plugin/prometheus/TestPrometheusBasicAuth.java index 940aff99d28a..900000864c47 100644 --- a/plugin/trino-prometheus/src/test/java/io/trino/plugin/prometheus/TestPrometheusBasicAuth.java +++ b/plugin/trino-prometheus/src/test/java/io/trino/plugin/prometheus/TestPrometheusBasicAuth.java @@ -13,14 +13,12 @@ */ package io.trino.plugin.prometheus; -import com.google.common.collect.ImmutableMap; import io.airlift.units.Duration; import io.trino.testing.AbstractTestQueryFramework; import io.trino.testing.QueryRunner; import org.junit.jupiter.api.Test; import static io.trino.plugin.prometheus.MetadataUtil.METRIC_CODEC; -import static io.trino.plugin.prometheus.PrometheusQueryRunner.createPrometheusQueryRunner; import static io.trino.plugin.prometheus.PrometheusServer.LATEST_VERSION; import static io.trino.plugin.prometheus.PrometheusServer.PASSWORD; import static io.trino.plugin.prometheus.PrometheusServer.USER; @@ -40,7 +38,10 @@ protected QueryRunner createQueryRunner() throws Exception { server = closeAfterClass(new PrometheusServer(LATEST_VERSION, true)); - return createPrometheusQueryRunner(server, ImmutableMap.of("prometheus.auth.user", USER, "prometheus.auth.password", PASSWORD)); + return PrometheusQueryRunner.builder(server) + .addConnectorProperty("prometheus.auth.user", USER) + .addConnectorProperty("prometheus.auth.password", PASSWORD) + .build(); } @Test diff --git a/plugin/trino-prometheus/src/test/java/io/trino/plugin/prometheus/TestPrometheusIntegration.java b/plugin/trino-prometheus/src/test/java/io/trino/plugin/prometheus/TestPrometheusIntegration.java index f5183156a659..cbed2cf69e9a 100644 --- a/plugin/trino-prometheus/src/test/java/io/trino/plugin/prometheus/TestPrometheusIntegration.java +++ b/plugin/trino-prometheus/src/test/java/io/trino/plugin/prometheus/TestPrometheusIntegration.java @@ -13,7 +13,6 @@ */ package io.trino.plugin.prometheus; -import com.google.common.collect.ImmutableMap; import io.airlift.units.Duration; import io.trino.spi.connector.ConnectorSplitSource; import io.trino.spi.connector.Constraint; @@ -26,7 +25,6 @@ import java.util.concurrent.TimeUnit; import static io.trino.plugin.prometheus.PrometheusQueryRunner.createPrometheusClient; -import static io.trino.plugin.prometheus.PrometheusQueryRunner.createPrometheusQueryRunner; import static java.util.concurrent.TimeUnit.DAYS; import static java.util.concurrent.TimeUnit.SECONDS; import static org.assertj.core.api.Assertions.assertThat; @@ -45,7 +43,7 @@ protected QueryRunner createQueryRunner() { this.server = closeAfterClass(new PrometheusServer()); this.client = createPrometheusClient(server); - return createPrometheusQueryRunner(server, ImmutableMap.of()); + return PrometheusQueryRunner.builder(server).build(); } @Test From 30850a5bd64e824ee40b5467a811812f59b3714c Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Thu, 30 May 2024 22:32:11 +0900 Subject: [PATCH 07/19] Convert ScyllaQueryRunner to builder --- .../plugin/cassandra/ScyllaQueryRunner.java | 113 ++++++++++-------- .../TestScyllaConnectorSmokeTest.java | 5 +- 2 files changed, 66 insertions(+), 52 deletions(-) diff --git a/plugin/trino-cassandra/src/test/java/io/trino/plugin/cassandra/ScyllaQueryRunner.java b/plugin/trino-cassandra/src/test/java/io/trino/plugin/cassandra/ScyllaQueryRunner.java index 03546d92f014..572563e55156 100644 --- a/plugin/trino-cassandra/src/test/java/io/trino/plugin/cassandra/ScyllaQueryRunner.java +++ b/plugin/trino-cassandra/src/test/java/io/trino/plugin/cassandra/ScyllaQueryRunner.java @@ -13,92 +13,105 @@ */ package io.trino.plugin.cassandra; -import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableList; +import com.google.errorprone.annotations.CanIgnoreReturnValue; import io.airlift.log.Logger; import io.airlift.log.Logging; -import io.trino.Session; +import io.trino.plugin.base.util.Closables; import io.trino.plugin.tpch.TpchPlugin; import io.trino.testing.DistributedQueryRunner; import io.trino.testing.QueryRunner; import io.trino.tpch.TpchTable; import java.util.HashMap; +import java.util.List; import java.util.Map; -import static io.airlift.testing.Closeables.closeAllSuppress; import static io.trino.plugin.cassandra.CassandraTestingUtils.createKeyspace; import static io.trino.plugin.tpch.TpchMetadata.TINY_SCHEMA_NAME; import static io.trino.testing.QueryAssertions.copyTpchTables; import static io.trino.testing.TestingSession.testSessionBuilder; +import static java.util.Objects.requireNonNull; public final class ScyllaQueryRunner { private ScyllaQueryRunner() {} - public static QueryRunner createScyllaQueryRunner( - TestingScyllaServer server, - Iterable> tables) - throws Exception + public static Builder builder(TestingScyllaServer server) { - return createScyllaQueryRunner(server, Map.of(), tables); + return new Builder(server) + .addConnectorProperty("cassandra.contact-points", server.getHost()) + .addConnectorProperty("cassandra.native-protocol-port", Integer.toString(server.getPort())) + .addConnectorProperty("cassandra.allow-drop-table", "true") + .addConnectorProperty("cassandra.load-policy.use-dc-aware", "true") + .addConnectorProperty("cassandra.load-policy.dc-aware.local-dc", "datacenter1"); } - // TODO convert to builder - private static QueryRunner createScyllaQueryRunner( - TestingScyllaServer server, - Map coordinatorProperties, - Iterable> tables) - throws Exception + public static class Builder + extends DistributedQueryRunner.Builder { - QueryRunner queryRunner = DistributedQueryRunner.builder(createSession("tpch")) - .setCoordinatorProperties(coordinatorProperties) - .build(); + private final TestingScyllaServer server; + private final Map connectorProperties = new HashMap<>(); + private List> initialTables = ImmutableList.of(); + + private Builder(TestingScyllaServer server) + { + super(testSessionBuilder() + .setCatalog("cassandra") + .setSchema("tpch") + .build()); + this.server = requireNonNull(server, "server is null"); + } + + @CanIgnoreReturnValue + public Builder addConnectorProperty(String key, String value) + { + this.connectorProperties.put(key, value); + return this; + } - try { - queryRunner.installPlugin(new TpchPlugin()); - queryRunner.createCatalog("tpch", "tpch"); + @CanIgnoreReturnValue + public Builder setInitialTables(List> initialTables) + { + this.initialTables = ImmutableList.copyOf(initialTables); + return this; + } - // note: additional copy via ImmutableList so that if fails on nulls - Map connectorProperties = new HashMap<>(); - connectorProperties.put("cassandra.contact-points", server.getHost()); - connectorProperties.put("cassandra.native-protocol-port", Integer.toString(server.getPort())); - connectorProperties.put("cassandra.allow-drop-table", "true"); - connectorProperties.put("cassandra.load-policy.use-dc-aware", "true"); - connectorProperties.put("cassandra.load-policy.dc-aware.local-dc", "datacenter1"); + @Override + public DistributedQueryRunner build() + throws Exception + { + DistributedQueryRunner queryRunner = super.build(); + try { + queryRunner.installPlugin(new TpchPlugin()); + queryRunner.createCatalog("tpch", "tpch"); - queryRunner.installPlugin(new CassandraPlugin()); - queryRunner.createCatalog("cassandra", "cassandra", connectorProperties); + queryRunner.installPlugin(new CassandraPlugin()); + queryRunner.createCatalog("cassandra", "cassandra", connectorProperties); - createKeyspace(server.getSession(), "tpch"); - copyTpchTables(queryRunner, "tpch", TINY_SCHEMA_NAME, tables); - for (TpchTable table : tables) { - server.refreshSizeEstimates("tpch", table.getTableName()); + createKeyspace(server.getSession(), "tpch"); + copyTpchTables(queryRunner, "tpch", TINY_SCHEMA_NAME, initialTables); + for (TpchTable table : initialTables) { + server.refreshSizeEstimates("tpch", table.getTableName()); + } + return queryRunner; + } + catch (Throwable e) { + Closables.closeAllSuppress(e, queryRunner); + throw e; } - return queryRunner; - } - catch (Throwable e) { - closeAllSuppress(e, queryRunner); - throw e; } } - public static Session createSession(String schema) - { - return testSessionBuilder() - .setCatalog("cassandra") - .setSchema(schema) - .build(); - } - public static void main(String[] args) throws Exception { Logging.initialize(); - QueryRunner queryRunner = createScyllaQueryRunner( - new TestingScyllaServer(), - ImmutableMap.of("http-server.http.port", "8080"), - TpchTable.getTables()); + QueryRunner queryRunner = builder(new TestingScyllaServer()) + .addCoordinatorProperty("http-server.http.port", "8080") + .setInitialTables(TpchTable.getTables()) + .build(); Logger log = Logger.get(ScyllaQueryRunner.class); log.info("======== SERVER STARTED ========"); diff --git a/plugin/trino-cassandra/src/test/java/io/trino/plugin/cassandra/TestScyllaConnectorSmokeTest.java b/plugin/trino-cassandra/src/test/java/io/trino/plugin/cassandra/TestScyllaConnectorSmokeTest.java index fc5a0e834365..34672f15a468 100644 --- a/plugin/trino-cassandra/src/test/java/io/trino/plugin/cassandra/TestScyllaConnectorSmokeTest.java +++ b/plugin/trino-cassandra/src/test/java/io/trino/plugin/cassandra/TestScyllaConnectorSmokeTest.java @@ -18,7 +18,6 @@ import java.sql.Timestamp; import static io.trino.plugin.cassandra.CassandraTestingUtils.createTestTables; -import static io.trino.plugin.cassandra.ScyllaQueryRunner.createScyllaQueryRunner; public class TestScyllaConnectorSmokeTest extends BaseCassandraConnectorSmokeTest @@ -30,6 +29,8 @@ protected QueryRunner createQueryRunner() TestingScyllaServer server = closeAfterClass(new TestingScyllaServer()); CassandraSession session = server.getSession(); createTestTables(session, KEYSPACE, Timestamp.from(TIMESTAMP_VALUE.toInstant())); - return createScyllaQueryRunner(server, REQUIRED_TPCH_TABLES); + return ScyllaQueryRunner.builder(server) + .setInitialTables(REQUIRED_TPCH_TABLES) + .build(); } } From da01a349a5f005863e0f323f8a9c74b8da6b9f97 Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Thu, 30 May 2024 22:50:29 +0900 Subject: [PATCH 08/19] Convert SheetsQueryRunner to builder --- plugin/trino-google-sheets/pom.xml | 6 ++ .../google/sheets/SheetsQueryRunner.java | 86 ++++++++++--------- .../google/sheets/TestGoogleSheets.java | 12 ++- ...estGoogleSheetsWithoutMetadataSheetId.java | 6 +- 4 files changed, 59 insertions(+), 51 deletions(-) diff --git a/plugin/trino-google-sheets/pom.xml b/plugin/trino-google-sheets/pom.xml index 4c78931ac5ca..90dc5357163d 100644 --- a/plugin/trino-google-sheets/pom.xml +++ b/plugin/trino-google-sheets/pom.xml @@ -163,6 +163,12 @@ provided + + com.google.errorprone + error_prone_annotations + runtime + + io.airlift log-manager diff --git a/plugin/trino-google-sheets/src/test/java/io/trino/plugin/google/sheets/SheetsQueryRunner.java b/plugin/trino-google-sheets/src/test/java/io/trino/plugin/google/sheets/SheetsQueryRunner.java index 3aa25ca2580f..d232658a32ec 100644 --- a/plugin/trino-google-sheets/src/test/java/io/trino/plugin/google/sheets/SheetsQueryRunner.java +++ b/plugin/trino-google-sheets/src/test/java/io/trino/plugin/google/sheets/SheetsQueryRunner.java @@ -14,9 +14,10 @@ package io.trino.plugin.google.sheets; import com.google.common.collect.ImmutableMap; +import com.google.errorprone.annotations.CanIgnoreReturnValue; import io.airlift.log.Logger; import io.airlift.log.Logging; -import io.trino.Session; +import io.trino.plugin.base.util.Closables; import io.trino.plugin.tpch.TpchPlugin; import io.trino.testing.DistributedQueryRunner; import io.trino.testing.QueryRunner; @@ -24,7 +25,6 @@ import java.util.HashMap; import java.util.Map; -import static io.airlift.testing.Closeables.closeAllSuppress; import static io.trino.plugin.google.sheets.TestSheetsPlugin.TEST_METADATA_SHEET_ID; import static io.trino.plugin.google.sheets.TestSheetsPlugin.getTestCredentialsPath; import static io.trino.testing.TestingSession.testSessionBuilder; @@ -35,51 +35,54 @@ private SheetsQueryRunner() {} static final String GOOGLE_SHEETS = "gsheets"; - public static QueryRunner createSheetsQueryRunner( - Map connectorProperties) + public static Builder builder() throws Exception { - return createSheetsQueryRunner( - ImmutableMap.of(), - connectorProperties); + return new Builder() + .addConnectorProperty("gsheets.credentials-path", getTestCredentialsPath()) + .addConnectorProperty("gsheets.max-data-cache-size", "1000") + .addConnectorProperty("gsheets.data-cache-ttl", "1m"); } - // TODO convert to builder - private static QueryRunner createSheetsQueryRunner( - Map coordinatorProperties, - Map connectorProperties) - throws Exception + public static class Builder + extends DistributedQueryRunner.Builder { - QueryRunner queryRunner = DistributedQueryRunner.builder(createSession()) - .setCoordinatorProperties(coordinatorProperties) - .build(); - try { - // note: additional copy via ImmutableList so that if fails on nulls - connectorProperties = new HashMap<>(ImmutableMap.copyOf(connectorProperties)); - connectorProperties.putIfAbsent("gsheets.credentials-path", getTestCredentialsPath()); - connectorProperties.putIfAbsent("gsheets.max-data-cache-size", "1000"); - connectorProperties.putIfAbsent("gsheets.data-cache-ttl", "1m"); - - queryRunner.installPlugin(new SheetsPlugin()); - queryRunner.createCatalog(GOOGLE_SHEETS, GOOGLE_SHEETS, connectorProperties); + private final Map connectorProperties = new HashMap<>(); - queryRunner.installPlugin(new TpchPlugin()); - queryRunner.createCatalog("tpch", "tpch", ImmutableMap.of()); - - return queryRunner; + protected Builder() + { + super(testSessionBuilder() + .setCatalog(GOOGLE_SHEETS) + .setSchema("default") + .build()); } - catch (Throwable e) { - closeAllSuppress(e, queryRunner); - throw e; + + @CanIgnoreReturnValue + public Builder addConnectorProperty(String key, String value) + { + this.connectorProperties.put(key, value); + return this; } - } - private static Session createSession() - { - return testSessionBuilder() - .setCatalog(GOOGLE_SHEETS) - .setSchema("default") - .build(); + @Override + public DistributedQueryRunner build() + throws Exception + { + DistributedQueryRunner queryRunner = super.build(); + try { + queryRunner.installPlugin(new SheetsPlugin()); + queryRunner.createCatalog(GOOGLE_SHEETS, GOOGLE_SHEETS, connectorProperties); + + queryRunner.installPlugin(new TpchPlugin()); + queryRunner.createCatalog("tpch", "tpch", ImmutableMap.of()); + + return queryRunner; + } + catch (Throwable e) { + Closables.closeAllSuppress(e, queryRunner); + throw e; + } + } } public static void main(String[] args) @@ -87,9 +90,10 @@ public static void main(String[] args) { Logging.initialize(); - QueryRunner queryRunner = createSheetsQueryRunner( - ImmutableMap.of("http-server.http.port", "8080"), - ImmutableMap.of("gsheets.metadata-sheet-id", TEST_METADATA_SHEET_ID)); + QueryRunner queryRunner = builder() + .addCoordinatorProperty("http-server.http.port", "8080") + .addConnectorProperty("gsheets.metadata-sheet-id", TEST_METADATA_SHEET_ID) + .build(); Logger log = Logger.get(SheetsQueryRunner.class); log.info("======== SERVER STARTED ========"); diff --git a/plugin/trino-google-sheets/src/test/java/io/trino/plugin/google/sheets/TestGoogleSheets.java b/plugin/trino-google-sheets/src/test/java/io/trino/plugin/google/sheets/TestGoogleSheets.java index 9cf417c4ed0d..47b3fe321915 100644 --- a/plugin/trino-google-sheets/src/test/java/io/trino/plugin/google/sheets/TestGoogleSheets.java +++ b/plugin/trino-google-sheets/src/test/java/io/trino/plugin/google/sheets/TestGoogleSheets.java @@ -24,7 +24,6 @@ import com.google.api.services.sheets.v4.model.UpdateValuesResponse; import com.google.api.services.sheets.v4.model.ValueRange; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import io.airlift.units.Duration; import io.trino.testing.AbstractTestQueryFramework; import io.trino.testing.QueryRunner; @@ -36,7 +35,6 @@ import java.util.concurrent.TimeUnit; import static com.google.api.client.googleapis.javanet.GoogleNetHttpTransport.newTrustedTransport; -import static io.trino.plugin.google.sheets.SheetsQueryRunner.createSheetsQueryRunner; import static io.trino.plugin.google.sheets.TestSheetsPlugin.DATA_SHEET_ID; import static io.trino.plugin.google.sheets.TestSheetsPlugin.getTestCredentialsPath; import static io.trino.testing.assertions.Assert.assertEventually; @@ -58,11 +56,11 @@ protected QueryRunner createQueryRunner() { sheetsService = getSheetsService(); spreadsheetId = createSpreadsheetWithTestdata(); - return createSheetsQueryRunner(ImmutableMap.of( - "gsheets.metadata-sheet-id", spreadsheetId + "#Metadata", - "gsheets.connection-timeout", "1m", - "gsheets.read-timeout", "1m", - "gsheets.write-timeout", "1m")); + return SheetsQueryRunner.builder() + .addConnectorProperty("gsheets.metadata-sheet-id", spreadsheetId + "#Metadata") + .addConnectorProperty("gsheets.connection-timeout", "1m") + .addConnectorProperty("gsheets.read-timeout", "1m") + .build(); } // This test currently only creates spreadsheets and does not delete them afterward. diff --git a/plugin/trino-google-sheets/src/test/java/io/trino/plugin/google/sheets/TestGoogleSheetsWithoutMetadataSheetId.java b/plugin/trino-google-sheets/src/test/java/io/trino/plugin/google/sheets/TestGoogleSheetsWithoutMetadataSheetId.java index b8aace2f50f4..09061238bbc8 100644 --- a/plugin/trino-google-sheets/src/test/java/io/trino/plugin/google/sheets/TestGoogleSheetsWithoutMetadataSheetId.java +++ b/plugin/trino-google-sheets/src/test/java/io/trino/plugin/google/sheets/TestGoogleSheetsWithoutMetadataSheetId.java @@ -13,12 +13,10 @@ */ package io.trino.plugin.google.sheets; -import com.google.common.collect.ImmutableMap; import io.trino.testing.AbstractTestQueryFramework; import io.trino.testing.QueryRunner; import org.junit.jupiter.api.Test; -import static io.trino.plugin.google.sheets.SheetsQueryRunner.createSheetsQueryRunner; import static io.trino.plugin.google.sheets.TestSheetsPlugin.DATA_SHEET_ID; public class TestGoogleSheetsWithoutMetadataSheetId @@ -28,7 +26,9 @@ public class TestGoogleSheetsWithoutMetadataSheetId protected QueryRunner createQueryRunner() throws Exception { - return createSheetsQueryRunner(ImmutableMap.of("gsheets.read-timeout", "1m")); + return SheetsQueryRunner.builder() + .addConnectorProperty("gsheets.read-timeout", "1m") + .build(); } @Test From 44b8ef52dc73af0aaabe8ecaf2684947f09ef853 Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Fri, 31 May 2024 08:43:48 +0900 Subject: [PATCH 09/19] Convert KuduQueryRunnerFactory to builder --- plugin/trino-kudu/pom.xml | 6 + .../kudu/BaseKuduConnectorSmokeTest.java | 8 +- .../plugin/kudu/KuduQueryRunnerFactory.java | 212 +++++++----------- .../plugin/kudu/TestKuduConnectorTest.java | 8 +- .../TestKuduIntegrationDecimalColumns.java | 3 +- .../TestKuduIntegrationDynamicFilter.java | 17 +- .../TestKuduIntegrationHashPartitioning.java | 3 +- .../TestKuduIntegrationIntegerColumns.java | 3 +- .../TestKuduIntegrationRangePartitioning.java | 3 +- 9 files changed, 102 insertions(+), 161 deletions(-) diff --git a/plugin/trino-kudu/pom.xml b/plugin/trino-kudu/pom.xml index ce1b38d23251..8b19b467e129 100644 --- a/plugin/trino-kudu/pom.xml +++ b/plugin/trino-kudu/pom.xml @@ -132,6 +132,12 @@ provided + + com.google.errorprone + error_prone_annotations + runtime + + io.airlift log-manager diff --git a/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/BaseKuduConnectorSmokeTest.java b/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/BaseKuduConnectorSmokeTest.java index bb539d8ed5f7..36894e32c691 100644 --- a/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/BaseKuduConnectorSmokeTest.java +++ b/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/BaseKuduConnectorSmokeTest.java @@ -20,7 +20,6 @@ import java.util.Optional; -import static io.trino.plugin.kudu.KuduQueryRunnerFactory.createKuduQueryRunnerTpch; import static io.trino.plugin.kudu.TestKuduConnectorTest.REGION_COLUMNS; import static io.trino.plugin.kudu.TestKuduConnectorTest.createKuduTableForWrites; import static io.trino.plugin.kudu.TestingKuduServer.EARLIEST_TAG; @@ -40,9 +39,10 @@ public abstract class BaseKuduConnectorSmokeTest protected QueryRunner createQueryRunner() throws Exception { - return createKuduQueryRunnerTpch( - closeAfterClass(new TestingKuduServer(getKuduServerVersion())), - getKuduSchemaEmulationPrefix(), REQUIRED_TPCH_TABLES); + return KuduQueryRunnerFactory.builder(closeAfterClass(new TestingKuduServer(getKuduServerVersion()))) + .setKuduSchemaEmulationPrefix(getKuduSchemaEmulationPrefix()) + .setInitialTables(REQUIRED_TPCH_TABLES) + .build(); } @Override diff --git a/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/KuduQueryRunnerFactory.java b/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/KuduQueryRunnerFactory.java index 519588a19949..16721472342d 100644 --- a/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/KuduQueryRunnerFactory.java +++ b/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/KuduQueryRunnerFactory.java @@ -14,176 +14,120 @@ package io.trino.plugin.kudu; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; -import com.google.common.net.HostAndPort; +import com.google.errorprone.annotations.CanIgnoreReturnValue; import io.airlift.log.Logger; import io.airlift.log.Logging; -import io.trino.Session; +import io.trino.plugin.base.util.Closables; import io.trino.plugin.tpch.TpchPlugin; import io.trino.testing.DistributedQueryRunner; import io.trino.testing.QueryRunner; import io.trino.tpch.TpchTable; +import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Optional; -import static io.airlift.testing.Closeables.closeAllSuppress; -import static io.trino.Session.SessionBuilder; import static io.trino.plugin.tpch.TpchMetadata.TINY_SCHEMA_NAME; import static io.trino.testing.QueryAssertions.copyTpchTables; import static io.trino.testing.TestingSession.testSessionBuilder; +import static java.util.Objects.requireNonNull; public final class KuduQueryRunnerFactory { private KuduQueryRunnerFactory() {} - // TODO convert to builder - public static QueryRunner createKuduQueryRunner(TestingKuduServer kuduServer, Session session) - throws Exception + public static Builder builder(TestingKuduServer kuduServer) { - QueryRunner runner = null; - try { - runner = DistributedQueryRunner.builder(session).build(); - - installKuduConnector(kuduServer.getMasterAddress(), runner, session.getSchema().orElse("kudu_smoke_test"), Optional.of("")); - - return runner; - } - catch (Throwable e) { - closeAllSuppress(e, runner); - throw e; - } + return new Builder(kuduServer); } - // TODO convert to builder - public static QueryRunner createKuduQueryRunner(TestingKuduServer kuduServer, String kuduSchema) - throws Exception + public static class Builder + extends DistributedQueryRunner.Builder { - QueryRunner runner = null; - try { - runner = DistributedQueryRunner.builder(createSession(kuduSchema)).build(); - - installKuduConnector(kuduServer.getMasterAddress(), runner, kuduSchema, Optional.of("")); - - return runner; - } - catch (Throwable e) { - closeAllSuppress(e, runner); - throw e; + private final TestingKuduServer kuduServer; + private final Map connectorProperties = new HashMap<>(); + private Optional kuduSchemaEmulationPrefix = Optional.empty(); + private List> initialTables = ImmutableList.of(); + + private Builder(TestingKuduServer kuduServer) + { + super(testSessionBuilder() + .setCatalog("kudu") + .setSchema("default") + .build()); + this.kuduServer = requireNonNull(kuduServer, "kuduServer is null"); } - } - - // TODO convert to builder - public static QueryRunner createKuduQueryRunnerTpch(TestingKuduServer kuduServer, Optional kuduSchemaEmulationPrefix, TpchTable... tables) - throws Exception - { - return createKuduQueryRunnerTpch(kuduServer, kuduSchemaEmulationPrefix, ImmutableList.copyOf(tables)); - } - - // TODO convert to builder - public static QueryRunner createKuduQueryRunnerTpch(TestingKuduServer kuduServer, Optional kuduSchemaEmulationPrefix, Iterable> tables) - throws Exception - { - return createKuduQueryRunnerTpch(kuduServer, kuduSchemaEmulationPrefix, ImmutableMap.of(), ImmutableMap.of(), tables); - } - - // TODO convert to builder - public static QueryRunner createKuduQueryRunnerTpch( - TestingKuduServer kuduServer, - Optional kuduSchemaEmulationPrefix, - Map kuduSessionProperties, - Map extraProperties, - Iterable> tables) - throws Exception - { - return createKuduQueryRunnerTpch(kuduServer, kuduSchemaEmulationPrefix, kuduSessionProperties, extraProperties, ImmutableMap.of(), tables); - } - // TODO convert to builder - private static QueryRunner createKuduQueryRunnerTpch( - TestingKuduServer kuduServer, - Optional kuduSchemaEmulationPrefix, - Map kuduSessionProperties, - Map extraProperties, - Map coordinatorProperties, - Iterable> tables) - throws Exception - { - QueryRunner runner = null; - try { - String kuduSchema = kuduSchemaEmulationPrefix.isPresent() ? "tpch" : "default"; - runner = DistributedQueryRunner.builder(createSession(kuduSchema, kuduSessionProperties)) - .setExtraProperties(extraProperties) - .setCoordinatorProperties(coordinatorProperties) - .build(); - - runner.installPlugin(new TpchPlugin()); - runner.createCatalog("tpch", "tpch"); - - installKuduConnector(kuduServer.getMasterAddress(), runner, kuduSchema, kuduSchemaEmulationPrefix); - - copyTpchTables(runner, "tpch", TINY_SCHEMA_NAME, tables); - - return runner; - } - catch (Throwable e) { - closeAllSuppress(e, runner); - throw e; + @CanIgnoreReturnValue + public Builder setKuduSchemaEmulationPrefix(Optional kuduSchemaEmulationPrefix) + { + this.kuduSchemaEmulationPrefix = requireNonNull(kuduSchemaEmulationPrefix, "kuduSchemaEmulationPrefix is null"); + return this; } - } - private static void installKuduConnector(HostAndPort masterAddress, QueryRunner runner, String kuduSchema, Optional kuduSchemaEmulationPrefix) - { - Map properties; - if (kuduSchemaEmulationPrefix.isPresent()) { - properties = ImmutableMap.of( - "kudu.schema-emulation.enabled", "true", - "kudu.schema-emulation.prefix", kuduSchemaEmulationPrefix.get(), - "kudu.client.master-addresses", masterAddress.toString()); - } - else { - properties = ImmutableMap.of( - "kudu.schema-emulation.enabled", "false", - "kudu.client.master-addresses", masterAddress.toString()); + @CanIgnoreReturnValue + public Builder addConnectorProperty(String key, String value) + { + this.connectorProperties.put(key, value); + return this; } - runner.installPlugin(new KuduPlugin()); - runner.createCatalog("kudu", "kudu", properties); - - if (kuduSchemaEmulationPrefix.isPresent()) { - runner.execute("DROP SCHEMA IF EXISTS " + kuduSchema); - runner.execute("CREATE SCHEMA " + kuduSchema); + @CanIgnoreReturnValue + public Builder setInitialTables(List> initialTables) + { + this.initialTables = ImmutableList.copyOf(initialTables); + return this; } - } - - public static Session createSession(String schema, Map kuduSessionProperties) - { - SessionBuilder builder = testSessionBuilder() - .setCatalog("kudu") - .setSchema(schema); - kuduSessionProperties.forEach((k, v) -> builder.setCatalogSessionProperty("kudu", k, v)); - return builder.build(); - } - public static Session createSession(String schema) - { - return testSessionBuilder() - .setCatalog("kudu") - .setSchema(schema) - .build(); + @Override + public DistributedQueryRunner build() + throws Exception + { + String kuduSchema = kuduSchemaEmulationPrefix.isPresent() ? "tpch" : "default"; + amendSession(sessionBuilder -> sessionBuilder.setSchema(kuduSchema)); + DistributedQueryRunner queryRunner = super.build(); + try { + queryRunner.installPlugin(new TpchPlugin()); + queryRunner.createCatalog("tpch", "tpch"); + + if (kuduSchemaEmulationPrefix.isPresent()) { + addConnectorProperty("kudu.schema-emulation.enabled", "true"); + addConnectorProperty("kudu.schema-emulation.prefix", kuduSchemaEmulationPrefix.get()); + addConnectorProperty("kudu.client.master-addresses", kuduServer.getMasterAddress().toString()); + } + else { + addConnectorProperty("kudu.schema-emulation.enabled", "false"); + addConnectorProperty("kudu.client.master-addresses", kuduServer.getMasterAddress().toString()); + } + + queryRunner.installPlugin(new KuduPlugin()); + queryRunner.createCatalog("kudu", "kudu", connectorProperties); + + if (kuduSchemaEmulationPrefix.isPresent()) { + queryRunner.execute("DROP SCHEMA IF EXISTS " + kuduSchema); + queryRunner.execute("CREATE SCHEMA " + kuduSchema); + } + + copyTpchTables(queryRunner, "tpch", TINY_SCHEMA_NAME, initialTables); + return queryRunner; + } + catch (Throwable e) { + Closables.closeAllSuppress(e, queryRunner); + throw e; + } + } } public static void main(String[] args) throws Exception { Logging.initialize(); - QueryRunner queryRunner = createKuduQueryRunnerTpch( - new TestingKuduServer(), - Optional.empty(), - ImmutableMap.of(), - ImmutableMap.of(), - ImmutableMap.of("http-server.http.port", "8080"), - TpchTable.getTables()); + QueryRunner queryRunner = builder(new TestingKuduServer()) + .addCoordinatorProperty("http-server.http.port", "8080") + .setInitialTables(TpchTable.getTables()) + .build(); + Logger log = Logger.get(KuduQueryRunnerFactory.class); log.info("======== SERVER STARTED ========"); log.info("\n====\n%s\n====", queryRunner.getCoordinator().getBaseUrl()); diff --git a/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/TestKuduConnectorTest.java b/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/TestKuduConnectorTest.java index 0a8fb17bfc14..28c33f135629 100644 --- a/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/TestKuduConnectorTest.java +++ b/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/TestKuduConnectorTest.java @@ -29,7 +29,6 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; -import static io.trino.plugin.kudu.KuduQueryRunnerFactory.createKuduQueryRunnerTpch; import static io.trino.spi.type.VarcharType.VARCHAR; import static io.trino.testing.MaterializedResult.resultBuilder; import static io.trino.testing.TestingNames.randomNameSuffix; @@ -51,10 +50,9 @@ public class TestKuduConnectorTest protected QueryRunner createQueryRunner() throws Exception { - return createKuduQueryRunnerTpch( - closeAfterClass(new TestingKuduServer()), - Optional.empty(), - REQUIRED_TPCH_TABLES); + return KuduQueryRunnerFactory.builder(closeAfterClass(new TestingKuduServer())) + .setInitialTables(REQUIRED_TPCH_TABLES) + .build(); } @Override diff --git a/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/TestKuduIntegrationDecimalColumns.java b/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/TestKuduIntegrationDecimalColumns.java index c2420d76be86..87c98224ab15 100644 --- a/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/TestKuduIntegrationDecimalColumns.java +++ b/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/TestKuduIntegrationDecimalColumns.java @@ -22,7 +22,6 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInstance; -import static io.trino.plugin.kudu.KuduQueryRunnerFactory.createKuduQueryRunner; import static java.lang.String.format; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.offset; @@ -49,7 +48,7 @@ public class TestKuduIntegrationDecimalColumns protected QueryRunner createQueryRunner() throws Exception { - return createKuduQueryRunner(closeAfterClass(new TestingKuduServer()), "decimal"); + return KuduQueryRunnerFactory.builder(closeAfterClass(new TestingKuduServer())).build(); } @AfterAll diff --git a/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/TestKuduIntegrationDynamicFilter.java b/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/TestKuduIntegrationDynamicFilter.java index 21e0da71d518..4c8ca8d418c1 100644 --- a/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/TestKuduIntegrationDynamicFilter.java +++ b/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/TestKuduIntegrationDynamicFilter.java @@ -13,7 +13,6 @@ */ package io.trino.plugin.kudu; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.primitives.Ints; import io.opentelemetry.api.trace.Span; @@ -47,7 +46,6 @@ import static com.google.common.collect.ImmutableList.toImmutableList; import static io.trino.SystemSessionProperties.JOIN_DISTRIBUTION_TYPE; import static io.trino.SystemSessionProperties.JOIN_REORDERING_STRATEGY; -import static io.trino.plugin.kudu.KuduQueryRunnerFactory.createKuduQueryRunnerTpch; import static io.trino.spi.connector.Constraint.alwaysTrue; import static io.trino.sql.planner.OptimizerConfig.JoinDistributionType.BROADCAST; import static io.trino.sql.planner.OptimizerConfig.JoinReorderingStrategy.NONE; @@ -64,14 +62,13 @@ public class TestKuduIntegrationDynamicFilter protected QueryRunner createQueryRunner() throws Exception { - return createKuduQueryRunnerTpch( - closeAfterClass(new TestingKuduServer()), - Optional.of(""), - ImmutableMap.of("dynamic_filtering_wait_timeout", "1h"), - ImmutableMap.of( - "dynamic-filtering.small.max-distinct-values-per-driver", "100", - "dynamic-filtering.small.range-row-limit-per-driver", "100"), - List.of(LINE_ITEM, ORDERS, PART)); + return KuduQueryRunnerFactory.builder(closeAfterClass(new TestingKuduServer())) + .setKuduSchemaEmulationPrefix(Optional.of("")) + .addConnectorProperty("kudu.dynamic-filtering.wait-timeout", "1h") + .addExtraProperty("dynamic-filtering.small.max-distinct-values-per-driver", "100") + .addExtraProperty("dynamic-filtering.small.range-row-limit-per-driver", "100") + .setInitialTables(List.of(LINE_ITEM, ORDERS, PART)) + .build(); } @Test diff --git a/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/TestKuduIntegrationHashPartitioning.java b/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/TestKuduIntegrationHashPartitioning.java index 3c0cb1b917ae..fde6eeffd8aa 100644 --- a/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/TestKuduIntegrationHashPartitioning.java +++ b/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/TestKuduIntegrationHashPartitioning.java @@ -19,7 +19,6 @@ import org.intellij.lang.annotations.Language; import org.junit.jupiter.api.Test; -import static io.trino.plugin.kudu.KuduQueryRunnerFactory.createKuduQueryRunner; import static org.assertj.core.api.Assertions.assertThat; public class TestKuduIntegrationHashPartitioning @@ -29,7 +28,7 @@ public class TestKuduIntegrationHashPartitioning protected QueryRunner createQueryRunner() throws Exception { - return createKuduQueryRunner(closeAfterClass(new TestingKuduServer()), "hash"); + return KuduQueryRunnerFactory.builder(closeAfterClass(new TestingKuduServer())).build(); } @Test diff --git a/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/TestKuduIntegrationIntegerColumns.java b/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/TestKuduIntegrationIntegerColumns.java index c20bddf65646..d87be9f3e72a 100644 --- a/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/TestKuduIntegrationIntegerColumns.java +++ b/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/TestKuduIntegrationIntegerColumns.java @@ -18,7 +18,6 @@ import io.trino.testing.QueryRunner; import org.junit.jupiter.api.Test; -import static io.trino.plugin.kudu.KuduQueryRunnerFactory.createKuduQueryRunner; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Fail.fail; @@ -36,7 +35,7 @@ public class TestKuduIntegrationIntegerColumns protected QueryRunner createQueryRunner() throws Exception { - return createKuduQueryRunner(closeAfterClass(new TestingKuduServer()), "test_integer"); + return KuduQueryRunnerFactory.builder(closeAfterClass(new TestingKuduServer())).build(); } @Test diff --git a/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/TestKuduIntegrationRangePartitioning.java b/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/TestKuduIntegrationRangePartitioning.java index 0e2975b1d0e0..6ac167647dfe 100644 --- a/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/TestKuduIntegrationRangePartitioning.java +++ b/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/TestKuduIntegrationRangePartitioning.java @@ -18,7 +18,6 @@ import io.trino.testing.QueryRunner; import org.junit.jupiter.api.Test; -import static io.trino.plugin.kudu.KuduQueryRunnerFactory.createKuduQueryRunner; import static java.lang.String.join; import static org.assertj.core.api.Assertions.assertThat; @@ -84,7 +83,7 @@ public class TestKuduIntegrationRangePartitioning protected QueryRunner createQueryRunner() throws Exception { - return createKuduQueryRunner(closeAfterClass(new TestingKuduServer()), "range_partitioning"); + return KuduQueryRunnerFactory.builder(closeAfterClass(new TestingKuduServer())).build(); } @Test From 1e4ec4997b4bee59c93f192a9e867ff8d165e2ed Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Fri, 31 May 2024 08:50:21 +0900 Subject: [PATCH 10/19] Convert PhoenixQueryRunner to builder --- .../plugin/phoenix5/PhoenixQueryRunner.java | 88 +++++++++++++------ .../phoenix5/TestPhoenixConnectorTest.java | 5 +- .../phoenix5/TestPhoenixTypeMapping.java | 4 +- 3 files changed, 65 insertions(+), 32 deletions(-) diff --git a/plugin/trino-phoenix5/src/test/java/io/trino/plugin/phoenix5/PhoenixQueryRunner.java b/plugin/trino-phoenix5/src/test/java/io/trino/plugin/phoenix5/PhoenixQueryRunner.java index fdabd9da6c49..864392fc8e78 100644 --- a/plugin/trino-phoenix5/src/test/java/io/trino/plugin/phoenix5/PhoenixQueryRunner.java +++ b/plugin/trino-phoenix5/src/test/java/io/trino/plugin/phoenix5/PhoenixQueryRunner.java @@ -13,10 +13,12 @@ */ package io.trino.plugin.phoenix5; -import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableList; +import com.google.errorprone.annotations.CanIgnoreReturnValue; import io.airlift.log.Logger; import io.trino.Session; import io.trino.metadata.QualifiedObjectName; +import io.trino.plugin.base.util.Closables; import io.trino.plugin.tpch.TpchPlugin; import io.trino.testing.DistributedQueryRunner; import io.trino.testing.QueryRunner; @@ -27,6 +29,7 @@ import java.sql.DriverManager; import java.sql.SQLException; import java.sql.Statement; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Properties; @@ -37,6 +40,7 @@ import static io.trino.tpch.TpchTable.ORDERS; import static io.trino.tpch.TpchTable.PART_SUPPLIER; import static java.lang.String.format; +import static java.util.Objects.requireNonNull; public final class PhoenixQueryRunner { @@ -47,36 +51,65 @@ private PhoenixQueryRunner() { } - // TODO convert to builder - public static QueryRunner createPhoenixQueryRunner(TestingPhoenixServer server, List> tables) - throws Exception + public static Builder builder(TestingPhoenixServer phoenixServer) { - return createPhoenixQueryRunner(server, ImmutableMap.of(), tables); + return new Builder(phoenixServer) + .addConnectorProperty("phoenix.connection-url", phoenixServer.getJdbcUrl()) + .addConnectorProperty("case-insensitive-name-matching", "true"); } - // TODO convert to builder - private static QueryRunner createPhoenixQueryRunner(TestingPhoenixServer server, Map coordinatorProperties, List> tables) - throws Exception + public static class Builder + extends DistributedQueryRunner.Builder { - QueryRunner queryRunner = DistributedQueryRunner.builder(createSession()) - .setCoordinatorProperties(coordinatorProperties) - .build(); - - queryRunner.installPlugin(new TpchPlugin()); - queryRunner.createCatalog("tpch", "tpch"); - - Map properties = ImmutableMap.builder() - .put("phoenix.connection-url", server.getJdbcUrl()) - .put("case-insensitive-name-matching", "true") - .buildOrThrow(); + private final TestingPhoenixServer phoenixServer; + private final Map connectorProperties = new HashMap<>(); + private List> initialTables = ImmutableList.of(); + + private Builder(TestingPhoenixServer phoenixServer) + { + super(testSessionBuilder() + .setCatalog("phoenix") + .setSchema(TPCH_SCHEMA) + .build()); + this.phoenixServer = requireNonNull(phoenixServer, "phoenixServer is null"); + } - queryRunner.installPlugin(new PhoenixPlugin()); - queryRunner.createCatalog("phoenix", "phoenix5", properties); + @CanIgnoreReturnValue + public Builder addConnectorProperty(String key, String value) + { + this.connectorProperties.put(key, value); + return this; + } - createSchema(server, TPCH_SCHEMA); - copyTpchTables(queryRunner, "tpch", TINY_SCHEMA_NAME, createSession(), tables); + @CanIgnoreReturnValue + public Builder setInitialTables(List> initialTables) + { + this.initialTables = ImmutableList.copyOf(initialTables); + return this; + } - return queryRunner; + @Override + public DistributedQueryRunner build() + throws Exception + { + DistributedQueryRunner queryRunner = super.build(); + try { + queryRunner.installPlugin(new TpchPlugin()); + queryRunner.createCatalog("tpch", "tpch"); + + queryRunner.installPlugin(new PhoenixPlugin()); + queryRunner.createCatalog("phoenix", "phoenix5", connectorProperties); + + createSchema(phoenixServer, TPCH_SCHEMA); + copyTpchTables(queryRunner, "tpch", TINY_SCHEMA_NAME, createSession(), initialTables); + + return queryRunner; + } + catch (Throwable e) { + Closables.closeAllSuppress(e, queryRunner); + throw e; + } + } } private static void createSchema(TestingPhoenixServer phoenixServer, String schema) @@ -141,10 +174,9 @@ private static Session createSession() public static void main(String[] args) throws Exception { - QueryRunner queryRunner = createPhoenixQueryRunner( - TestingPhoenixServer.getInstance().get(), - ImmutableMap.of("http-server.http.port", "8080"), - TpchTable.getTables()); + QueryRunner queryRunner = PhoenixQueryRunner.builder(TestingPhoenixServer.getInstance().get()) + .addCoordinatorProperty("http-server.http.port", "8080") + .build(); Logger log = Logger.get(PhoenixQueryRunner.class); log.info("======== SERVER STARTED ========"); diff --git a/plugin/trino-phoenix5/src/test/java/io/trino/plugin/phoenix5/TestPhoenixConnectorTest.java b/plugin/trino-phoenix5/src/test/java/io/trino/plugin/phoenix5/TestPhoenixConnectorTest.java index 7e1dd2962b45..34daac40f239 100644 --- a/plugin/trino-phoenix5/src/test/java/io/trino/plugin/phoenix5/TestPhoenixConnectorTest.java +++ b/plugin/trino-phoenix5/src/test/java/io/trino/plugin/phoenix5/TestPhoenixConnectorTest.java @@ -43,7 +43,6 @@ import static io.trino.plugin.jdbc.JdbcMetadataSessionProperties.DOMAIN_COMPACTION_THRESHOLD; import static io.trino.plugin.jdbc.TypeHandlingJdbcSessionProperties.UNSUPPORTED_TYPE_HANDLING; import static io.trino.plugin.jdbc.UnsupportedTypeHandling.CONVERT_TO_VARCHAR; -import static io.trino.plugin.phoenix5.PhoenixQueryRunner.createPhoenixQueryRunner; import static io.trino.sql.planner.assertions.PlanMatchPattern.exchange; import static io.trino.sql.planner.assertions.PlanMatchPattern.limit; import static io.trino.sql.planner.assertions.PlanMatchPattern.output; @@ -77,7 +76,9 @@ protected QueryRunner createQueryRunner() throws Exception { testingPhoenixServer = closeAfterClass(TestingPhoenixServer.getInstance()).get(); - return createPhoenixQueryRunner(testingPhoenixServer, REQUIRED_TPCH_TABLES); + return PhoenixQueryRunner.builder(testingPhoenixServer) + .setInitialTables(REQUIRED_TPCH_TABLES) + .build(); } @Override diff --git a/plugin/trino-phoenix5/src/test/java/io/trino/plugin/phoenix5/TestPhoenixTypeMapping.java b/plugin/trino-phoenix5/src/test/java/io/trino/plugin/phoenix5/TestPhoenixTypeMapping.java index e6a901ad6d5e..19b9a0879327 100644 --- a/plugin/trino-phoenix5/src/test/java/io/trino/plugin/phoenix5/TestPhoenixTypeMapping.java +++ b/plugin/trino-phoenix5/src/test/java/io/trino/plugin/phoenix5/TestPhoenixTypeMapping.java @@ -47,7 +47,6 @@ import static io.trino.plugin.jdbc.DecimalSessionSessionProperties.DECIMAL_ROUNDING_MODE; import static io.trino.plugin.jdbc.TypeHandlingJdbcSessionProperties.UNSUPPORTED_TYPE_HANDLING; import static io.trino.plugin.jdbc.UnsupportedTypeHandling.CONVERT_TO_VARCHAR; -import static io.trino.plugin.phoenix5.PhoenixQueryRunner.createPhoenixQueryRunner; import static io.trino.spi.type.BigintType.BIGINT; import static io.trino.spi.type.BooleanType.BOOLEAN; import static io.trino.spi.type.CharType.createCharType; @@ -108,7 +107,8 @@ protected QueryRunner createQueryRunner() throws Exception { phoenixServer = closeAfterClass(TestingPhoenixServer.getInstance()).get(); - return createPhoenixQueryRunner(phoenixServer, ImmutableList.of()); + return PhoenixQueryRunner.builder(phoenixServer) + .build(); } @Test From 8d7c90648f75340b596fe20cbe82c67097a4b2bd Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Fri, 31 May 2024 08:56:24 +0900 Subject: [PATCH 11/19] Convert DruidQueryRunner to builder --- plugin/trino-druid/pom.xml | 6 + .../trino/plugin/druid/DruidQueryRunner.java | 125 +++++++++--------- .../TestDruidCaseInsensitiveMapping.java | 15 +-- .../plugin/druid/TestDruidConnectorTest.java | 9 +- .../TestDruidLatestConnectorSmokeTest.java | 9 +- .../plugin/druid/TestDruidTypeMapping.java | 4 +- 6 files changed, 83 insertions(+), 85 deletions(-) diff --git a/plugin/trino-druid/pom.xml b/plugin/trino-druid/pom.xml index a0bb3c9a03a8..054ad72056d5 100644 --- a/plugin/trino-druid/pom.xml +++ b/plugin/trino-druid/pom.xml @@ -94,6 +94,12 @@ runtime + + com.google.errorprone + error_prone_annotations + runtime + + io.airlift log diff --git a/plugin/trino-druid/src/test/java/io/trino/plugin/druid/DruidQueryRunner.java b/plugin/trino-druid/src/test/java/io/trino/plugin/druid/DruidQueryRunner.java index 597ebe1550bb..7b16f294c4ad 100644 --- a/plugin/trino-druid/src/test/java/io/trino/plugin/druid/DruidQueryRunner.java +++ b/plugin/trino-druid/src/test/java/io/trino/plugin/druid/DruidQueryRunner.java @@ -14,10 +14,10 @@ package io.trino.plugin.druid; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import com.google.common.io.Resources; +import com.google.errorprone.annotations.CanIgnoreReturnValue; import io.airlift.log.Logger; -import io.trino.Session; +import io.trino.plugin.base.util.Closables; import io.trino.plugin.tpch.TpchPlugin; import io.trino.testing.DistributedQueryRunner; import io.trino.testing.MaterializedResult; @@ -37,7 +37,6 @@ import java.util.stream.Collectors; import static com.google.common.io.Resources.getResource; -import static io.airlift.testing.Closeables.closeAllSuppress; import static io.airlift.units.Duration.nanosSince; import static io.trino.testing.TestingSession.testSessionBuilder; import static io.trino.tpch.TpchTable.CUSTOMER; @@ -48,6 +47,7 @@ import static io.trino.tpch.TpchTable.REGION; import static java.lang.String.format; import static java.nio.charset.StandardCharsets.UTF_8; +import static java.util.Objects.requireNonNull; import static java.util.concurrent.TimeUnit.SECONDS; public final class DruidQueryRunner @@ -58,56 +58,71 @@ private DruidQueryRunner() {} private static final String SCHEMA = "druid"; - public static QueryRunner createDruidQueryRunnerTpch( - TestingDruidServer testingDruidServer, - Map connectorProperties, - Iterable> tables) - throws Exception + public static Builder builder(TestingDruidServer druidServer) { - return createDruidQueryRunnerTpch( - testingDruidServer, - ImmutableMap.of(), - connectorProperties, - tables); + return new Builder(druidServer) + .addConnectorProperty("connection-url", druidServer.getJdbcUrl()); } - // TODO convert to builder - private static QueryRunner createDruidQueryRunnerTpch( - TestingDruidServer testingDruidServer, - Map coordinatorProperties, - Map connectorProperties, - Iterable> tables) - throws Exception + public static class Builder + extends DistributedQueryRunner.Builder { - QueryRunner queryRunner = null; - try { - queryRunner = DistributedQueryRunner.builder(createSession()) - .setCoordinatorProperties(coordinatorProperties) - .build(); - queryRunner.installPlugin(new TpchPlugin()); - queryRunner.createCatalog("tpch", "tpch"); - - connectorProperties = new HashMap<>(ImmutableMap.copyOf(connectorProperties)); - connectorProperties.putIfAbsent("connection-url", testingDruidServer.getJdbcUrl()); - queryRunner.installPlugin(new DruidJdbcPlugin()); - queryRunner.createCatalog("druid", "druid", connectorProperties); - - log.info("Loading data from druid.%s...", SCHEMA); - long startTime = System.nanoTime(); - for (TpchTable table : tables) { - long start = System.nanoTime(); - log.info("Running import for %s", table.getTableName()); - MaterializedResult rows = queryRunner.execute(DruidTpchTables.getSelectQuery(table.getTableName())); - copyAndIngestTpchData(rows, testingDruidServer, table.getTableName()); - log.info("Imported %s rows for %s in %s", rows.getRowCount(), table.getTableName(), nanosSince(start).convertToMostSuccinctTimeUnit()); - } - log.info("Loading from druid.%s complete in %s", SCHEMA, nanosSince(startTime).toString(SECONDS)); + private final TestingDruidServer druidServer; + private final Map connectorProperties = new HashMap<>(); + private List> initialTables = ImmutableList.of(); + + private Builder(TestingDruidServer druidServer) + { + super(testSessionBuilder() + .setCatalog("druid") + .setSchema(SCHEMA) + .build()); + this.druidServer = requireNonNull(druidServer, "druidServer is null"); + } + + @CanIgnoreReturnValue + public Builder addConnectorProperty(String key, String value) + { + this.connectorProperties.put(key, value); + return this; + } - return queryRunner; + @CanIgnoreReturnValue + public Builder setInitialTables(List> initialTables) + { + this.initialTables = ImmutableList.copyOf(initialTables); + return this; } - catch (Throwable e) { - closeAllSuppress(e, queryRunner); - throw e; + + @Override + public DistributedQueryRunner build() + throws Exception + { + DistributedQueryRunner queryRunner = super.build(); + try { + queryRunner.installPlugin(new TpchPlugin()); + queryRunner.createCatalog("tpch", "tpch"); + + queryRunner.installPlugin(new DruidJdbcPlugin()); + queryRunner.createCatalog("druid", "druid", connectorProperties); + + log.info("Loading data from druid.%s...", SCHEMA); + long startTime = System.nanoTime(); + for (TpchTable table : initialTables) { + long start = System.nanoTime(); + log.info("Running import for %s", table.getTableName()); + MaterializedResult rows = queryRunner.execute(DruidTpchTables.getSelectQuery(table.getTableName())); + copyAndIngestTpchData(rows, druidServer, table.getTableName()); + log.info("Imported %s rows for %s in %s", rows.getRowCount(), table.getTableName(), nanosSince(start).convertToMostSuccinctTimeUnit()); + } + log.info("Loading from druid.%s complete in %s", SCHEMA, nanosSince(startTime).toString(SECONDS)); + + return queryRunner; + } + catch (Throwable e) { + Closables.closeAllSuppress(e, queryRunner); + throw e; + } } } @@ -149,14 +164,6 @@ private static String getIngestionSpecFileName(String datasource) return format("druid-tpch-ingest-%s.json", datasource); } - private static Session createSession() - { - return testSessionBuilder() - .setCatalog("druid") - .setSchema(SCHEMA) - .build(); - } - private static void writeDataAsTsv(MaterializedResult rows, String dataFile) throws IOException { @@ -179,11 +186,9 @@ private static String convertToTSV(List data) public static void main(String[] args) throws Exception { - QueryRunner queryRunner = createDruidQueryRunnerTpch( - new TestingDruidServer(), - ImmutableMap.of("http-server.http.port", "8080"), - ImmutableMap.of(), - ImmutableList.of(ORDERS, LINE_ITEM, NATION, REGION, PART, CUSTOMER)); + QueryRunner queryRunner = builder(new TestingDruidServer()) + .setInitialTables(ImmutableList.of(ORDERS, LINE_ITEM, NATION, REGION, PART, CUSTOMER)) + .build(); Logger log = Logger.get(DruidQueryRunner.class); log.info("======== SERVER STARTED ========"); diff --git a/plugin/trino-druid/src/test/java/io/trino/plugin/druid/TestDruidCaseInsensitiveMapping.java b/plugin/trino-druid/src/test/java/io/trino/plugin/druid/TestDruidCaseInsensitiveMapping.java index 2724311a134a..3f9cb7adf0b2 100644 --- a/plugin/trino-druid/src/test/java/io/trino/plugin/druid/TestDruidCaseInsensitiveMapping.java +++ b/plugin/trino-druid/src/test/java/io/trino/plugin/druid/TestDruidCaseInsensitiveMapping.java @@ -14,7 +14,6 @@ package io.trino.plugin.druid; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import io.trino.plugin.base.mapping.TableMappingRule; import io.trino.plugin.jdbc.BaseCaseInsensitiveMappingTest; import io.trino.testing.MaterializedResult; @@ -31,12 +30,9 @@ import static io.trino.plugin.base.mapping.RuleBasedIdentifierMappingUtils.createRuleBasedIdentifierMappingFile; import static io.trino.plugin.base.mapping.RuleBasedIdentifierMappingUtils.updateRuleBasedIdentifierMappingFile; import static io.trino.plugin.druid.DruidQueryRunner.copyAndIngestTpchDataFromSourceToTarget; -import static io.trino.plugin.druid.DruidQueryRunner.createDruidQueryRunnerTpch; import static io.trino.plugin.druid.DruidTpchTables.SELECT_FROM_ORDERS; import static io.trino.plugin.druid.DruidTpchTables.SELECT_FROM_REGION; import static io.trino.spi.type.VarcharType.VARCHAR; -import static io.trino.tpch.TpchTable.ORDERS; -import static io.trino.tpch.TpchTable.REGION; import static java.util.Objects.requireNonNull; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assumptions.abort; @@ -64,12 +60,11 @@ protected QueryRunner createQueryRunner() { druidServer = new TestingDruidServer(); mappingFile = createRuleBasedIdentifierMappingFile(); - QueryRunner queryRunner = createDruidQueryRunnerTpch( - druidServer, - ImmutableMap.of("case-insensitive-name-matching", "true", - "case-insensitive-name-matching.config-file", mappingFile.toFile().getAbsolutePath(), - "case-insensitive-name-matching.config-file.refresh-period", "1ms"), // ~always refresh - ImmutableList.of(ORDERS, REGION)); + QueryRunner queryRunner = DruidQueryRunner.builder(druidServer) + .addConnectorProperty("case-insensitive-name-matching", "true") + .addConnectorProperty("case-insensitive-name-matching.config-file", mappingFile.toFile().getAbsolutePath()) + .addConnectorProperty("case-insensitive-name-matching.config-file.refresh-period", "1ms") // ~always refresh + .build(); copyAndIngestTpchDataFromSourceToTarget(queryRunner.execute(SELECT_FROM_ORDERS + " LIMIT 10"), this.druidServer, "orders", "MiXeD_CaSe", Optional.empty()); return queryRunner; diff --git a/plugin/trino-druid/src/test/java/io/trino/plugin/druid/TestDruidConnectorTest.java b/plugin/trino-druid/src/test/java/io/trino/plugin/druid/TestDruidConnectorTest.java index 027745779b8b..840e0f7867fe 100644 --- a/plugin/trino-druid/src/test/java/io/trino/plugin/druid/TestDruidConnectorTest.java +++ b/plugin/trino-druid/src/test/java/io/trino/plugin/druid/TestDruidConnectorTest.java @@ -14,7 +14,6 @@ package io.trino.plugin.druid; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import io.trino.Session; import io.trino.plugin.jdbc.BaseJdbcConnectorTest; import io.trino.plugin.jdbc.JdbcTableHandle; @@ -39,7 +38,6 @@ import java.sql.SQLException; import static io.trino.plugin.druid.DruidQueryRunner.copyAndIngestTpchData; -import static io.trino.plugin.druid.DruidQueryRunner.createDruidQueryRunnerTpch; import static io.trino.plugin.druid.DruidTpchTables.SELECT_FROM_ORDERS; import static io.trino.spi.type.VarcharType.VARCHAR; import static io.trino.sql.planner.assertions.PlanMatchPattern.anyTree; @@ -70,10 +68,9 @@ protected QueryRunner createQueryRunner() throws Exception { druidServer = closeAfterClass(new TestingDruidServer()); - return createDruidQueryRunnerTpch( - druidServer, - ImmutableMap.of(), - ImmutableList.of(ORDERS, LINE_ITEM, NATION, REGION, PART, CUSTOMER)); + return DruidQueryRunner.builder(druidServer) + .setInitialTables(ImmutableList.of(ORDERS, LINE_ITEM, NATION, REGION, PART, CUSTOMER)) + .build(); } @AfterAll diff --git a/plugin/trino-druid/src/test/java/io/trino/plugin/druid/TestDruidLatestConnectorSmokeTest.java b/plugin/trino-druid/src/test/java/io/trino/plugin/druid/TestDruidLatestConnectorSmokeTest.java index 515597ef7152..0290d8bf95b3 100644 --- a/plugin/trino-druid/src/test/java/io/trino/plugin/druid/TestDruidLatestConnectorSmokeTest.java +++ b/plugin/trino-druid/src/test/java/io/trino/plugin/druid/TestDruidLatestConnectorSmokeTest.java @@ -13,7 +13,6 @@ */ package io.trino.plugin.druid; -import com.google.common.collect.ImmutableMap; import io.trino.plugin.jdbc.BaseJdbcConnectorSmokeTest; import io.trino.testing.QueryRunner; import io.trino.testing.TestingConnectorBehavior; @@ -21,7 +20,6 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInstance; -import static io.trino.plugin.druid.DruidQueryRunner.createDruidQueryRunnerTpch; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS; @@ -36,10 +34,9 @@ protected QueryRunner createQueryRunner() throws Exception { druidServer = closeAfterClass(new TestingDruidServer("apache/druid:0.20.0")); - return createDruidQueryRunnerTpch( - druidServer, - ImmutableMap.of(), - REQUIRED_TPCH_TABLES); + return DruidQueryRunner.builder(druidServer) + .setInitialTables(REQUIRED_TPCH_TABLES) + .build(); } @AfterAll diff --git a/plugin/trino-druid/src/test/java/io/trino/plugin/druid/TestDruidTypeMapping.java b/plugin/trino-druid/src/test/java/io/trino/plugin/druid/TestDruidTypeMapping.java index 8d7c1a0b8450..efab10b17477 100644 --- a/plugin/trino-druid/src/test/java/io/trino/plugin/druid/TestDruidTypeMapping.java +++ b/plugin/trino-druid/src/test/java/io/trino/plugin/druid/TestDruidTypeMapping.java @@ -14,7 +14,6 @@ package io.trino.plugin.druid; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import io.trino.plugin.druid.ingestion.IndexTaskBuilder; import io.trino.plugin.druid.ingestion.TimestampSpec; import io.trino.testing.AbstractTestQueryFramework; @@ -31,7 +30,6 @@ import java.util.List; import java.util.Optional; -import static io.trino.plugin.druid.DruidQueryRunner.createDruidQueryRunnerTpch; import static io.trino.spi.type.BigintType.BIGINT; import static io.trino.spi.type.DoubleType.DOUBLE; import static io.trino.spi.type.RealType.REAL; @@ -57,7 +55,7 @@ protected QueryRunner createQueryRunner() throws Exception { this.druidServer = new TestingDruidServer(DRUID_DOCKER_IMAGE); - return createDruidQueryRunnerTpch(druidServer, ImmutableMap.of(), ImmutableList.of()); + return DruidQueryRunner.builder(druidServer).build(); } @AfterAll From 3f7ea3065187bdfdd6c241b23b62c650bf56407e Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Fri, 31 May 2024 09:01:51 +0900 Subject: [PATCH 12/19] Convert IgniteQueryRunner to builder --- .../plugin/ignite/IgniteQueryRunner.java | 98 ++++++++++--------- .../TestIgniteCaseInsensitiveMapping.java | 15 +-- .../ignite/TestIgniteConnectorTest.java | 9 +- .../plugin/ignite/TestIgniteTypeMapping.java | 12 +-- 4 files changed, 64 insertions(+), 70 deletions(-) diff --git a/plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/IgniteQueryRunner.java b/plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/IgniteQueryRunner.java index d39e5c02066e..759126a0e306 100644 --- a/plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/IgniteQueryRunner.java +++ b/plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/IgniteQueryRunner.java @@ -13,10 +13,10 @@ */ package io.trino.plugin.ignite; -import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableList; +import com.google.errorprone.annotations.CanIgnoreReturnValue; import io.airlift.log.Logger; import io.airlift.log.Logging; -import io.trino.Session; import io.trino.plugin.tpch.TpchPlugin; import io.trino.testing.DistributedQueryRunner; import io.trino.testing.QueryRunner; @@ -30,6 +30,7 @@ import static io.trino.plugin.tpch.TpchMetadata.TINY_SCHEMA_NAME; import static io.trino.testing.QueryAssertions.copyTpchTables; import static io.trino.testing.TestingSession.testSessionBuilder; +import static java.util.Objects.requireNonNull; public final class IgniteQueryRunner { @@ -37,53 +38,61 @@ private IgniteQueryRunner() {} private static final String IGNITE_SCHEMA = "public"; - public static QueryRunner createIgniteQueryRunner( - TestingIgniteServer server, - Map connectorProperties, - List> tables) - throws Exception + public static Builder builder(TestingIgniteServer server) { - return createIgniteQueryRunner(server, ImmutableMap.of(), connectorProperties, tables); + return new Builder() + .addConnectorProperty("connection-url", server.getJdbcUrl()); } - // TODO convert to builder - private static QueryRunner createIgniteQueryRunner( - TestingIgniteServer server, - Map coordinatorProperties, - Map connectorProperties, - List> tables) - throws Exception + public static final class Builder + extends DistributedQueryRunner.Builder { - QueryRunner queryRunner = null; - try { - queryRunner = DistributedQueryRunner.builder(createSession()) - .setCoordinatorProperties(coordinatorProperties) - .build(); - - queryRunner.installPlugin(new TpchPlugin()); - queryRunner.createCatalog("tpch", "tpch"); - - connectorProperties = new HashMap<>(ImmutableMap.copyOf(connectorProperties)); - connectorProperties.putIfAbsent("connection-url", server.getJdbcUrl()); + private final Map connectorProperties = new HashMap<>(); + private List> initialTables = ImmutableList.of(); - queryRunner.installPlugin(new IgnitePlugin()); - queryRunner.createCatalog("ignite", "ignite", connectorProperties); + private Builder() + { + super(testSessionBuilder() + .setCatalog("ignite") + .setSchema(IGNITE_SCHEMA) + .build()); + } - copyTpchTables(queryRunner, "tpch", TINY_SCHEMA_NAME, tables); - return queryRunner; + @CanIgnoreReturnValue + public Builder addConnectorProperty(String key, String value) + { + this.connectorProperties.put(key, value); + return this; } - catch (Throwable e) { - closeAllSuppress(e, queryRunner); - throw e; + + @CanIgnoreReturnValue + public Builder setInitialTables(Iterable> initialTables) + { + this.initialTables = ImmutableList.copyOf(requireNonNull(initialTables, "initialTables is null")); + return this; } - } - public static Session createSession() - { - return testSessionBuilder() - .setCatalog("ignite") - .setSchema(IGNITE_SCHEMA) - .build(); + @Override + public DistributedQueryRunner build() + throws Exception + { + DistributedQueryRunner queryRunner = super.build(); + try { + queryRunner.installPlugin(new TpchPlugin()); + queryRunner.createCatalog("tpch", "tpch"); + + queryRunner.installPlugin(new IgnitePlugin()); + queryRunner.createCatalog("ignite", "ignite", connectorProperties); + + copyTpchTables(queryRunner, "tpch", TINY_SCHEMA_NAME, initialTables); + + return queryRunner; + } + catch (Throwable e) { + closeAllSuppress(e, queryRunner); + throw e; + } + } } public static void main(String[] args) @@ -91,11 +100,10 @@ public static void main(String[] args) { Logging.initialize(); - QueryRunner queryRunner = createIgniteQueryRunner( - TestingIgniteServer.getInstance().get(), - ImmutableMap.of("http-server.http.port", "8080"), - ImmutableMap.of(), - TpchTable.getTables()); + QueryRunner queryRunner = builder(TestingIgniteServer.getInstance().get()) + .addCoordinatorProperty("http-server.http.port", "8080") + .setInitialTables(TpchTable.getTables()) + .build(); Logger log = Logger.get(IgniteQueryRunner.class); log.info("======== SERVER STARTED ========"); diff --git a/plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteCaseInsensitiveMapping.java b/plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteCaseInsensitiveMapping.java index b9003cdf9d43..ff421052fc35 100644 --- a/plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteCaseInsensitiveMapping.java +++ b/plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteCaseInsensitiveMapping.java @@ -14,7 +14,6 @@ package io.trino.plugin.ignite; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import io.trino.plugin.jdbc.BaseCaseInsensitiveMappingTest; import io.trino.testing.MaterializedRow; @@ -28,7 +27,6 @@ import static com.google.common.collect.ImmutableSet.toImmutableSet; import static io.trino.plugin.base.mapping.RuleBasedIdentifierMappingUtils.createRuleBasedIdentifierMappingFile; -import static io.trino.plugin.ignite.IgniteQueryRunner.createIgniteQueryRunner; import static java.util.Locale.ENGLISH; import static java.util.Objects.requireNonNull; import static org.assertj.core.api.Assertions.assertThat; @@ -48,14 +46,11 @@ protected QueryRunner createQueryRunner() { mappingFile = createRuleBasedIdentifierMappingFile(); igniteServer = closeAfterClass(TestingIgniteServer.getInstance()).get(); - return createIgniteQueryRunner( - igniteServer, - ImmutableMap.builder() - .put("case-insensitive-name-matching", "true") - .put("case-insensitive-name-matching.config-file", mappingFile.toFile().getAbsolutePath()) - .put("case-insensitive-name-matching.config-file.refresh-period", "1ms") // ~always refresh - .buildOrThrow(), - ImmutableList.of()); + return IgniteQueryRunner.builder(igniteServer) + .addConnectorProperty("case-insensitive-name-matching", "true") + .addConnectorProperty("case-insensitive-name-matching.config-file", mappingFile.toFile().getAbsolutePath()) + .addConnectorProperty("case-insensitive-name-matching.config-file.refresh-period", "1ms") // ~always refresh + .build(); } @Override 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 b07d98755f7f..ebd1116bb8d6 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 @@ -14,7 +14,6 @@ package io.trino.plugin.ignite; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import io.trino.plugin.jdbc.BaseJdbcConnectorTest; import io.trino.sql.planner.plan.FilterNode; import io.trino.sql.planner.plan.TableScanNode; @@ -30,7 +29,6 @@ import java.util.Optional; import static com.google.common.base.Strings.nullToEmpty; -import static io.trino.plugin.ignite.IgniteQueryRunner.createIgniteQueryRunner; import static io.trino.sql.planner.assertions.PlanMatchPattern.node; import static io.trino.testing.TestingNames.randomNameSuffix; import static java.lang.String.format; @@ -49,10 +47,9 @@ protected QueryRunner createQueryRunner() throws Exception { this.igniteServer = closeAfterClass(TestingIgniteServer.getInstance()).get(); - return createIgniteQueryRunner( - igniteServer, - ImmutableMap.of(), - REQUIRED_TPCH_TABLES); + return IgniteQueryRunner.builder(igniteServer) + .setInitialTables(REQUIRED_TPCH_TABLES) + .build(); } @Override diff --git a/plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteTypeMapping.java b/plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteTypeMapping.java index 2732de1b7006..02bace045b3d 100644 --- a/plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteTypeMapping.java +++ b/plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteTypeMapping.java @@ -14,7 +14,6 @@ package io.trino.plugin.ignite; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import io.trino.Session; import io.trino.spi.type.TimeZoneKey; import io.trino.testing.AbstractTestQueryFramework; @@ -36,8 +35,6 @@ import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Verify.verify; -import static io.trino.plugin.ignite.IgniteQueryRunner.createIgniteQueryRunner; -import static io.trino.plugin.ignite.IgniteQueryRunner.createSession; import static io.trino.spi.type.BigintType.BIGINT; import static io.trino.spi.type.BooleanType.BOOLEAN; import static io.trino.spi.type.DateType.DATE; @@ -69,10 +66,7 @@ protected QueryRunner createQueryRunner() throws Exception { this.igniteServer = closeAfterClass(TestingIgniteServer.getInstance()).get(); - return createIgniteQueryRunner( - igniteServer, - ImmutableMap.of(), - ImmutableList.of()); + return IgniteQueryRunner.builder(igniteServer).build(); } @BeforeAll @@ -234,7 +228,7 @@ public void testDouble() .addRoundTrip("double", "+infinity()", DOUBLE, "+infinity()") .addRoundTrip("double", "-infinity()", DOUBLE, "-infinity()") .execute(getQueryRunner(), trinoCreateAsSelect("trino_test_double")) - .execute(getQueryRunner(), trinoCreateAndInsert(createSession(), "trino_test_double")); + .execute(getQueryRunner(), trinoCreateAndInsert("trino_test_double")); } @Test @@ -424,7 +418,7 @@ public void testUnboundedVarchar() private DataSetup trinoCreateAndInsert(String tableNamePrefix) { - return trinoCreateAndInsert(createSession(), tableNamePrefix); + return trinoCreateAndInsert(getSession(), tableNamePrefix); } private DataSetup trinoCreateAndInsert(Session session, String tableNamePrefix) From aafb1f5ace340686b337c8efc7d9cd687c2df42b Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Fri, 31 May 2024 09:07:58 +0900 Subject: [PATCH 13/19] Convert MariaDbQueryRunner to builder --- plugin/trino-mariadb/pom.xml | 6 + .../BaseMariaDbTableStatisticsTest.java | 9 +- .../plugin/mariadb/MariaDbQueryRunner.java | 104 ++++++++++-------- .../TestMariaDbCaseInsensitiveMapping.java | 16 +-- .../mariadb/TestMariaDbConnectorTest.java | 7 +- .../TestMariaDbLatestConnectorSmokeTest.java | 6 +- .../mariadb/TestMariaDbTypeMapping.java | 5 +- 7 files changed, 78 insertions(+), 75 deletions(-) diff --git a/plugin/trino-mariadb/pom.xml b/plugin/trino-mariadb/pom.xml index 5932031799ee..2a771218d746 100644 --- a/plugin/trino-mariadb/pom.xml +++ b/plugin/trino-mariadb/pom.xml @@ -99,6 +99,12 @@ provided + + com.google.errorprone + error_prone_annotations + runtime + + io.airlift log-manager diff --git a/plugin/trino-mariadb/src/test/java/io/trino/plugin/mariadb/BaseMariaDbTableStatisticsTest.java b/plugin/trino-mariadb/src/test/java/io/trino/plugin/mariadb/BaseMariaDbTableStatisticsTest.java index 0f1ef8197116..24dc5dc3411a 100644 --- a/plugin/trino-mariadb/src/test/java/io/trino/plugin/mariadb/BaseMariaDbTableStatisticsTest.java +++ b/plugin/trino-mariadb/src/test/java/io/trino/plugin/mariadb/BaseMariaDbTableStatisticsTest.java @@ -32,7 +32,6 @@ import static com.google.common.base.Verify.verify; import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.Streams.stream; -import static io.trino.plugin.mariadb.MariaDbQueryRunner.createMariaDbQueryRunner; import static io.trino.testing.TestingNames.randomNameSuffix; import static io.trino.testing.sql.TestTable.fromColumns; import static io.trino.tpch.TpchTable.ORDERS; @@ -67,10 +66,10 @@ protected QueryRunner createQueryRunner() { mariaDbServer = closeAfterClass(new TestingMariaDbServer(dockerImageName)); - return createMariaDbQueryRunner( - mariaDbServer, - Map.of("case-insensitive-name-matching", "true"), - List.of(ORDERS)); + return MariaDbQueryRunner.builder(mariaDbServer) + .addConnectorProperty("case-insensitive-name-matching", "true") + .setInitialTables(List.of(ORDERS)) + .build(); } @Test diff --git a/plugin/trino-mariadb/src/test/java/io/trino/plugin/mariadb/MariaDbQueryRunner.java b/plugin/trino-mariadb/src/test/java/io/trino/plugin/mariadb/MariaDbQueryRunner.java index ca4ee8acc6d0..a3874e998391 100644 --- a/plugin/trino-mariadb/src/test/java/io/trino/plugin/mariadb/MariaDbQueryRunner.java +++ b/plugin/trino-mariadb/src/test/java/io/trino/plugin/mariadb/MariaDbQueryRunner.java @@ -13,23 +13,25 @@ */ package io.trino.plugin.mariadb; -import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableList; +import com.google.errorprone.annotations.CanIgnoreReturnValue; import io.airlift.log.Level; import io.airlift.log.Logger; import io.airlift.log.Logging; -import io.trino.Session; import io.trino.plugin.tpch.TpchPlugin; import io.trino.testing.DistributedQueryRunner; import io.trino.testing.QueryRunner; import io.trino.tpch.TpchTable; import java.util.HashMap; +import java.util.List; import java.util.Map; import static io.airlift.testing.Closeables.closeAllSuppress; import static io.trino.plugin.tpch.TpchMetadata.TINY_SCHEMA_NAME; import static io.trino.testing.QueryAssertions.copyTpchTables; import static io.trino.testing.TestingSession.testSessionBuilder; +import static java.util.Objects.requireNonNull; public final class MariaDbQueryRunner { @@ -42,66 +44,72 @@ public final class MariaDbQueryRunner private MariaDbQueryRunner() {} - // TODO convert to builder - public static QueryRunner createMariaDbQueryRunner( - TestingMariaDbServer server, - Map connectorProperties, - Iterable> tables) - throws Exception + public static Builder builder(TestingMariaDbServer mariaDbServer) { - return createMariaDbQueryRunner(server, ImmutableMap.of(), connectorProperties, tables); + return new Builder() + .addConnectorProperty("connection-url", mariaDbServer.getJdbcUrl()) + .addConnectorProperty("connection-user", mariaDbServer.getUsername()) + .addConnectorProperty("connection-password", mariaDbServer.getPassword()); } - // TODO convert to builder - private static QueryRunner createMariaDbQueryRunner( - TestingMariaDbServer server, - Map coordinatorProperties, - Map connectorProperties, - Iterable> tables) - throws Exception + public static final class Builder + extends DistributedQueryRunner.Builder { - QueryRunner queryRunner = DistributedQueryRunner.builder(createSession()) - .setCoordinatorProperties(coordinatorProperties) - .build(); - try { - queryRunner.installPlugin(new TpchPlugin()); - queryRunner.createCatalog("tpch", "tpch"); - - // note: additional copy via ImmutableList so that if fails on nulls - connectorProperties = new HashMap<>(ImmutableMap.copyOf(connectorProperties)); - connectorProperties.putIfAbsent("connection-url", server.getJdbcUrl()); - connectorProperties.putIfAbsent("connection-user", server.getUsername()); - connectorProperties.putIfAbsent("connection-password", server.getPassword()); + private final Map connectorProperties = new HashMap<>(); + private List> initialTables = ImmutableList.of(); - queryRunner.installPlugin(new MariaDbPlugin()); - queryRunner.createCatalog("mariadb", "mariadb", connectorProperties); - - copyTpchTables(queryRunner, "tpch", TINY_SCHEMA_NAME, tables); + private Builder() + { + super(testSessionBuilder() + .setCatalog("mariadb") + .setSchema(TPCH_SCHEMA) + .build()); + } - return queryRunner; + @CanIgnoreReturnValue + public Builder addConnectorProperty(String key, String value) + { + this.connectorProperties.put(key, value); + return this; } - catch (Throwable e) { - closeAllSuppress(e, queryRunner); - throw e; + + @CanIgnoreReturnValue + public Builder setInitialTables(Iterable> initialTables) + { + this.initialTables = ImmutableList.copyOf(requireNonNull(initialTables, "initialTables is null")); + return this; } - } - private static Session createSession() - { - return testSessionBuilder() - .setCatalog("mariadb") - .setSchema(TPCH_SCHEMA) - .build(); + @Override + public DistributedQueryRunner build() + throws Exception + { + DistributedQueryRunner queryRunner = super.build(); + try { + queryRunner.installPlugin(new TpchPlugin()); + queryRunner.createCatalog("tpch", "tpch"); + + queryRunner.installPlugin(new MariaDbPlugin()); + queryRunner.createCatalog("mariadb", "mariadb", connectorProperties); + + copyTpchTables(queryRunner, "tpch", TINY_SCHEMA_NAME, initialTables); + + return queryRunner; + } + catch (Throwable e) { + closeAllSuppress(e, queryRunner); + throw e; + } + } } public static void main(String[] args) throws Exception { - QueryRunner queryRunner = createMariaDbQueryRunner( - new TestingMariaDbServer(), - ImmutableMap.of("http-server.http.port", "8080"), - ImmutableMap.of(), - TpchTable.getTables()); + QueryRunner queryRunner = builder(new TestingMariaDbServer()) + .addCoordinatorProperty("http-server.http.port", "8080") + .setInitialTables(TpchTable.getTables()) + .build(); Logger log = Logger.get(MariaDbQueryRunner.class); log.info("======== SERVER STARTED ========"); diff --git a/plugin/trino-mariadb/src/test/java/io/trino/plugin/mariadb/TestMariaDbCaseInsensitiveMapping.java b/plugin/trino-mariadb/src/test/java/io/trino/plugin/mariadb/TestMariaDbCaseInsensitiveMapping.java index 2cfd87ac2e80..593ca323a093 100644 --- a/plugin/trino-mariadb/src/test/java/io/trino/plugin/mariadb/TestMariaDbCaseInsensitiveMapping.java +++ b/plugin/trino-mariadb/src/test/java/io/trino/plugin/mariadb/TestMariaDbCaseInsensitiveMapping.java @@ -13,8 +13,6 @@ */ package io.trino.plugin.mariadb; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import io.trino.plugin.jdbc.BaseCaseInsensitiveMappingTest; import io.trino.testing.QueryRunner; import io.trino.testing.sql.SqlExecutor; @@ -22,7 +20,6 @@ import java.nio.file.Path; import static io.trino.plugin.base.mapping.RuleBasedIdentifierMappingUtils.createRuleBasedIdentifierMappingFile; -import static io.trino.plugin.mariadb.MariaDbQueryRunner.createMariaDbQueryRunner; import static java.util.Objects.requireNonNull; // With case-insensitive-name-matching enabled colliding schema/table names are considered as errors. @@ -39,14 +36,11 @@ protected QueryRunner createQueryRunner() { mappingFile = createRuleBasedIdentifierMappingFile(); server = closeAfterClass(new TestingMariaDbServer()); - return createMariaDbQueryRunner( - server, - ImmutableMap.builder() - .put("case-insensitive-name-matching", "true") - .put("case-insensitive-name-matching.config-file", mappingFile.toFile().getAbsolutePath()) - .put("case-insensitive-name-matching.config-file.refresh-period", "1ms") // ~always refresh - .buildOrThrow(), - ImmutableList.of()); + return MariaDbQueryRunner.builder(server) + .addConnectorProperty("case-insensitive-name-matching", "true") + .addConnectorProperty("case-insensitive-name-matching.config-file", mappingFile.toFile().getAbsolutePath()) + .addConnectorProperty("case-insensitive-name-matching.config-file.refresh-period", "1ms") // ~always refresh + .build(); } @Override diff --git a/plugin/trino-mariadb/src/test/java/io/trino/plugin/mariadb/TestMariaDbConnectorTest.java b/plugin/trino-mariadb/src/test/java/io/trino/plugin/mariadb/TestMariaDbConnectorTest.java index ffbaebf19860..7532cc2c2f84 100644 --- a/plugin/trino-mariadb/src/test/java/io/trino/plugin/mariadb/TestMariaDbConnectorTest.java +++ b/plugin/trino-mariadb/src/test/java/io/trino/plugin/mariadb/TestMariaDbConnectorTest.java @@ -13,12 +13,9 @@ */ package io.trino.plugin.mariadb; -import com.google.common.collect.ImmutableMap; import io.trino.testing.QueryRunner; import io.trino.testing.sql.SqlExecutor; -import static io.trino.plugin.mariadb.MariaDbQueryRunner.createMariaDbQueryRunner; - public class TestMariaDbConnectorTest extends BaseMariaDbConnectorTest { @@ -27,7 +24,9 @@ protected QueryRunner createQueryRunner() throws Exception { server = closeAfterClass(new TestingMariaDbServer()); - return createMariaDbQueryRunner(server, ImmutableMap.of(), REQUIRED_TPCH_TABLES); + return MariaDbQueryRunner.builder(server) + .setInitialTables(REQUIRED_TPCH_TABLES) + .build(); } @Override diff --git a/plugin/trino-mariadb/src/test/java/io/trino/plugin/mariadb/TestMariaDbLatestConnectorSmokeTest.java b/plugin/trino-mariadb/src/test/java/io/trino/plugin/mariadb/TestMariaDbLatestConnectorSmokeTest.java index 53152cecfd57..d55d52afb476 100644 --- a/plugin/trino-mariadb/src/test/java/io/trino/plugin/mariadb/TestMariaDbLatestConnectorSmokeTest.java +++ b/plugin/trino-mariadb/src/test/java/io/trino/plugin/mariadb/TestMariaDbLatestConnectorSmokeTest.java @@ -13,12 +13,10 @@ */ package io.trino.plugin.mariadb; -import com.google.common.collect.ImmutableMap; import io.trino.plugin.jdbc.BaseJdbcConnectorSmokeTest; import io.trino.testing.QueryRunner; import io.trino.testing.TestingConnectorBehavior; -import static io.trino.plugin.mariadb.MariaDbQueryRunner.createMariaDbQueryRunner; import static io.trino.plugin.mariadb.TestingMariaDbServer.LATEST_VERSION; public class TestMariaDbLatestConnectorSmokeTest @@ -38,6 +36,8 @@ protected QueryRunner createQueryRunner() throws Exception { TestingMariaDbServer server = closeAfterClass(new TestingMariaDbServer(LATEST_VERSION)); - return createMariaDbQueryRunner(server, ImmutableMap.of(), REQUIRED_TPCH_TABLES); + return MariaDbQueryRunner.builder(server) + .setInitialTables(REQUIRED_TPCH_TABLES) + .build(); } } diff --git a/plugin/trino-mariadb/src/test/java/io/trino/plugin/mariadb/TestMariaDbTypeMapping.java b/plugin/trino-mariadb/src/test/java/io/trino/plugin/mariadb/TestMariaDbTypeMapping.java index a5a77f5baa20..8ec8b95227c7 100644 --- a/plugin/trino-mariadb/src/test/java/io/trino/plugin/mariadb/TestMariaDbTypeMapping.java +++ b/plugin/trino-mariadb/src/test/java/io/trino/plugin/mariadb/TestMariaDbTypeMapping.java @@ -13,8 +13,6 @@ */ package io.trino.plugin.mariadb; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import io.trino.Session; import io.trino.plugin.jdbc.UnsupportedTypeHandling; import io.trino.spi.type.TimeZoneKey; @@ -46,7 +44,6 @@ import static io.trino.plugin.jdbc.DecimalSessionSessionProperties.DECIMAL_ROUNDING_MODE; import static io.trino.plugin.jdbc.TypeHandlingJdbcSessionProperties.UNSUPPORTED_TYPE_HANDLING; import static io.trino.plugin.jdbc.UnsupportedTypeHandling.CONVERT_TO_VARCHAR; -import static io.trino.plugin.mariadb.MariaDbQueryRunner.createMariaDbQueryRunner; import static io.trino.spi.type.BigintType.BIGINT; import static io.trino.spi.type.CharType.createCharType; import static io.trino.spi.type.DateType.DATE; @@ -100,7 +97,7 @@ protected QueryRunner createQueryRunner() throws Exception { server = closeAfterClass(new TestingMariaDbServer()); - return createMariaDbQueryRunner(server, ImmutableMap.of(), ImmutableList.of()); + return MariaDbQueryRunner.builder(server).build(); } @Test From a71b0a350db7eb60a717d99788be898dba2e2d37 Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Fri, 31 May 2024 09:21:36 +0900 Subject: [PATCH 14/19] Convert RaptorQueryRunner to builder --- .../raptor/legacy/RaptorQueryRunner.java | 110 ++++++++++++------ .../TestRaptorBucketedConnectorTest.java | 8 +- .../legacy/TestRaptorConnectorTest.java | 8 +- .../security/TestRaptorFileBasedSecurity.java | 12 +- .../security/TestRaptorReadOnlySecurity.java | 6 +- 5 files changed, 89 insertions(+), 55 deletions(-) diff --git a/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/RaptorQueryRunner.java b/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/RaptorQueryRunner.java index bce36811ee78..d0e567f220d3 100644 --- a/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/RaptorQueryRunner.java +++ b/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/RaptorQueryRunner.java @@ -17,6 +17,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Maps; +import com.google.errorprone.annotations.CanIgnoreReturnValue; import io.airlift.log.Logger; import io.trino.Session; import io.trino.SystemSessionProperties; @@ -32,18 +33,22 @@ import org.intellij.lang.annotations.Language; import java.io.IOException; +import java.io.UncheckedIOException; import java.nio.file.Files; import java.nio.file.Path; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Map.Entry; +import static io.airlift.testing.Closeables.closeAllSuppress; import static io.airlift.units.Duration.nanosSince; import static io.trino.plugin.tpch.TpchMetadata.TINY_SCHEMA_NAME; import static io.trino.testing.QueryAssertions.copyTpchTables; import static io.trino.testing.TestingHandles.createTestCatalogHandle; import static io.trino.testing.TestingSession.testSessionBuilder; import static java.lang.String.format; +import static java.util.Objects.requireNonNull; public final class RaptorQueryRunner { @@ -51,47 +56,71 @@ private RaptorQueryRunner() {} private static final Logger log = Logger.get(RaptorQueryRunner.class); - // TODO convert to builder - public static QueryRunner createRaptorQueryRunner( - List> tablesToLoad, - boolean bucketed, - Map extraRaptorProperties) - throws Exception + public static Builder builder() { - return createRaptorQueryRunner(ImmutableMap.of(), tablesToLoad, bucketed, extraRaptorProperties); + return new Builder() + .addConnectorProperty("metadata.db.type", "h2") + .addConnectorProperty("metadata.db.filename", createTempDirectory("raptor-db").toString()) + .addConnectorProperty("storage.data-directory", createTempDirectory("raptor-data").toString()) + .addConnectorProperty("storage.max-shard-rows", "2000") + .addConnectorProperty("backup.provider", "file") + .addConnectorProperty("backup.directory", createTempDirectory("raptor-backup").toString()); } - // TODO convert to builder - private static QueryRunner createRaptorQueryRunner( - Map extraProperties, - List> tablesToLoad, - boolean bucketed, - Map extraRaptorProperties) - throws Exception + public static final class Builder + extends DistributedQueryRunner.Builder { - QueryRunner queryRunner = DistributedQueryRunner.builder(createSession("tpch")) - .setExtraProperties(extraProperties) - .build(); + private final Map connectorProperties = new HashMap<>(); + private boolean bucketed; + private List> initialTables = ImmutableList.of(); + + private Builder() + { + super(createSession()); + } + + @CanIgnoreReturnValue + public Builder addConnectorProperty(String key, String value) + { + this.connectorProperties.put(key, value); + return this; + } + + @CanIgnoreReturnValue + public Builder enableBucketed() + { + this.bucketed = true; + return this; + } - queryRunner.installPlugin(new TpchPlugin()); - queryRunner.createCatalog("tpch", "tpch"); + @CanIgnoreReturnValue + public Builder setInitialTables(Iterable> initialTables) + { + this.initialTables = ImmutableList.copyOf(requireNonNull(initialTables, "initialTables is null")); + return this; + } - queryRunner.installPlugin(new RaptorPlugin()); - Map raptorProperties = ImmutableMap.builder() - .putAll(extraRaptorProperties) - .put("metadata.db.type", "h2") - .put("metadata.db.filename", createTempDirectory("raptor-db").toString()) - .put("storage.data-directory", createTempDirectory("raptor-data").toString()) - .put("storage.max-shard-rows", "2000") - .put("backup.provider", "file") - .put("backup.directory", createTempDirectory("raptor-backup").toString()) - .buildOrThrow(); + @Override + public DistributedQueryRunner build() + throws Exception + { + DistributedQueryRunner queryRunner = super.build(); + try { + queryRunner.installPlugin(new TpchPlugin()); + queryRunner.createCatalog("tpch", "tpch"); - queryRunner.createCatalog("raptor", "raptor_legacy", raptorProperties); + queryRunner.installPlugin(new RaptorPlugin()); + queryRunner.createCatalog("raptor", "raptor_legacy", connectorProperties); - copyTables(queryRunner, "tpch", createSession(), bucketed, tablesToLoad); + copyTables(queryRunner, "tpch", createSession(), bucketed, initialTables); - return queryRunner; + return queryRunner; + } + catch (Throwable e) { + closeAllSuppress(e, queryRunner); + throw e; + } + } } public static void copyTables(QueryRunner queryRunner, String catalog, Session session, boolean bucketed, List> tables) @@ -176,18 +205,23 @@ public static Session createSession(String schema) } public static Path createTempDirectory(String name) - throws IOException { - Path tempDirectory = Files.createTempDirectory(name); - tempDirectory.toFile().deleteOnExit(); - return tempDirectory.toAbsolutePath(); + try { + Path tempDirectory = Files.createTempDirectory(name); + tempDirectory.toFile().deleteOnExit(); + return tempDirectory.toAbsolutePath(); + } + catch (IOException e) { + throw new UncheckedIOException(e); + } } public static void main(String[] args) throws Exception { - Map properties = ImmutableMap.of("http-server.http.port", "8080"); - QueryRunner queryRunner = createRaptorQueryRunner(properties, ImmutableList.of(), false, ImmutableMap.of()); + QueryRunner queryRunner = RaptorQueryRunner.builder() + .addCoordinatorProperty("http-server.http.port", "8080") + .build(); Logger log = Logger.get(RaptorQueryRunner.class); log.info("======== SERVER STARTED ========"); log.info("\n====\n%s\n====", queryRunner.getCoordinator().getBaseUrl()); diff --git a/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/TestRaptorBucketedConnectorTest.java b/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/TestRaptorBucketedConnectorTest.java index 6f9a65374e82..c1a1ceaff756 100644 --- a/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/TestRaptorBucketedConnectorTest.java +++ b/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/TestRaptorBucketedConnectorTest.java @@ -13,11 +13,9 @@ */ package io.trino.plugin.raptor.legacy; -import com.google.common.collect.ImmutableMap; import io.trino.testing.QueryRunner; import org.junit.jupiter.api.Test; -import static io.trino.plugin.raptor.legacy.RaptorQueryRunner.createRaptorQueryRunner; import static org.assertj.core.api.Assertions.assertThat; public class TestRaptorBucketedConnectorTest @@ -27,7 +25,11 @@ public class TestRaptorBucketedConnectorTest protected QueryRunner createQueryRunner() throws Exception { - return createRaptorQueryRunner(REQUIRED_TPCH_TABLES, true, ImmutableMap.of("storage.compaction-enabled", "false")); + return RaptorQueryRunner.builder() + .enableBucketed() + .addConnectorProperty("storage.compaction-enabled", "false") + .setInitialTables(REQUIRED_TPCH_TABLES) + .build(); } @Test diff --git a/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/TestRaptorConnectorTest.java b/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/TestRaptorConnectorTest.java index 0850fa03caf2..acdb1bd522c3 100644 --- a/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/TestRaptorConnectorTest.java +++ b/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/TestRaptorConnectorTest.java @@ -13,11 +13,8 @@ */ package io.trino.plugin.raptor.legacy; -import com.google.common.collect.ImmutableMap; import io.trino.testing.QueryRunner; -import static io.trino.plugin.raptor.legacy.RaptorQueryRunner.createRaptorQueryRunner; - public class TestRaptorConnectorTest extends BaseRaptorConnectorTest { @@ -25,6 +22,9 @@ public class TestRaptorConnectorTest protected QueryRunner createQueryRunner() throws Exception { - return createRaptorQueryRunner(REQUIRED_TPCH_TABLES, false, ImmutableMap.of("storage.compaction-enabled", "false")); + return RaptorQueryRunner.builder() + .addConnectorProperty("storage.compaction-enabled", "false") + .setInitialTables(REQUIRED_TPCH_TABLES) + .build(); } } diff --git a/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/security/TestRaptorFileBasedSecurity.java b/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/security/TestRaptorFileBasedSecurity.java index d28bc328abe6..dade69191d45 100644 --- a/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/security/TestRaptorFileBasedSecurity.java +++ b/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/security/TestRaptorFileBasedSecurity.java @@ -13,9 +13,9 @@ */ package io.trino.plugin.raptor.legacy.security; -import com.google.common.collect.ImmutableMap; import com.google.common.io.Resources; import io.trino.Session; +import io.trino.plugin.raptor.legacy.RaptorQueryRunner; import io.trino.spi.security.Identity; import io.trino.testing.QueryRunner; import io.trino.tpch.TpchTable; @@ -26,7 +26,6 @@ import java.io.File; -import static io.trino.plugin.raptor.legacy.RaptorQueryRunner.createRaptorQueryRunner; import static io.trino.testing.TestingSession.testSessionBuilder; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS; @@ -42,10 +41,11 @@ public TestRaptorFileBasedSecurity() throws Exception { String path = new File(Resources.getResource(getClass(), "security.json").toURI()).getPath(); - queryRunner = createRaptorQueryRunner( - TpchTable.getTables(), - false, - ImmutableMap.of("security.config-file", path, "raptor.security", "file")); + queryRunner = RaptorQueryRunner.builder() + .addConnectorProperty("security.config-file", path) + .addConnectorProperty("raptor.security", "file") + .setInitialTables(TpchTable.getTables()) + .build(); } @AfterAll diff --git a/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/security/TestRaptorReadOnlySecurity.java b/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/security/TestRaptorReadOnlySecurity.java index f07af0ec8c6c..65e647a8ba40 100644 --- a/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/security/TestRaptorReadOnlySecurity.java +++ b/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/security/TestRaptorReadOnlySecurity.java @@ -13,12 +13,10 @@ */ package io.trino.plugin.raptor.legacy.security; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; +import io.trino.plugin.raptor.legacy.RaptorQueryRunner; import io.trino.testing.QueryRunner; import org.junit.jupiter.api.Test; -import static io.trino.plugin.raptor.legacy.RaptorQueryRunner.createRaptorQueryRunner; import static org.assertj.core.api.Assertions.assertThatThrownBy; public class TestRaptorReadOnlySecurity @@ -27,7 +25,7 @@ public class TestRaptorReadOnlySecurity public void testCannotWrite() throws Exception { - try (QueryRunner queryRunner = createRaptorQueryRunner(ImmutableList.of(), false, ImmutableMap.of("raptor.security", "read-only"))) { + try (QueryRunner queryRunner = RaptorQueryRunner.builder().addConnectorProperty("raptor.security", "read-only").build()) { assertThatThrownBy(() -> { queryRunner.execute("CREATE TABLE test_create (a bigint, b double, c varchar)"); }) From aeec1a96a040cbe047457b3b6ff8d4f9423b1a44 Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Fri, 31 May 2024 09:32:53 +0900 Subject: [PATCH 15/19] Convert SingleStoreQueryRunner to builder --- plugin/trino-singlestore/pom.xml | 6 + .../singlestore/SingleStoreQueryRunner.java | 105 ++++++++++-------- ...TestSingleStoreCaseInsensitiveMapping.java | 16 +-- .../TestSingleStoreConnectorTest.java | 6 +- ...stSingleStoreLatestConnectorSmokeTest.java | 7 +- .../TestSingleStoreTypeMapping.java | 4 +- 6 files changed, 74 insertions(+), 70 deletions(-) diff --git a/plugin/trino-singlestore/pom.xml b/plugin/trino-singlestore/pom.xml index 958036531178..084c7f14d3f7 100644 --- a/plugin/trino-singlestore/pom.xml +++ b/plugin/trino-singlestore/pom.xml @@ -95,6 +95,12 @@ provided + + com.google.errorprone + error_prone_annotations + runtime + + io.airlift bootstrap diff --git a/plugin/trino-singlestore/src/test/java/io/trino/plugin/singlestore/SingleStoreQueryRunner.java b/plugin/trino-singlestore/src/test/java/io/trino/plugin/singlestore/SingleStoreQueryRunner.java index 470c9e3e9aa0..58c6bb804a74 100644 --- a/plugin/trino-singlestore/src/test/java/io/trino/plugin/singlestore/SingleStoreQueryRunner.java +++ b/plugin/trino-singlestore/src/test/java/io/trino/plugin/singlestore/SingleStoreQueryRunner.java @@ -13,21 +13,23 @@ */ package io.trino.plugin.singlestore; -import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableList; +import com.google.errorprone.annotations.CanIgnoreReturnValue; import io.airlift.log.Logger; -import io.trino.Session; import io.trino.plugin.tpch.TpchPlugin; import io.trino.testing.DistributedQueryRunner; import io.trino.testing.QueryRunner; import io.trino.tpch.TpchTable; import java.util.HashMap; +import java.util.List; import java.util.Map; import static io.airlift.testing.Closeables.closeAllSuppress; import static io.trino.plugin.tpch.TpchMetadata.TINY_SCHEMA_NAME; import static io.trino.testing.QueryAssertions.copyTpchTables; import static io.trino.testing.TestingSession.testSessionBuilder; +import static java.util.Objects.requireNonNull; public final class SingleStoreQueryRunner { @@ -35,69 +37,74 @@ private SingleStoreQueryRunner() {} private static final String TPCH_SCHEMA = "tpch"; - // TODO convert to builder - public static QueryRunner createSingleStoreQueryRunner( - TestingSingleStoreServer server, - Map connectorProperties, - Iterable> tables) - throws Exception + public static Builder builder(TestingSingleStoreServer server) { - return createSingleStoreQueryRunner(server, ImmutableMap.of(), connectorProperties, tables); + return new Builder() + .addConnectorProperty("connection-url", server.getJdbcUrl()) + .addConnectorProperty("connection-user", server.getUsername()) + .addConnectorProperty("connection-password", server.getPassword()); } - // TODO convert to builder - private static QueryRunner createSingleStoreQueryRunner( - TestingSingleStoreServer server, - Map coordinatorProperties, - Map connectorProperties, - Iterable> tables) - throws Exception + public static final class Builder + extends DistributedQueryRunner.Builder { - QueryRunner queryRunner = DistributedQueryRunner.builder(createSession()) - .setCoordinatorProperties(coordinatorProperties) - .build(); - try { - queryRunner.installPlugin(new TpchPlugin()); - queryRunner.createCatalog("tpch", "tpch"); + private final Map connectorProperties = new HashMap<>(); + private List> initialTables = ImmutableList.of(); - // note: additional copy via ImmutableList so that if fails on nulls - connectorProperties = new HashMap<>(ImmutableMap.copyOf(connectorProperties)); - connectorProperties.putIfAbsent("connection-url", server.getJdbcUrl()); - connectorProperties.putIfAbsent("connection-user", server.getUsername()); - connectorProperties.putIfAbsent("connection-password", server.getPassword()); + private Builder() + { + super(testSessionBuilder() + .setCatalog("singlestore") + .setSchema(TPCH_SCHEMA) + .build()); + } - server.execute("CREATE SCHEMA tpch"); + @CanIgnoreReturnValue + public Builder addConnectorProperty(String key, String value) + { + this.connectorProperties.put(key, value); + return this; + } - queryRunner.installPlugin(new SingleStorePlugin()); - queryRunner.createCatalog("singlestore", "singlestore", connectorProperties); + @CanIgnoreReturnValue + public Builder setInitialTables(Iterable> initialTables) + { + this.initialTables = ImmutableList.copyOf(requireNonNull(initialTables, "initialTables is null")); + return this; + } - copyTpchTables(queryRunner, "tpch", TINY_SCHEMA_NAME, tables); + @Override + public DistributedQueryRunner build() + throws Exception + { + DistributedQueryRunner queryRunner = super.build(); + try { + queryRunner.installPlugin(new TpchPlugin()); + queryRunner.createCatalog("tpch", "tpch"); - return queryRunner; - } - catch (Throwable e) { - closeAllSuppress(e, queryRunner); - throw e; - } - } + queryRunner.installPlugin(new SingleStorePlugin()); + queryRunner.createCatalog("singlestore", "singlestore", connectorProperties); - private static Session createSession() - { - return testSessionBuilder() - .setCatalog("singlestore") - .setSchema(TPCH_SCHEMA) - .build(); + queryRunner.execute("CREATE SCHEMA tpch"); + copyTpchTables(queryRunner, "tpch", TINY_SCHEMA_NAME, initialTables); + + return queryRunner; + } + catch (Throwable e) { + closeAllSuppress(e, queryRunner); + throw e; + } + } } public static void main(String[] args) throws Exception { // You need to set 'memsql.license' to VM options - QueryRunner queryRunner = createSingleStoreQueryRunner( - new TestingSingleStoreServer(), - ImmutableMap.of("http-server.http.port", "8080"), - ImmutableMap.of(), - TpchTable.getTables()); + QueryRunner queryRunner = builder(new TestingSingleStoreServer()) + .addCoordinatorProperty("http-server.http.port", "8080") + .setInitialTables(TpchTable.getTables()) + .build(); Logger log = Logger.get(SingleStoreQueryRunner.class); log.info("======== SERVER STARTED ========"); diff --git a/plugin/trino-singlestore/src/test/java/io/trino/plugin/singlestore/TestSingleStoreCaseInsensitiveMapping.java b/plugin/trino-singlestore/src/test/java/io/trino/plugin/singlestore/TestSingleStoreCaseInsensitiveMapping.java index 8f9b86b2e756..b0d68a0db5d0 100644 --- a/plugin/trino-singlestore/src/test/java/io/trino/plugin/singlestore/TestSingleStoreCaseInsensitiveMapping.java +++ b/plugin/trino-singlestore/src/test/java/io/trino/plugin/singlestore/TestSingleStoreCaseInsensitiveMapping.java @@ -13,8 +13,6 @@ */ package io.trino.plugin.singlestore; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import io.trino.plugin.jdbc.BaseCaseInsensitiveMappingTest; import io.trino.testing.QueryRunner; import io.trino.testing.sql.SqlExecutor; @@ -22,7 +20,6 @@ import java.nio.file.Path; import static io.trino.plugin.base.mapping.RuleBasedIdentifierMappingUtils.createRuleBasedIdentifierMappingFile; -import static io.trino.plugin.singlestore.SingleStoreQueryRunner.createSingleStoreQueryRunner; import static java.util.Objects.requireNonNull; // With case-insensitive-name-matching enabled colliding schema/table names are considered as errors. @@ -39,14 +36,11 @@ protected QueryRunner createQueryRunner() { mappingFile = createRuleBasedIdentifierMappingFile(); singleStoreServer = closeAfterClass(new TestingSingleStoreServer()); - return createSingleStoreQueryRunner( - singleStoreServer, - ImmutableMap.builder() - .put("case-insensitive-name-matching", "true") - .put("case-insensitive-name-matching.config-file", mappingFile.toFile().getAbsolutePath()) - .put("case-insensitive-name-matching.config-file.refresh-period", "1ms") // ~always refresh - .buildOrThrow(), - ImmutableList.of()); + return SingleStoreQueryRunner.builder(singleStoreServer) + .addConnectorProperty("case-insensitive-name-matching", "true") + .addConnectorProperty("case-insensitive-name-matching.config-file", mappingFile.toFile().getAbsolutePath()) + .addConnectorProperty("case-insensitive-name-matching.config-file.refresh-period", "1ms") // ~always refresh + .build(); } @Override diff --git a/plugin/trino-singlestore/src/test/java/io/trino/plugin/singlestore/TestSingleStoreConnectorTest.java b/plugin/trino-singlestore/src/test/java/io/trino/plugin/singlestore/TestSingleStoreConnectorTest.java index 8d79f6b50430..1bf5e4b26736 100644 --- a/plugin/trino-singlestore/src/test/java/io/trino/plugin/singlestore/TestSingleStoreConnectorTest.java +++ b/plugin/trino-singlestore/src/test/java/io/trino/plugin/singlestore/TestSingleStoreConnectorTest.java @@ -13,7 +13,6 @@ */ package io.trino.plugin.singlestore; -import com.google.common.collect.ImmutableMap; import io.trino.plugin.jdbc.BaseJdbcConnectorTest; import io.trino.sql.planner.plan.AggregationNode; import io.trino.sql.planner.plan.FilterNode; @@ -31,7 +30,6 @@ import java.util.OptionalInt; import static com.google.common.collect.Iterables.getOnlyElement; -import static io.trino.plugin.singlestore.SingleStoreQueryRunner.createSingleStoreQueryRunner; import static io.trino.spi.connector.ConnectorMetadata.MODIFYING_ROWS_MESSAGE; import static io.trino.spi.type.VarcharType.VARCHAR; import static io.trino.testing.MaterializedResult.resultBuilder; @@ -54,7 +52,9 @@ protected QueryRunner createQueryRunner() throws Exception { singleStoreServer = new TestingSingleStoreServer(); - return createSingleStoreQueryRunner(singleStoreServer, ImmutableMap.of(), REQUIRED_TPCH_TABLES); + return SingleStoreQueryRunner.builder(singleStoreServer) + .setInitialTables(REQUIRED_TPCH_TABLES) + .build(); } @AfterAll diff --git a/plugin/trino-singlestore/src/test/java/io/trino/plugin/singlestore/TestSingleStoreLatestConnectorSmokeTest.java b/plugin/trino-singlestore/src/test/java/io/trino/plugin/singlestore/TestSingleStoreLatestConnectorSmokeTest.java index d98fe08c98d3..8c8aad5c53e1 100644 --- a/plugin/trino-singlestore/src/test/java/io/trino/plugin/singlestore/TestSingleStoreLatestConnectorSmokeTest.java +++ b/plugin/trino-singlestore/src/test/java/io/trino/plugin/singlestore/TestSingleStoreLatestConnectorSmokeTest.java @@ -13,13 +13,10 @@ */ package io.trino.plugin.singlestore; -import com.google.common.collect.ImmutableMap; import io.trino.plugin.jdbc.BaseJdbcConnectorSmokeTest; import io.trino.testing.QueryRunner; import io.trino.testing.TestingConnectorBehavior; -import static io.trino.plugin.singlestore.SingleStoreQueryRunner.createSingleStoreQueryRunner; - public class TestSingleStoreLatestConnectorSmokeTest extends BaseJdbcConnectorSmokeTest { @@ -28,7 +25,9 @@ protected QueryRunner createQueryRunner() throws Exception { TestingSingleStoreServer singleStoreServer = closeAfterClass(new TestingSingleStoreServer(TestingSingleStoreServer.LATEST_TESTED_TAG)); - return createSingleStoreQueryRunner(singleStoreServer, ImmutableMap.of(), REQUIRED_TPCH_TABLES); + return SingleStoreQueryRunner.builder(singleStoreServer) + .setInitialTables(REQUIRED_TPCH_TABLES) + .build(); } @Override diff --git a/plugin/trino-singlestore/src/test/java/io/trino/plugin/singlestore/TestSingleStoreTypeMapping.java b/plugin/trino-singlestore/src/test/java/io/trino/plugin/singlestore/TestSingleStoreTypeMapping.java index 9d6c3d043647..0d8b5ea97223 100644 --- a/plugin/trino-singlestore/src/test/java/io/trino/plugin/singlestore/TestSingleStoreTypeMapping.java +++ b/plugin/trino-singlestore/src/test/java/io/trino/plugin/singlestore/TestSingleStoreTypeMapping.java @@ -14,7 +14,6 @@ package io.trino.plugin.singlestore; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import io.trino.Session; import io.trino.plugin.jdbc.UnsupportedTypeHandling; import io.trino.spi.type.TimeZoneKey; @@ -49,7 +48,6 @@ import static io.trino.plugin.jdbc.TypeHandlingJdbcSessionProperties.UNSUPPORTED_TYPE_HANDLING; import static io.trino.plugin.jdbc.UnsupportedTypeHandling.CONVERT_TO_VARCHAR; import static io.trino.plugin.singlestore.SingleStoreClient.SINGLESTORE_VARCHAR_MAX_LENGTH; -import static io.trino.plugin.singlestore.SingleStoreQueryRunner.createSingleStoreQueryRunner; import static io.trino.spi.type.BigintType.BIGINT; import static io.trino.spi.type.BooleanType.BOOLEAN; import static io.trino.spi.type.CharType.createCharType; @@ -102,7 +100,7 @@ protected QueryRunner createQueryRunner() throws Exception { singleStoreServer = closeAfterClass(new TestingSingleStoreServer()); - return createSingleStoreQueryRunner(singleStoreServer, ImmutableMap.of(), ImmutableList.of()); + return SingleStoreQueryRunner.builder(singleStoreServer).build(); } @BeforeAll From 74339d52f321ac7c048b3ac2bfcf3ce14daafbc6 Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Fri, 31 May 2024 09:37:46 +0900 Subject: [PATCH 16/19] Convert SnowflakeQueryRunner to builder --- plugin/trino-snowflake/pom.xml | 6 + .../snowflake/SnowflakeQueryRunner.java | 106 ++++++++++-------- .../snowflake/TestSnowflakeConnectorTest.java | 6 +- .../snowflake/TestSnowflakeTypeMapping.java | 9 +- 4 files changed, 70 insertions(+), 57 deletions(-) diff --git a/plugin/trino-snowflake/pom.xml b/plugin/trino-snowflake/pom.xml index e191b8d61592..f20a07cc40bb 100644 --- a/plugin/trino-snowflake/pom.xml +++ b/plugin/trino-snowflake/pom.xml @@ -86,6 +86,12 @@ provided + + com.google.errorprone + error_prone_annotations + runtime + + io.airlift log diff --git a/plugin/trino-snowflake/src/test/java/io/trino/plugin/snowflake/SnowflakeQueryRunner.java b/plugin/trino-snowflake/src/test/java/io/trino/plugin/snowflake/SnowflakeQueryRunner.java index d991a4872d95..97bca10337d4 100644 --- a/plugin/trino-snowflake/src/test/java/io/trino/plugin/snowflake/SnowflakeQueryRunner.java +++ b/plugin/trino-snowflake/src/test/java/io/trino/plugin/snowflake/SnowflakeQueryRunner.java @@ -14,20 +14,21 @@ package io.trino.plugin.snowflake; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; +import com.google.errorprone.annotations.CanIgnoreReturnValue; import io.airlift.log.Logger; -import io.trino.Session; import io.trino.plugin.tpch.TpchPlugin; import io.trino.testing.DistributedQueryRunner; import io.trino.tpch.TpchTable; import java.util.HashMap; +import java.util.List; import java.util.Map; import static io.airlift.testing.Closeables.closeAllSuppress; import static io.trino.plugin.tpch.TpchMetadata.TINY_SCHEMA_NAME; import static io.trino.testing.QueryAssertions.copyTpchTables; import static io.trino.testing.TestingSession.testSessionBuilder; +import static java.util.Objects.requireNonNull; public final class SnowflakeQueryRunner { @@ -35,66 +36,75 @@ private SnowflakeQueryRunner() {} public static final String TPCH_SCHEMA = "tpch"; - // TODO convert to builder - public static DistributedQueryRunner createSnowflakeQueryRunner( - Map connectorProperties, - Iterable> tables) - throws Exception + public static Builder builder() { - return createSnowflakeQueryRunner(ImmutableMap.of(), connectorProperties, tables); + return new Builder() + .addConnectorProperty("connection-url", TestingSnowflakeServer.TEST_URL) + .addConnectorProperty("connection-user", TestingSnowflakeServer.TEST_USER) + .addConnectorProperty("connection-password", TestingSnowflakeServer.TEST_PASSWORD) + .addConnectorProperty("snowflake.database", TestingSnowflakeServer.TEST_DATABASE) + .addConnectorProperty("snowflake.role", TestingSnowflakeServer.TEST_ROLE) + .addConnectorProperty("snowflake.warehouse", TestingSnowflakeServer.TEST_WAREHOUSE); } - // TODO convert to builder - private static DistributedQueryRunner createSnowflakeQueryRunner( - Map coordinatorProperties, - Map connectorProperties, - Iterable> tables) - throws Exception + public static final class Builder + extends DistributedQueryRunner.Builder { - DistributedQueryRunner queryRunner = DistributedQueryRunner.builder(createSession()) - .setCoordinatorProperties(coordinatorProperties) - .build(); - try { - queryRunner.installPlugin(new TpchPlugin()); - queryRunner.createCatalog("tpch", "tpch"); - - connectorProperties = new HashMap<>(ImmutableMap.copyOf(connectorProperties)); - connectorProperties.putIfAbsent("connection-url", TestingSnowflakeServer.TEST_URL); - connectorProperties.putIfAbsent("connection-user", TestingSnowflakeServer.TEST_USER); - connectorProperties.putIfAbsent("connection-password", TestingSnowflakeServer.TEST_PASSWORD); - connectorProperties.putIfAbsent("snowflake.database", TestingSnowflakeServer.TEST_DATABASE); - connectorProperties.putIfAbsent("snowflake.role", TestingSnowflakeServer.TEST_ROLE); - connectorProperties.putIfAbsent("snowflake.warehouse", TestingSnowflakeServer.TEST_WAREHOUSE); + private final Map connectorProperties = new HashMap<>(); + private List> initialTables = ImmutableList.of(); - queryRunner.installPlugin(new SnowflakePlugin()); - queryRunner.createCatalog("snowflake", "snowflake", connectorProperties); - - queryRunner.execute(createSession(), "CREATE SCHEMA IF NOT EXISTS " + TPCH_SCHEMA); - copyTpchTables(queryRunner, "tpch", TINY_SCHEMA_NAME, tables); + private Builder() + { + super(testSessionBuilder() + .setCatalog("snowflake") + .setSchema(TPCH_SCHEMA) + .build()); + } - return queryRunner; + @CanIgnoreReturnValue + public Builder addConnectorProperty(String key, String value) + { + this.connectorProperties.put(key, value); + return this; } - catch (Throwable e) { - closeAllSuppress(e, queryRunner); - throw e; + + @CanIgnoreReturnValue + public Builder setInitialTables(Iterable> initialTables) + { + this.initialTables = ImmutableList.copyOf(requireNonNull(initialTables, "initialTables is null")); + return this; } - } - public static Session createSession() - { - return testSessionBuilder() - .setCatalog("snowflake") - .setSchema(TPCH_SCHEMA) - .build(); + @Override + public DistributedQueryRunner build() + throws Exception + { + DistributedQueryRunner queryRunner = super.build(); + try { + queryRunner.installPlugin(new TpchPlugin()); + queryRunner.createCatalog("tpch", "tpch"); + + queryRunner.installPlugin(new SnowflakePlugin()); + queryRunner.createCatalog("snowflake", "snowflake", connectorProperties); + + queryRunner.execute("CREATE SCHEMA IF NOT EXISTS " + TPCH_SCHEMA); + copyTpchTables(queryRunner, "tpch", TINY_SCHEMA_NAME, initialTables); + + return queryRunner; + } + catch (Throwable e) { + closeAllSuppress(e, queryRunner); + throw e; + } + } } public static void main(String[] args) throws Exception { - DistributedQueryRunner queryRunner = createSnowflakeQueryRunner( - ImmutableMap.of("http-server.http.port", "8080"), - ImmutableMap.of(), - ImmutableList.of()); + DistributedQueryRunner queryRunner = builder() + .addCoordinatorProperty("http-server.http.port", "8080") + .build(); Logger log = Logger.get(SnowflakeQueryRunner.class); log.info("======== SERVER STARTED ========"); diff --git a/plugin/trino-snowflake/src/test/java/io/trino/plugin/snowflake/TestSnowflakeConnectorTest.java b/plugin/trino-snowflake/src/test/java/io/trino/plugin/snowflake/TestSnowflakeConnectorTest.java index ad08a3a5ed80..365bd78160af 100644 --- a/plugin/trino-snowflake/src/test/java/io/trino/plugin/snowflake/TestSnowflakeConnectorTest.java +++ b/plugin/trino-snowflake/src/test/java/io/trino/plugin/snowflake/TestSnowflakeConnectorTest.java @@ -13,7 +13,6 @@ */ package io.trino.plugin.snowflake; -import com.google.common.collect.ImmutableMap; import io.trino.plugin.jdbc.BaseJdbcConnectorTest; import io.trino.sql.planner.plan.AggregationNode; import io.trino.sql.planner.plan.ProjectNode; @@ -31,7 +30,6 @@ import java.util.OptionalInt; import static com.google.common.base.Strings.nullToEmpty; -import static io.trino.plugin.snowflake.SnowflakeQueryRunner.createSnowflakeQueryRunner; import static io.trino.plugin.snowflake.TestingSnowflakeServer.TEST_SCHEMA; import static io.trino.spi.connector.ConnectorMetadata.MODIFYING_ROWS_MESSAGE; import static io.trino.spi.type.VarcharType.VARCHAR; @@ -55,7 +53,9 @@ public class TestSnowflakeConnectorTest protected QueryRunner createQueryRunner() throws Exception { - return createSnowflakeQueryRunner(ImmutableMap.of(), REQUIRED_TPCH_TABLES); + return SnowflakeQueryRunner.builder() + .setInitialTables(REQUIRED_TPCH_TABLES) + .build(); } @Override diff --git a/plugin/trino-snowflake/src/test/java/io/trino/plugin/snowflake/TestSnowflakeTypeMapping.java b/plugin/trino-snowflake/src/test/java/io/trino/plugin/snowflake/TestSnowflakeTypeMapping.java index 0eee20f93a11..59ea2f6bd68c 100644 --- a/plugin/trino-snowflake/src/test/java/io/trino/plugin/snowflake/TestSnowflakeTypeMapping.java +++ b/plugin/trino-snowflake/src/test/java/io/trino/plugin/snowflake/TestSnowflakeTypeMapping.java @@ -13,8 +13,6 @@ */ package io.trino.plugin.snowflake; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import io.trino.Session; import io.trino.plugin.jdbc.UnsupportedTypeHandling; import io.trino.spi.type.TimeZoneKey; @@ -40,7 +38,6 @@ import static io.trino.plugin.jdbc.TypeHandlingJdbcSessionProperties.UNSUPPORTED_TYPE_HANDLING; import static io.trino.plugin.jdbc.UnsupportedTypeHandling.CONVERT_TO_VARCHAR; import static io.trino.plugin.jdbc.UnsupportedTypeHandling.IGNORE; -import static io.trino.plugin.snowflake.SnowflakeQueryRunner.createSnowflakeQueryRunner; import static io.trino.spi.type.BooleanType.BOOLEAN; import static io.trino.spi.type.DateType.DATE; import static io.trino.spi.type.DecimalType.createDecimalType; @@ -78,9 +75,9 @@ public void setUp() protected QueryRunner createQueryRunner() throws Exception { - return createSnowflakeQueryRunner( - ImmutableMap.of("jdbc-types-mapped-to-varchar", "ARRAY"), - ImmutableList.of()); + return SnowflakeQueryRunner.builder() + .addConnectorProperty("jdbc-types-mapped-to-varchar", "ARRAY") + .build(); } @Test From 1f560b231fa37a7b4ff9c07e3172a2f64aa4e86d Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Fri, 31 May 2024 10:09:43 +0900 Subject: [PATCH 17/19] Convert ThriftQueryRunner to builder --- plugin/trino-thrift/pom.xml | 6 + .../integration/TestThriftConnectorTest.java | 6 +- .../TestThriftDistributedQueriesIndexed.java | 6 +- .../thrift/integration/ThriftQueryRunner.java | 110 ++++++++---------- 4 files changed, 65 insertions(+), 63 deletions(-) diff --git a/plugin/trino-thrift/pom.xml b/plugin/trino-thrift/pom.xml index 9f8db34dd494..2cbd61d5f666 100644 --- a/plugin/trino-thrift/pom.xml +++ b/plugin/trino-thrift/pom.xml @@ -145,6 +145,12 @@ provided + + com.google.errorprone + error_prone_annotations + runtime + + io.airlift log diff --git a/plugin/trino-thrift/src/test/java/io/trino/plugin/thrift/integration/TestThriftConnectorTest.java b/plugin/trino-thrift/src/test/java/io/trino/plugin/thrift/integration/TestThriftConnectorTest.java index 24f479dcbff1..5eaad0e939c3 100644 --- a/plugin/trino-thrift/src/test/java/io/trino/plugin/thrift/integration/TestThriftConnectorTest.java +++ b/plugin/trino-thrift/src/test/java/io/trino/plugin/thrift/integration/TestThriftConnectorTest.java @@ -19,7 +19,7 @@ import io.trino.testing.TestingConnectorBehavior; import org.junit.jupiter.api.Test; -import static io.trino.plugin.thrift.integration.ThriftQueryRunner.createThriftQueryRunner; +import static io.trino.plugin.thrift.integration.ThriftQueryRunner.startThriftServers; import static io.trino.spi.type.VarcharType.VARCHAR; import static io.trino.testing.QueryAssertions.assertContains; @@ -55,7 +55,9 @@ protected boolean hasBehavior(TestingConnectorBehavior connectorBehavior) protected QueryRunner createQueryRunner() throws Exception { - return createThriftQueryRunner(3, false); + ThriftQueryRunner.StartedServers servers = startThriftServers(3, false); + servers.resources().forEach(this::closeAfterClass); + return ThriftQueryRunner.builder(servers).build(); } @Override diff --git a/plugin/trino-thrift/src/test/java/io/trino/plugin/thrift/integration/TestThriftDistributedQueriesIndexed.java b/plugin/trino-thrift/src/test/java/io/trino/plugin/thrift/integration/TestThriftDistributedQueriesIndexed.java index 6682ad22c344..fa8eb5a3ad98 100644 --- a/plugin/trino-thrift/src/test/java/io/trino/plugin/thrift/integration/TestThriftDistributedQueriesIndexed.java +++ b/plugin/trino-thrift/src/test/java/io/trino/plugin/thrift/integration/TestThriftDistributedQueriesIndexed.java @@ -17,7 +17,7 @@ import io.trino.testing.QueryRunner; import org.junit.jupiter.api.Test; -import static io.trino.plugin.thrift.integration.ThriftQueryRunner.createThriftQueryRunner; +import static io.trino.plugin.thrift.integration.ThriftQueryRunner.startThriftServers; public class TestThriftDistributedQueriesIndexed extends AbstractTestIndexedQueries @@ -26,7 +26,9 @@ public class TestThriftDistributedQueriesIndexed protected QueryRunner createQueryRunner() throws Exception { - return createThriftQueryRunner(2, true); + ThriftQueryRunner.StartedServers servers = startThriftServers(2, true); + servers.resources().forEach(this::closeAfterClass); + return ThriftQueryRunner.builder(servers).build(); } @Test diff --git a/plugin/trino-thrift/src/test/java/io/trino/plugin/thrift/integration/ThriftQueryRunner.java b/plugin/trino-thrift/src/test/java/io/trino/plugin/thrift/integration/ThriftQueryRunner.java index c05538fcb886..ef92bdc410c0 100644 --- a/plugin/trino-thrift/src/test/java/io/trino/plugin/thrift/integration/ThriftQueryRunner.java +++ b/plugin/trino-thrift/src/test/java/io/trino/plugin/thrift/integration/ThriftQueryRunner.java @@ -14,8 +14,8 @@ package io.trino.plugin.thrift.integration; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.errorprone.annotations.CanIgnoreReturnValue; import io.airlift.drift.codec.ThriftCodecManager; import io.airlift.drift.server.DriftServer; import io.airlift.drift.server.DriftService; @@ -25,7 +25,6 @@ import io.airlift.drift.transport.netty.server.DriftNettyServerTransportFactory; import io.airlift.log.Logger; import io.airlift.log.Logging; -import io.trino.Session; import io.trino.plugin.thrift.ThriftPlugin; import io.trino.plugin.thrift.server.ThriftIndexedTpchService; import io.trino.plugin.thrift.server.ThriftTpchService; @@ -34,9 +33,11 @@ import io.trino.testing.QueryRunner; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; import java.util.Map; +import static io.airlift.testing.Closeables.closeAllSuppress; import static io.trino.testing.TestingSession.testSessionBuilder; import static java.util.stream.Collectors.joining; @@ -46,41 +47,13 @@ public final class ThriftQueryRunner private ThriftQueryRunner() {} - // TODO convert to builder - public static QueryRunner createThriftQueryRunner(int thriftServers, boolean enableIndexJoin) - throws Exception - { - return createThriftQueryRunner(thriftServers, enableIndexJoin, Map.of()); - } - - // TODO convert to builder - private static QueryRunner createThriftQueryRunner(int thriftServers, boolean enableIndexJoin, Map coordinatorProperties) - throws Exception - { - StartedServers startedServers = null; - try { - startedServers = startThriftServers(thriftServers, enableIndexJoin); - return createThriftQueryRunnerInternal(startedServers.servers(), startedServers.resources(), coordinatorProperties); - } - catch (Throwable t) { - if (startedServers != null) { - for (DriftServer server : startedServers.servers()) { - server.shutdown(); - } - for (AutoCloseable resource : startedServers.resources()) { - resource.close(); - } - } - throw t; - } - } - public static void main(String[] args) throws Exception { Logging.initialize(); - Map coordinatorProperties = ImmutableMap.of("http-server.http.port", "8080"); - QueryRunner queryRunner = createThriftQueryRunner(3, true, coordinatorProperties); + QueryRunner queryRunner = builder(startThriftServers(3, true)) + .addCoordinatorProperty("http-server.http.port", "8080") + .build(); Logger log = Logger.get(ThriftQueryRunner.class); log.info("======== SERVER STARTED ========"); log.info("\n====\n%s\n====", queryRunner.getCoordinator().getBaseUrl()); @@ -105,36 +78,55 @@ static StartedServers startThriftServers(int thriftServers, boolean enableIndexJ return new StartedServers(servers, resources); } - private static QueryRunner createThriftQueryRunnerInternal(List servers, List resources, Map coordinatorProperties) - throws Exception + public static Builder builder(StartedServers startedServers) { - String addresses = servers.stream() - .map(server -> "localhost:" + driftServerPort(server)) - .collect(joining(",")); + return new Builder() + .addConnectorProperty("trino.thrift.client.addresses", startedServers.servers.stream() + .map(server -> "localhost:" + driftServerPort(server)) + .collect(joining(","))) + .addConnectorProperty("trino.thrift.client.connect-timeout", "30s") + .addConnectorProperty("trino-thrift.lookup-requests-concurrency", "2"); + } - Session defaultSession = testSessionBuilder() - .setCatalog("thrift") - .setSchema("tiny") - .build(); + public static final class Builder + extends DistributedQueryRunner.Builder + { + private final Map connectorProperties = new HashMap<>(); - QueryRunner queryRunner = DistributedQueryRunner.builder(defaultSession) - .setCoordinatorProperties(coordinatorProperties) - .registerResources(servers.stream().map(server -> (AutoCloseable) server::shutdown).toList()) - .registerResources(resources) - .build(); + private Builder() + { + super(testSessionBuilder() + .setCatalog("thrift") + .setSchema("tiny") + .build()); + } - queryRunner.installPlugin(new ThriftPlugin()); - Map connectorProperties = ImmutableMap.builder() - .put("trino.thrift.client.addresses", addresses) - .put("trino.thrift.client.connect-timeout", "30s") - .put("trino-thrift.lookup-requests-concurrency", "2") - .buildOrThrow(); - queryRunner.createCatalog("thrift", "trino_thrift", connectorProperties); + @CanIgnoreReturnValue + public Builder addConnectorProperty(String key, String value) + { + this.connectorProperties.put(key, value); + return this; + } + + @Override + public DistributedQueryRunner build() + throws Exception + { + DistributedQueryRunner queryRunner = super.build(); + try { + queryRunner.installPlugin(new ThriftPlugin()); + queryRunner.createCatalog("thrift", "trino_thrift", connectorProperties); - queryRunner.installPlugin(new TpchPlugin()); - queryRunner.createCatalog("tpch", "tpch"); + queryRunner.installPlugin(new TpchPlugin()); + queryRunner.createCatalog("tpch", "tpch"); - return queryRunner; + return queryRunner; + } + catch (Throwable e) { + closeAllSuppress(e, queryRunner); + throw e; + } + } } static int driftServerPort(DriftServer server) @@ -142,9 +134,9 @@ static int driftServerPort(DriftServer server) return ((DriftNettyServerTransport) server.getServerTransport()).getPort(); } - record StartedServers(List servers, List resources) + public record StartedServers(List servers, List resources) { - StartedServers + public StartedServers { servers = ImmutableList.copyOf(servers); resources = ImmutableList.copyOf(resources); From 310139ff34a3fb0146ec1d339ab289a2451a4396 Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Fri, 31 May 2024 10:12:59 +0900 Subject: [PATCH 18/19] Convert TpcdsQueryRunner to builder --- plugin/trino-tpcds/pom.xml | 6 ++ .../java/io/trino/plugin/tpcds/TestTpcds.java | 2 +- .../trino/plugin/tpcds/TpcdsQueryRunner.java | 59 ++++++++++--------- 3 files changed, 37 insertions(+), 30 deletions(-) diff --git a/plugin/trino-tpcds/pom.xml b/plugin/trino-tpcds/pom.xml index e686bc938c98..70523e8f6da5 100644 --- a/plugin/trino-tpcds/pom.xml +++ b/plugin/trino-tpcds/pom.xml @@ -133,6 +133,12 @@ test + + io.airlift + testing + test + + io.trino trino-main diff --git a/plugin/trino-tpcds/src/test/java/io/trino/plugin/tpcds/TestTpcds.java b/plugin/trino-tpcds/src/test/java/io/trino/plugin/tpcds/TestTpcds.java index 5116058c2841..0db7e417a67c 100644 --- a/plugin/trino-tpcds/src/test/java/io/trino/plugin/tpcds/TestTpcds.java +++ b/plugin/trino-tpcds/src/test/java/io/trino/plugin/tpcds/TestTpcds.java @@ -37,7 +37,7 @@ public class TestTpcds protected QueryRunner createQueryRunner() throws Exception { - return TpcdsQueryRunner.createQueryRunner(); + return TpcdsQueryRunner.builder().build(); } @Test diff --git a/plugin/trino-tpcds/src/test/java/io/trino/plugin/tpcds/TpcdsQueryRunner.java b/plugin/trino-tpcds/src/test/java/io/trino/plugin/tpcds/TpcdsQueryRunner.java index 5d4acf210deb..df522c405b20 100644 --- a/plugin/trino-tpcds/src/test/java/io/trino/plugin/tpcds/TpcdsQueryRunner.java +++ b/plugin/trino-tpcds/src/test/java/io/trino/plugin/tpcds/TpcdsQueryRunner.java @@ -13,56 +13,57 @@ */ package io.trino.plugin.tpcds; -import com.google.common.collect.ImmutableMap; import io.airlift.log.Logger; -import io.trino.Session; import io.trino.testing.DistributedQueryRunner; import io.trino.testing.QueryRunner; -import java.util.Map; - +import static io.airlift.testing.Closeables.closeAllSuppress; import static io.trino.testing.TestingSession.testSessionBuilder; public final class TpcdsQueryRunner { private TpcdsQueryRunner() {} - // TODO convert to builder - public static QueryRunner createQueryRunner() - throws Exception + public static Builder builder() { - return createQueryRunner(ImmutableMap.of()); + return new Builder(); } - // TODO convert to builder - private static QueryRunner createQueryRunner(Map coordinatorProperties) - throws Exception + public static final class Builder + extends DistributedQueryRunner.Builder { - Session session = testSessionBuilder() - .setSource("test") - .setCatalog("tpcds") - .setSchema("sf1") - .build(); - - QueryRunner queryRunner = DistributedQueryRunner.builder(session) - .setCoordinatorProperties(coordinatorProperties) - .build(); - - try { - queryRunner.installPlugin(new TpcdsPlugin()); - queryRunner.createCatalog("tpcds", "tpcds"); - return queryRunner; + private Builder() + { + super(testSessionBuilder() + .setSource("test") + .setCatalog("tpcds") + .setSchema("sf1") + .build()); } - catch (Exception e) { - queryRunner.close(); - throw e; + + @Override + public DistributedQueryRunner build() + throws Exception + { + DistributedQueryRunner queryRunner = super.build(); + try { + queryRunner.installPlugin(new TpcdsPlugin()); + queryRunner.createCatalog("tpcds", "tpcds"); + return queryRunner; + } + catch (Throwable e) { + closeAllSuppress(e, queryRunner); + throw e; + } } } public static void main(String[] args) throws Exception { - QueryRunner queryRunner = createQueryRunner(ImmutableMap.of("http-server.http.port", "8080")); + QueryRunner queryRunner = builder() + .addCoordinatorProperty("http-server.http.port", "8080") + .build(); Logger log = Logger.get(TpcdsQueryRunner.class); log.info("======== SERVER STARTED ========"); log.info("\n====\n%s\n====", queryRunner.getCoordinator().getBaseUrl()); From ae29415a6a4245afffb8ca37b964237abe67062b Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Fri, 31 May 2024 10:21:01 +0900 Subject: [PATCH 19/19] Convert S3HudiQueryRunner to builder --- .../trino/plugin/hudi/S3HudiQueryRunner.java | 136 ++++++++++-------- ...udiCopyOnWriteMinioConnectorSmokeTest.java | 9 +- ...udiMergeOnReadMinioConnectorSmokeTest.java | 9 +- 3 files changed, 81 insertions(+), 73 deletions(-) diff --git a/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/S3HudiQueryRunner.java b/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/S3HudiQueryRunner.java index 1d99c7ea776a..0f862a5cf2fe 100644 --- a/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/S3HudiQueryRunner.java +++ b/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/S3HudiQueryRunner.java @@ -13,11 +13,11 @@ */ package io.trino.plugin.hudi; -import com.google.common.collect.ImmutableMap; +import com.google.errorprone.annotations.CanIgnoreReturnValue; import io.airlift.log.Logger; import io.airlift.log.Logging; -import io.trino.Session; import io.trino.filesystem.Location; +import io.trino.plugin.base.util.Closables; import io.trino.plugin.hive.containers.HiveMinioDataLake; import io.trino.plugin.hive.metastore.Database; import io.trino.plugin.hive.metastore.HiveMetastoreFactory; @@ -28,6 +28,7 @@ import io.trino.testing.QueryRunner; import io.trino.tpch.TpchTable; +import java.util.HashMap; import java.util.Map; import java.util.Optional; @@ -35,74 +36,84 @@ import static io.trino.testing.containers.Minio.MINIO_ACCESS_KEY; import static io.trino.testing.containers.Minio.MINIO_REGION; import static io.trino.testing.containers.Minio.MINIO_SECRET_KEY; +import static java.util.Objects.requireNonNull; +// TODO merge with HudiQueryRunner public final class S3HudiQueryRunner { private static final String TPCH_SCHEMA = "tpch"; private S3HudiQueryRunner() {} - // TODO convert to builder, merge with HudiQueryRunner - public static QueryRunner create( - Map connectorProperties, - HudiTablesInitializer dataLoader, - HiveMinioDataLake hiveMinioDataLake) - throws Exception + public static Builder builder(HiveMinioDataLake hiveMinioDataLake) { - return create( - ImmutableMap.of(), - connectorProperties, - dataLoader, - hiveMinioDataLake); + return new Builder(hiveMinioDataLake) + .addConnectorProperty("fs.hadoop.enabled", "false") + .addConnectorProperty("fs.native-s3.enabled", "true") + .addConnectorProperty("s3.aws-access-key", MINIO_ACCESS_KEY) + .addConnectorProperty("s3.aws-secret-key", MINIO_SECRET_KEY) + .addConnectorProperty("s3.region", MINIO_REGION) + .addConnectorProperty("s3.endpoint", hiveMinioDataLake.getMinio().getMinioAddress()) + .addConnectorProperty("s3.path-style-access", "true"); } - // TODO convert to builder, merge with HudiQueryRunner - private static QueryRunner create( - Map coordinatorProperties, - Map connectorProperties, - HudiTablesInitializer dataLoader, - HiveMinioDataLake hiveMinioDataLake) - throws Exception + public static class Builder + extends DistributedQueryRunner.Builder { - QueryRunner queryRunner = DistributedQueryRunner.builder(createSession()) - .setCoordinatorProperties(coordinatorProperties) - .build(); - queryRunner.installPlugin(new TestingHudiPlugin(queryRunner.getCoordinator().getBaseDataDir().resolve("hudi_data"))); - queryRunner.createCatalog( - "hudi", - "hudi", - ImmutableMap.builder() - .put("fs.hadoop.enabled", "false") - .put("fs.native-s3.enabled", "true") - .put("s3.aws-access-key", MINIO_ACCESS_KEY) - .put("s3.aws-secret-key", MINIO_SECRET_KEY) - .put("s3.region", MINIO_REGION) - .put("s3.endpoint", hiveMinioDataLake.getMinio().getMinioAddress()) - .put("s3.path-style-access", "true") - .putAll(connectorProperties) - .buildOrThrow()); - - // Hudi connector does not support creating schema or any other write operations - ((HudiConnector) queryRunner.getCoordinator().getConnector("hudi")).getInjector() - .getInstance(HiveMetastoreFactory.class) - .createMetastore(Optional.empty()) - .createDatabase(Database.builder() - .setDatabaseName(TPCH_SCHEMA) - .setOwnerName(Optional.of("public")) - .setOwnerType(Optional.of(PrincipalType.ROLE)) - .build()); - - dataLoader.initializeTables(queryRunner, Location.of("s3://" + hiveMinioDataLake.getBucketName() + "/"), TPCH_SCHEMA); - - return queryRunner; - } + private final HiveMinioDataLake hiveMinioDataLake; + private HudiTablesInitializer dataLoader; + private final Map connectorProperties = new HashMap<>(); - private static Session createSession() - { - return testSessionBuilder() - .setCatalog("hudi") - .setSchema(TPCH_SCHEMA) - .build(); + protected Builder(HiveMinioDataLake hiveMinioDataLake) + { + super(testSessionBuilder() + .setCatalog("hudi") + .setSchema(TPCH_SCHEMA) + .build()); + this.hiveMinioDataLake = requireNonNull(hiveMinioDataLake, "hiveMinioDataLake is null"); + } + + @CanIgnoreReturnValue + public Builder setDataLoader(HudiTablesInitializer dataLoader) + { + this.dataLoader = dataLoader; + return this; + } + + @CanIgnoreReturnValue + public Builder addConnectorProperty(String key, String value) + { + this.connectorProperties.put(key, value); + return this; + } + + @Override + public DistributedQueryRunner build() + throws Exception + { + DistributedQueryRunner queryRunner = super.build(); + try { + queryRunner.installPlugin(new TestingHudiPlugin(queryRunner.getCoordinator().getBaseDataDir().resolve("hudi_data"))); + queryRunner.createCatalog("hudi", "hudi", connectorProperties); + + // Hudi connector does not support creating schema or any other write operations + ((HudiConnector) queryRunner.getCoordinator().getConnector("hudi")).getInjector() + .getInstance(HiveMetastoreFactory.class) + .createMetastore(Optional.empty()) + .createDatabase(Database.builder() + .setDatabaseName(TPCH_SCHEMA) + .setOwnerName(Optional.of("public")) + .setOwnerType(Optional.of(PrincipalType.ROLE)) + .build()); + + dataLoader.initializeTables(queryRunner, Location.of("s3://" + hiveMinioDataLake.getBucketName() + "/"), TPCH_SCHEMA); + return queryRunner; + } + catch (Throwable e) { + Closables.closeAllSuppress(e, queryRunner); + throw e; + } + } } public static void main(String[] args) @@ -114,11 +125,10 @@ public static void main(String[] args) String bucketName = "test-bucket"; HiveMinioDataLake hiveMinioDataLake = new HiveMinioDataLake(bucketName); hiveMinioDataLake.start(); - QueryRunner queryRunner = create( - ImmutableMap.of("http-server.http.port", "8080"), - ImmutableMap.of(), - new TpchHudiTablesInitializer(TpchTable.getTables()), - hiveMinioDataLake); + QueryRunner queryRunner = builder(hiveMinioDataLake) + .addCoordinatorProperty("http-server.http.port", "8080") + .setDataLoader(new TpchHudiTablesInitializer(TpchTable.getTables())) + .build(); log.info("======== SERVER STARTED ========"); log.info("\n====\n%s\n====", queryRunner.getCoordinator().getBaseUrl()); diff --git a/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/TestHudiCopyOnWriteMinioConnectorSmokeTest.java b/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/TestHudiCopyOnWriteMinioConnectorSmokeTest.java index 15a7f3f0ea1a..e7b9241e1ac0 100644 --- a/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/TestHudiCopyOnWriteMinioConnectorSmokeTest.java +++ b/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/TestHudiCopyOnWriteMinioConnectorSmokeTest.java @@ -13,7 +13,6 @@ */ package io.trino.plugin.hudi; -import com.google.common.collect.ImmutableMap; import io.trino.plugin.hive.containers.HiveMinioDataLake; import io.trino.plugin.hudi.testing.TpchHudiTablesInitializer; import io.trino.testing.QueryRunner; @@ -34,9 +33,9 @@ protected QueryRunner createQueryRunner() hiveMinioDataLake.start(); hiveMinioDataLake.getMinioClient().ensureBucketExists(bucketName); - return S3HudiQueryRunner.create( - ImmutableMap.of("hudi.columns-to-hide", COLUMNS_TO_HIDE), - new TpchHudiTablesInitializer(REQUIRED_TPCH_TABLES), - hiveMinioDataLake); + return S3HudiQueryRunner.builder(hiveMinioDataLake) + .addConnectorProperty("hudi.columns-to-hide", COLUMNS_TO_HIDE) + .setDataLoader(new TpchHudiTablesInitializer(REQUIRED_TPCH_TABLES)) + .build(); } } diff --git a/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/TestHudiMergeOnReadMinioConnectorSmokeTest.java b/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/TestHudiMergeOnReadMinioConnectorSmokeTest.java index 9cba3282e2d3..4a5e5c02b42a 100644 --- a/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/TestHudiMergeOnReadMinioConnectorSmokeTest.java +++ b/plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/TestHudiMergeOnReadMinioConnectorSmokeTest.java @@ -13,7 +13,6 @@ */ package io.trino.plugin.hudi; -import com.google.common.collect.ImmutableMap; import io.trino.plugin.hive.containers.HiveMinioDataLake; import io.trino.plugin.hudi.testing.TpchHudiTablesInitializer; import io.trino.testing.QueryRunner; @@ -34,9 +33,9 @@ protected QueryRunner createQueryRunner() hiveMinioDataLake.start(); hiveMinioDataLake.getMinioClient().ensureBucketExists(bucketName); - return S3HudiQueryRunner.create( - ImmutableMap.of("hudi.columns-to-hide", COLUMNS_TO_HIDE), - new TpchHudiTablesInitializer(REQUIRED_TPCH_TABLES), - hiveMinioDataLake); + return S3HudiQueryRunner.builder(hiveMinioDataLake) + .setDataLoader(new TpchHudiTablesInitializer(REQUIRED_TPCH_TABLES)) + .addConnectorProperty("hudi.columns-to-hide", COLUMNS_TO_HIDE) + .build(); } }