voonhous commented on code in PR #19809:
URL: https://github.com/apache/hudi/pull/19809#discussion_r3916404564
##########
hudi-common/src/main/java/org/apache/hudi/common/schema/HoodieSchemaUtils.java:
##########
@@ -769,47 +788,27 @@ public static String getRecordQualifiedName(String
tableName) {
}
public static boolean hasDecimalField(HoodieSchema schema) {
- return hasDecimalWithCondition(schema, unused -> true);
- }
-
- /**
- * Checks whether the provided schema contains a decimal with a precision
less than or equal to 18,
- * which allows the decimal to be stored as int/long instead of a fixed size
byte array in
- * <a
href="https://github.com/apache/parquet-format/blob/master/LogicalTypes.md";>parquet
logical types</a>
- * @param schema the input schema to search
- * @return true if the schema contains a small precision decimal field and
false otherwise
- */
- public static boolean hasSmallPrecisionDecimalField(HoodieSchema schema) {
- return hasDecimalWithCondition(schema,
HoodieSchemaUtils::isSmallPrecisionDecimalField);
- }
-
- private static boolean hasDecimalWithCondition(HoodieSchema schema,
Function<HoodieSchema.Decimal, Boolean> condition) {
switch (schema.getType()) {
case RECORD:
for (HoodieSchemaField field : schema.getFields()) {
- if (hasDecimalWithCondition(field.schema(), condition)) {
+ if (hasDecimalField(field.schema())) {
return true;
}
}
return false;
case ARRAY:
- return hasDecimalWithCondition(schema.getElementType(), condition);
+ return hasDecimalField(schema.getElementType());
case MAP:
- return hasDecimalWithCondition(schema.getValueType(), condition);
+ return hasDecimalField(schema.getValueType());
case UNION:
- return hasDecimalWithCondition(schema.getNonNullType(), condition);
+ return hasDecimalField(schema.getNonNullType());
Review Comment:
Filed as #19825 (with the `HoodieSchemaRepair.hasTimestampMillisField`
twin); fix goes in a separate PR, this one keeps the arm as on master.
##########
hudi-common/src/test/java/org/apache/hudi/common/schema/TestHoodieSchemaUtils.java:
##########
@@ -1047,6 +1043,11 @@ public void testGenerateProjectionSchema() {
assertTrue(fieldNames1.contains("_row_key"));
assertTrue(fieldNames1.contains("timestamp"));
+ // Field names are matched case-insensitively; HiveHoodieReaderContext
lowercases names before calling this.
+ HoodieSchema schema2 =
HoodieSchemaUtils.generateProjectionSchema(originalSchema,
Arrays.asList("_ROW_KEY"));
Review Comment:
Filed as #19826; fix in a separate PR. Correction to the claim above:
`generateProjectionSchema` has six callers, `HiveHoodieReaderContext` is the
one that pre-lowercases (with `Locale.ROOT`); the others lowercase both sides
through this call and stay consistent.
--
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]
