vinishjail97 commented on code in PR #940:
URL: https://github.com/apache/incubator-xtable/pull/940#discussion_r4018602561
##########
xtable-aws/src/test/java/org/apache/xtable/glue/table/TestDeltaGlueCatalogTableBuilder.java:
##########
@@ -92,12 +95,57 @@ void testGetUpdateTableInput() {
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("org.apache.hadoop.hive.ql.io.parquet.MapredParquetInputFormat")
+
.outputFormat("org.apache.hadoop.hive.ql.io.parquet.MapredParquetOutputFormat")
+ .serdeInfo(
+ SerDeInfo.builder()
+ .serializationLibrary(
+
"org.apache.hadoop.hive.ql.io.parquet.serde.ParquetHiveSerDe")
+ .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());
+ }
+
private StorageDescriptor getTestStorageDescriptor(List<Column> columns) {
return StorageDescriptor.builder()
.columns(columns)
.location(TEST_BASE_PATH)
+
.inputFormat("org.apache.hadoop.hive.ql.io.parquet.MapredParquetInputFormat")
Review Comment:
Can you add a refresh case with null input/output formats and a null
serialization library? Both refresh tests currently start with all three
Parquet values already set, so they would still pass if refresh stopped filling
these fields.
An input matching the old XTable descriptor would protect the repair
behavior for existing tables. A separate case with no SerDeInfo would also
cover the new fallback branch and will ensure 100% test coverage.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
