hudi-agent commented on code in PR #19289:
URL: https://github.com/apache/hudi/pull/19289#discussion_r3581138444
##########
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/ddl/HMSDDLExecutor.java:
##########
@@ -282,6 +288,24 @@ public void updateTableComments(String tableName,
Map<String, Pair<String, Strin
}
}
+ private static void setFieldComments(List<FieldSchema> fields, Map<String,
Pair<String, String>> alterSchema) {
+ for (FieldSchema fieldSchema : fields) {
+ if (alterSchema.containsKey(fieldSchema.getName())) {
+ String comment = alterSchema.get(fieldSchema.getName()).getRight();
+ fieldSchema.setComment(comment);
+ }
+ }
+ }
+
+ private static void setFieldDocsAsComments(List<FieldSchema> fields,
Map<String, String> fieldDocs) {
Review Comment:
🤖 nit: the name `setFieldDocsAsComments` is asymmetric with
`setFieldComments` directly above it — both methods do the same thing (iterate
fields and call `setComment`), the only difference is the input map type. The
"DocsAsComments" phrasing implies a bigger conceptual distinction than exists.
Something like `applyFieldDocs` would parallel `setFieldComments` more cleanly.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/util/HiveSchemaUtil.java:
##########
@@ -336,30 +338,57 @@ public static String generateSchemaString(HoodieSchema
storageSchema, List<Strin
}
public static String generateSchemaString(HoodieSchema storageSchema,
List<String> colsToSkip, boolean supportTimestamp) throws IOException {
+ return generateSchemaString(storageSchema, colsToSkip, supportTimestamp,
Collections.emptyMap());
+ }
+
+ public static String generateSchemaString(HoodieSchema storageSchema,
List<String> colsToSkip, boolean supportTimestamp,
+ Map<String, String> fieldDocs)
throws IOException {
Map<String, String> hiveSchema = convertSchemaToHiveSchema(storageSchema,
supportTimestamp);
StringBuilder columns = new StringBuilder();
for (Map.Entry<String, String> hiveSchemaEntry : hiveSchema.entrySet()) {
- if
(!colsToSkip.contains(removeSurroundingTick(hiveSchemaEntry.getKey()))) {
+ String fieldName = removeSurroundingTick(hiveSchemaEntry.getKey());
+ if (!colsToSkip.contains(fieldName)) {
columns.append(hiveSchemaEntry.getKey()).append(" ");
- columns.append(hiveSchemaEntry.getValue()).append(", ");
+ columns.append(hiveSchemaEntry.getValue());
+ String doc = fieldDocs.get(fieldName.toLowerCase(Locale.ROOT));
+ if (doc != null) {
+ columns.append(" COMMENT '").append(doc.replace("'",
"")).append("'");
Review Comment:
🤖 nit: `doc.replace("'", "")` silently strips apostrophes from field
comments with no explanation, and the same pattern is repeated verbatim in
`generateCreateDDL`. A brief inline comment (e.g., `// strip single quotes to
avoid breaking the SQL string literal`) — or a tiny private helper like
`escapeSqlComment(String doc)` shared by both call sites — would make the
intent clear and avoid the duplication.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/util/HiveSchemaUtil.java:
##########
@@ -336,30 +338,57 @@ public static String generateSchemaString(HoodieSchema
storageSchema, List<Strin
}
public static String generateSchemaString(HoodieSchema storageSchema,
List<String> colsToSkip, boolean supportTimestamp) throws IOException {
+ return generateSchemaString(storageSchema, colsToSkip, supportTimestamp,
Collections.emptyMap());
+ }
+
+ public static String generateSchemaString(HoodieSchema storageSchema,
List<String> colsToSkip, boolean supportTimestamp,
+ Map<String, String> fieldDocs)
throws IOException {
Map<String, String> hiveSchema = convertSchemaToHiveSchema(storageSchema,
supportTimestamp);
StringBuilder columns = new StringBuilder();
for (Map.Entry<String, String> hiveSchemaEntry : hiveSchema.entrySet()) {
- if
(!colsToSkip.contains(removeSurroundingTick(hiveSchemaEntry.getKey()))) {
+ String fieldName = removeSurroundingTick(hiveSchemaEntry.getKey());
+ if (!colsToSkip.contains(fieldName)) {
columns.append(hiveSchemaEntry.getKey()).append(" ");
- columns.append(hiveSchemaEntry.getValue()).append(", ");
+ columns.append(hiveSchemaEntry.getValue());
+ String doc = fieldDocs.get(fieldName.toLowerCase(Locale.ROOT));
+ if (doc != null) {
+ columns.append(" COMMENT '").append(doc.replace("'",
"")).append("'");
+ }
+ columns.append(", ");
}
}
// Remove the last ", "
columns.delete(columns.length() - 2, columns.length());
return columns.toString();
}
+ /**
+ * Returns the doc of each field of the schema keyed by lower-cased field
name, fields without doc are skipped.
+ */
+ public static Map<String, String> getFieldDocs(HoodieSchema schema) {
+ return schema.getFields().stream()
+ .filter(field -> field.doc().map(doc -> !doc.isEmpty()).orElse(false))
+ .collect(Collectors.toMap(field ->
field.name().toLowerCase(Locale.ROOT), field -> field.doc().get(), (first,
second) -> first));
Review Comment:
🤖 nit: the `(first, second) -> first` merge function in `toMap` quietly
handles duplicate field names, but neither this line nor the Javadoc explains
when duplicates can occur. Renaming the lambda params to `(existing, duplicate)
-> existing` would make the intent self-documenting without adding a comment.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
