zhuxiangyi commented on code in PR #9423: URL: https://github.com/apache/paimon/pull/9423#discussion_r3891223995
########## 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: Confirmed. In the end-to-end test the matching row is lost every time. The ordered components are retained now, as part of the change above. For the parquet side this PR takes the "at minimum" option you offered: pushdown is declined when any component contains a dot. Building the `ColumnPath` from the array does look feasible — `Operators.*Column(ColumnPath)` is package-private and `ParquetFilters` lives in that package — but it means routing components instead of a joined name through `findFileColumn` and the column-construction sites, which also changes the flat path. My instinct was to keep this PR to the correctness fixes and send that separately, especially since the full version would also change behaviour for flat columns whose names contain a dot. But I do not feel strongly — if you would rather have it here, I will do it. While fixing this I ran into a related case: the dot can also sit in the top-level column's own name. `ROW<"a.b" ROW<city STRING>>` joins to `a.b.city`, which splits into `[a, b, city]` and misses the real `["a.b", "city"]` — the same silent loss, and with an unlucky schema it could resolve to a genuinely different column rather than none. The guard rejects both spellings. Tests: - `ParquetFiltersTest.testNestedComponentContainingADotIsNotPushedDown` - `ParquetFiltersTest.testNestedFieldUnderATopLevelNameContainingADotIsNotPushedDown` - `ParquetFormatReadWriteTest.testNestedComponentContainingADotKeepsMatchingRows` - `ParquetFormatReadWriteTest.testTopLevelNameContainingADotKeepsMatchingRows` `SparkV2FilterConverterTestBase` keeps the control that a flat column named `` `a.b` `` still resolves to a `FieldTransform` and is unaffected by the guard. -- 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]
