JingsongLi commented on code in PR #8399:
URL: https://github.com/apache/paimon/pull/8399#discussion_r3504902810
##########
paimon-common/src/main/java/org/apache/paimon/predicate/FieldRef.java:
##########
@@ -34,19 +37,31 @@ public class FieldRef implements Serializable {
private static final String FIELD_INDEX = "index";
private static final String FIELD_NAME = "name";
private static final String FIELD_TYPE = "type";
+ private static final String FIELD_NESTED_INDEXES = "nestedIndexes";
+ private static final String FIELD_NESTED_ARITIES = "nestedArities";
private final int index;
private final String name;
private final DataType type;
+ @Nullable private final int[] nestedIndexes;
+ @Nullable private final int[] nestedArities;
+
+ public FieldRef(int index, String name, DataType type) {
+ this(index, name, type, null, null);
+ }
@JsonCreator
public FieldRef(
@JsonProperty(FIELD_INDEX) int index,
@JsonProperty(FIELD_NAME) String name,
- @JsonProperty(FIELD_TYPE) DataType type) {
+ @JsonProperty(FIELD_TYPE) DataType type,
+ @JsonProperty(FIELD_NESTED_INDEXES) @Nullable int[] nestedIndexes,
+ @JsonProperty(FIELD_NESTED_ARITIES) @Nullable int[] nestedArities)
{
this.index = index;
this.name = name;
this.type = type;
+ this.nestedIndexes = nestedIndexes;
Review Comment:
Because `FieldRef` now carries `nestedIndexes` / `nestedArities`, every
place that remaps a `FieldRef` has to preserve them. Existing remappers such as
`PredicateProjectionConverter`, `PartitionValuePredicateVisitor`, and
`TableQueryAuthResult` still rebuild refs with the 3-arg constructor, so a
pushed predicate like `a.b = 1` loses its nested path after
projection/auth/partition remapping. The resulting `FieldTransform` then reads
top-level `a` as the leaf type instead of traversing to `b`, which can produce
wrong filtering or runtime failures. Please add a copy/remap helper that
preserves the nested metadata and use it for all `FieldRef` rewrites before
enabling nested predicates.
##########
paimon-spark/paimon-spark-common/src/main/java/org/apache/paimon/spark/SparkFilterConverter.java:
##########
@@ -107,53 +110,55 @@ public Predicate convert(Filter filter) {
return PredicateBuilder.alwaysFalse();
} else if (filter instanceof EqualTo) {
EqualTo eq = (EqualTo) filter;
- int index = fieldIndex(eq.attribute());
+ FieldInfo fieldInfo = resolveField(eq.attribute());
if (isNaN(eq.value())) {
- return builder.isNaN(index);
+ return builder.isNaN(fieldInfo.transform());
}
- Object literal = convertLiteral(index, eq.value());
- return builder.equal(index, literal);
+ Object literal = convertLiteral(fieldInfo.type(), eq.value());
+ return builder.equal(fieldInfo.transform(), literal);
} else if (filter instanceof EqualNullSafe) {
EqualNullSafe eq = (EqualNullSafe) filter;
- int index = fieldIndex(eq.attribute());
+ FieldInfo fieldInfo = resolveField(eq.attribute());
if (eq.value() == null) {
- return builder.isNull(index);
+ return builder.isNull(fieldInfo.transform());
} else {
- Object literal = convertLiteral(index, eq.value());
- return builder.equal(index, literal);
+ Object literal = convertLiteral(fieldInfo.type(), eq.value());
+ return builder.equal(fieldInfo.transform(), literal);
}
} else if (filter instanceof GreaterThan) {
GreaterThan gt = (GreaterThan) filter;
- int index = fieldIndex(gt.attribute());
- Object literal = convertLiteral(index, gt.value());
- return builder.greaterThan(index, literal);
+ FieldInfo fieldInfo = resolveField(gt.attribute());
+ Object literal = convertLiteral(fieldInfo.type(), gt.value());
+ return builder.greaterThan(fieldInfo.transform(), literal);
} else if (filter instanceof GreaterThanOrEqual) {
GreaterThanOrEqual gt = (GreaterThanOrEqual) filter;
- int index = fieldIndex(gt.attribute());
- Object literal = convertLiteral(index, gt.value());
- return builder.greaterOrEqual(index, literal);
+ FieldInfo fieldInfo = resolveField(gt.attribute());
+ Object literal = convertLiteral(fieldInfo.type(), gt.value());
+ return builder.greaterOrEqual(fieldInfo.transform(), literal);
} else if (filter instanceof LessThan) {
LessThan lt = (LessThan) filter;
- int index = fieldIndex(lt.attribute());
- Object literal = convertLiteral(index, lt.value());
- return builder.lessThan(index, literal);
+ FieldInfo fieldInfo = resolveField(lt.attribute());
+ Object literal = convertLiteral(fieldInfo.type(), lt.value());
+ return builder.lessThan(fieldInfo.transform(), literal);
} else if (filter instanceof LessThanOrEqual) {
LessThanOrEqual lt = (LessThanOrEqual) filter;
- int index = fieldIndex(lt.attribute());
- Object literal = convertLiteral(index, lt.value());
- return builder.lessOrEqual(index, literal);
+ FieldInfo fieldInfo = resolveField(lt.attribute());
+ Object literal = convertLiteral(fieldInfo.type(), lt.value());
+ return builder.lessOrEqual(fieldInfo.transform(), literal);
} else if (filter instanceof In) {
In in = (In) filter;
- int index = fieldIndex(in.attribute());
+ FieldInfo fieldInfo = resolveField(in.attribute());
return builder.in(
Review Comment:
This changes empty `IN` behavior even for top-level columns.
`PredicateBuilder.in(int, ...)` explicitly handles an empty literal list by
creating an `In` leaf predicate, but `PredicateBuilder.in(Transform, ...)`
falls through to `or(empty)` and throws. As a result, an empty Spark `In`
filter is no longer converted consistently. Please make the transform overload
handle empty lists the same way as the index overload.
##########
paimon-spark/paimon-spark-common/src/main/java/org/apache/paimon/spark/SparkFilterConverter.java:
##########
@@ -198,26 +203,90 @@ private static boolean isNaN(Object value) {
}
public Object convertLiteral(String field, Object value) {
- return convertLiteral(fieldIndex(field), value);
+ FieldInfo fieldInfo = resolveField(field);
+ return convertLiteral(fieldInfo.type(), value);
}
public String convertString(String field, Object value) {
Object literal = convertLiteral(field, value);
return literal == null ? null : literal.toString();
}
- private int fieldIndex(String field) {
- int index = rowType.getFieldIndex(field);
- // TODO: support nested field
- if (index == -1) {
+ private static class FieldInfo {
+ private final Transform transform;
+ private final DataType type;
+
+ public FieldInfo(Transform transform, DataType type) {
+ this.transform = transform;
+ this.type = type;
+ }
+
+ public Transform transform() {
+ return transform;
+ }
+
+ public DataType type() {
+ return type;
+ }
+ }
+
+ private FieldInfo resolveField(String field) {
+ String[] parts = field.split("\\.");
Review Comment:
This unconditionally treats any dot as a nested path. Paimon schema
validation does not forbid top-level column names containing `.`, and the
previous converter resolved those fields with `rowType.getFieldIndex(field)`.
With a top-level column named `a.b`, this now looks for top-level `a` and
either fails or resolves a different field. Please try an exact top-level field
lookup first, and only split as a nested path when no exact field exists.
--
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]
