fresh-borzoni commented on code in PR #3447:
URL: https://github.com/apache/fluss/pull/3447#discussion_r3446992723
##########
fluss-client/src/main/java/org/apache/fluss/client/converter/FlussTypeToPojoTypeConverter.java:
##########
@@ -74,6 +75,17 @@ static Object convertTextValue(
ConverterCommons.charLengthExceptionMessage(fieldName,
v.length()));
}
return v.charAt(0);
+ } else if (pojoType.isEnum()) {
+ return Arrays.stream(pojoType.getEnumConstants())
Review Comment:
getEnumConstants() clones the array and scans on every row. Could build a
name -> constant map once at converter creation instead.
##########
fluss-client/src/main/java/org/apache/fluss/client/converter/FlussTypeToPojoTypeConverter.java:
##########
@@ -74,6 +75,17 @@ static Object convertTextValue(
ConverterCommons.charLengthExceptionMessage(fieldName,
v.length()));
}
return v.charAt(0);
+ } else if (pojoType.isEnum()) {
+ return Arrays.stream(pojoType.getEnumConstants())
+ .filter(e -> e.toString().equals(v.toUpperCase()))
Review Comment:
Works for uppercase enums, but breaks for any enum whose constants aren't
all-uppercase (write/read mismatch)
##########
fluss-client/src/main/java/org/apache/fluss/client/converter/FlussTypeToPojoTypeConverter.java:
##########
@@ -74,6 +75,17 @@ static Object convertTextValue(
ConverterCommons.charLengthExceptionMessage(fieldName,
v.length()));
}
return v.charAt(0);
+ } else if (pojoType.isEnum()) {
+ return Arrays.stream(pojoType.getEnumConstants())
+ .filter(e -> e.toString().equals(v.toUpperCase()))
+ .findAny()
+ .orElseThrow(
+ () ->
+ new IllegalArgumentException(
+ String.format(
+ "Could not parse value for
enum %s. Expected one of: [%s]",
Review Comment:
nit: Would it be double brackets here with Arrays.toString?
##########
fluss-client/src/test/java/org/apache/fluss/client/converter/FlussTypeToPojoTypeConverterTest.java:
##########
@@ -0,0 +1,250 @@
+/*
Review Comment:
All the enum tests use an uppercase enum, which is the case that works.
Could you add a lowercase enum with a full POJO→Row→POJO round-trip? And
ideally a List<MyEnum>, so collections hit the same path
--
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]
