prashantwason commented on code in PR #20354:
URL: https://github.com/apache/hudi/pull/20354#discussion_r4290210694
##########
hudi-sync/hudi-sync-common/src/main/java/org/apache/hudi/sync/common/util/SparkSchemaUtils.java:
##########
@@ -41,7 +41,8 @@ public static String convertToSparkSchemaJson(HoodieSchema
schema) {
* as {@code comment}, which spark displays as the
column comment
*/
public static String convertToSparkSchemaJson(HoodieSchema schema, boolean
includeFieldDocs) {
- String fieldsJsonString = schema.getFields().stream().map(field -> {
+ // Fields of the "null" type always hold null and are not written to the
data files, so they are skipped
+ String fieldsJsonString = schema.getFields().stream().filter(field ->
!field.schema().isSchemaNull()).map(field -> {
Review Comment:
Done in 37cc5b6096be: the chain is now split with `.filter(...)`,
`.map(...)` and `.reduce(...)` each on its own line.
##########
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/util/HiveSchemaUtil.java:
##########
@@ -285,6 +291,14 @@ private static String
createHiveStruct(List<HoodieSchemaField> fields, boolean s
return finalStr;
}
+ /**
Review Comment:
Done in 37cc5b6096be: removed `isNullTypeField` and inlined
`field.schema().isSchemaNull()`, which matches `SparkSchemaUtils`. The comment
moved to the first call site.
##########
hudi-sync/hudi-hive-sync/src/test/java/org/apache/hudi/hive/util/TestHiveSchemaUtil.java:
##########
@@ -198,4 +198,55 @@ void testSchemaTypeConversion() throws IOException {
"STRUCT< `title` : string, `variant_data` : STRUCT< `metadata` :
binary, `value` : binary>>");
assertEquals(expected, actual);
}
+
+ /**
+ * Fields of the Avro primitive type "null" (not a nullable union) are valid
Avro but carry no data.
+ * They are not written to parquet and have no Hive equivalent, so Hive sync
must skip them,
+ * at the top level as well as inside nested records.
+ */
+ @Test
+ void testNullTypeFieldsAreSkipped() throws IOException {
Review Comment:
Added `TestHiveSyncTool#testSyncWithNullTypeFields` in 04efffcc6965. It
creates a table with a top-level and a nested `"null"` type field and runs the
full sync for each sync mode. It checks that the Hive columns, the nested
struct type and `spark.sql.sources.schema` all leave those fields out. Without
the fix it fails in `syncProperties` with `Cannot convert NULL to spark sql
type`.
##########
hudi-sync/hudi-hive-sync/src/test/java/org/apache/hudi/hive/TestHiveSyncTool.java:
##########
@@ -1141,6 +1141,36 @@ public void testSyncWithSchema(String syncMode, String
enablePushDown) throws Ex
"The last commit that was synced should be updated in the
TBLPROPERTIES");
}
+ @ParameterizedTest
+ @MethodSource("syncMode")
+ public void testSyncWithNullTypeFields(String syncMode) throws Exception {
+ hiveSyncProps.setProperty(HIVE_SYNC_MODE.key(), syncMode);
+ String commitTime = "100";
+ HiveTestUtil.createCOWTableWithSchema(commitTime,
HoodieSchema.parse("{\"type\": \"record\", \"name\": \"User\", "
+ + "\"namespace\": \"example.avro\", \"fields\": ["
+ + "{\"name\": \"name\", \"type\": \"string\"},"
+ + "{\"name\": \"clientPromotion\", \"type\": \"null\", \"default\":
null},"
+ + "{\"name\": \"address\", \"type\": {\"type\": \"record\", \"name\":
\"Address\", \"fields\": ["
+ + "{\"name\": \"city\", \"type\": \"string\"},"
+ + "{\"name\": \"nestedPromotion\", \"type\": \"null\", \"default\":
null}]}}]}"));
+
+ reInitHiveSyncClient();
+ reSyncHiveTable();
+ assertEquals(commitTime,
hiveClient.getLastCommitTimeSynced(HiveTestUtil.TABLE_NAME).get(),
+ "The last commit that was synced should be updated in the
TBLPROPERTIES");
+ Map<String, String> hiveSchema =
hiveClient.getMetastoreSchema(HiveTestUtil.TABLE_NAME);
+ assertEquals("string", hiveSchema.get("name").toLowerCase(Locale.ROOT));
+ assertFalse(hiveSchema.containsKey("clientpromotion"), "null type field
should not be synced");
Review Comment:
Good point. The test now uses lowercase names (`clientpromotion`,
`nestedpromotion`) in the schema too, so the schema and the assertions agree
regardless of Hive lowercasing. Fixed in 3c2e0f18aa4d.
--
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]
