feat(format): feature flag for a fragment reuse index on stable row id tables - #9119
Conversation
github-actions
Bot
added
A-format
f1655f5 to
059faf8
Compare
|
Important Format specification voteThis PR modifies the Lance format specification, so it requires 3 binding +1 votes from PMC members (excluding the proposer), at least one of them on the latest commit, and a minimum 72-hour voting period, weekends excluded, before it can merge. Vote by approving this PR (+1) or requesting changes (−1, a veto). See the voting process. Approvals carry over across pushes, so a rebase or a typo fix does not send everyone back to re-vote. Whoever approves the latest commit is vouching that nothing substantive has changed since the earlier approvals; if something has, ask for fresh votes. Status: ✅ Vote passed — 3 PMC approvals, voting period elapsed
Updated automatically by the format-spec vote gate, which re-checks every 15 minutes — just voted? Re-check now (press Run workflow; leave the input blank to re-check every open format PR). A PMC member may apply the |
![lance-gatekeeper[bot]](https://avatars.githubusercontent.com/in/4433141?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Gate recommendation: approve.
The format-only reservation establishes the necessary compatibility fence: bit 9 is documented for stable-row-ID fragment reuse, while this revision still rejects bits 8 and 9 until their implementations land. PR #9120 is correctly separated as the dependent implementation.
lance-gatekeeper
Bot
added
the
K-approved
Bit 9 is taken by the stable-row-id FRI compatibility flag (#9119), so the tagged-history capability moves to bit 1024. A rewrite and its transition may land in separate commits: transitions reference only committed fragments, and unmapped destinations are served by scanning, so correctness never depends on the mapping being present; atomic composition is deferred to a future composite transaction mechanism. Tables using stable row IDs do not support tagged histories. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016RynpAxtwGB9Q9CL4JCvR4
Bit 9 is taken by the stable-row-id FRI compatibility flag (#9119). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016RynpAxtwGB9Q9CL4JCvR4
|
updated my flag number to avoid contradiction with #9119 |
059faf8 to
4de4ade
Compare
lance-gatekeeper
Bot
removed
the
K-approved
There was a problem hiding this comment.
✅ Gate recommendation: approve.
The rebase preserves the compatibility fence: bit 8 is now implemented, while reserved bit 9 remains explicitly excluded from supported flags until the domain-aware implementation lands. PR #9120 remains correctly separated as that dependent implementation.
lance-gatekeeper
Bot
added
the
K-approved
| /// Reserved ahead of its implementation: refused for reading and writing | ||
| /// until domain-aware index loading lands, or a build from the gap would open | ||
| /// the table and remap stable row ids as if they were row addresses. |
There was a problem hiding this comment.
thought: I was wondering why this was necessary. I wonder if we can still avoid it. What if we recorded in each index whether it was address or row-id domain, and have defaults that match the current behavior. What do you think of that? @westonpace
There was a problem hiding this comment.
I like the per-index domain field but don't we need the flag anyways? My understanding is the index reader today applies the FRI to every index it loads and an older build opening the stable-row-id table that has an FRI is going to remap the stable Ids as if they were addresses and return wrong or missing rows and the only thing today's readers are going to consult is a feature flag.
There was a problem hiding this comment.
I think what we are saying here is:
- SRID + FRI is not writable today (we block that)
- We want to start writing SRID + FRI
- There is no way we can think of to do this that would allow the table to be safely written by old writers.
Without a feature flag an old writer could create an old-style index, that has no awareness of this concern. A compaction job (again old writer) would then silently corrupt the table.
westonpace
left a comment
westonpace
left a comment
There was a problem hiding this comment.
This is fine but let's avoid talking about "some indexes should apply the FRI and some should not". We don't know the final state. The current plan is the final state is that all indexes are based on addresses and do not apply the FRI so this advice would be potentially confusing to a future user.
For now let's just limit the scope to "this means SRID + FRI which was previously unsupported and old readers/writers could corrupt these tables"
| /// Reserved ahead of its implementation: refused for reading and writing | ||
| /// until domain-aware index loading lands, or a build from the gap would open | ||
| /// the table and remap stable row ids as if they were row addresses. |
There was a problem hiding this comment.
I think what we are saying here is:
- SRID + FRI is not writable today (we block that)
- We want to start writing SRID + FRI
- There is no way we can think of to do this that would allow the table to be safely written by old writers.
Without a feature flag an old writer could create an old-style index, that has no awareness of this concern. A compaction job (again old writer) would then silently corrupt the table.
…d tables Reserve reader/writer feature flag 1 << 9 for a table version that uses stable row ids and carries a fragment reuse index. The index records how physical row addresses moved; entries of an index that stores stable row ids must not be remapped through it, so builds without domain-aware index loading must refuse such a table. This build defines the bit but does not support it yet; support lands with the implementation. Part of lance-format#8977
4de4ade to
0c87706
Compare
lance-gatekeeper
Bot
removed
the
K-approved
thank you. i've updated the PR |
FLAG_FRAG_REUSE_WITH_STABLE_ROW_IDS (#9119) took bit 9, which pushed FLAG_UNKNOWN up to bit 10 -- the same bit FLAG_FRAGMENT_REUSE_INDEX (#9136) already occupies. Move FLAG_UNKNOWN to bit 11 so the unknown boundary sits above every named flag again. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgAshYD7wVzPVdjXRuWPPs
FLAG_FRAG_REUSE_WITH_STABLE_ROW_IDS (#9119) took bit 9, which pushed FLAG_UNKNOWN up to bit 10 -- the same bit FLAG_FRAGMENT_REUSE_INDEX (#9136) already occupies. On main the collision is latent because the tagged FRI bit stays below the FLAG_UNKNOWN boundary and is never marked supported. Move FLAG_UNKNOWN to bit 11 so it sits above every named flag again, add a const assert pinning FLAG_FRAGMENT_REUSE_INDEX below the boundary so an insertion cannot recreate the collision silently, and explicitly keep the tagged FRI bit unsupported (as the boundary used to do implicitly) until its reader/writer handling lands. No behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgAshYD7wVzPVdjXRuWPPs
FLAG_FRAG_REUSE_WITH_STABLE_ROW_IDS (#9119) took bit 9, which pushed FLAG_UNKNOWN up to bit 10 -- the same bit FLAG_FRAGMENT_REUSE_INDEX (#9136) already occupies. On main the collision is latent because the tagged FRI bit stays below the FLAG_UNKNOWN boundary and is never marked supported. Move FLAG_UNKNOWN to bit 11 so it sits above every named flag again, reorder FLAG_FRAGMENT_REUSE_INDEX to its bit-order position (after bit 9, before the unknown boundary), add a const assert pinning it below the boundary so an insertion cannot recreate the collision silently, and explicitly keep the tagged FRI bit unsupported (as the boundary used to do implicitly) until its reader/writer handling lands. No behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgAshYD7wVzPVdjXRuWPPs
## Problem Two feature-flag constants collide on bit 10: ``` FLAG_FRAG_REUSE_WITH_STABLE_ROW_IDS = 1 << 9 // #9119 FLAG_UNKNOWN = 1 << 10 // collides FLAG_FRAGMENT_REUSE_INDEX = 1 << 10 // #9136 ``` `FLAG_FRAGMENT_REUSE_INDEX` (#9136) was defined at bit 10 while `FLAG_UNKNOWN` was at bit 9. Later #9119 inserted `FLAG_FRAG_REUSE_WITH_STABLE_ROW_IDS` at bit 9 and bumped `FLAG_UNKNOWN` up one -- but only to bit 10, which #9136 had already claimed. So on current main both live on the same bit. The collision is **latent** on main: `supported_flags_when` uses `FLAG_UNKNOWN - 1` as the base mask, so bit 10 is currently *below* the boundary being masked out, and the tagged FRI bit is never marked supported. But the moment any build activates the tagged FRI flag (adds it to the supported set), the `FLAG_UNKNOWN`-based "reject unknown flags" tests break, because `FLAG_UNKNOWN` **is** the FRI bit -- the rejection path would silently accept it. ## Fix - Move `FLAG_UNKNOWN` to `1 << 11` so it sits above every named flag again. - Add `const _: () = assert!(FLAG_FRAGMENT_REUSE_INDEX < FLAG_UNKNOWN)` to pin the invariant so a future insertion can't recreate the collision silently. - Explicitly `mark_supported(FLAG_FRAGMENT_REUSE_INDEX, false)` -- with the boundary moved, bit 10 would otherwise fall inside `FLAG_UNKNOWN - 1` and become supported; keep tagged FRI unsupported (exactly as the boundary did implicitly) until its reader/writer handling lands. **No behavior change**: tagged FRI stays unsupported, unknown-flag rejection still fires on the correct bit. ## Test - `cargo build -p lance-table` (const asserts pass) - `cargo test -p lance-table --lib feature_flags` -- 14 passed, incl. `test_frag_reuse_with_stable_row_ids_flag_is_reserved_not_supported` 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
6 participants
Reserves the reader/writer feature flag FLAG_FRAG_REUSE_WITH_STABLE_ROW_IDS for table versions that use stable row IDs and carry a Fragment Reuse Index (FRI).
This combination was previously unsupported. Older readers and writers are not prevented from opening such tables and could silently corrupt them. Setting the flag in both feature fields makes those clients reject the table with an unsupported error. This PR defines and documents the flag but keeps it unsupported. Support will be enabled in a follow-up PR alongside the implementation.
Part of #8977