Skip to content

fix(format): move FLAG_UNKNOWN off the tagged FRI bit - #9420

Merged
LuQQiu merged 1 commit into
mainfrom
lu/fix-unknown-flag-bit
Sep 18, 2026
Merged

LuQQiu merged 1 commit into
mainfrom
lu/fix-unknown-flag-bit

Conversation

@LuQQiu

@LuQQiu LuQQiu commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

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

@github-actions github-actions Bot added the bug Something isn't working label Sep 18, 2026
@LuQQiu

LuQQiu commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

cc @jackye1995 @Xuanwo @BubbleCal -- you reviewed #9136 / #9119 which introduced these two flags, so flagging this for your eyes.

This is a correctness fix for a latent bit collision, not a format/spec change: the on-disk flag values are unchanged (FLAG_FRAG_REUSE_WITH_STABLE_ROW_IDS stays bit 9, FLAG_FRAGMENT_REUSE_INDEX stays bit 10) and tagged FRI remains unsupported, exactly as on main today. The only change is moving the internal FLAG_UNKNOWN boundary off bit 10 (where #9119 accidentally parked it, on top of #9136's FRI bit) up to bit 11, plus a const-assert so this can't silently recur. No wire/spec behavior changes, so this does not need a format vote.

@LuQQiu
LuQQiu marked this pull request as ready for review September 18, 2026 19:18
@LuQQiu
LuQQiu force-pushed the lu/fix-unknown-flag-bit branch 2 times, most recently from e32a303 to 992c685 Compare September 18, 2026 19:23
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

@lance-gatekeeper lance-gatekeeper Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gate recommendation: approve.

This restores the feature-flag boundary invariant without changing the format-assigned bits: tagged FRI remains explicitly unsupported at bit 10, and the unknown sentinel moves above every named flag.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Sep 18, 2026
@LuQQiu
LuQQiu merged commit c8f1821 into main Sep 18, 2026
38 checks passed
@LuQQiu
LuQQiu deleted the lu/fix-unknown-flag-bit branch September 18, 2026 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working K-approved Latest Gatekeeper recommendation permits acceptance.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants