rangareddy commented on code in PR #19488:
URL: https://github.com/apache/hudi/pull/19488#discussion_r4011887401
##########
hudi-aws/src/main/java/org/apache/hudi/aws/sync/AWSGlueCatalogSyncClient.java:
##########
@@ -682,12 +729,21 @@ public void createTable(String tableName,
try {
Map<String, String> mapSchema = hoodieSchemaToMapSchema(storageSchema,
config.getBoolean(HIVE_SUPPORT_TIMESTAMP_TYPE), false);
- List<Column> schemaWithoutPartitionKeys =
getColumnsFromSchema(mapSchema);
+ // Populate comments at create time rather than leaving them to the next
updateTableComments pass:
+ // HiveSyncTool.syncHoodieTable runs syncFirstTime without syncSchema,
so a table created with
+ // empty comments would only pick them up on the second sync. Mirrors
HiveSchemaUtil.generateCreateDDL
+ // on the HMS side, which gates the same lookup on HIVE_SYNC_COMMENT
(#19289).
+ Map<String, String> fieldDocs = config.getBoolean(HIVE_SYNC_COMMENT)
Review Comment:
Done - pulled the gate into a `getFieldDocsIfEnabled(HoodieSchema)` helper,
so `createTable` and `updateTableSchema` both go through it.
##########
hudi-aws/src/main/java/org/apache/hudi/aws/sync/AWSGlueCatalogSyncClient.java:
##########
@@ -506,19 +541,25 @@ public List<FieldSchema> getStorageFieldSchemas() {
public boolean updateTableComments(String tableName, List<FieldSchema>
fromMetastore, List<FieldSchema> fromStorage) {
Table table = getTable(awsGlue, databaseName, tableName);
- Map<String, Option<String>> commentsMap =
fromStorage.stream().collect(Collectors.toMap(FieldSchema::getName,
FieldSchema::getComment));
+ Map<String, Option<String>> commentsMap = fromStorage.stream()
+ .collect(Collectors.toMap(f -> f.getName().toLowerCase(Locale.ROOT),
FieldSchema::getComment, (existing, duplicate) -> existing));
StorageDescriptor storageDescriptor = table.storageDescriptor();
- List<Column> columns = storageDescriptor.columns();
- setComments(columns, commentsMap);
-
- List<Column> partitionKeys = table.partitionKeys();
- setComments(partitionKeys, commentsMap);
+ List<Column> partitionKeys = withComments(table.partitionKeys(),
commentsMap);
Review Comment:
Done - renamed to `updatedPartitionKeys`.
##########
hudi-aws/src/main/java/org/apache/hudi/aws/sync/AWSGlueCatalogSyncClient.java:
##########
@@ -474,11 +478,42 @@ public boolean updateTableProperties(String tableName,
Map<String, String> table
}
}
- private void setComments(List<Column> columns, Map<String, Option<String>>
commentsMap) {
- columns.forEach(column -> {
- String comment = commentsMap.getOrDefault(column.name(),
Option.empty()).orElse(null);
- Column.builder().comment(comment).build();
- });
+ /**
+ * Returns {@code columns} with the comment of every column the storage
schema knows about replaced by the
+ * one the schema carries, clearing it when the schema has none.
+ *
+ * <p>Columns the schema says nothing about are left untouched rather than
cleared. The pre-SDK-v2 code
+ * cleared them, but only nominally: it built a {@code Column} and discarded
it, so no comment was ever
+ * applied and nothing can depend on that behaviour. Clearing is also the
more dangerous reading - the
+ * storage field names keep the Avro schema's case while a catalog may hold
them lowercased, and a name
+ * that fails to match would silently wipe a comment. This matches
+ * {@code HMSDDLExecutor.applyFieldComments} on the Hive side, which only
touches known columns.
+ *
+ * <p>SDK v2 model classes are immutable and their getters return
unmodifiable lists, so the columns cannot
+ * be edited in place; a new list of rebuilt columns is returned instead.
+ */
+ @VisibleForTesting
+ static List<Column> withComments(List<Column> columns, Map<String,
Option<String>> commentsMap) {
Review Comment:
Done - trimmed the javadoc to the current contract (kept only the
case-matching rationale, since that is why unknown columns are left alone) and
dropped the #19488 reference from the test javadoc. The pre-SDK-v2 history is
in the PR description.
--
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]
