sundapeng commented on code in PR #1014:
URL: https://github.com/apache/paimon-rust/pull/1014#discussion_r4371820257
##########
crates/paimon/src/table/data_evolution_reader.rs:
##########
@@ -1397,6 +1397,8 @@ async fn resolve_descriptor_columns(
file_io: &FileIO,
limiter: &BlobReadLimiter,
) -> crate::Result<RecordBatch> {
+ // Descriptor fields reference objects the table does not own, which may
be replaced.
+ let file_io = file_io.origin_only();
Review Comment:
Thanks, reproduced and fixed in bbc4a6932c, for every reader of such URIs
rather than for this resolver only.
The previous round chose the FileIO at three call sites, and this PK path
(`read_pk_with_blob` → `managed_blob_reader::resolve_batch`, payload and
predicate resolution alike) was a fourth. Now a table's view of the FileIO
carries the table's location, and a request through it may use a target only
for a path under that location; anything else, such as an object a
`blob-descriptor-field` value points to, goes to the origin. The table's own
`.managed.blob` packs and `.blob` files lie under its location and keep the
target, so a single `resolve_batch` call can mix both. The three call-site
views of the previous round are gone; those files are back to what main has.
Auditing the other readers of descriptors and the other paths outside a
table turned up two more:
- `BlobReader::from_file_io(table.file_io())`, which
`paimon_table_new_blob_reader` (C, and Go through it) uses, routed external
descriptors by name. It follows the table's rule now, and a `BlobReader` built
from storage options, which serves no table, reads every descriptor from the
origin (`UriInput` does the same for any FileIO without a table). The
DataFusion and Python binding registries already resolve a URI to a table's
FileIO only under that table's location.
- Data files written under `data-file.external-paths` went to the target,
which serves the table's location and may not serve those paths at all. They
now use the origin, as without routing.
Tests: `primary_key_descriptor_fields_read_their_sources_from_origin` is
your case (origin NEW, target OLD; the read returns NEW, the source GET reaches
only the origin, and the managed pack of a second BLOB column is read from the
target); `blob_readers_read_objects_outside_the_table_from_origin` covers both
`BlobReader` constructors; `data_files_under_external_paths_use_the_origin` the
external path. With the location rule disabled these three fail, and so do
`blob_descriptor_sources_are_read_from_origin` and
`blob_descriptor_fields_read_their_sources_from_origin` from the previous round.
apache/paimon#10350 and #10351 got the same rule in this round; Java had
both the PK descriptor-field read and the BLOB-source copy gap as well.
##########
crates/paimon/src/table/mod.rs:
##########
@@ -1125,6 +1167,7 @@ impl Table {
} else {
schema_manager.with_branch(&branch)
};
+ table.file_io = with_schema_history(&table.file_io,
&table.schema_manager);
Review Comment:
Thanks, reproduced and fixed in 04517aa703.
The view used to be rebuilt in each constructor and copy, and the REST
branch arm only attached the (empty) schema history to the source table's
FileIO. Now `table_view` builds it for every constructor and for every copy
that moves to another branch: origin only for a Format Table, otherwise the
table's location, the prefixes of the schema it uses and the lazily loaded,
origin-only history of that branch. The REST branch copy keeps the original
provider/FUSE FileIO and passes the branch schema the catalog returned. Copies
within a branch keep their loaded history and add the prefixes of their options.
Tests: `rest_branch_copies_route_files_named_with_the_branch_prefix` serves
`main-` and `branch-` schemas from a REST catalog with no branch schema
directory in storage, and reads `branch-<uuid>-1.parquet` through the directly
loaded branch, `copy_with_branch("dev")` and the `branch` dynamic option; all
reach the target (the two copies fail without the fix).
`every_table_copy_routes_files_named_with_the_schema_prefix` walks every other
way to derive a table (`Table::from_resolved_schema`, `copy_with_options`,
snapshot and tag time travel, `copy_with_resolved_snapshot`,
`copy_with_pinned_snapshot`, `copy_with_resolved_schema` on main and on a
branch, a filesystem `copy_with_branch` and `branch` option) and checks that
each reads its custom-prefixed data file from the target.
One limit remains: a REST table whose older schemas exist only in the
catalog has no API to list them, so files under those older prefixes are still
read from the origin. That only loses the cache, never data.
--
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]
