danny0405 commented on code in PR #19762:
URL: https://github.com/apache/hudi/pull/19762#discussion_r3868393932
##########
hudi-aws/src/test/java/org/apache/hudi/aws/sync/TestAWSGlueSyncClient.java:
##########
@@ -909,4 +912,81 @@ private CompletableFuture<GetTableResponse>
getTableWithDefaultProps(String tabl
.build();
return CompletableFuture.completedFuture(response);
}
+
+ @ParameterizedTest
+ @ValueSource(strings = {"2024-01-15", "datestr=2024-01-15", "2024/01/15"})
+ void
testUpdateTableSchema_cascadePreservesGlueRecordedPartitionLocation(String
partitionDir) {
+ String tableName = GlueTestUtil.TABLE_NAME;
+ String basePath =
GlueTestUtil.getHiveSyncConfig().getString(META_SYNC_BASE_PATH);
+ String partitionLocation = new StoragePath(basePath,
partitionDir).toString();
+
+ Table table = Table.builder()
+ .name(tableName)
+ .databaseName(GlueTestUtil.DB_NAME)
+ .storageDescriptor(StorageDescriptor.builder()
+ .location(basePath)
+ .columns(Column.builder().name("name").type("string").build())
+ .build())
+ .partitionKeys(Column.builder().name("datestr").type("string").build())
+ .build();
+ when(mockAwsGlue.getTable(any(GetTableRequest.class)))
+
.thenReturn(CompletableFuture.completedFuture(GetTableResponse.builder().table(table).build()));
+ when(mockAwsGlue.updateTable(any(UpdateTableRequest.class)))
+
.thenReturn(CompletableFuture.completedFuture(UpdateTableResponse.builder().build()));
+
+ software.amazon.awssdk.services.glue.model.Partition gluePartition =
+ software.amazon.awssdk.services.glue.model.Partition.builder()
+ .values("2024-01-15")
+
.storageDescriptor(StorageDescriptor.builder().location(partitionLocation).build())
+ .build();
+ when(mockAwsGlue.getPartitions(any(GetPartitionsRequest.class)))
+ .thenReturn(CompletableFuture.completedFuture(
+
GetPartitionsResponse.builder().partitions(gluePartition).nextToken(null).build()));
+
+ ArgumentCaptor<BatchUpdatePartitionRequest> captor =
ArgumentCaptor.forClass(BatchUpdatePartitionRequest.class);
+ when(mockAwsGlue.batchUpdatePartition(captor.capture()))
+
.thenReturn(CompletableFuture.completedFuture(BatchUpdatePartitionResponse.builder().build()));
+
+ HoodieSchema schema = GlueTestUtil.getSimpleSchema();
+ SchemaDifference schemaDiff = SchemaDifference.newBuilder(schema, new
HashMap<>())
+ .updateTableColumn("name", "string")
+ .build();
+
+ awsGlueSyncClient.updateTableSchema(tableName, schema, schemaDiff);
+
+ List<BatchUpdatePartitionRequestEntry> entries =
captor.getValue().entries();
+ assertEquals(1, entries.size());
+ assertEquals(partitionLocation,
entries.get(0).partitionInput().storageDescriptor().location());
+ assertEquals(Collections.singletonList("2024-01-15"),
entries.get(0).partitionValueList());
+ assertEquals(table.storageDescriptor().columns(),
+ entries.get(0).partitionInput().storageDescriptor().columns());
Review Comment:
[P2] Please make this assertion verify the updated columns, not the
pre-update table schema. `getSimpleSchema()` contains both `id` and `name`, but
the `getTable` stub always returns `table`, whose descriptor contains only
`name`; consequently the cascade currently propagates that stale list and this
test explicitly accepts it. This misses the ordering invariant used to justify
keeping the first `UpdateTable` call. Could the first `getTable` return the
original table and the cascade re-read return a table built from the captured
update request (or otherwise containing the new columns), then assert those new
columns here? An `InOrder` check for `updateTable` before
`batchUpdatePartition` would cover the same dependency.
--
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]
