JingsongLi commented on code in PR #10415:
URL: https://github.com/apache/paimon/pull/10415#discussion_r4248753911
##########
paimon-core/src/main/java/org/apache/paimon/operation/DataEvolutionSplitRead.java:
##########
@@ -1078,6 +1095,20 @@ private Builder formatBuilder(
nestedFieldEnabled);
}
+ // Row sidecars use positional decoding and contain exactly the physical
write schema.
+ // Row tracking fields are supplied by DataFileRecordReader from the
manifest entry.
+ private Builder rowSidecarFormatBuilder(
+ RowType readRowType, @Nullable List<Predicate> filters, boolean
nestedFieldEnabled) {
+ return new Builder(
+ formatDiscover,
+ readRowType.getFields(),
+ TableSchema::fields,
Review Comment:
[P1] Preserve manifest tracking fields through schema casts
Using the physical schema is necessary for positional decoding, but it
leaves `_ROW_ID` and `_SEQUENCE_NUMBER` absent from `readDataFields`.
`SchemaEvolutionUtil.createCastFieldGetterMapping` creates a getter returning
null for each missing field. When another column requires a schema-evolution
cast, the entire cast mapping becomes active. `DataFileRecordReader` currently
restores manifest tracking values before applying `CastedRow`, so those values
are then hidden by the null getters. `isNullAt()` still delegates to the
underlying row and returns false, but `getLong()` unboxes null and throws a
NullPointerException.
I reproduced this using real table writes: write 100 rows with `f0 INT`, a
bitmap index on `f1`, and row sidecars enabled; ALTER `f0` to BIGINT; then read
the widened column and tracking fields with a selective `f1 = 'a050'` filter
through `executeFilter()`. Both the ordinary bitmap read and the variant with
`_ROW_ID BETWEEN 0 AND 99` throw NPE. The same cases pass on the pre-change
implementation.
Please preserve tracking values through casting, for example by restoring
non-columnar tracking fields after applying the cast, or by using identity
getters for the tracking slots. These virtual fields should remain excluded
from the physical row decoder schema.
##########
paimon-core/src/main/java/org/apache/paimon/operation/DataEvolutionSplitRead.java:
##########
@@ -767,6 +763,27 @@ private FileRecordReader<InternalRow> createFileReader(
}
}
+ FileReadTarget readTarget =
+ readTarget(file, dataFilePathFactory, rowRanges,
fileIndexResult);
+ String formatIdentifier = readTarget.formatIdentifier;
+ boolean readRowSidecar = ROW_SIDECAR_FORMAT.equals(formatIdentifier);
+ Builder targetFormatBuilder =
+ readRowSidecar
+ ? rowSidecarFormatBuilder(readRowType, fileFilters,
nestedFieldEnabled)
+ : formatBuilder(readRowType, fileFilters,
nestedFieldEnabled);
+ TableSchema readerDataSchema = readRowSidecar ? physicalDataSchema :
fileSchema;
Review Comment:
[P2] Restore partition values independently of the physical row schema
A valid partial-column bundle write can provide the partition explicitly
without storing its column in the physical file. With a data-evolution table
partitioned by `pt`, I used the public `BatchTableWrite` API with
`withWriteType(table.rowType().project("f1"))`, then `writeBundle(partitionP1,
0, bundle)` and a normal prepare/commit. The bundle contains 100 `f1` values,
with a bitmap index and row sidecars enabled. Querying the matching row for `f1
= 'a050'` now returns `pt = NULL`, although the split's partition is `p1`. The
same test returns `p1` on the pre-change implementation.
Here, `physicalDataSchema` contains only `f1`.
`FormatReaderMapping.readDataFields` drops the requested `pt` field and gives
it an index mapping of -1. `PartitionUtils` only creates restoration mappings
for partition columns present in those data fields, so it never restores `pt`
from the split. The main-file path uses the full file schema and still restores
it correctly.
Please retain partition restoration based on the full file schema and
requested output fields, and supply missing partition values from the split
independently of the physical decoder schema. Adding the absent partition
column to the row decoder itself would reintroduce positional decoding errors.
--
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]
