Skip to content

Flatten Iceberg catalog/metastore configuration #9577

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
findepi merged 2 commits into from
Oct 12, 2021
Merged

Flatten Iceberg catalog/metastore configuration #9577

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8d3eb27
Flatten Iceberg catalog/metastore configuration
findepi Oct 11, 2021
0da3e53
empty
findepi Oct 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/CatalogType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,8 @@

public enum CatalogType
{
HIVE,
// TODO: dummy type to pass IcebergConfig test, remove it after adding actual catalog types
UNKNOWN,

TESTING_FILE_METASTORE,
HIVE_METASTORE,
GLUE,
/**/;
}
4 changes: 2 additions & 2 deletions plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import javax.validation.constraints.NotNull;

import static io.trino.plugin.hive.HiveCompressionCodec.GZIP;
import static io.trino.plugin.iceberg.CatalogType.HIVE;
import static io.trino.plugin.iceberg.CatalogType.HIVE_METASTORE;
import static io.trino.plugin.iceberg.IcebergFileFormat.ORC;

public class IcebergConfig
Expand All @@ -32,7 +32,7 @@ public class IcebergConfig
private boolean useFileSizeFromMetadata = true;
private int maxPartitionsPerWriter = 100;
private boolean uniqueTableLocation;
private CatalogType catalogType = HIVE;
private CatalogType catalogType = HIVE_METASTORE;

public CatalogType getCatalogType()
{
Expand Down
13 changes: 7 additions & 6 deletions plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetastoreModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import com.google.inject.Module;
import io.airlift.configuration.AbstractConfigurationAwareModule;
import io.trino.plugin.hive.metastore.HiveMetastore;
import io.trino.plugin.hive.metastore.MetastoreTypeConfig;
import io.trino.plugin.hive.metastore.cache.CachingHiveMetastore;
import io.trino.plugin.hive.metastore.cache.CachingHiveMetastoreModule;
import io.trino.plugin.hive.metastore.cache.ForCachingHiveMetastore;
Expand All @@ -29,6 +28,8 @@
import java.util.Optional;

import static io.airlift.configuration.ConditionalModule.conditionalModule;
import static io.trino.plugin.iceberg.CatalogType.HIVE_METASTORE;
import static io.trino.plugin.iceberg.CatalogType.TESTING_FILE_METASTORE;
import static java.util.Objects.requireNonNull;

public class IcebergMetastoreModule
Expand All @@ -49,8 +50,8 @@ protected void setup(Binder binder)
install(new CachingHiveMetastoreModule());
}
else {
bindMetastoreModule("thrift", new ThriftMetastoreModule());
bindMetastoreModule("file", new FileMetastoreModule());
bindMetastoreModule(HIVE_METASTORE, new ThriftMetastoreModule());
bindMetastoreModule(TESTING_FILE_METASTORE, new FileMetastoreModule());
// TODO add support for Glue metastore
}

Expand All @@ -68,11 +69,11 @@ public MetastoreValidator(HiveMetastore metastore)
}
}

private void bindMetastoreModule(String name, Module module)
private void bindMetastoreModule(CatalogType catalogType, Module module)
{
install(conditionalModule(
MetastoreTypeConfig.class,
metastore -> name.equalsIgnoreCase(metastore.getMetastoreType()),
IcebergConfig.class,
config -> config.getCatalogType() == catalogType,
module));
}
}
1 change: 1 addition & 0 deletions plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ public void configure(Binder binder)
binder.bind(FileFormatDataSourceStats.class).in(Scopes.SINGLETON);
newExporter(binder).export(FileFormatDataSourceStats.class).withGeneratedName();

// TODO inject table operations based on IcebergConfig.getCatalogType
binder.bind(HiveTableOperationsProvider.class).in(Scopes.SINGLETON);

binder.bind(IcebergFileWriterFactory.class).in(Scopes.SINGLETON);
Expand Down
6 changes: 4 additions & 2 deletions plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/TrinoCatalogFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,11 @@ public TrinoCatalogFactory(
public TrinoCatalog create()
{
switch (catalogType) {
case HIVE:
case TESTING_FILE_METASTORE:
case HIVE_METASTORE:
return new TrinoHiveCatalog(catalogName, memoizeMetastore(metastore, 1000), hdfsEnvironment, typeManager, tableOperationsProvider, trinoVersion, isUniqueTableLocation);
case UNKNOWN:
case GLUE:
// TODO not supported yet
throw new TrinoException(NOT_SUPPORTED, "Unknown Trino Iceberg catalog type");
}
throw new TrinoException(NOT_SUPPORTED, "Unsupported Trino Iceberg catalog type " + catalogType);
Expand Down
2 changes: 1 addition & 1 deletion plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/IcebergQueryRunner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public static DistributedQueryRunner createIcebergQueryRunner(

queryRunner.installPlugin(new IcebergPlugin());
connectorProperties = new HashMap<>(ImmutableMap.copyOf(connectorProperties));
connectorProperties.putIfAbsent("hive.metastore", "file");
connectorProperties.putIfAbsent("iceberg.catalog.type", "TESTING_FILE_METASTORE");
connectorProperties.putIfAbsent("hive.metastore.catalog.dir", dataDir.toString());
queryRunner.createCatalog(ICEBERG_CATALOG, "iceberg", connectorProperties);

Expand Down
10 changes: 5 additions & 5 deletions plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
import static io.airlift.configuration.testing.ConfigAssertions.assertRecordedDefaults;
import static io.airlift.configuration.testing.ConfigAssertions.recordDefaults;
import static io.trino.plugin.hive.HiveCompressionCodec.GZIP;
import static io.trino.plugin.iceberg.CatalogType.HIVE;
import static io.trino.plugin.iceberg.CatalogType.UNKNOWN;
import static io.trino.plugin.iceberg.CatalogType.GLUE;
import static io.trino.plugin.iceberg.CatalogType.HIVE_METASTORE;
import static io.trino.plugin.iceberg.IcebergFileFormat.ORC;
import static io.trino.plugin.iceberg.IcebergFileFormat.PARQUET;

Expand All @@ -39,7 +39,7 @@ public void testDefaults()
.setUseFileSizeFromMetadata(true)
.setMaxPartitionsPerWriter(100)
.setUniqueTableLocation(false)
.setCatalogType(HIVE));
.setCatalogType(HIVE_METASTORE));
}

@Test
Expand All @@ -51,7 +51,7 @@ public void testExplicitPropertyMappings()
.put("iceberg.use-file-size-from-metadata", "false")
.put("iceberg.max-partitions-per-writer", "222")
.put("iceberg.unique-table-location", "true")
.put("iceberg.catalog.type", "UNKNOWN")
.put("iceberg.catalog.type", "GLUE")
.build();

IcebergConfig expected = new IcebergConfig()
Expand All @@ -60,7 +60,7 @@ public void testExplicitPropertyMappings()
.setUseFileSizeFromMetadata(false)
.setMaxPartitionsPerWriter(222)
.setUniqueTableLocation(true)
.setCatalogType(UNKNOWN);
.setCatalogType(GLUE);

assertFullMapping(properties, expected);
}
Expand Down
24 changes: 19 additions & 5 deletions plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,27 +40,41 @@ public void testThriftMetastore()
factory.create(
"test",
Map.of(
"hive.metastore", "thrift",
"iceberg.catalog.type", "HIVE_METASTORE",
"hive.metastore.uri", "thrift://foo:1234"),
new TestingConnectorContext())
.shutdown();
}

@Test
public void testHiveMetastoreRejected()
{
ConnectorFactory factory = getConnectorFactory();

assertThatThrownBy(() -> factory.create(
"test",
Map.of(
"hive.metastore", "thrift",
"hive.metastore.uri", "thrift://foo:1234"),
new TestingConnectorContext()))
.hasMessageContaining("Error: Configuration property 'hive.metastore' was not used");
}

@Test
public void testGlueMetastore()
{
ConnectorFactory factory = getConnectorFactory();

assertThatThrownBy(() -> factory.create(
"test",
Map.of("hive.metastore", "glue"),
Map.of("iceberg.catalog.type", "glue"),
new TestingConnectorContext()))
.hasMessageContaining("Explicit bindings are required and HiveMetastore is not explicitly bound");

assertThatThrownBy(() -> factory.create(
"test",
Map.of(
"hive.metastore", "glue",
"iceberg.catalog.type", "glue",
"hive.metastore.uri", "thrift://foo:1234"),
new TestingConnectorContext()))
.hasMessageContaining("Error: Configuration property 'hive.metastore.uri' was not used");
Expand All @@ -75,7 +89,7 @@ public void testRecordingMetastore()
factory.create(
"test",
Map.of(
"hive.metastore", "thrift",
"iceberg.catalog.type", "HIVE_METASTORE",
"hive.metastore.uri", "thrift://foo:1234",
"hive.metastore-recording-path", "/tmp"),
new TestingConnectorContext())
Expand All @@ -85,7 +99,7 @@ public void testRecordingMetastore()
assertThatThrownBy(() -> factory.create(
"test",
Map.of(
"hive.metastore", "glue",
"iceberg.catalog.type", "glue",
"hive.metastore.glue.region", "us-east-2",
"hive.metastore-recording-path", "/tmp"),
new TestingConnectorContext()))
Expand Down