JingsongLi commented on PR #8826:
URL: https://github.com/apache/paimon/pull/8826#issuecomment-5213515512
I think this can be simplified further by keeping row-id range planning as a
passive planning primitive, instead of making `RowIdRangeContext` own the
read/update/skip sequence and commit lifecycle.
The existing row-id update path already carries the information needed by
the normal commit conflict detector:
- `CommitMessage.check_from_snapshot` identifies the snapshot from which the
update was planned.
- The new files in the message carry `first_row_id`, `row_count`, and
`write_cols`.
- `FileStoreCommit` can therefore derive row-range × write-column conflicts
and detect missing or replaced anchor files from the ordinary commit messages.
The important requirement is that the writer must build its `CommitMessage`
from the same pinned snapshot/file information used by the range planner. It
must not re-plan against the latest snapshot after the user has read an older
pinned snapshot. This is a writer/planner concern; it does not require
`commit()` to accept additional range arguments.
A simpler API shape would be:
```python
with plan_row_id_ranges(...) as plan:
for row_range in plan:
source = build_read(
snapshot_id=row_range.snapshot_id,
row_id_range=row_range,
)
updates = user_process(source)
update_by_row_id_from_plan(row_range, updates, update_cols)
```
Here, `RowIdRange` can be immutable metadata (plus an opaque internal
planning token if needed for efficient file selection). The user controls read
and processing, while the update writer creates the same normal `CommitMessage`
that existing row-id updates already use.
The current `protected_columns` and context-side read tracking introduce a
stronger semantic contract. For example:
```text
read text -> compute embedding -> write embedding
```
If another writer changes `text`, the existing row-id update semantics allow
the `embedding` write because the actual write columns are disjoint. Tracking
`text` through the context changes this into a transform/read-dependency
conflict. That may be useful, but it is a new consistency contract rather than
something required by row-id range commits themselves. If we need that
contract, I think it should be specified and justified independently, and the
dependency columns could be supplied explicitly rather than inferred through a
stateful context.
If we retain the existing row-id update conflict semantics for the first
version, we should be able to remove most of the operation-specific machinery:
- context-side `_read_columns`, `_updated`, `skip()`, and sequence state;
- the operation checkpoint and exact-snapshot return path;
- `protect_from_external_rewrites` state added to the generic commit layer.
The range planner, pinned-snapshot tag, file-group-aligned packing, and
normal `CommitMessage` conflict detection should be sufficient. This keeps the
abstraction focused: planning returns ranges, reading reads a pinned range, and
committing commits ordinary row-id update messages.
--
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]
