JingsongLi commented on code in PR #9423:
URL: https://github.com/apache/paimon/pull/9423#discussion_r3879048033
##########
paimon-format/src/main/java/org/apache/parquet/filter2/predicate/ParquetFilters.java:
##########
@@ -273,9 +278,19 @@ public FilterPredicate visitNotIn(FieldRef fieldRef,
List<Object> literals) {
throw new UnsupportedOperationException();
}
+ /**
+ * A nested field carries no index into the file, only a path, so it
is re-dispatched under
+ * a {@link FieldRef} naming that path. Every other transform - casts,
string functions -
+ * has no column of its own to filter on and is given up here.
+ */
@Override
public FilterPredicate visitNonFieldLeaf(LeafPredicate predicate) {
- throw new UnsupportedOperationException();
+ if (!(predicate.transform() instanceof NestedFieldTransform)) {
+ throw new UnsupportedOperationException();
+ }
+ NestedFieldTransform nested = (NestedFieldTransform)
predicate.transform();
+ FieldRef pathRef = new FieldRef(UNUSED_INDEX, nested.fieldName(),
nested.outputType());
Review Comment:
[P1] Preserve the full nested path for DECIMAL and TIMESTAMP predicates
This re-dispatches the nested transform as a FieldRef named
"payload.amount", but the decimal, timestamp, and local-zoned-timestamp
visitors later build FilterApi columns from primitiveType.getName(), which is
only "amount". For a predicate such as payload.amount = 12.34, parquet-mr
therefore receives a missing top-level column and its statistics filter can
drop every row group as all-null, producing an empty result. Please keep the
resolved FileColumn.path when validating these physical types and use that full
path to construct the predicate column; regression tests should cover nested
DECIMAL and both timestamp variants against actual row groups.
##########
paimon-common/src/main/java/org/apache/paimon/predicate/NestedFieldTransform.java:
##########
@@ -0,0 +1,182 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.paimon.predicate;
+
+import org.apache.paimon.data.InternalRow;
+import org.apache.paimon.types.DataType;
+import org.apache.paimon.types.RowType;
+
+import
org.apache.paimon.shade.jackson2.com.fasterxml.jackson.annotation.JsonCreator;
+import
org.apache.paimon.shade.jackson2.com.fasterxml.jackson.annotation.JsonIgnore;
+import
org.apache.paimon.shade.jackson2.com.fasterxml.jackson.annotation.JsonProperty;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+
+import static org.apache.paimon.utils.InternalRowUtils.get;
+import static org.apache.paimon.utils.Preconditions.checkArgument;
+
+/**
+ * Transform that extracts a field nested inside a row-typed column, for
example {@code addr.city}.
+ *
+ * <p>The transform keeps the enclosing top-level column as its only {@link
#inputs() input}, so
+ * anything that rewrites field indices (schema projection, for instance)
keeps working without
+ * knowing about nesting. The positions below that column are held separately
in {@link #path()}.
+ *
+ * <p>Deliberately <b>not</b> a {@link FieldTransform}: {@link
LeafPredicate#fieldRefOptional()}
+ * returns empty for it, which is what keeps every consumer that equates a
leaf with a top-level
+ * column — min/max pruning, file index lookup, ORC pushdown, schema evolution
— from silently
+ * reading the enclosing column's metadata as if it belonged to the nested
field. Those consumers
+ * give up on this transform instead, which costs pruning but never rows.
+ */
+public class NestedFieldTransform implements Transform {
+
+ private static final long serialVersionUID = 1L;
+
+ public static final String NAME = "NESTED_FIELD_REF";
+
+ public static final String FIELD_FIELD_REF = "fieldRef";
+ public static final String FIELD_PATH = "path";
+
+ /** The top-level row-typed column the nested field lives in. */
+ private final FieldRef fieldRef;
+
+ /** Positions to descend, relative to {@code fieldRef}'s row type. Never
empty. */
+ private final List<Integer> path;
+
+ private final String name;
+ private final DataType outputType;
+
+ @JsonCreator
+ public NestedFieldTransform(
+ @JsonProperty(FIELD_FIELD_REF) FieldRef fieldRef,
+ @JsonProperty(FIELD_PATH) List<Integer> path) {
+ checkArgument(path != null && !path.isEmpty(), "Nested field path must
not be empty.");
+ this.fieldRef = fieldRef;
+ this.path = Collections.unmodifiableList(new ArrayList<>(path));
+
+ StringBuilder nameBuilder = new StringBuilder(fieldRef.name());
+ DataType current = fieldRef.type();
+ for (int position : this.path) {
+ checkArgument(
+ current instanceof RowType,
+ "Nested field path of '%s' descends into a non-row type
%s.",
+ fieldRef.name(),
+ current);
+ RowType rowType = (RowType) current;
+ checkArgument(
+ position >= 0 && position < rowType.getFieldCount(),
+ "Nested field position %s is out of range for %s.",
+ position,
+ rowType);
+
nameBuilder.append('.').append(rowType.getFields().get(position).name());
+ current = rowType.getTypeAt(position);
+ }
+ this.name = nameBuilder.toString();
+ this.outputType = current;
+ }
+
+ @Override
+ public String name() {
+ return NAME;
+ }
+
+ @JsonProperty(FIELD_FIELD_REF)
+ public FieldRef fieldRef() {
+ return fieldRef;
+ }
+
+ @JsonProperty(FIELD_PATH)
+ public List<Integer> path() {
+ return path;
+ }
+
+ /** Dot-separated name from the top-level column down to the nested field,
{@code addr.city}. */
+ @JsonIgnore
+ public String fieldName() {
+ return name;
+ }
+
+ @Override
+ @JsonIgnore
+ public List<Object> inputs() {
+ return Collections.singletonList(fieldRef);
+ }
+
+ @Override
+ @JsonIgnore
+ public DataType outputType() {
+ return outputType;
+ }
+
+ /**
+ * Reads the nested field out of {@code row}, which must match the row
type {@link #fieldRef}
+ * was built against. A null anywhere along the path yields null, matching
SQL semantics for
+ * field access on a null struct.
+ */
+ @Override
+ public Object transform(InternalRow row) {
+ int position = fieldRef.index();
+ if (row.isNullAt(position)) {
+ return null;
+ }
+ RowType currentType = (RowType) fieldRef.type();
+ InternalRow current = row.getRow(position,
currentType.getFieldCount());
+
+ for (int i = 0; i < path.size() - 1; i++) {
+ position = path.get(i);
+ if (current.isNullAt(position)) {
+ return null;
+ }
+ RowType nextType = (RowType) currentType.getTypeAt(position);
+ current = current.getRow(position, nextType.getFieldCount());
+ currentType = nextType;
+ }
+
+ int leaf = path.get(path.size() - 1);
+ return get(current, leaf, currentType.getTypeAt(leaf));
+ }
+
+ @Override
+ public Transform copyWithNewInputs(List<Object> inputs) {
+ checkArgument(inputs.size() == 1);
Review Comment:
[P1] Re-resolve nested identity when inputs are remapped
This preserves an ordinal path even when the replacement FieldRef has a
different nested RowType. Nested transforms are now JSON-serializable and can
be used by REST row filters, so a policy on info.secret with path [0] against
ROW<secret, region> can be remapped against a Spark-pruned ROW<region> and
silently evaluate info.region instead. With same-typed fields this does not
fail closed and can admit unauthorized rows. Please persist stable nested names
or field IDs and re-resolve them during remapping, while ensuring auth reads
the full nested dependencies; alternatively, reject nested transforms in row
filters until their identity can be preserved.
##########
paimon-common/src/main/java/org/apache/paimon/predicate/NestedFieldTransform.java:
##########
@@ -0,0 +1,182 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.paimon.predicate;
+
+import org.apache.paimon.data.InternalRow;
+import org.apache.paimon.types.DataType;
+import org.apache.paimon.types.RowType;
+
+import
org.apache.paimon.shade.jackson2.com.fasterxml.jackson.annotation.JsonCreator;
+import
org.apache.paimon.shade.jackson2.com.fasterxml.jackson.annotation.JsonIgnore;
+import
org.apache.paimon.shade.jackson2.com.fasterxml.jackson.annotation.JsonProperty;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+
+import static org.apache.paimon.utils.InternalRowUtils.get;
+import static org.apache.paimon.utils.Preconditions.checkArgument;
+
+/**
+ * Transform that extracts a field nested inside a row-typed column, for
example {@code addr.city}.
+ *
+ * <p>The transform keeps the enclosing top-level column as its only {@link
#inputs() input}, so
+ * anything that rewrites field indices (schema projection, for instance)
keeps working without
+ * knowing about nesting. The positions below that column are held separately
in {@link #path()}.
+ *
+ * <p>Deliberately <b>not</b> a {@link FieldTransform}: {@link
LeafPredicate#fieldRefOptional()}
+ * returns empty for it, which is what keeps every consumer that equates a
leaf with a top-level
+ * column — min/max pruning, file index lookup, ORC pushdown, schema evolution
— from silently
+ * reading the enclosing column's metadata as if it belonged to the nested
field. Those consumers
+ * give up on this transform instead, which costs pruning but never rows.
+ */
+public class NestedFieldTransform implements Transform {
+
+ private static final long serialVersionUID = 1L;
+
+ public static final String NAME = "NESTED_FIELD_REF";
+
+ public static final String FIELD_FIELD_REF = "fieldRef";
+ public static final String FIELD_PATH = "path";
+
+ /** The top-level row-typed column the nested field lives in. */
+ private final FieldRef fieldRef;
+
+ /** Positions to descend, relative to {@code fieldRef}'s row type. Never
empty. */
+ private final List<Integer> path;
+
+ private final String name;
+ private final DataType outputType;
+
+ @JsonCreator
+ public NestedFieldTransform(
+ @JsonProperty(FIELD_FIELD_REF) FieldRef fieldRef,
+ @JsonProperty(FIELD_PATH) List<Integer> path) {
+ checkArgument(path != null && !path.isEmpty(), "Nested field path must
not be empty.");
+ this.fieldRef = fieldRef;
+ this.path = Collections.unmodifiableList(new ArrayList<>(path));
+
+ StringBuilder nameBuilder = new StringBuilder(fieldRef.name());
+ DataType current = fieldRef.type();
+ for (int position : this.path) {
+ checkArgument(
+ current instanceof RowType,
+ "Nested field path of '%s' descends into a non-row type
%s.",
+ fieldRef.name(),
+ current);
+ RowType rowType = (RowType) current;
+ checkArgument(
+ position >= 0 && position < rowType.getFieldCount(),
+ "Nested field position %s is out of range for %s.",
+ position,
+ rowType);
+
nameBuilder.append('.').append(rowType.getFields().get(position).name());
Review Comment:
[P2] Preserve multipart field-name boundaries
Joining the resolved components with dots loses identifier boundaries. For a
valid schema such as ROW<s ROW<"a.b" STRING>>, Spark supplies the parts [s,
a.b], but this transform emits s.a.b and ParquetFilters later splits it into
[s, a, b]. parquet-mr then treats the real [s, a.b] column as missing and may
prune matching row groups. Please retain the ordered components and construct
the Parquet ColumnPath from that array; at minimum, decline Parquet pushdown
whenever a nested component contains a dot.
--
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]
