voonhous commented on code in PR #17599:
URL: https://github.com/apache/hudi/pull/17599#discussion_r2643583351
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/index/HoodieIndexUtils.java:
##########
@@ -150,50 +149,38 @@ static boolean
validateDataTypeForSecondaryIndex(List<String> sourceFields, Sche
* @param tableSchema table schema
* @return true if each field's data types are supported, false otherwise
*/
- public static boolean
validateDataTypeForSecondaryOrExpressionIndex(List<String> sourceFields, Schema
tableSchema) {
+ public static boolean
validateDataTypeForSecondaryOrExpressionIndex(List<String> sourceFields,
HoodieSchema tableSchema) {
return sourceFields.stream().anyMatch(fieldToIndex -> {
- Schema schema = getNestedFieldSchemaFromWriteSchema(tableSchema,
fieldToIndex);
- return schema.getType() != Schema.Type.RECORD && schema.getType() !=
Schema.Type.ARRAY && schema.getType() != Schema.Type.MAP;
+ Option<Pair<String, HoodieSchemaField>> nestedFieldOpt =
HoodieSchemaUtils.getNestedField(tableSchema, fieldToIndex);
+ HoodieSchema fieldSchema = nestedFieldOpt.get().getRight().schema();
+ return fieldSchema.getType() != HoodieSchemaType.RECORD &&
fieldSchema.getType() != HoodieSchemaType.ARRAY && fieldSchema.getType() !=
HoodieSchemaType.MAP;
});
}
/**
* Check if the given schema type is supported for secondary index.
* Supported types are: String (including CHAR), Integer types (Int, BigInt,
Long, Short), and timestamp
*/
- private static boolean isSecondaryIndexSupportedType(Schema schema) {
+ private static boolean isSecondaryIndexSupportedType(HoodieSchema schema) {
// Handle union types (nullable fields)
- if (schema.getType() == Schema.Type.UNION) {
+ if (schema.getType() == HoodieSchemaType.UNION) {
// For union types, check if any of the types is supported
return schema.getTypes().stream()
- .anyMatch(s -> s.getType() != Schema.Type.NULL &&
isSecondaryIndexSupportedType(s));
+ .anyMatch(s -> s.getType() != HoodieSchemaType.NULL &&
isSecondaryIndexSupportedType(s));
}
// Check basic types
switch (schema.getType()) {
case STRING:
- // STRING type can have UUID logical type which we don't support
- return schema.getLogicalType() == null; // UUID and other string-based
logical types are not supported
- // Regular STRING (includes CHAR)
case INT:
- // INT type can represent regular integers or dates/times with logical
types
- if (schema.getLogicalType() != null) {
- // Support date and time-millis logical types
- return schema.getLogicalType() == LogicalTypes.date()
- || schema.getLogicalType() == LogicalTypes.timeMillis();
- }
- return true; // Regular INT
case LONG:
- // LONG type can represent regular longs or timestamps with logical
types
- if (schema.getLogicalType() != null) {
- // Support timestamp logical types
- return schema.getLogicalType() == LogicalTypes.timestampMillis()
- || schema.getLogicalType() == LogicalTypes.timestampMicros()
- || schema.getLogicalType() == LogicalTypes.timeMicros();
- }
- return true; // Regular LONG
case DOUBLE:
- return true; // Support DOUBLE type
+ case DATE:
+ case TIME:
+ return true;
+ case TIMESTAMP:
+ // LOCAL timestamps are not supported
Review Comment:
Yeap, we can. I'm just transferring the test expectations to actual code
since i don't recall seeing it documented anywhere other than tests here. (Have
tagged u separately for this)
--
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]
