voonhous commented on code in PR #19289:
URL: https://github.com/apache/hudi/pull/19289#discussion_r3594583946
##########
hudi-sync/hudi-hive-sync/src/test/java/org/apache/hudi/hive/TestSparkSchemaUtils.java:
##########
@@ -120,6 +120,22 @@ public void testConvertBasicTypes() {
assertEquals(sparkSchema.json(), convertedSparkSchema.json());
}
+ @Test
+ public void testConvertWithFieldDocs() {
+ HoodieSchema schema = HoodieSchema.createRecord("root", null, null, false,
Arrays.asList(
+ HoodieSchemaField.of("id",
HoodieSchema.create(HoodieSchemaType.STRING), "Unique \"id\"\nof the record",
null),
Review Comment:
These are all flat fields, so the `includeFieldDocs` threading through the
array/map/record recursion never runs in this test. Could you add a nested
record with a doc on a sub-field and assert the nested `getComment()`? That
recursion is the easiest part of the change to break.
##########
hudi-sync/hudi-hive-sync/src/test/java/org/apache/hudi/hive/TestHiveSyncTool.java:
##########
@@ -1032,6 +1032,67 @@ public void testSyncWithCommentedSchema(String syncMode)
throws Exception {
assertEquals(2, commentCnt, "hive schema field comment numbers should
match the avro schema field doc numbers");
}
+ @ParameterizedTest
+ @MethodSource("syncMode")
+ public void testSyncCommentsForPartitionColumns(String syncMode) throws
Exception {
+ hiveSyncProps.setProperty(HIVE_SYNC_MODE.key(), syncMode);
+ hiveSyncProps.setProperty(HIVE_SYNC_COMMENT.key(), "true");
+ String commitTime = "100";
+ HiveTestUtil.createCOWTableWithSchema(commitTime,
"/partition-doced-test.avsc");
Review Comment:
The fixture has a single partition column, so the per-key partition loops
(and the jdbc/hiveql skip) only ever see one key. A second doced partition
field would cover ordering and the multi-key skip path.
##########
hudi-sync/hudi-hive-sync/src/test/java/org/apache/hudi/hive/TestHiveSyncTool.java:
##########
@@ -1032,6 +1032,67 @@ public void testSyncWithCommentedSchema(String syncMode)
throws Exception {
assertEquals(2, commentCnt, "hive schema field comment numbers should
match the avro schema field doc numbers");
}
+ @ParameterizedTest
+ @MethodSource("syncMode")
+ public void testSyncCommentsForPartitionColumns(String syncMode) throws
Exception {
+ hiveSyncProps.setProperty(HIVE_SYNC_MODE.key(), syncMode);
+ hiveSyncProps.setProperty(HIVE_SYNC_COMMENT.key(), "true");
+ String commitTime = "100";
+ HiveTestUtil.createCOWTableWithSchema(commitTime,
"/partition-doced-test.avsc");
+
+ reInitHiveSyncClient();
+ reSyncHiveTable();
+
+ Map<String, String> commentsByField =
hiveClient.getMetastoreFieldSchemas(HiveTestUtil.TABLE_NAME)
Review Comment:
nit: this `getMetastoreFieldSchemas(...).collect(toMap(getName,
getCommentOrEmpty))` block is copy-pasted in both new tests (again at 1082).
Worth a small helper like `metastoreFieldComments(table)`.
--
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]
