This is an automated email from the ASF dual-hosted git repository.
vinishjail97 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-xtable.git
The following commit(s) were added to refs/heads/main by this push:
new 35eecfa5 fix(xtable-aws): Delta table StorageDescriptor for Glue
Catalog Federation compatibility (#940)
35eecfa5 is described below
commit 35eecfa510cd0f5c241bdc6525ee828da7bad042
Author: Abdul Megoon <[email protected]>
AuthorDate: Sat Sep 19 00:00:13 2026 +0530
fix(xtable-aws): Delta table StorageDescriptor for Glue Catalog Federation
compatibility (#940)
* fix catalog
* Preserve existing StorageDescriptor settings on Delta Glue table refresh
getUpdateTableRequest rebuilt the StorageDescriptor from scratch, same as
getCreateTableRequest, silently dropping additionalLocations and any custom
serde parameters/settings a user had set directly in Glue. Build off the
existing descriptor via toBuilder() instead, only adding the Parquet
input/output format and serde class this fix requires.
* Cover the legacy StorageDescriptor repair and the null SerDeInfo fallback
Both existing refresh tests start from a descriptor that already carries the
three Parquet values, so they would still pass if refresh stopped filling
them
in. Add two cases that pin the repair behaviour:
- testGetUpdateTableInputRepairsLegacyStorageDescriptor: input has no
inputFormat, no outputFormat and a null serializationLibrary, matching a
table registered by an earlier XTable version. Asserts refresh fills all
three and preserves the columns, location and custom serde parameters.
- testGetUpdateTableInputPopulatesMissingSerDeInfo: exercises the
existing.serdeInfo() == null branch of getUpdatedStorageDescriptor.
Also lift the repeated Hive Parquet class names into constants.
---------
Co-authored-by: Vinish Reddy <[email protected]>
---
.../glue/table/DeltaGlueCatalogTableBuilder.java | 58 ++++++++--
.../table/TestDeltaGlueCatalogTableBuilder.java | 120 +++++++++++++++++++++
2 files changed, 170 insertions(+), 8 deletions(-)
diff --git
a/xtable-aws/src/main/java/org/apache/xtable/glue/table/DeltaGlueCatalogTableBuilder.java
b/xtable-aws/src/main/java/org/apache/xtable/glue/table/DeltaGlueCatalogTableBuilder.java
index 015bde7b..15385d33 100644
---
a/xtable-aws/src/main/java/org/apache/xtable/glue/table/DeltaGlueCatalogTableBuilder.java
+++
b/xtable-aws/src/main/java/org/apache/xtable/glue/table/DeltaGlueCatalogTableBuilder.java
@@ -27,6 +27,7 @@ import static
org.apache.xtable.catalog.Constants.PROP_SPARK_SQL_SOURCES_PROVIDE
import static
org.apache.xtable.glue.GlueCatalogSyncClient.GLUE_EXTERNAL_TABLE_TYPE;
import java.util.HashMap;
+import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
@@ -48,6 +49,20 @@ import software.amazon.awssdk.services.glue.model.TableInput;
/** Delta specific table operations for Glue catalog sync */
public class DeltaGlueCatalogTableBuilder implements
CatalogTableBuilder<TableInput, Table> {
+ // Standard Hive-compatible Parquet classes (Delta's on-disk file format).
+ // A generic Hive-Metastore-compatible reader (e.g. a UC Glue Federation
+ // foreign catalog) validates every table's StorageDescriptor when listing a
+ // database and throws InvalidObjectException for the whole database on the
+ // first table with a null SerDe/format -- these must be set even though
+ // Athena's own Delta reader (which keys off the table_type/
+ // spark.sql.sources.provider parameters instead) tolerates them being null.
+ private static final String PARQUET_INPUT_FORMAT =
+ "org.apache.hadoop.hive.ql.io.parquet.MapredParquetInputFormat";
+ private static final String PARQUET_OUTPUT_FORMAT =
+ "org.apache.hadoop.hive.ql.io.parquet.MapredParquetOutputFormat";
+ private static final String PARQUET_SERDE_CLASS =
+ "org.apache.hadoop.hive.ql.io.parquet.serde.ParquetHiveSerDe";
+
private final GlueSchemaExtractor schemaExtractor;
private static final String tableFormat = TableFormat.DELTA;
@@ -68,11 +83,7 @@ public class DeltaGlueCatalogTableBuilder implements
CatalogTableBuilder<TableIn
.tableType(GLUE_EXTERNAL_TABLE_TYPE)
.parameters(getTableParameters())
.storageDescriptor(
- StorageDescriptor.builder()
- .columns(schemaExtractor.getNonPartitionColumns(table,
columnsMap))
- .location(table.getBasePath())
-
.serdeInfo(SerDeInfo.builder().parameters(getSerDeParameters(table)).build())
- .build())
+ getStorageDescriptor(table,
schemaExtractor.getNonPartitionColumns(table, columnsMap)))
.partitionKeys(schemaExtractor.getPartitionColumns(table, columnsMap))
.build();
}
@@ -90,13 +101,44 @@ public class DeltaGlueCatalogTableBuilder implements
CatalogTableBuilder<TableIn
.tableType(GLUE_EXTERNAL_TABLE_TYPE)
.parameters(parameters)
.storageDescriptor(
- catalogTable.storageDescriptor().toBuilder()
- .columns(schemaExtractor.getNonPartitionColumns(table,
columnsMap))
- .build())
+ getUpdatedStorageDescriptor(
+ table, catalogTable,
schemaExtractor.getNonPartitionColumns(table, columnsMap)))
.partitionKeys(schemaExtractor.getPartitionColumns(table, columnsMap))
.build();
}
+ private StorageDescriptor getStorageDescriptor(InternalTable table,
List<Column> columns) {
+ return StorageDescriptor.builder()
+ .columns(columns)
+ .location(table.getBasePath())
+ .inputFormat(PARQUET_INPUT_FORMAT)
+ .outputFormat(PARQUET_OUTPUT_FORMAT)
+ .serdeInfo(
+ SerDeInfo.builder()
+ .serializationLibrary(PARQUET_SERDE_CLASS)
+ .parameters(getSerDeParameters(table))
+ .build())
+ .build();
+ }
+
+ // Refresh must build off the existing StorageDescriptor rather than
rebuilding one from
+ // scratch (like getStorageDescriptor does for create) -- a from-scratch
rebuild silently drops
+ // additionalLocations and any custom serde parameters/settings a user set
directly in Glue.
+ private StorageDescriptor getUpdatedStorageDescriptor(
+ InternalTable table, Table catalogTable, List<Column> columns) {
+ StorageDescriptor existing = catalogTable.storageDescriptor();
+ SerDeInfo.Builder serdeInfo =
+ existing.serdeInfo() == null
+ ? SerDeInfo.builder().parameters(getSerDeParameters(table))
+ : existing.serdeInfo().toBuilder();
+ return existing.toBuilder()
+ .columns(columns)
+ .inputFormat(PARQUET_INPUT_FORMAT)
+ .outputFormat(PARQUET_OUTPUT_FORMAT)
+ .serdeInfo(serdeInfo.serializationLibrary(PARQUET_SERDE_CLASS).build())
+ .build();
+ }
+
@VisibleForTesting
Map<String, String> getTableParameters() {
Map<String, String> parameters = new HashMap<>();
diff --git
a/xtable-aws/src/test/java/org/apache/xtable/glue/table/TestDeltaGlueCatalogTableBuilder.java
b/xtable-aws/src/test/java/org/apache/xtable/glue/table/TestDeltaGlueCatalogTableBuilder.java
index 59b3452d..a8c8eeaf 100644
---
a/xtable-aws/src/test/java/org/apache/xtable/glue/table/TestDeltaGlueCatalogTableBuilder.java
+++
b/xtable-aws/src/test/java/org/apache/xtable/glue/table/TestDeltaGlueCatalogTableBuilder.java
@@ -21,7 +21,10 @@ package org.apache.xtable.glue.table;
import static
org.apache.xtable.glue.GlueCatalogSyncClient.GLUE_EXTERNAL_TABLE_TYPE;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import java.util.Collections;
+import java.util.HashMap;
import java.util.List;
+import java.util.Map;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
@@ -38,6 +41,13 @@ import software.amazon.awssdk.services.glue.model.TableInput;
@ExtendWith(MockitoExtension.class)
public class TestDeltaGlueCatalogTableBuilder extends GlueCatalogSyncTestBase {
+ private static final String PARQUET_INPUT_FORMAT =
+ "org.apache.hadoop.hive.ql.io.parquet.MapredParquetInputFormat";
+ private static final String PARQUET_OUTPUT_FORMAT =
+ "org.apache.hadoop.hive.ql.io.parquet.MapredParquetOutputFormat";
+ private static final String PARQUET_SERDE_CLASS =
+ "org.apache.hadoop.hive.ql.io.parquet.serde.ParquetHiveSerDe";
+
private DeltaGlueCatalogTableBuilder deltaGlueCatalogTableBuilder;
private DeltaGlueCatalogTableBuilder createDeltaGlueCatalogSyncHelper() {
@@ -92,12 +102,122 @@ public class TestDeltaGlueCatalogTableBuilder extends
GlueCatalogSyncTestBase {
assertEquals(expected, output);
}
+ @Test
+ void testGetUpdateTableInputPreservesExistingStorageSettings() {
+ // Regression test: a from-scratch StorageDescriptor rebuild (like
getCreateTableRequest
+ // uses) would silently drop additionalLocations and any custom serde
parameter set directly
+ // in Glue outside of XTable -- refresh must instead build off the
existing descriptor.
+ setupCommonMocks();
+ List<String> additionalLocations =
Collections.singletonList("s3://base-path/extra-location");
+ Map<String, String> existingSerdeParams =
+ new
HashMap<>(deltaGlueCatalogTableBuilder.getSerDeParameters(TEST_DELTA_INTERNAL_TABLE));
+ existingSerdeParams.put("custom.serde.param", "custom-value");
+
+ StorageDescriptor existingStorageDescriptor =
+ StorageDescriptor.builder()
+ .columns(DELTA_GLUE_SCHEMA)
+ .location(TEST_BASE_PATH)
+ .additionalLocations(additionalLocations)
+ .inputFormat(PARQUET_INPUT_FORMAT)
+ .outputFormat(PARQUET_OUTPUT_FORMAT)
+ .serdeInfo(
+ SerDeInfo.builder()
+ .serializationLibrary(PARQUET_SERDE_CLASS)
+ .parameters(existingSerdeParams)
+ .build())
+ .build();
+ Table glueTable =
+ Table.builder()
+ .parameters(deltaGlueCatalogTableBuilder.getTableParameters())
+ .storageDescriptor(existingStorageDescriptor)
+ .partitionKeys(PARTITION_KEYS)
+ .build();
+
+ TableInput output =
+ deltaGlueCatalogTableBuilder.getUpdateTableRequest(
+ TEST_UPDATED_DELTA_INTERNAL_TABLE, glueTable,
TEST_CATALOG_TABLE_IDENTIFIER);
+
+ StorageDescriptor outputStorageDescriptor = output.storageDescriptor();
+ assertEquals(additionalLocations,
outputStorageDescriptor.additionalLocations());
+ assertEquals(existingSerdeParams,
outputStorageDescriptor.serdeInfo().parameters());
+ assertEquals(UPDATED_DELTA_GLUE_SCHEMA, outputStorageDescriptor.columns());
+ }
+
+ @Test
+ void testGetUpdateTableInputRepairsLegacyStorageDescriptor() {
+ // A table registered by an earlier XTable version carries no input/output
format and no
+ // serialization library, which is what breaks Glue Catalog Federation.
Refresh must fill all
+ // three in, and must leave the columns, location and existing serde
parameters intact.
+ setupCommonMocks();
+ Map<String, String> existingSerdeParams =
+ new
HashMap<>(deltaGlueCatalogTableBuilder.getSerDeParameters(TEST_DELTA_INTERNAL_TABLE));
+ existingSerdeParams.put("custom.serde.param", "custom-value");
+
+ StorageDescriptor legacyStorageDescriptor =
+ StorageDescriptor.builder()
+ .columns(DELTA_GLUE_SCHEMA)
+ .location(TEST_BASE_PATH)
+
.serdeInfo(SerDeInfo.builder().parameters(existingSerdeParams).build())
+ .build();
+ Table glueTable =
+ Table.builder()
+ .parameters(deltaGlueCatalogTableBuilder.getTableParameters())
+ .storageDescriptor(legacyStorageDescriptor)
+ .partitionKeys(PARTITION_KEYS)
+ .build();
+
+ StorageDescriptor output =
+ deltaGlueCatalogTableBuilder
+ .getUpdateTableRequest(
+ TEST_UPDATED_DELTA_INTERNAL_TABLE, glueTable,
TEST_CATALOG_TABLE_IDENTIFIER)
+ .storageDescriptor();
+
+ assertEquals(PARQUET_INPUT_FORMAT, output.inputFormat());
+ assertEquals(PARQUET_OUTPUT_FORMAT, output.outputFormat());
+ assertEquals(PARQUET_SERDE_CLASS,
output.serdeInfo().serializationLibrary());
+ assertEquals(existingSerdeParams, output.serdeInfo().parameters());
+ assertEquals(UPDATED_DELTA_GLUE_SCHEMA, output.columns());
+ assertEquals(TEST_BASE_PATH, output.location());
+ }
+
+ @Test
+ void testGetUpdateTableInputPopulatesMissingSerDeInfo() {
+ // Covers the null-SerDeInfo branch of the refresh path: an existing
descriptor with no
+ // SerDeInfo at all gets a fresh one derived from the table.
+ setupCommonMocks();
+ StorageDescriptor storageDescriptorWithoutSerDeInfo =
+
StorageDescriptor.builder().columns(DELTA_GLUE_SCHEMA).location(TEST_BASE_PATH).build();
+ Table glueTable =
+ Table.builder()
+ .parameters(deltaGlueCatalogTableBuilder.getTableParameters())
+ .storageDescriptor(storageDescriptorWithoutSerDeInfo)
+ .partitionKeys(PARTITION_KEYS)
+ .build();
+
+ StorageDescriptor output =
+ deltaGlueCatalogTableBuilder
+ .getUpdateTableRequest(
+ TEST_UPDATED_DELTA_INTERNAL_TABLE, glueTable,
TEST_CATALOG_TABLE_IDENTIFIER)
+ .storageDescriptor();
+
+ assertEquals(PARQUET_INPUT_FORMAT, output.inputFormat());
+ assertEquals(PARQUET_OUTPUT_FORMAT, output.outputFormat());
+ assertEquals(PARQUET_SERDE_CLASS,
output.serdeInfo().serializationLibrary());
+ assertEquals(
+
deltaGlueCatalogTableBuilder.getSerDeParameters(TEST_UPDATED_DELTA_INTERNAL_TABLE),
+ output.serdeInfo().parameters());
+ assertEquals(UPDATED_DELTA_GLUE_SCHEMA, output.columns());
+ }
+
private StorageDescriptor getTestStorageDescriptor(List<Column> columns) {
return StorageDescriptor.builder()
.columns(columns)
.location(TEST_BASE_PATH)
+ .inputFormat(PARQUET_INPUT_FORMAT)
+ .outputFormat(PARQUET_OUTPUT_FORMAT)
.serdeInfo(
SerDeInfo.builder()
+ .serializationLibrary(PARQUET_SERDE_CLASS)
.parameters(
deltaGlueCatalogTableBuilder.getSerDeParameters(TEST_DELTA_INTERNAL_TABLE))
.build())
