zhuxiangyi commented on code in PR #9423:
URL: https://github.com/apache/paimon/pull/9423#discussion_r4056327736
##########
paimon-format/src/main/java/org/apache/parquet/filter2/predicate/ParquetFilters.java:
##########
@@ -534,35 +573,100 @@ private static PrimitiveType timestampPrimitiveType(
|| timestampType.isAdjustedToUTC() != expectedAdjustedToUtc) {
throw new UnsupportedOperationException();
}
- return primitiveType;
+ return column;
}
- private static PrimitiveType primitiveType(
+ /**
+ * The column the file holds for {@code fieldRef}, with the file's own
spelling of the path.
+ * Callers that build a parquet column must use {@link FileColumn#path}: a
{@link PrimitiveType}
+ * only knows its own leaf name, so rebuilding the column from it drops
the enclosing path and
+ * addresses a column the file does not have.
+ */
+ private static FileColumn fileColumn(
FieldRef fieldRef, MessageType fileSchema, boolean caseSensitive) {
- PrimitiveType matched = findPrimitiveType(fieldRef, fileSchema,
caseSensitive);
+ FileColumn matched = findFileColumn(fieldRef, fileSchema,
caseSensitive);
if (matched == null) {
throw new UnsupportedOperationException();
}
return matched;
}
+ /** A column the file actually holds: its own spelling of the path, and
its physical type. */
+ private static class FileColumn {
+
+ private final String path;
+ private final PrimitiveType type;
+
+ private FileColumn(String path, PrimitiveType type) {
+ this.path = path;
+ this.type = type;
+ }
+ }
+
/**
* The file's column for {@code fieldRef}, or null when the file has no
such column. A column
* that exists but is not primitive cannot carry a predicate at all, so it
is rejected outright.
+ *
+ * <p>{@code fieldRef} names a nested field with dots ({@code addr.city}),
which is resolved by
+ * descending the file's groups. A top-level column matching the whole
name wins over that walk,
+ * keeping flat columns spelled with dots resolving as they always did.
parquet-mr identifies
+ * columns by dot-joined path too, so it cannot tell the two apart either
way.
*/
@Nullable
- private static PrimitiveType findPrimitiveType(
+ private static FileColumn findFileColumn(
FieldRef fieldRef, MessageType fileSchema, boolean caseSensitive) {
- // Paimon predicates currently reference top-level fields only. Nested
field
- // predicates are rejected before reaching the format reader.
- for (Type field : fileSchema.getFields()) {
+ Type matched = findChild(fileSchema, fieldRef.name(), caseSensitive);
+ if (matched != null) {
+ return toFileColumn(matched.getName(), matched);
+ }
+
+ String[] parts = fieldRef.name().split("\\.");
Review Comment:
Confirmed, and reproduced exactly as described: a declared top-level `s.a`
(BIGINT) absent from a file that only holds nested `s -> a` (INT32), `s.a IS
NULL` pruned both rows instead of keeping them.
`findFileColumn`'s walk now only descends groups for a `FieldRef`
synthesized from `NestedFieldTransform` (its index left at `UNUSED_INDEX`); an
ordinary field's dotted name with no exact match is treated as a genuinely
missing column. My first attempt stopped there, but that alone wasn't enough -
whatever name ends up handed to `FilterApi` gets re-split by parquet-mr the
same way regardless of how we resolved it on our side, so a
missing-but-walkable name was still silently binding to the unrelated column
(this time as a type mismatch crash rather than the earlier read). The walk
stays in place as a collision check: for an ordinary field, if it would still
find something down there, the pushdown is refused outright instead of risking
it; only when the walk also finds nothing is the column safe to leave to the
existing missing-column fallback.
Tests:
-
`ParquetFormatReadWriteTest.testMissingDottedTopLevelFieldIsTreatedAsNullNotAsANestedPath`
- reproduces the false pruning before the fix.
-
`ParquetFormatReadWriteTest.testMissingDottedTopLevelFieldWithNoCollisionIsStillPushedDown`
- companion control: a dotted, missing column with nothing to collide with
must still be pushed down, not rejected outright.
##########
paimon-common/src/main/java/org/apache/paimon/predicate/PredicateBuilder.java:
##########
@@ -286,6 +286,10 @@ public Predicate notIn(int idx, List<Object> literals) {
return in(idx, literals).negate().get();
}
+ public Predicate notIn(Transform transform, List<Object> literals) {
+ return in(transform, literals).negate().get();
Review Comment:
Confirmed. `in(int, List)` special-cases an empty list; `in(Transform,
List)` never did, so it fell through to `or(equals)` with an empty `equals`,
which throws - and `notIn` inherits that through `negate()`.
`in(Transform, List)` now takes the same branch on an empty list that
`in(int, List)` does, so both `in` and `notIn` on the `Transform` overload
match their `idx` counterparts.
Test:
`PredicateBuilderTest.testInAndNotInTransformWithEmptyLiteralsMatchTheIdxOverloads`.
--
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]
