gripleaf commented on PR #323:
URL: https://github.com/apache/paimon-cpp/pull/323#issuecomment-5653041009
> Thanks for the contribution and the performance improvement. Could we
refactor this before merging?
>
> Java’s `ManifestAvroReader` only uses the top-level `bucket` and
`totalBuckets` fields for filtering. I’m not sure why `_FILE._SCHEMA_ID` is
needed here, since `bucket-key` is immutable. Schema ID also seems too broad as
a bucket-layout identifier.
>
> Could we reuse or extend the existing late-materialization framework
instead of implementing another probe/bitmap/reread flow in `ManifestFile`? The
selector is roughly:
>
> ```
> _BUCKET = target
> OR _TOTAL_BUCKETS IS NULL
> OR _TOTAL_BUCKETS != expectedTotalBuckets
> ```
>
> `_VERSION = 2` may still be validated for every row.
>
> This would also keep Manifest-specific logic out of the generic Avro
reader.
Thanks for the suggestions. I’ve refactored the PR to reuse
LateMaterializingFileBatchReader.
ManifestFile now builds the selector you proposed:
_BUCKET = target
OR _TOTAL_BUCKETS IS NULL
OR _TOTAL_BUCKETS != expectedTotalBuckets
The existing framework handles probing, bitmap construction, and the
payload pass. The Avro changes provide generic bitmap selection, physical
row-ID tracking, and a bounded block index; they contain no Manifest field
names or bucket-specific logic. The two-pass path runs only when the manifest
bytes are retained in memory, avoiding a second remote read.
I also agree that schema ID is too broad to identify a bucket layout.
_FILE._SCHEMA_ID is no longer part of the probe. However, unchanged bucket-key
names do not necessarily imply unchanged hashing: the regression test covers an
append table whose bucket-key type changes from INT to BIGINT, where the same
value maps to different buckets despite an unchanged
bucket count.
To preserve the scan’s conservative behavior, inferred-bucket pruning now
has a scan-level compatibility check. It uses the maximum schema ID from the
already-loaded manifest metadata as an upper bound and checks historical
schemas against the current bucket-key field IDs, order, types, and bucket
function. Compatible histories retain selective decoding; an
incompatibility or schema-read failure triggers fallback. This is
deliberately conservative and reuses the existing per-scan caches. Cold schema
checks can add schema-file reads.
One detail about _VERSION: the current implementation validates retained
entries through the existing serializer, rather than validating every row
during probing. Unsupported versions in retained entries still fail.
--
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]
