Hello Yuxia,
I sent an email to your FIP-47 discussion in mailing list. But I it is
not delivered since.
In any case, please find my findings below:
---
Hello Yuxia,
Thanks for the FIP! Please find my findings below.
1) The __rowid Introduction
Does __rowid actually differ from the existing __offset system column?
Every Fluss-tiered Paimon table already gets __bucket, __offset, and
__timestamp appended, PK tables included. The __offset is filled per row
from `logRecord.logOffset()`. So after DEDUPLICATE merge, the surviving
row's __offset is the offset of the +I/+U that produced that version.
That's similar to the FIP's definition of __rowid.
Can we reuse __offset instead? If not, the FIP should say why, because
right now it looks like we're paying 8 bytes per row on every DV table
for a column we already have.
2) New Format Change
I think the new format change is visible to clients than `@Internal`
suggestion.
KV values don't stay on the server. SnapshotFilesReader in fluss-client
reads raw KV snapshot files and decodes `[schemaId][BinaryRow]`
directly, so prepending 8 bytes changes a format that ships to clients.
The compatibility section says `opt-in, no migration`, maybe we can
update it with what an older client does when it reads a DV-enabled
table. Does it fail on a version check, or does it silently misparse the
row as if the RowId bytes were column data? We can extend the the FIP to
answer this with a concrete protocol or version gate, not just the
opt-in flag.
3) Validating Correctness Invariants
I think this was one of the points from Leonard's comments.
From the FIP, I understood that we have two assumptions: Fluss stays the
only writer, and compaction preserves __rowid.
Nothing enforces either one, because a Paimon table is openly writable
and Spark, Trino, or Flink can write to it at any time. A single
external INSERT puts rows with null __rowid into the table; an external
rewrite that drops the column loses the mapping entirely. Either way the
index is corrupted and nothing in the FIP would notice.
I don't think documenting these would be enough. Maybe we can extend the
FIP to specify a detection mechanism. One option: during the file scan,
check for null __rowid values, and if any turn up, halt and alert.
Best,
Muhammet
