fresh-borzoni commented on code in PR #3444:
URL: https://github.com/apache/fluss/pull/3444#discussion_r3446920575
##########
fluss-client/src/main/java/org/apache/fluss/client/converter/PojoType.java:
##########
@@ -73,39 +75,53 @@ static <T> PojoType<T> of(Class<T> pojoClass) {
Map<String, Property> props = new LinkedHashMap<>();
for (Map.Entry<String, Field> e : allFields.entrySet()) {
- String name = e.getKey();
+ String fieldName = e.getKey();
Field field = e.getValue();
// Enforce nullable fields: primitives are not allowed in POJO
definitions.
if (field.getType().isPrimitive()) {
throw new IllegalArgumentException(
String.format(
"POJO class %s has primitive field '%s' of
type %s. Primitive types are not allowed; all fields must be nullable (use
wrapper types).",
- pojoClass.getName(), name,
field.getType().getName()));
+ pojoClass.getName(), fieldName,
field.getType().getName()));
}
+ // Check for @ColumnName annotation to determine the mapped column
name
+ ColumnName columnNameAnnotation =
field.getAnnotation(ColumnName.class);
+ String mappedColumnName =
+ columnNameAnnotation != null ?
columnNameAnnotation.value() : fieldName;
+ checkArgument(
+ !mappedColumnName.isEmpty(),
+ "Column name cannot be empty for field '%s' in POJO class
%s",
+ fieldName,
+ pojoClass.getName());
+
// use boxed type as effective type
Class<?> effectiveType = boxIfPrimitive(field.getType());
boolean publicField = Modifier.isPublic(field.getModifiers());
- Method getter = getters.get(name);
- Method setter = setters.get(name);
+ Method getter = getters.get(fieldName);
+ Method setter = setters.get(fieldName);
if (!publicField) {
// When not a public field, require both getter and setter
if (getter == null || setter == null) {
- final String capitalizedName = capitalize(name);
+ final String capitalizedName = capitalize(fieldName);
throw new IllegalArgumentException(
String.format(
"POJO class %s field '%s' must be public
or have both getter and setter (get%s/set%s).",
- pojoClass.getName(), name,
capitalizedName, capitalizedName));
+ pojoClass.getName(),
+ fieldName,
+ capitalizedName,
+ capitalizedName));
}
}
props.put(
Review Comment:
If two fields resolve to the same column (e.g. @ColumnName("name") on one
field plus another field literally called name, or two fields annotated the
same), this silently overwrites and one field just disappears and no error.
Before, field names were unique by construction, so we lost that guarantee
here. Could we fail fast instead?
##########
website/docs/apis/java-client.md:
##########
@@ -336,20 +336,23 @@ The Typed API provides a more user-friendly experience
but comes with a performa
### Defining POJOs
-To use the Typed API, define a Java class where the field names and types
match your Fluss table schema.
+To use the Typed API, define a Java class where the field names and types
match your Fluss table schema. You can use
+@ColumnName to properly map the name.
```java
public class User {
public Integer id;
- public String name;
- public Integer age;
+ @ColumnName("first_name")
+ public String firstName;
+ @ColumnName("last_name")
+ public String lastName;
public User() {}
- public User(Integer id, String name, Integer age) {
+ public User(Integer id, String firstName, Sting lastName) {
Review Comment:
typo: Sting
##########
fluss-client/src/test/java/org/apache/fluss/client/converter/PojoTypeTest.java:
##########
@@ -61,6 +62,27 @@ void test() {
PojoType.of(PublicWithPublicNonPrimitive.class);
}
+ @Test
+ void testColumnNameAnnotation() {
Review Comment:
This only checks the property mapping in PojoType. The actual point of the
PR is that the converters honor @ColumnName.
Could we add a round-trip test through PojoToRowConverter +
RowToPojoConverter against a schema with the snake_case column names? A
negative test for the duplicate case above would be good too once it throws.
--
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]
