From 686e6b36ccecda1784750fef2ce5cde2adfdecf6 Mon Sep 17 00:00:00 2001 From: Zac Blanco Date: Fri, 7 Feb 2025 09:52:46 -0800 Subject: [PATCH] Use builder pattern for IcebergQueryRunner Additionally, add support for tracking catalog properties added to iceberg. This can be useful for tests that may require their own catalogs --- .../BenchmarkIcebergHadoopCatalog.java | 17 +- .../iceberg/BenchmarkIcebergHiveCatalog.java | 17 +- .../IcebergDistributedSmokeTestBase.java | 3 +- .../iceberg/IcebergDistributedTestBase.java | 5 +- .../presto/iceberg/IcebergQueryRunner.java | 299 ++++++++++-------- .../TestIcebergDistributedQueries.java | 5 +- .../iceberg/TestIcebergLogicalPlanner.java | 6 +- .../TestIcebergParquetMetadataCaching.java | 23 +- .../iceberg/TestIcebergSplitManager.java | 13 +- .../iceberg/TestIcebergTableChangelog.java | 8 +- .../presto/iceberg/TestIcebergTypes.java | 7 +- .../hive/TestIcebergDistributedHive.java | 6 +- .../hive/TestIcebergHiveStatistics.java | 77 +++-- .../presto/iceberg/nessie/NessieTestUtil.java | 4 +- .../nessie/TestIcebergDistributedNessie.java | 5 +- ...cebergNessieCatalogDistributedQueries.java | 6 +- .../nessie/TestIcebergSmokeNessie.java | 6 +- .../nessie/TestIcebergSystemTablesNessie.java | 3 + .../nessie/TestNessieMultiBranching.java | 10 +- .../TestExpireSnapshotProcedure.java | 2 +- .../TestFastForwardBranchProcedure.java | 7 +- .../TestRemoveOrphanFilesProcedureBase.java | 5 +- .../TestRollbackToTimestampProcedure.java | 2 +- .../TestSetCurrentSnapshotProcedure.java | 2 +- .../TestSetTablePropertyProcedure.java | 4 +- .../iceberg/rest/IcebergRestTestUtil.java | 3 +- .../rest/TestIcebergDistributedRest.java | 18 +- ...tIcebergRestCatalogDistributedQueries.java | 19 +- .../iceberg/rest/TestIcebergSmokeRest.java | 16 +- .../TestIcebergSmokeRestNestedNamespace.java | 31 +- .../PrestoNativeQueryRunnerUtils.java | 52 ++- 31 files changed, 345 insertions(+), 336 deletions(-) diff --git a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/BenchmarkIcebergHadoopCatalog.java b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/BenchmarkIcebergHadoopCatalog.java index f791d936f8980..c9b275ca56fb8 100644 --- a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/BenchmarkIcebergHadoopCatalog.java +++ b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/BenchmarkIcebergHadoopCatalog.java @@ -14,7 +14,6 @@ package com.facebook.presto.iceberg; import com.facebook.presto.tests.DistributedQueryRunner; -import com.google.common.collect.ImmutableMap; import org.openjdk.jmh.annotations.BenchmarkMode; import org.openjdk.jmh.annotations.Fork; import org.openjdk.jmh.annotations.Measurement; @@ -27,12 +26,9 @@ import org.openjdk.jmh.runner.options.OptionsBuilder; import org.openjdk.jmh.runner.options.VerboseMode; -import java.util.Optional; import java.util.OptionalInt; import static com.facebook.presto.iceberg.CatalogType.HADOOP; -import static com.facebook.presto.iceberg.FileFormat.PARQUET; -import static com.facebook.presto.iceberg.IcebergQueryRunner.createIcebergQueryRunner; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static org.openjdk.jmh.annotations.Mode.AverageTime; import static org.openjdk.jmh.annotations.Scope.Benchmark; @@ -50,14 +46,11 @@ public class BenchmarkIcebergHadoopCatalog public DistributedQueryRunner getQueryRunner() { try { - return createIcebergQueryRunner( - ImmutableMap.of(), - ImmutableMap.of("iceberg.catalog.type", HADOOP.name()), - PARQUET, - false, - true, - OptionalInt.of(1), - Optional.empty()); + return IcebergQueryRunner.builder() + .setCatalogType(HADOOP) + .setNodeCount(OptionalInt.of(4)) + .build() + .getQueryRunner(); } catch (Exception e) { e.printStackTrace(); diff --git a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/BenchmarkIcebergHiveCatalog.java b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/BenchmarkIcebergHiveCatalog.java index 9ce08c8e9b4c4..5a986a74ffe2f 100644 --- a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/BenchmarkIcebergHiveCatalog.java +++ b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/BenchmarkIcebergHiveCatalog.java @@ -14,7 +14,6 @@ package com.facebook.presto.iceberg; import com.facebook.presto.tests.DistributedQueryRunner; -import com.google.common.collect.ImmutableMap; import org.openjdk.jmh.annotations.BenchmarkMode; import org.openjdk.jmh.annotations.Fork; import org.openjdk.jmh.annotations.Measurement; @@ -27,11 +26,9 @@ import org.openjdk.jmh.runner.options.OptionsBuilder; import org.openjdk.jmh.runner.options.VerboseMode; -import java.util.Optional; import java.util.OptionalInt; -import static com.facebook.presto.iceberg.FileFormat.PARQUET; -import static com.facebook.presto.iceberg.IcebergQueryRunner.createIcebergQueryRunner; +import static com.facebook.presto.iceberg.CatalogType.HIVE; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static org.openjdk.jmh.annotations.Mode.AverageTime; import static org.openjdk.jmh.annotations.Scope.Benchmark; @@ -49,14 +46,10 @@ public class BenchmarkIcebergHiveCatalog public DistributedQueryRunner getQueryRunner() { try { - return createIcebergQueryRunner( - ImmutableMap.of(), - ImmutableMap.of(), - PARQUET, - false, - true, - OptionalInt.of(1), - Optional.empty()); + return IcebergQueryRunner.builder() + .setCatalogType(HIVE) + .setNodeCount(OptionalInt.of(1)) + .build().getQueryRunner(); } catch (Exception e) { e.printStackTrace(); diff --git a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/IcebergDistributedSmokeTestBase.java b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/IcebergDistributedSmokeTestBase.java index da2c1c41a5a40..0e49ed20b78f5 100644 --- a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/IcebergDistributedSmokeTestBase.java +++ b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/IcebergDistributedSmokeTestBase.java @@ -23,7 +23,6 @@ import com.facebook.presto.testing.QueryRunner; import com.facebook.presto.testing.assertions.Assert; import com.facebook.presto.tests.AbstractTestIntegrationSmokeTest; -import com.google.common.collect.ImmutableMap; import org.apache.iceberg.Table; import org.apache.iceberg.UpdateProperties; import org.intellij.lang.annotations.Language; @@ -74,7 +73,7 @@ protected IcebergDistributedSmokeTestBase(CatalogType catalogType) protected QueryRunner createQueryRunner() throws Exception { - return IcebergQueryRunner.createIcebergQueryRunner(ImmutableMap.of(), catalogType); + return IcebergQueryRunner.builder().setCatalogType(catalogType).build().getQueryRunner(); } @Test diff --git a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/IcebergDistributedTestBase.java b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/IcebergDistributedTestBase.java index 64736be5edc56..1587cf6621a4b 100644 --- a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/IcebergDistributedTestBase.java +++ b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/IcebergDistributedTestBase.java @@ -193,7 +193,10 @@ protected IcebergDistributedTestBase(CatalogType catalogType) protected QueryRunner createQueryRunner() throws Exception { - return IcebergQueryRunner.createIcebergQueryRunner(ImmutableMap.of(), catalogType, extraConnectorProperties); + return IcebergQueryRunner.builder() + .setCatalogType(catalogType) + .setExtraConnectorProperties(extraConnectorProperties) + .build().getQueryRunner(); } @Test diff --git a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/IcebergQueryRunner.java b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/IcebergQueryRunner.java index 08bf8ae9eeb72..08bddbb6822f7 100644 --- a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/IcebergQueryRunner.java +++ b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/IcebergQueryRunner.java @@ -46,20 +46,25 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.Collections; +import java.util.HashMap; import java.util.Map; import java.util.Optional; import java.util.OptionalInt; +import java.util.concurrent.ConcurrentHashMap; import java.util.function.BiFunction; import static com.facebook.airlift.log.Level.ERROR; import static com.facebook.airlift.log.Level.WARN; import static com.facebook.presto.hive.HiveTestUtils.getDataDirectoryPath; import static com.facebook.presto.iceberg.CatalogType.HIVE; +import static com.facebook.presto.iceberg.FileFormat.PARQUET; import static com.facebook.presto.spi.StandardErrorCode.GENERIC_INTERNAL_ERROR; import static com.facebook.presto.spi.StandardErrorCode.NOT_SUPPORTED; import static com.facebook.presto.testing.TestingSession.testSessionBuilder; import static com.facebook.presto.tests.QueryAssertions.copyTpchTables; import static com.facebook.presto.tpch.TpchMetadata.TINY_SCHEMA_NAME; +import static com.google.common.base.Preconditions.checkArgument; +import static java.util.Objects.requireNonNull; public final class IcebergQueryRunner { @@ -69,179 +74,193 @@ public final class IcebergQueryRunner public static final String TEST_DATA_DIRECTORY = "iceberg_data"; public static final MetastoreContext METASTORE_CONTEXT = new MetastoreContext("test_user", "test_queryId", Optional.empty(), Collections.emptySet(), Optional.empty(), Optional.empty(), false, HiveColumnConverterProvider.DEFAULT_COLUMN_CONVERTER_PROVIDER, WarningCollector.NOOP, new RuntimeStats()); - private IcebergQueryRunner() {} + private DistributedQueryRunner queryRunner; + private Map> icebergCatalogs; - public static DistributedQueryRunner createIcebergQueryRunner(Map extraProperties, Optional dataDirectory) - throws Exception + private IcebergQueryRunner(DistributedQueryRunner queryRunner, Map> icebergCatalogs) { - return createIcebergQueryRunner(extraProperties, ImmutableMap.of(), dataDirectory); + this.queryRunner = requireNonNull(queryRunner, "queryRunner is null"); + this.icebergCatalogs = new ConcurrentHashMap<>(requireNonNull(icebergCatalogs, "icebergCatalogs is null")); } - public static DistributedQueryRunner createIcebergQueryRunner(Map extraProperties, CatalogType catalogType) - throws Exception + public DistributedQueryRunner getQueryRunner() { - return createIcebergQueryRunner(extraProperties, ImmutableMap.of("iceberg.catalog.type", catalogType.name()), Optional.empty()); + return queryRunner; } - public static DistributedQueryRunner createIcebergQueryRunner(Map extraProperties, CatalogType catalogType, Map extraConnectorProperties) - throws Exception + public Map> getIcebergCatalogs() { - return createIcebergQueryRunner( - extraProperties, - ImmutableMap.builder() - .putAll(extraConnectorProperties) - .put("iceberg.catalog.type", catalogType.name()) - .build(), - Optional.empty()); + return icebergCatalogs; } - public static DistributedQueryRunner createIcebergQueryRunner(Map extraProperties, Map extraConnectorProperties) - throws Exception + public void addCatalog(String name, Map properties) { - return createIcebergQueryRunner(extraProperties, extraConnectorProperties, Optional.empty()); + queryRunner.createCatalog(name, "iceberg", properties); + icebergCatalogs.put(name, properties); } - public static DistributedQueryRunner createIcebergQueryRunner(Map extraProperties, Map extraConnectorProperties, Optional dataDirectory) - throws Exception + public static Builder builder() { - FileFormat defaultFormat = new IcebergConfig().getFileFormat(); - return createIcebergQueryRunner(extraProperties, extraConnectorProperties, defaultFormat, true, dataDirectory); + return new Builder(); } - public static DistributedQueryRunner createIcebergQueryRunner(Map extraProperties, Map extraConnectorProperties, FileFormat format) - throws Exception + public static class Builder { - return createIcebergQueryRunner(extraProperties, extraConnectorProperties, format, true, Optional.empty()); - } + private Builder() {} + + private CatalogType catalogType = HIVE; + private Map> icebergCatalogs = new HashMap<>(); + private Map extraProperties = new HashMap<>(); + private Map extraConnectorProperties = new HashMap<>(); + private FileFormat format = PARQUET; + private boolean createTpchTables = true; + private boolean addJmxPlugin = true; + private OptionalInt nodeCount = OptionalInt.of(4); + private Optional> externalWorkerLauncher = Optional.empty(); + private Optional dataDirectory = Optional.empty(); + private boolean addStorageFormatToPath; + private Optional schemaName = Optional.empty(); + + public Builder setFormat(FileFormat format) + { + this.format = format; + return this; + } - public static DistributedQueryRunner createIcebergQueryRunner( - Map extraProperties, - Map extraConnectorProperties, - FileFormat format, - boolean createTpchTables, - Optional dataDirectory) - throws Exception - { - return createIcebergQueryRunner(extraProperties, extraConnectorProperties, format, createTpchTables, false, OptionalInt.empty(), dataDirectory); - } + public Builder setExternalWorkerLauncher(Optional> externalWorkerLauncher) + { + this.externalWorkerLauncher = externalWorkerLauncher; + return this; + } - public static DistributedQueryRunner createIcebergQueryRunner( - Map extraProperties, - Map extraConnectorProperties, - FileFormat format, - boolean createTpchTables, - boolean addJmxPlugin, - OptionalInt nodeCount, - Optional dataDirectory) - throws Exception - { - return createIcebergQueryRunner(extraProperties, extraConnectorProperties, format, createTpchTables, addJmxPlugin, nodeCount, Optional.empty(), dataDirectory); - } + public Builder setSchemaName(String schemaName) + { + this.schemaName = Optional.of(schemaName); + return this; + } - public static DistributedQueryRunner createIcebergQueryRunner( - Map extraProperties, - Map extraConnectorProperties, - FileFormat format, - boolean createTpchTables, - boolean addJmxPlugin, - OptionalInt nodeCount, - Optional> externalWorkerLauncher, - Optional dataDirectory) - throws Exception - { - return createIcebergQueryRunner(extraProperties, extraConnectorProperties, format, createTpchTables, addJmxPlugin, nodeCount, externalWorkerLauncher, dataDirectory, false, Optional.empty()); - } + public Builder setCreateTpchTables(boolean createTpchTables) + { + this.createTpchTables = createTpchTables; + return this; + } - public static DistributedQueryRunner createIcebergQueryRunner( - Map extraProperties, - Map extraConnectorProperties, - FileFormat format, - boolean createTpchTables, - boolean addJmxPlugin, - OptionalInt nodeCount, - Optional> externalWorkerLauncher, - Optional dataDirectory, - boolean addStorageFormatToPath) - throws Exception - { - return createIcebergQueryRunner(extraProperties, extraConnectorProperties, format, createTpchTables, addJmxPlugin, nodeCount, externalWorkerLauncher, dataDirectory, addStorageFormatToPath, Optional.empty()); - } + public Builder setAddJmxPlugin(boolean addJmxPlugin) + { + this.addJmxPlugin = addJmxPlugin; + return this; + } - public static DistributedQueryRunner createIcebergQueryRunner( - Map extraProperties, - Map extraConnectorProperties, - FileFormat format, - boolean createTpchTables, - boolean addJmxPlugin, - OptionalInt nodeCount, - Optional> externalWorkerLauncher, - Optional dataDirectory, - boolean addStorageFormatToPath, - Optional schemaName) - throws Exception - { - setupLogging(); + public Builder setNodeCount(OptionalInt nodeCount) + { + this.nodeCount = nodeCount; + return this; + } - Session session = testSessionBuilder() - .setCatalog(ICEBERG_CATALOG) - .setSchema(schemaName.orElse("tpch")) - .build(); + public Builder setExtraProperties(Map extraProperties) + { + this.extraProperties = extraProperties; + return this; + } - DistributedQueryRunner queryRunner = DistributedQueryRunner.builder(session) - .setExtraProperties(extraProperties) - .setDataDirectory(dataDirectory) - .setNodeCount(nodeCount.orElse(4)) - .setExternalWorkerLauncher(externalWorkerLauncher) - .build(); + public Builder setCatalogType(CatalogType catalogType) + { + this.catalogType = catalogType; + return this; + } - queryRunner.installPlugin(new TpchPlugin()); - queryRunner.createCatalog("tpch", "tpch"); + public Builder setDataDirectory(Optional dataDirectory) + { + this.dataDirectory = dataDirectory; + return this; + } - queryRunner.installPlugin(new TpcdsPlugin()); - queryRunner.createCatalog("tpcds", "tpcds"); + public Builder setExtraConnectorProperties(Map extraConnectorProperties) + { + this.extraConnectorProperties = extraConnectorProperties; + return this; + } - queryRunner.getServers().forEach(server -> { - MBeanServer mBeanServer = MBeanServerFactory.newMBeanServer(); - server.installPlugin(new IcebergPlugin(mBeanServer)); - if (addJmxPlugin) { - server.installPlugin(new JmxPlugin(mBeanServer)); - } - }); + public Builder setAddStorageFormatToPath(boolean addStorageFormatToPath) + { + this.addStorageFormatToPath = addStorageFormatToPath; + return this; + } - String catalogType = extraConnectorProperties.getOrDefault("iceberg.catalog.type", HIVE.name()); - Path icebergDataDirectory = getIcebergDataDirectoryPath(queryRunner.getCoordinator().getDataDirectory(), catalogType, format, addStorageFormatToPath); + public IcebergQueryRunner build() + throws Exception + { + setupLogging(); + + checkArgument(this.catalogType.name().equals(extraConnectorProperties.getOrDefault("iceberg.catalog.type", this.catalogType.name())), + "catalog type must match catalog type in extraConnectorProperties"); + checkArgument(this.format.name().equals(extraConnectorProperties.getOrDefault("iceberg.file-format", this.format.name())), + "file format must match iceberg.file-format type in extraConnectorProperties"); + + ImmutableMap.Builder> icebergCatalogs = ImmutableMap.builder(); + + Session session = testSessionBuilder() + .setCatalog(ICEBERG_CATALOG) + .setSchema(schemaName.orElse("tpch")) + .build(); + + DistributedQueryRunner queryRunner = DistributedQueryRunner.builder(session) + .setExtraProperties(extraProperties) + .setDataDirectory(dataDirectory) + .setNodeCount(nodeCount.orElse(4)) + .setExternalWorkerLauncher(externalWorkerLauncher) + .build(); + + queryRunner.installPlugin(new TpchPlugin()); + queryRunner.createCatalog("tpch", "tpch"); + + queryRunner.installPlugin(new TpcdsPlugin()); + queryRunner.createCatalog("tpcds", "tpcds"); + + queryRunner.getServers().forEach(server -> { + MBeanServer mBeanServer = MBeanServerFactory.newMBeanServer(); + server.installPlugin(new IcebergPlugin(mBeanServer)); + if (addJmxPlugin) { + server.installPlugin(new JmxPlugin(mBeanServer)); + } + }); - Map icebergProperties = ImmutableMap.builder() - .put("iceberg.file-format", format.name()) - .putAll(getConnectorProperties(CatalogType.valueOf(catalogType), icebergDataDirectory)) - .putAll(extraConnectorProperties) - .build(); + Path icebergDataDirectory = getIcebergDataDirectoryPath(queryRunner.getCoordinator().getDataDirectory(), catalogType.name(), format, addStorageFormatToPath); - queryRunner.createCatalog(ICEBERG_CATALOG, "iceberg", icebergProperties); + Map icebergProperties = ImmutableMap.builder() + .put("iceberg.file-format", format.name()) + .put("iceberg.catalog.type", catalogType.name()) + .putAll(getConnectorProperties(catalogType, icebergDataDirectory)) + .putAll(extraConnectorProperties) + .build(); - if (addJmxPlugin) { - queryRunner.createCatalog("jmx", "jmx"); - } + queryRunner.createCatalog(ICEBERG_CATALOG, "iceberg", icebergProperties); + icebergCatalogs.put(ICEBERG_CATALOG, icebergProperties); - if (catalogType.equals(HIVE.name())) { - ExtendedHiveMetastore metastore = getFileHiveMetastore(icebergDataDirectory); - if (!metastore.getDatabase(METASTORE_CONTEXT, "tpch").isPresent()) { - queryRunner.execute("CREATE SCHEMA tpch"); + if (addJmxPlugin) { + queryRunner.createCatalog("jmx", "jmx"); } - if (!metastore.getDatabase(METASTORE_CONTEXT, "tpcds").isPresent()) { + + if (catalogType == HIVE) { + ExtendedHiveMetastore metastore = getFileHiveMetastore(icebergDataDirectory); + if (!metastore.getDatabase(METASTORE_CONTEXT, "tpch").isPresent()) { + queryRunner.execute("CREATE SCHEMA tpch"); + } + if (!metastore.getDatabase(METASTORE_CONTEXT, "tpcds").isPresent()) { + queryRunner.execute("CREATE SCHEMA tpcds"); + } + } + else { + queryRunner.execute("CREATE SCHEMA tpch"); queryRunner.execute("CREATE SCHEMA tpcds"); } - } - else { - queryRunner.execute("CREATE SCHEMA tpch"); - queryRunner.execute("CREATE SCHEMA tpcds"); - } - if (createTpchTables) { - copyTpchTables(queryRunner, "tpch", TINY_SCHEMA_NAME, session, TpchTable.getTables(), true); - } + if (createTpchTables) { + copyTpchTables(queryRunner, "tpch", TINY_SCHEMA_NAME, session, TpchTable.getTables(), true); + } - return queryRunner; + return new IcebergQueryRunner(queryRunner, icebergCatalogs.build()); + } } private static ExtendedHiveMetastore getFileHiveMetastore(Path dataDirectory) @@ -261,7 +280,7 @@ public static Path getIcebergDataDirectoryPath(Path dataDirectory, String catalo return icebergCatalogDirectory; } - private static Map getConnectorProperties(CatalogType icebergCatalogType, Path icebergDataDirectory) + public static Map getConnectorProperties(CatalogType icebergCatalogType, Path icebergDataDirectory) { switch (icebergCatalogType) { case HADOOP: @@ -322,7 +341,11 @@ public static void main(String[] args) Map properties = ImmutableMap.of("http-server.http.port", "8080"); DistributedQueryRunner queryRunner = null; try { - queryRunner = createIcebergQueryRunner(properties, dataDirectory); + queryRunner = builder() + .setExtraProperties(properties) + .setDataDirectory(dataDirectory) + .build() + .getQueryRunner(); } catch (Throwable t) { log.error(t); diff --git a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergDistributedQueries.java b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergDistributedQueries.java index 8bfb68d48872d..a8ff74d23f379 100644 --- a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergDistributedQueries.java +++ b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergDistributedQueries.java @@ -56,7 +56,10 @@ protected TestIcebergDistributedQueries(CatalogType catalogType) @Override protected QueryRunner createQueryRunner() throws Exception { - return IcebergQueryRunner.createIcebergQueryRunner(ImmutableMap.of(), catalogType, extraConnectorProperties); + return IcebergQueryRunner.builder() + .setCatalogType(catalogType) + .setExtraConnectorProperties(extraConnectorProperties) + .build().getQueryRunner(); } @Override diff --git a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergLogicalPlanner.java b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergLogicalPlanner.java index 9617c527ad1c7..f96d2f08b11fe 100644 --- a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergLogicalPlanner.java +++ b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergLogicalPlanner.java @@ -87,7 +87,6 @@ import static com.facebook.presto.iceberg.IcebergColumnHandle.getSynthesizedIcebergColumnHandle; import static com.facebook.presto.iceberg.IcebergColumnHandle.isPushedDownSubfield; import static com.facebook.presto.iceberg.IcebergQueryRunner.ICEBERG_CATALOG; -import static com.facebook.presto.iceberg.IcebergQueryRunner.createIcebergQueryRunner; import static com.facebook.presto.iceberg.IcebergSessionProperties.PARQUET_DEREFERENCE_PUSHDOWN_ENABLED; import static com.facebook.presto.iceberg.IcebergSessionProperties.PUSHDOWN_FILTER_ENABLED; import static com.facebook.presto.iceberg.IcebergSessionProperties.isPushdownFilterEnabled; @@ -130,7 +129,10 @@ protected TestIcebergLogicalPlanner() {} protected QueryRunner createQueryRunner() throws Exception { - return createIcebergQueryRunner(ImmutableMap.of("experimental.pushdown-subfields-enabled", "true"), ImmutableMap.of()); + return IcebergQueryRunner.builder() + .setExtraProperties(ImmutableMap.of("experimental.pushdown-subfields-enabled", "true")) + .build() + .getQueryRunner(); } @DataProvider(name = "push_down_filter_enabled") diff --git a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergParquetMetadataCaching.java b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergParquetMetadataCaching.java index 65d4158f18dfa..5fe812dd06c21 100644 --- a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergParquetMetadataCaching.java +++ b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergParquetMetadataCaching.java @@ -21,11 +21,8 @@ import org.testng.annotations.BeforeClass; import org.testng.annotations.Test; -import java.util.Optional; import java.util.OptionalInt; -import static com.facebook.presto.iceberg.FileFormat.PARQUET; -import static com.facebook.presto.iceberg.IcebergQueryRunner.createIcebergQueryRunner; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertTrue; @@ -36,17 +33,15 @@ public class TestIcebergParquetMetadataCaching protected QueryRunner createQueryRunner() throws Exception { - return createIcebergQueryRunner( - ImmutableMap.of(), - ImmutableMap.of( - "iceberg.parquet.metadata-cache-enabled", "true", - "iceberg.parquet.metadata-cache-size", "100MB", - "iceberg.parquet.metadata-cache-ttl-since-last-access", "1h"), - PARQUET, - false, - true, - OptionalInt.of(2), - Optional.empty()); + return IcebergQueryRunner.builder() + .setExtraConnectorProperties( + ImmutableMap.of( + "iceberg.parquet.metadata-cache-enabled", "true", + "iceberg.parquet.metadata-cache-size", "100MB", + "iceberg.parquet.metadata-cache-ttl-since-last-access", "1h")) + .setNodeCount(OptionalInt.of(2)) + .build() + .getQueryRunner(); } @BeforeClass diff --git a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergSplitManager.java b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergSplitManager.java index bb7f2f26ff5f8..d49121f26dd81 100644 --- a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergSplitManager.java +++ b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergSplitManager.java @@ -29,15 +29,12 @@ import com.facebook.presto.testing.QueryRunner; import com.facebook.presto.tests.AbstractTestQueryFramework; import com.facebook.presto.transaction.TransactionManager; -import com.google.common.collect.ImmutableMap; import org.testng.annotations.Test; import java.util.ArrayList; import java.util.List; -import java.util.Optional; import static com.facebook.presto.iceberg.IcebergQueryRunner.ICEBERG_CATALOG; -import static com.facebook.presto.iceberg.IcebergQueryRunner.createIcebergQueryRunner; import static com.facebook.presto.iceberg.IcebergSessionProperties.PUSHDOWN_FILTER_ENABLED; import static com.facebook.presto.spi.connector.NotPartitionedPartitionHandle.NOT_PARTITIONED; import static org.testng.Assert.assertEquals; @@ -54,7 +51,7 @@ protected TestIcebergSplitManager() {} protected QueryRunner createQueryRunner() throws Exception { - return createIcebergQueryRunner(ImmutableMap.of(), Optional.empty()); + return IcebergQueryRunner.builder().build().getQueryRunner(); } @Test @@ -174,10 +171,10 @@ private Session sessionWithFilterPushdown(boolean pushdown) } private void validateSplitsPlannedForSql(SplitManager splitManager, - TransactionManager transactionManager, - boolean filterPushdown, - String sql, - int expectedSplitCount) + TransactionManager transactionManager, + boolean filterPushdown, + String sql, + int expectedSplitCount) { Session session = sessionWithFilterPushdown(filterPushdown); List tableScanNodes = getTableScanFromOptimizedPlanOfSql(sql, session); diff --git a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergTableChangelog.java b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergTableChangelog.java index 9adce1fe7deb0..4c45b70080488 100644 --- a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergTableChangelog.java +++ b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergTableChangelog.java @@ -16,7 +16,6 @@ import com.facebook.presto.testing.QueryRunner; import com.facebook.presto.tests.AbstractTestQueryFramework; import com.google.common.base.Joiner; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; import org.testng.annotations.BeforeClass; import org.testng.annotations.Test; @@ -24,7 +23,7 @@ import java.util.Arrays; import java.util.stream.Collectors; -import static com.facebook.presto.iceberg.IcebergQueryRunner.createIcebergQueryRunner; +import static com.facebook.presto.iceberg.CatalogType.HADOOP; public class TestIcebergTableChangelog extends AbstractTestQueryFramework @@ -33,7 +32,10 @@ public class TestIcebergTableChangelog protected QueryRunner createQueryRunner() throws Exception { - return createIcebergQueryRunner(ImmutableMap.of(), CatalogType.HADOOP); + return IcebergQueryRunner.builder() + .setCatalogType(HADOOP) + .build() + .getQueryRunner(); } private long[] snapshots = new long[0]; diff --git a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergTypes.java b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergTypes.java index 28265c9289c9b..e6e43a2e89d8b 100644 --- a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergTypes.java +++ b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergTypes.java @@ -21,23 +21,22 @@ import com.facebook.presto.testing.MaterializedRow; import com.facebook.presto.testing.QueryRunner; import com.facebook.presto.tests.AbstractTestQueryFramework; -import com.google.common.collect.ImmutableMap; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import java.util.List; import static com.facebook.presto.hive.HiveCommonSessionProperties.PARQUET_BATCH_READ_OPTIMIZATION_ENABLED; -import static com.facebook.presto.iceberg.IcebergQueryRunner.createIcebergQueryRunner; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertTrue; public class TestIcebergTypes extends AbstractTestQueryFramework { - protected QueryRunner createQueryRunner() throws Exception + protected QueryRunner createQueryRunner() + throws Exception { - return createIcebergQueryRunner(ImmutableMap.of(), ImmutableMap.of()); + return IcebergQueryRunner.builder().build().getQueryRunner(); } @DataProvider(name = "testTimestampWithTimezone") diff --git a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/hive/TestIcebergDistributedHive.java b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/hive/TestIcebergDistributedHive.java index 3954524b959b2..36ffd2965d508 100644 --- a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/hive/TestIcebergDistributedHive.java +++ b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/hive/TestIcebergDistributedHive.java @@ -21,16 +21,12 @@ import com.facebook.presto.metadata.CatalogManager; import com.facebook.presto.spi.ConnectorId; import com.facebook.presto.spi.SchemaTableName; -import com.google.common.base.Joiner; -import com.google.common.collect.ImmutableMap; import org.apache.iceberg.Table; import org.testng.annotations.Test; import static com.facebook.presto.hive.metastore.InMemoryCachingHiveMetastore.memoizeMetastore; import static com.facebook.presto.iceberg.CatalogType.HIVE; import static com.facebook.presto.iceberg.IcebergQueryRunner.ICEBERG_CATALOG; -import static com.facebook.presto.spi.statistics.ColumnStatisticType.NUMBER_OF_DISTINCT_VALUES; -import static com.facebook.presto.spi.statistics.ColumnStatisticType.TOTAL_SIZE_IN_BYTES; @Test public class TestIcebergDistributedHive @@ -38,7 +34,7 @@ public class TestIcebergDistributedHive { public TestIcebergDistributedHive() { - super(HIVE, ImmutableMap.of("iceberg.hive-statistics-merge-strategy", Joiner.on(",").join(NUMBER_OF_DISTINCT_VALUES.name(), TOTAL_SIZE_IN_BYTES.name()))); + super(HIVE); } @Override diff --git a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/hive/TestIcebergHiveStatistics.java b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/hive/TestIcebergHiveStatistics.java index 9bbac609739e1..b3f278b8e5ce0 100644 --- a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/hive/TestIcebergHiveStatistics.java +++ b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/hive/TestIcebergHiveStatistics.java @@ -35,6 +35,7 @@ import com.facebook.presto.iceberg.IcebergColumnHandle; import com.facebook.presto.iceberg.IcebergHiveTableOperationsConfig; import com.facebook.presto.iceberg.IcebergMetadataColumn; +import com.facebook.presto.iceberg.IcebergQueryRunner; import com.facebook.presto.iceberg.IcebergUtil; import com.facebook.presto.metadata.CatalogManager; import com.facebook.presto.metadata.Metadata; @@ -54,7 +55,6 @@ import com.facebook.presto.testing.MaterializedResult; import com.facebook.presto.testing.QueryRunner; import com.facebook.presto.tests.AbstractTestQueryFramework; -import com.facebook.presto.tests.DistributedQueryRunner; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -86,7 +86,6 @@ import static com.facebook.presto.iceberg.CatalogType.HIVE; import static com.facebook.presto.iceberg.IcebergQueryRunner.ICEBERG_CATALOG; import static com.facebook.presto.iceberg.IcebergQueryRunner.TEST_DATA_DIRECTORY; -import static com.facebook.presto.iceberg.IcebergQueryRunner.createIcebergQueryRunner; import static com.facebook.presto.iceberg.IcebergSessionProperties.HIVE_METASTORE_STATISTICS_MERGE_STRATEGY; import static com.facebook.presto.iceberg.IcebergSessionProperties.PUSHDOWN_FILTER_ENABLED; import static com.facebook.presto.iceberg.IcebergSessionProperties.STATISTICS_KLL_SKETCH_K_PARAMETER; @@ -109,11 +108,16 @@ public class TestIcebergHiveStatistics extends AbstractTestQueryFramework { + private IcebergQueryRunner icebergQueryRunner; + @Override protected QueryRunner createQueryRunner() throws Exception { - return createIcebergQueryRunner(ImmutableMap.of(), ImmutableMap.of("iceberg.hive-statistics-merge-strategy", NUMBER_OF_DISTINCT_VALUES.name())); + icebergQueryRunner = IcebergQueryRunner.builder() + .setExtraConnectorProperties(ImmutableMap.of("iceberg.hive-statistics-merge-strategy", NUMBER_OF_DISTINCT_VALUES.name())) + .build(); + return icebergQueryRunner.getQueryRunner(); } private static final Set NUMERIC_ORDERS_COLUMNS = ImmutableSet.builder() @@ -412,35 +416,40 @@ public void testPredicateOnlyColumnInStatisticsOutput(boolean pushdownFilterEnab public void testStatisticsCachePartialEviction() throws Exception { - try (DistributedQueryRunner queryRunner = createIcebergQueryRunner(ImmutableMap.of(), ImmutableMap.of("iceberg.max-statistics-file-cache-size", "1024B"))) { - Session session = Session.builder(queryRunner.getDefaultSession()) - // set histograms enabled - .setSystemProperty(OPTIMIZER_USE_HISTOGRAMS, "true") - .setCatalogSessionProperty("iceberg", STATISTICS_KLL_SKETCH_K_PARAMETER, "32768") - .build(); - - queryRunner.execute(session, "ANALYZE lineitem"); - // get table statistics, to populate some of the cache - TableStatistics statistics = getTableStatistics(queryRunner, session, "lineitem"); - RuntimeStats runtimeStats = session.getRuntimeStats(); - runtimeStats.getMetrics().keySet().stream().filter(name -> name.contains("ColumnCount")).findFirst() - .ifPresent(stat -> assertEquals(32, runtimeStats.getMetric(stat).getSum())); - runtimeStats.getMetrics().keySet().stream().filter(name -> name.contains("PuffinFileSize")).findFirst() - .ifPresent(stat -> assertTrue(runtimeStats.getMetric(stat).getSum() > 1024)); - // get them again to trigger retrieval of _some_ cached statistics - statistics = getTableStatistics(queryRunner, session, "lineitem"); - RuntimeMetric partialMiss = runtimeStats.getMetrics().keySet().stream().filter(name -> name.contains("PartialMiss")).findFirst() - .map(runtimeStats::getMetric) - .orElseThrow(() -> new RuntimeException("partial miss on statistics cache should have occurred")); - assertTrue(partialMiss.getCount() > 0); - - statistics.getColumnStatistics().forEach((handle, stats) -> { - assertFalse(stats.getDistinctValuesCount().isUnknown()); - if (isKllHistogramSupportedType(((IcebergColumnHandle) handle).getType())) { - assertTrue(stats.getHistogram().isPresent()); - } - }); - } + String catalogName = "ice_stat_file_cache"; + Map catalogProperties = ImmutableMap.builder().putAll(icebergQueryRunner.getIcebergCatalogs().get("iceberg")) + .put("iceberg.max-statistics-file-cache-size", "1024B") + .build(); + getQueryRunner().createCatalog(catalogName, "iceberg", catalogProperties); + Session session = Session.builder(getQueryRunner().getDefaultSession()) + .setCatalog(catalogName) + // set histograms enabled to increase statistics cache size + .setSystemProperty(OPTIMIZER_USE_HISTOGRAMS, "true") + .setCatalogSessionProperty("iceberg", STATISTICS_KLL_SKETCH_K_PARAMETER, "32768") + .build(); + + assertQuerySucceeds(session, "ANALYZE lineitem"); + // get table statistics, to populate some of the cache + TableStatistics statistics = getTableStatistics(getQueryRunner(), session, "lineitem"); + assertTrue(statistics.getColumnStatistics().values().stream().map(ColumnStatistics::getHistogram).anyMatch(Optional::isPresent)); + RuntimeStats runtimeStats = session.getRuntimeStats(); + runtimeStats.getMetrics().keySet().stream().filter(name -> name.contains("ColumnCount")).findFirst() + .ifPresent(stat -> assertEquals(32, runtimeStats.getMetric(stat).getSum())); + runtimeStats.getMetrics().keySet().stream().filter(name -> name.contains("PuffinFileSize")).findFirst() + .ifPresent(stat -> assertTrue(runtimeStats.getMetric(stat).getSum() > 1024)); + // get them again to trigger retrieval of _some_ cached statistics + statistics = getTableStatistics(getQueryRunner(), session, "lineitem"); + RuntimeMetric partialMiss = runtimeStats.getMetrics().keySet().stream().filter(name -> name.contains("PartialMiss")).findFirst() + .map(runtimeStats::getMetric) + .orElseThrow(() -> new RuntimeException("partial miss on statistics cache should have occurred")); + assertTrue(partialMiss.getCount() > 0); + + statistics.getColumnStatistics().forEach((handle, stats) -> { + assertFalse(stats.getDistinctValuesCount().isUnknown()); + if (isKllHistogramSupportedType(((IcebergColumnHandle) handle).getType())) { + assertTrue(stats.getHistogram().isPresent()); + } + }); } private TableStatistics getScanStatsEstimate(Session session, @Language("SQL") String sql) @@ -505,7 +514,7 @@ private static TableHandle getAnalyzeTableHandle(QueryRunner queryRunner, String Metadata meta = queryRunner.getMetadata(); return meta.getTableHandleForStatisticsCollection( session, - new QualifiedObjectName("iceberg", "tpch", tableName.toLowerCase(Locale.US)), + new QualifiedObjectName(session.getCatalog().get(), session.getSchema().get(), tableName.toLowerCase(Locale.US)), Collections.emptyMap()).get(); } @@ -517,7 +526,7 @@ private TableHandle getAnalyzeTableHandle(String tableName, Session session) private static TableHandle getTableHandle(QueryRunner queryRunner, String tableName, Session session) { MetadataResolver resolver = queryRunner.getMetadata().getMetadataResolver(session); - return resolver.getTableHandle(new QualifiedObjectName("iceberg", "tpch", tableName.toLowerCase(Locale.US))).get(); + return resolver.getTableHandle(new QualifiedObjectName(session.getCatalog().get(), session.getSchema().get(), tableName.toLowerCase(Locale.US))).get(); } private static Map getColumnHandles(QueryRunner queryRunner, String tableName, Session session) diff --git a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/nessie/NessieTestUtil.java b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/nessie/NessieTestUtil.java index f495a62d79ff5..ad2031168f7e3 100644 --- a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/nessie/NessieTestUtil.java +++ b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/nessie/NessieTestUtil.java @@ -17,8 +17,6 @@ import java.util.Map; -import static com.facebook.presto.iceberg.CatalogType.NESSIE; - public class NessieTestUtil { private NessieTestUtil() @@ -27,6 +25,6 @@ private NessieTestUtil() public static Map nessieConnectorProperties(String serverUri) { - return ImmutableMap.of("iceberg.catalog.type", NESSIE.name(), "iceberg.nessie.uri", serverUri); + return ImmutableMap.of("iceberg.nessie.uri", serverUri); } } diff --git a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/nessie/TestIcebergDistributedNessie.java b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/nessie/TestIcebergDistributedNessie.java index d9a5884ad52c2..3aacc290d2055 100644 --- a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/nessie/TestIcebergDistributedNessie.java +++ b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/nessie/TestIcebergDistributedNessie.java @@ -69,7 +69,10 @@ public void tearDown() protected QueryRunner createQueryRunner() throws Exception { - return IcebergQueryRunner.createIcebergQueryRunner(ImmutableMap.of(), nessieConnectorProperties(nessieContainer.getRestApiUri())); + return IcebergQueryRunner.builder() + .setCatalogType(NESSIE) + .setExtraConnectorProperties(nessieConnectorProperties(nessieContainer.getRestApiUri())) + .build().getQueryRunner(); } @Override diff --git a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/nessie/TestIcebergNessieCatalogDistributedQueries.java b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/nessie/TestIcebergNessieCatalogDistributedQueries.java index cb238c0aa044b..35de3b9e71eb8 100644 --- a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/nessie/TestIcebergNessieCatalogDistributedQueries.java +++ b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/nessie/TestIcebergNessieCatalogDistributedQueries.java @@ -17,7 +17,6 @@ import com.facebook.presto.iceberg.TestIcebergDistributedQueries; import com.facebook.presto.testing.QueryRunner; import com.facebook.presto.testing.containers.NessieContainer; -import com.google.common.collect.ImmutableMap; import org.testng.annotations.AfterClass; import org.testng.annotations.BeforeClass; @@ -56,7 +55,10 @@ public void tearDown() protected QueryRunner createQueryRunner() throws Exception { - return IcebergQueryRunner.createIcebergQueryRunner(ImmutableMap.of(), nessieConnectorProperties(nessieContainer.getRestApiUri())); + return IcebergQueryRunner.builder() + .setCatalogType(NESSIE) + .setExtraConnectorProperties(nessieConnectorProperties(nessieContainer.getRestApiUri())) + .build().getQueryRunner(); } @Override diff --git a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/nessie/TestIcebergSmokeNessie.java b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/nessie/TestIcebergSmokeNessie.java index efebec74b12b5..1e9e882eb0a36 100644 --- a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/nessie/TestIcebergSmokeNessie.java +++ b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/nessie/TestIcebergSmokeNessie.java @@ -28,7 +28,6 @@ import com.facebook.presto.testing.QueryRunner; import com.facebook.presto.testing.containers.NessieContainer; import com.facebook.presto.tests.DistributedQueryRunner; -import com.google.common.collect.ImmutableMap; import org.apache.iceberg.Table; import org.testng.annotations.AfterClass; import org.testng.annotations.BeforeClass; @@ -95,7 +94,10 @@ protected String getLocation(String schema, String table) protected QueryRunner createQueryRunner() throws Exception { - return IcebergQueryRunner.createIcebergQueryRunner(ImmutableMap.of(), nessieConnectorProperties(nessieContainer.getRestApiUri())); + return IcebergQueryRunner.builder() + .setCatalogType(NESSIE) + .setExtraConnectorProperties(nessieConnectorProperties(nessieContainer.getRestApiUri())) + .build().getQueryRunner(); } @Override diff --git a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/nessie/TestIcebergSystemTablesNessie.java b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/nessie/TestIcebergSystemTablesNessie.java index 5f606777596d0..9bd1d48ea4ec7 100644 --- a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/nessie/TestIcebergSystemTablesNessie.java +++ b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/nessie/TestIcebergSystemTablesNessie.java @@ -30,6 +30,7 @@ import java.util.List; import java.util.Map; +import static com.facebook.presto.iceberg.CatalogType.NESSIE; import static com.facebook.presto.iceberg.IcebergQueryRunner.ICEBERG_CATALOG; import static com.facebook.presto.iceberg.IcebergQueryRunner.getIcebergDataDirectoryPath; import static com.facebook.presto.iceberg.nessie.NessieTestUtil.nessieConnectorProperties; @@ -68,6 +69,7 @@ protected QueryRunner createQueryRunner() Session session = testSessionBuilder() .setCatalog(ICEBERG_CATALOG) .build(); + DistributedQueryRunner queryRunner = DistributedQueryRunner.builder(session).build(); Path dataDirectory = queryRunner.getCoordinator().getDataDirectory(); @@ -75,6 +77,7 @@ protected QueryRunner createQueryRunner() queryRunner.installPlugin(new IcebergPlugin()); Map icebergProperties = ImmutableMap.builder() + .put("iceberg.catalog.type", String.valueOf(NESSIE)) .putAll(nessieConnectorProperties(nessieContainer.getRestApiUri())) .put("iceberg.catalog.warehouse", catalogDirectory.getParent().toFile().toURI().toString()) .build(); diff --git a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/nessie/TestNessieMultiBranching.java b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/nessie/TestNessieMultiBranching.java index 4ff896164702b..1cde1e1549f97 100644 --- a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/nessie/TestNessieMultiBranching.java +++ b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/nessie/TestNessieMultiBranching.java @@ -19,7 +19,6 @@ import com.facebook.presto.testing.QueryRunner; import com.facebook.presto.testing.containers.NessieContainer; import com.facebook.presto.tests.AbstractTestQueryFramework; -import com.google.common.collect.ImmutableMap; import org.projectnessie.client.api.NessieApiV1; import org.projectnessie.client.http.HttpClientBuilder; import org.projectnessie.error.NessieConflictException; @@ -35,6 +34,7 @@ import java.util.List; +import static com.facebook.presto.iceberg.CatalogType.NESSIE; import static com.facebook.presto.iceberg.nessie.NessieTestUtil.nessieConnectorProperties; import static com.facebook.presto.testing.MaterializedResult.resultBuilder; import static com.facebook.presto.tests.QueryAssertions.assertEqualsIgnoreOrder; @@ -70,7 +70,8 @@ public void tearDown() } @AfterMethod - public void resetData() throws NessieNotFoundException, NessieConflictException + public void resetData() + throws NessieNotFoundException, NessieConflictException { Branch defaultBranch = nessieApiV1.getDefaultBranch(); for (Reference r : nessieApiV1.getAllReferences().get().getReferences()) { @@ -87,7 +88,10 @@ public void resetData() throws NessieNotFoundException, NessieConflictException protected QueryRunner createQueryRunner() throws Exception { - return IcebergQueryRunner.createIcebergQueryRunner(ImmutableMap.of(), nessieConnectorProperties(nessieContainer.getRestApiUri())); + return IcebergQueryRunner.builder() + .setCatalogType(NESSIE) + .setExtraConnectorProperties(nessieConnectorProperties(nessieContainer.getRestApiUri())) + .build().getQueryRunner(); } @Test diff --git a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/procedure/TestExpireSnapshotProcedure.java b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/procedure/TestExpireSnapshotProcedure.java index 65aba0580a411..4307a3ed251ea 100644 --- a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/procedure/TestExpireSnapshotProcedure.java +++ b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/procedure/TestExpireSnapshotProcedure.java @@ -57,7 +57,7 @@ public class TestExpireSnapshotProcedure protected QueryRunner createQueryRunner() throws Exception { - return IcebergQueryRunner.createIcebergQueryRunner(ImmutableMap.of(), HADOOP, ImmutableMap.of()); + return IcebergQueryRunner.builder().setCatalogType(HADOOP).build().getQueryRunner(); } public void dropTable(String tableName) diff --git a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/procedure/TestFastForwardBranchProcedure.java b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/procedure/TestFastForwardBranchProcedure.java index deaf56d8066cd..964760610a0f3 100644 --- a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/procedure/TestFastForwardBranchProcedure.java +++ b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/procedure/TestFastForwardBranchProcedure.java @@ -14,6 +14,7 @@ package com.facebook.presto.iceberg.procedure; import com.facebook.presto.iceberg.IcebergConfig; +import com.facebook.presto.iceberg.IcebergQueryRunner; import com.facebook.presto.testing.QueryRunner; import com.facebook.presto.tests.AbstractTestQueryFramework; import com.google.common.collect.ImmutableMap; @@ -30,7 +31,6 @@ import java.util.Map; import static com.facebook.presto.iceberg.CatalogType.HADOOP; -import static com.facebook.presto.iceberg.IcebergQueryRunner.createIcebergQueryRunner; import static com.facebook.presto.iceberg.IcebergQueryRunner.getIcebergDataDirectoryPath; import static java.lang.String.format; @@ -44,7 +44,10 @@ public class TestFastForwardBranchProcedure protected QueryRunner createQueryRunner() throws Exception { - return createIcebergQueryRunner(ImmutableMap.of(), HADOOP, ImmutableMap.of()); + return IcebergQueryRunner.builder() + .setCatalogType(HADOOP) + .build() + .getQueryRunner(); } public void createTable(String tableName) diff --git a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/procedure/TestRemoveOrphanFilesProcedureBase.java b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/procedure/TestRemoveOrphanFilesProcedureBase.java index 794cd3f8e4499..8807ab0307885 100644 --- a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/procedure/TestRemoveOrphanFilesProcedureBase.java +++ b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/procedure/TestRemoveOrphanFilesProcedureBase.java @@ -82,7 +82,10 @@ protected TestRemoveOrphanFilesProcedureBase(CatalogType catalogType, Map restConnectorProperties(String serverUri) { - return ImmutableMap.of("iceberg.catalog.type", REST.name(), "iceberg.rest.uri", serverUri); + return ImmutableMap.of("iceberg.rest.uri", serverUri); } public static TestingHttpServer getRestServer(String location) diff --git a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/rest/TestIcebergDistributedRest.java b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/rest/TestIcebergDistributedRest.java index 06965eb0e32ed..baa105bc79f0b 100644 --- a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/rest/TestIcebergDistributedRest.java +++ b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/rest/TestIcebergDistributedRest.java @@ -16,6 +16,7 @@ import com.facebook.airlift.http.server.testing.TestingHttpServer; import com.facebook.presto.Session; import com.facebook.presto.iceberg.IcebergDistributedTestBase; +import com.facebook.presto.iceberg.IcebergQueryRunner; import com.facebook.presto.spi.security.Identity; import com.facebook.presto.testing.QueryRunner; import com.google.common.collect.ImmutableMap; @@ -27,12 +28,9 @@ import java.io.File; import java.util.Map; import java.util.Optional; -import java.util.OptionalInt; import static com.facebook.presto.iceberg.CatalogType.REST; -import static com.facebook.presto.iceberg.FileFormat.PARQUET; import static com.facebook.presto.iceberg.IcebergQueryRunner.ICEBERG_CATALOG; -import static com.facebook.presto.iceberg.IcebergQueryRunner.createIcebergQueryRunner; import static com.facebook.presto.iceberg.rest.IcebergRestTestUtil.getRestServer; import static com.facebook.presto.iceberg.rest.IcebergRestTestUtil.restConnectorProperties; import static com.facebook.presto.testing.TestingSession.testSessionBuilder; @@ -92,14 +90,12 @@ protected QueryRunner createQueryRunner() .put("iceberg.rest.session.type", SessionType.USER.name()) .build(); - return createIcebergQueryRunner( - ImmutableMap.of(), - connectorProperties, - PARQUET, - true, - false, - OptionalInt.empty(), - Optional.of(warehouseLocation.toPath())); + return IcebergQueryRunner.builder() + .setExtraConnectorProperties(connectorProperties) + .setCatalogType(REST) + .setDataDirectory(Optional.of(warehouseLocation.toPath())) + .build() + .getQueryRunner(); } @Test diff --git a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/rest/TestIcebergRestCatalogDistributedQueries.java b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/rest/TestIcebergRestCatalogDistributedQueries.java index ab76a96a77da8..88112e51b8497 100644 --- a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/rest/TestIcebergRestCatalogDistributedQueries.java +++ b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/rest/TestIcebergRestCatalogDistributedQueries.java @@ -14,20 +14,18 @@ package com.facebook.presto.iceberg.rest; import com.facebook.airlift.http.server.testing.TestingHttpServer; +import com.facebook.presto.iceberg.IcebergQueryRunner; import com.facebook.presto.iceberg.TestIcebergDistributedQueries; import com.facebook.presto.testing.QueryRunner; -import com.google.common.collect.ImmutableMap; import org.assertj.core.util.Files; import org.testng.annotations.AfterClass; import org.testng.annotations.BeforeClass; import java.io.File; import java.util.Optional; -import java.util.OptionalInt; import static com.facebook.presto.iceberg.CatalogType.REST; import static com.facebook.presto.iceberg.FileFormat.PARQUET; -import static com.facebook.presto.iceberg.IcebergQueryRunner.createIcebergQueryRunner; import static com.facebook.presto.iceberg.rest.IcebergRestTestUtil.getRestServer; import static com.facebook.presto.iceberg.rest.IcebergRestTestUtil.restConnectorProperties; import static com.google.common.io.MoreFiles.deleteRecursively; @@ -73,14 +71,13 @@ public void tearDown() protected QueryRunner createQueryRunner() throws Exception { - return createIcebergQueryRunner( - ImmutableMap.of(), - restConnectorProperties(serverUri), - PARQUET, - true, - false, - OptionalInt.empty(), - Optional.of(warehouseLocation.toPath())); + return IcebergQueryRunner.builder() + .setCatalogType(REST) + .setExtraConnectorProperties(restConnectorProperties(serverUri)) + .setFormat(PARQUET) + .setDataDirectory(Optional.of(warehouseLocation.toPath())) + .build() + .getQueryRunner(); } @Override diff --git a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/rest/TestIcebergSmokeRest.java b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/rest/TestIcebergSmokeRest.java index d74c11c4ad063..e5a7fb6a6e191 100644 --- a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/rest/TestIcebergSmokeRest.java +++ b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/rest/TestIcebergSmokeRest.java @@ -27,7 +27,6 @@ import com.facebook.presto.spi.ConnectorSession; import com.facebook.presto.spi.SchemaTableName; import com.facebook.presto.testing.QueryRunner; -import com.google.common.collect.ImmutableMap; import org.apache.iceberg.Table; import org.apache.iceberg.rest.RESTCatalog; import org.assertj.core.util.Files; @@ -37,10 +36,8 @@ import java.io.File; import java.util.Optional; -import java.util.OptionalInt; import static com.facebook.presto.iceberg.CatalogType.REST; -import static com.facebook.presto.iceberg.FileFormat.PARQUET; import static com.facebook.presto.iceberg.IcebergQueryRunner.ICEBERG_CATALOG; import static com.facebook.presto.iceberg.IcebergUtil.getNativeIcebergTable; import static com.facebook.presto.iceberg.rest.AuthenticationType.OAUTH2; @@ -100,14 +97,11 @@ protected String getLocation(String schema, String table) protected QueryRunner createQueryRunner() throws Exception { - return IcebergQueryRunner.createIcebergQueryRunner( - ImmutableMap.of(), - restConnectorProperties(serverUri), - PARQUET, - true, - false, - OptionalInt.empty(), - Optional.of(warehouseLocation.toPath())); + return IcebergQueryRunner.builder() + .setCatalogType(REST) + .setExtraConnectorProperties(restConnectorProperties(serverUri)) + .setDataDirectory(Optional.of(warehouseLocation.toPath())) + .build().getQueryRunner(); } protected IcebergNativeCatalogFactory getCatalogFactory(IcebergRestConfig restConfig) diff --git a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/rest/TestIcebergSmokeRestNestedNamespace.java b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/rest/TestIcebergSmokeRestNestedNamespace.java index 0cfa1b751bb50..3fe39ab1da7ff 100644 --- a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/rest/TestIcebergSmokeRestNestedNamespace.java +++ b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/rest/TestIcebergSmokeRestNestedNamespace.java @@ -29,7 +29,6 @@ import com.facebook.presto.spi.SchemaTableName; import com.facebook.presto.testing.MaterializedResult; import com.facebook.presto.testing.QueryRunner; -import com.facebook.presto.tests.DistributedQueryRunner; import com.google.common.collect.ImmutableMap; import org.apache.iceberg.Table; import org.assertj.core.util.Files; @@ -41,10 +40,8 @@ import java.io.File; import java.util.Map; import java.util.Optional; -import java.util.OptionalInt; import static com.facebook.presto.iceberg.CatalogType.REST; -import static com.facebook.presto.iceberg.FileFormat.PARQUET; import static com.facebook.presto.iceberg.IcebergQueryRunner.ICEBERG_CATALOG; import static com.facebook.presto.iceberg.IcebergUtil.getNativeIcebergTable; import static com.facebook.presto.iceberg.rest.IcebergRestTestUtil.getRestServer; @@ -110,26 +107,22 @@ protected QueryRunner createQueryRunner() throws Exception { Map restConnectorProperties = restConnectorProperties(serverUri); - DistributedQueryRunner icebergQueryRunner = IcebergQueryRunner.createIcebergQueryRunner( - ImmutableMap.of(), - restConnectorProperties, - PARQUET, - true, - false, - OptionalInt.empty(), - Optional.empty(), - Optional.of(warehouseLocation.toPath()), - false, - Optional.of("ns1.ns2")); + IcebergQueryRunner icebergQueryRunner = IcebergQueryRunner.builder() + .setCatalogType(REST) + .setExtraConnectorProperties(restConnectorProperties(serverUri)) + .setDataDirectory(Optional.of(warehouseLocation.toPath())) + .setSchemaName("ns1.ns2") + .build(); // additional catalog for testing nested namespace disabled - icebergQueryRunner.createCatalog(ICEBERG_NESTED_NAMESPACE_DISABLED_CATALOG, "iceberg", + icebergQueryRunner.addCatalog(ICEBERG_NESTED_NAMESPACE_DISABLED_CATALOG, new ImmutableMap.Builder() - .putAll(restConnectorProperties) - .put("iceberg.rest.nested.namespace.enabled", "false") - .build()); + .putAll(restConnectorProperties) + .put("iceberg.catalog.type", REST.name()) + .put("iceberg.rest.nested.namespace.enabled", "false") + .build()); - return icebergQueryRunner; + return icebergQueryRunner.getQueryRunner(); } protected IcebergNativeCatalogFactory getCatalogFactory(IcebergRestConfig restConfig) diff --git a/presto-native-execution/src/test/java/com/facebook/presto/nativeworker/PrestoNativeQueryRunnerUtils.java b/presto-native-execution/src/test/java/com/facebook/presto/nativeworker/PrestoNativeQueryRunnerUtils.java index 825fbb1bc4387..90a7a0fe42cd2 100644 --- a/presto-native-execution/src/test/java/com/facebook/presto/nativeworker/PrestoNativeQueryRunnerUtils.java +++ b/presto-native-execution/src/test/java/com/facebook/presto/nativeworker/PrestoNativeQueryRunnerUtils.java @@ -25,6 +25,7 @@ import com.facebook.presto.hive.metastore.StorageFormat; import com.facebook.presto.hive.metastore.Table; import com.facebook.presto.iceberg.FileFormat; +import com.facebook.presto.iceberg.IcebergQueryRunner; import com.facebook.presto.spi.PrestoException; import com.facebook.presto.testing.QueryRunner; import com.facebook.presto.tests.DistributedQueryRunner; @@ -55,7 +56,6 @@ import static com.facebook.presto.hive.HiveQueryRunner.getFileHiveMetastore; import static com.facebook.presto.hive.HiveTestUtils.getProperty; import static com.facebook.presto.hive.metastore.PrestoTableType.EXTERNAL_TABLE; -import static com.facebook.presto.iceberg.IcebergQueryRunner.createIcebergQueryRunner; import static com.facebook.presto.nativeworker.NativeQueryRunnerUtils.getNativeSidecarProperties; import static com.facebook.presto.nativeworker.NativeQueryRunnerUtils.getNativeWorkerHiveProperties; import static com.facebook.presto.nativeworker.NativeQueryRunnerUtils.getNativeWorkerIcebergProperties; @@ -86,6 +86,7 @@ public class PrestoNativeQueryRunnerUtils ParquetHiveSerDe.class.getName(), SymlinkTextInputFormat.class.getName(), HiveIgnoreKeyTextOutputFormat.class.getName()); + private PrestoNativeQueryRunnerUtils() {} public static QueryRunner createQueryRunner(boolean addStorageFormatToPath, boolean isCoordinatorSidecarEnabled) @@ -225,21 +226,18 @@ public static QueryRunner createJavaIcebergQueryRunner(Optional baseDataDi ImmutableMap.Builder icebergPropertiesBuilder = new ImmutableMap.Builder<>(); icebergPropertiesBuilder.put("hive.parquet.writer.version", "PARQUET_1_0"); - DistributedQueryRunner queryRunner = createIcebergQueryRunner( - ImmutableMap.of( + return IcebergQueryRunner.builder() + .setExtraProperties(ImmutableMap.of( "regex-library", "RE2J", "offset-clause-enabled", "true", - "query.max-stage-count", "110"), - icebergPropertiesBuilder.build(), - FileFormat.valueOf(storageFormat), - false, - false, - OptionalInt.empty(), - Optional.empty(), - baseDataDirectory, - addStorageFormatToPath); - - return queryRunner; + "query.max-stage-count", "110")) + .setExtraConnectorProperties(icebergPropertiesBuilder.build()) + .setAddJmxPlugin(false) + .setCreateTpchTables(false) + .setDataDirectory(baseDataDirectory) + .setAddStorageFormatToPath(addStorageFormatToPath) + .setFormat(FileFormat.valueOf(storageFormat)) + .build().getQueryRunner(); } public static QueryRunner createNativeIcebergQueryRunner(boolean useThrift) @@ -298,21 +296,21 @@ public static QueryRunner createNativeIcebergQueryRunner( .build(); // Make query runner with external workers for tests - return createIcebergQueryRunner( - ImmutableMap.builder() + return IcebergQueryRunner.builder() + .setExtraProperties(ImmutableMap.builder() .put("http-server.http.port", "8080") .put("experimental.internal-communication.thrift-transport-enabled", String.valueOf(useThrift)) .put("query.max-stage-count", "110") .putAll(getNativeWorkerSystemProperties()) - .build(), - icebergProperties, - FileFormat.valueOf(storageFormat), - false, - false, - OptionalInt.of(workerCount.orElse(4)), - getExternalWorkerLauncher("iceberg", prestoServerPath, cacheMaxSize, remoteFunctionServerUds, false, false), - dataDirectory, - addStorageFormatToPath); + .build()) + .setFormat(FileFormat.valueOf(storageFormat)) + .setCreateTpchTables(false) + .setAddJmxPlugin(false) + .setNodeCount(OptionalInt.of(workerCount.orElse(4))) + .setExternalWorkerLauncher(getExternalWorkerLauncher("iceberg", prestoServerPath, cacheMaxSize, remoteFunctionServerUds, false, false)) + .setAddStorageFormatToPath(addStorageFormatToPath) + .setDataDirectory(dataDirectory) + .build().getQueryRunner(); } public static QueryRunner createNativeQueryRunner( @@ -475,10 +473,10 @@ public static String startRemoteFunctionServer(String remoteFunctionServerBinary public static NativeQueryRunnerParameters getNativeQueryRunnerParameters() { Path prestoServerPath = Paths.get(getProperty("PRESTO_SERVER") - .orElse("_build/debug/presto_cpp/main/presto_server")) + .orElse("_build/debug/presto_cpp/main/presto_server")) .toAbsolutePath(); Path dataDirectory = Paths.get(getProperty("DATA_DIR") - .orElse("target/velox_data")) + .orElse("target/velox_data")) .toAbsolutePath(); Optional workerCount = getProperty("WORKER_COUNT").map(Integer::parseInt);