Skip to content

Commit

Permalink
Rename object-store.enabled to object-store-layout.enabled in Iceberg
Browse files Browse the repository at this point in the history
  • Loading branch information
ebyhr committed Nov 26, 2024
1 parent d3c084c commit e13f637
Show file tree
Hide file tree
Showing 9 changed files with 46 additions and 45 deletions.
6 changes: 3 additions & 3 deletions docs/src/main/sphinx/connector/iceberg.md
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ implementation is used:
- Set to `false` to disable in-memory caching of metadata files on the
coordinator. This cache is not used when `fs.cache.enabled` is set to true.
- `true`
* - `iceberg.object-store.enabled`
* - `iceberg.object-store-layout.enabled`
- Set to `true` to enable Iceberg's [object store file layout](https://iceberg.apache.org/docs/latest/aws/#object-store-file-layout).
Enabling the object store file layout appends a deterministic hash directly
after the data write path.
Expand Down Expand Up @@ -816,7 +816,7 @@ The following table properties can be updated after a table is created:
- `format_version`
- `partitioning`
- `sorted_by`
- `object_store_enabled`
- `object_store_layout_enabled`
- `data_location`

For example, to update a table from v1 of the Iceberg specification to v2:
Expand Down Expand Up @@ -880,7 +880,7 @@ connector using a {doc}`WITH </sql/create-table-as>` clause.
- Comma-separated list of columns to use for Parquet bloom filter. It improves
the performance of queries using Equality and IN predicates when reading
Parquet files. Requires Parquet format. Defaults to `[]`.
* - `object_store_enabled`
* - `object_store_layout_enabled`
- Whether Iceberg's [object store file layout](https://iceberg.apache.org/docs/latest/aws/#object-store-file-layout) is enabled.
* - `data_location`
- Optionally specifies the file system location URI for the table's data files
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ public class IcebergConfig
private List<String> allowedExtraProperties = ImmutableList.of();
private boolean incrementalRefreshEnabled = true;
private boolean metadataCacheEnabled = true;
private boolean objectStoreEnabled;
private boolean objectStoreLayoutEnabled;
private int metadataParallelism = 8;

public CatalogType getCatalogType()
Expand Down Expand Up @@ -522,16 +522,17 @@ public IcebergConfig setMetadataCacheEnabled(boolean metadataCacheEnabled)
return this;
}

public boolean isObjectStoreEnabled()
public boolean isObjectStoreLayoutEnabled()
{
return objectStoreEnabled;
return objectStoreLayoutEnabled;
}

@Config("iceberg.object-store.enabled")
@Config("iceberg.object-store-layout.enabled")
@LegacyConfig("iceberg.object-store.enabled")
@ConfigDescription("Enable the Iceberg object store file layout")
public IcebergConfig setObjectStoreEnabled(boolean objectStoreEnabled)
public IcebergConfig setObjectStoreLayoutEnabled(boolean objectStoreLayoutEnabled)
{
this.objectStoreEnabled = objectStoreEnabled;
this.objectStoreLayoutEnabled = objectStoreLayoutEnabled;
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@
import static io.trino.plugin.iceberg.IcebergTableProperties.EXTRA_PROPERTIES_PROPERTY;
import static io.trino.plugin.iceberg.IcebergTableProperties.FILE_FORMAT_PROPERTY;
import static io.trino.plugin.iceberg.IcebergTableProperties.FORMAT_VERSION_PROPERTY;
import static io.trino.plugin.iceberg.IcebergTableProperties.OBJECT_STORE_ENABLED_PROPERTY;
import static io.trino.plugin.iceberg.IcebergTableProperties.OBJECT_STORE_LAYOUT_ENABLED_PROPERTY;
import static io.trino.plugin.iceberg.IcebergTableProperties.PARTITIONING_PROPERTY;
import static io.trino.plugin.iceberg.IcebergTableProperties.SORTED_BY_PROPERTY;
import static io.trino.plugin.iceberg.IcebergTableProperties.getPartitioning;
Expand Down Expand Up @@ -380,7 +380,7 @@ public class IcebergMetadata
.add(EXTRA_PROPERTIES_PROPERTY)
.add(FILE_FORMAT_PROPERTY)
.add(FORMAT_VERSION_PROPERTY)
.add(OBJECT_STORE_ENABLED_PROPERTY)
.add(OBJECT_STORE_LAYOUT_ENABLED_PROPERTY)
.add(DATA_LOCATION_PROPERTY)
.add(PARTITIONING_PROPERTY)
.add(SORTED_BY_PROPERTY)
Expand Down Expand Up @@ -2161,8 +2161,8 @@ public void setTableProperties(ConnectorSession session, ConnectorTableHandle ta
updateProperties.set(FORMAT_VERSION, Integer.toString(formatVersion));
}

if (properties.containsKey(OBJECT_STORE_ENABLED_PROPERTY)) {
boolean objectStoreEnabled = (boolean) properties.get(OBJECT_STORE_ENABLED_PROPERTY)
if (properties.containsKey(OBJECT_STORE_LAYOUT_ENABLED_PROPERTY)) {
boolean objectStoreEnabled = (boolean) properties.get(OBJECT_STORE_LAYOUT_ENABLED_PROPERTY)
.orElseThrow(() -> new IllegalArgumentException("The object_store_enabled property cannot be empty"));
updateProperties.set(OBJECT_STORE_ENABLED, Boolean.toString(objectStoreEnabled));
}
Expand All @@ -2171,10 +2171,10 @@ public void setTableProperties(ConnectorSession session, ConnectorTableHandle ta
String dataLocation = (String) properties.get(DATA_LOCATION_PROPERTY)
.orElseThrow(() -> new IllegalArgumentException("The data_location property cannot be empty"));
boolean objectStoreEnabled = (boolean) properties.getOrDefault(
OBJECT_STORE_ENABLED_PROPERTY,
OBJECT_STORE_LAYOUT_ENABLED_PROPERTY,
Optional.of(Boolean.parseBoolean(icebergTable.properties().get(OBJECT_STORE_ENABLED)))).orElseThrow();
if (!objectStoreEnabled) {
throw new TrinoException(INVALID_TABLE_PROPERTY, "Data location can only be set when object store is enabled");
throw new TrinoException(INVALID_TABLE_PROPERTY, "Data location can only be set when object store layout is enabled");
}
updateProperties.set(WRITE_DATA_LOCATION, dataLocation);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public class IcebergTableProperties
public static final String ORC_BLOOM_FILTER_COLUMNS_PROPERTY = "orc_bloom_filter_columns";
public static final String ORC_BLOOM_FILTER_FPP_PROPERTY = "orc_bloom_filter_fpp";
public static final String PARQUET_BLOOM_FILTER_COLUMNS_PROPERTY = "parquet_bloom_filter_columns";
public static final String OBJECT_STORE_ENABLED_PROPERTY = "object_store_enabled";
public static final String OBJECT_STORE_LAYOUT_ENABLED_PROPERTY = "object_store_layout_enabled";
public static final String DATA_LOCATION_PROPERTY = "data_location";
public static final String EXTRA_PROPERTIES_PROPERTY = "extra_properties";

Expand All @@ -71,7 +71,7 @@ public class IcebergTableProperties
.add(FORMAT_VERSION_PROPERTY)
.add(ORC_BLOOM_FILTER_COLUMNS_PROPERTY)
.add(ORC_BLOOM_FILTER_FPP_PROPERTY)
.add(OBJECT_STORE_ENABLED_PROPERTY)
.add(OBJECT_STORE_LAYOUT_ENABLED_PROPERTY)
.add(DATA_LOCATION_PROPERTY)
.add(EXTRA_PROPERTIES_PROPERTY)
.add(PARQUET_BLOOM_FILTER_COLUMNS_PROPERTY)
Expand Down Expand Up @@ -181,9 +181,9 @@ public IcebergTableProperties(
},
value -> value))
.add(booleanProperty(
OBJECT_STORE_ENABLED_PROPERTY,
OBJECT_STORE_LAYOUT_ENABLED_PROPERTY,
"Set to true to enable Iceberg object store file layout",
icebergConfig.isObjectStoreEnabled(),
icebergConfig.isObjectStoreLayoutEnabled(),
false))
.add(stringProperty(
DATA_LOCATION_PROPERTY,
Expand Down Expand Up @@ -264,9 +264,9 @@ public static List<String> getParquetBloomFilterColumns(Map<String, Object> tabl
return parquetBloomFilterColumns == null ? ImmutableList.of() : ImmutableList.copyOf(parquetBloomFilterColumns);
}

public static boolean getObjectStoreEnabled(Map<String, Object> tableProperties)
public static boolean getObjectStoreLayoutEnabled(Map<String, Object> tableProperties)
{
return (boolean) tableProperties.getOrDefault(OBJECT_STORE_ENABLED_PROPERTY, false);
return (boolean) tableProperties.getOrDefault(OBJECT_STORE_LAYOUT_ENABLED_PROPERTY, false);
}

public static Optional<String> getDataLocation(Map<String, Object> tableProperties)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@
import static io.trino.plugin.iceberg.IcebergTableProperties.FILE_FORMAT_PROPERTY;
import static io.trino.plugin.iceberg.IcebergTableProperties.FORMAT_VERSION_PROPERTY;
import static io.trino.plugin.iceberg.IcebergTableProperties.LOCATION_PROPERTY;
import static io.trino.plugin.iceberg.IcebergTableProperties.OBJECT_STORE_ENABLED_PROPERTY;
import static io.trino.plugin.iceberg.IcebergTableProperties.OBJECT_STORE_LAYOUT_ENABLED_PROPERTY;
import static io.trino.plugin.iceberg.IcebergTableProperties.ORC_BLOOM_FILTER_COLUMNS_PROPERTY;
import static io.trino.plugin.iceberg.IcebergTableProperties.ORC_BLOOM_FILTER_FPP_PROPERTY;
import static io.trino.plugin.iceberg.IcebergTableProperties.PARQUET_BLOOM_FILTER_COLUMNS_PROPERTY;
Expand Down Expand Up @@ -337,7 +337,7 @@ public static Map<String, Object> getIcebergTableProperties(Table icebergTable)
}

if (parseBoolean(icebergTable.properties().getOrDefault(OBJECT_STORE_ENABLED, "false"))) {
properties.put(OBJECT_STORE_ENABLED_PROPERTY, true);
properties.put(OBJECT_STORE_LAYOUT_ENABLED_PROPERTY, true);
}

Optional<String> dataLocation = Optional.ofNullable(icebergTable.properties().get(WRITE_DATA_LOCATION));
Expand Down Expand Up @@ -851,14 +851,14 @@ public static Map<String, String> createTableProperties(ConnectorTableMetadata t
propertiesBuilder.put(DEFAULT_FILE_FORMAT, fileFormat.toIceberg().toString());
propertiesBuilder.put(FORMAT_VERSION, Integer.toString(IcebergTableProperties.getFormatVersion(tableMetadata.getProperties())));

boolean objectStoreEnabled = IcebergTableProperties.getObjectStoreEnabled(tableMetadata.getProperties());
if (objectStoreEnabled) {
boolean objectStoreLayoutEnabled = IcebergTableProperties.getObjectStoreLayoutEnabled(tableMetadata.getProperties());
if (objectStoreLayoutEnabled) {
propertiesBuilder.put(OBJECT_STORE_ENABLED, "true");
}
Optional<String> dataLocation = IcebergTableProperties.getDataLocation(tableMetadata.getProperties());
dataLocation.ifPresent(location -> {
if (!objectStoreEnabled) {
throw new TrinoException(INVALID_TABLE_PROPERTY, "Data location can only be set when object store is enabled");
if (!objectStoreLayoutEnabled) {
throw new TrinoException(INVALID_TABLE_PROPERTY, "Data location can only be set when object store layout is enabled");
}
propertiesBuilder.put(WRITE_DATA_LOCATION, location);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8540,11 +8540,11 @@ public void testSetIllegalExtraPropertyKey()
}

@Test
public void testObjectStoreEnabledAndDataLocation()
public void testObjectStoreLayoutEnabledAndDataLocation()
throws Exception
{
String tableName = "test_object_store_enabled_data_location" + randomNameSuffix();
assertUpdate("CREATE TABLE " + tableName + " WITH (object_store_enabled = true, data_location = 'local:///data-location/xyz') AS SELECT 1 AS val", 1);
String tableName = "test_object_store_layout_enabled_data_location" + randomNameSuffix();
assertUpdate("CREATE TABLE " + tableName + " WITH (object_store_layout_enabled = true, data_location = 'local:///data-location/xyz') AS SELECT 1 AS val", 1);

Location tableLocation = Location.of(getTableLocation(tableName));
assertThat(fileSystem.directoryExists(tableLocation).get()).isTrue();
Expand All @@ -8560,11 +8560,11 @@ public void testObjectStoreEnabledAndDataLocation()
}

@Test
public void testCreateTableWithDataLocationButObjectStoreDisabled()
public void testCreateTableWithDataLocationButObjectStoreLayoutDisabled()
{
assertQueryFails(
"CREATE TABLE test_data_location WITH (data_location = 'local:///data-location/xyz') AS SELECT 1 AS val",
"Data location can only be set when object store is enabled");
"Data location can only be set when object store layout is enabled");
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public void testDefaults()
.setIncrementalRefreshEnabled(true)
.setMetadataCacheEnabled(true)
.setIncrementalRefreshEnabled(true)
.setObjectStoreEnabled(false)
.setObjectStoreLayoutEnabled(false)
.setMetadataParallelism(8));
}

Expand Down Expand Up @@ -114,7 +114,7 @@ public void testExplicitPropertyMappings()
.put("iceberg.allowed-extra-properties", "propX,propY")
.put("iceberg.incremental-refresh-enabled", "false")
.put("iceberg.metadata-cache.enabled", "false")
.put("iceberg.object-store.enabled", "true")
.put("iceberg.object-store-layout.enabled", "true")
.put("iceberg.metadata.parallelism", "10")
.buildOrThrow();

Expand Down Expand Up @@ -150,7 +150,7 @@ public void testExplicitPropertyMappings()
.setIncrementalRefreshEnabled(false)
.setMetadataCacheEnabled(false)
.setIncrementalRefreshEnabled(false)
.setObjectStoreEnabled(true)
.setObjectStoreLayoutEnabled(true)
.setMetadataParallelism(10);

assertFullMapping(properties, expected);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
import static io.trino.testing.TestingConnectorSession.SESSION;
import static org.assertj.core.api.Assertions.assertThat;

final class TestIcebergTableWithObjectStore
final class TestIcebergTableWithObjectStoreLayout
extends AbstractTestQueryFramework
{
private HiveMetastore metastore;
Expand All @@ -41,7 +41,7 @@ protected DistributedQueryRunner createQueryRunner()
throws Exception
{
DistributedQueryRunner queryRunner = IcebergQueryRunner.builder()
.addIcebergProperty("iceberg.object-store.enabled", "true")
.addIcebergProperty("iceberg.object-store-layout.enabled", "true")
.build();

metastore = ((IcebergConnector) queryRunner.getCoordinator().getConnector(ICEBERG_CATALOG)).getInjector()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,17 +183,17 @@ public void testDefaultFormatVersion()
}

@Test
public void testSetPropertiesObjectStoreEnabled()
public void testSetPropertiesObjectStoreLayoutEnabled()
{
try (TestTable table = new TestTable(getQueryRunner()::execute, "test_object_store", "(x int) WITH (object_store_enabled = false)")) {
try (TestTable table = new TestTable(getQueryRunner()::execute, "test_object_store", "(x int) WITH (object_store_layout_enabled = false)")) {
assertThat((String) computeScalar("SHOW CREATE TABLE " + table.getName()))
.doesNotContain("object_store_enabled");
.doesNotContain("object_store_layout_enabled");
assertThat(loadTable(table.getName()).properties())
.doesNotContainKey("write.object-storage.enabled");

assertUpdate("ALTER TABLE " + table.getName() + " SET PROPERTIES object_store_enabled = true");
assertUpdate("ALTER TABLE " + table.getName() + " SET PROPERTIES object_store_layout_enabled = true");
assertThat((String) computeScalar("SHOW CREATE TABLE " + table.getName()))
.contains("object_store_enabled = true");
.contains("object_store_layout_enabled = true");
assertThat(loadTable(table.getName()).properties())
.containsEntry("write.object-storage.enabled", "true");
}
Expand All @@ -210,11 +210,11 @@ public void testSetPropertiesDataLocation()

assertQueryFails(
"ALTER TABLE " + table.getName() + " SET PROPERTIES data_location = 'local:///data-location'",
"Data location can only be set when object store is enabled");
"Data location can only be set when object store layout is enabled");

assertUpdate("ALTER TABLE " + table.getName() + " SET PROPERTIES object_store_enabled = true, data_location = 'local:///data-location'");
assertUpdate("ALTER TABLE " + table.getName() + " SET PROPERTIES object_store_layout_enabled = true, data_location = 'local:///data-location'");
assertThat((String) computeScalar("SHOW CREATE TABLE " + table.getName()))
.contains("object_store_enabled = true")
.contains("object_store_layout_enabled = true")
.contains("data_location = 'local:///data-location'");
assertThat(loadTable(table.getName()).properties())
.containsEntry("write.object-storage.enabled", "true")
Expand Down

0 comments on commit e13f637

Please sign in to comment.