Repository navigation
Conversation
Draft RFC: allow schema apply on main while other branches are live. Branches keep their schema contract until a merge brings main's in; a merge across contracts is allowed only when every migrated table is unchanged since the merge base on the older side, and schema apply refuses when a live branch already changed a table it would migrate.
…ons-active-branches-fb6101 # Conflicts: # docs/rfcs/README.md
Order contracts by a stored schema sequence instead of content identity, which a reversed change reproduces. Judge each table by its stored signature across the base and both sides, so an older criss-cross base cannot hide a write to a re-created property. Count full-text @index declarations as table effects. Fence older binaries with storage stamp 17, budget the apply-time branch check, and align with the open historical-read, change-feed, polymorphic-types, actor-views and branch-operations proposals.
ragnorc
left a comment
ragnorc
left a comment
There was a problem hiding this comment.
Recommendation: approve this draft RFC. I found no blocking defect in the proposed design. Keep the stated cost gate and implementation proof requirements. This approval does not qualify an implementation for release.
The PR addresses a practical restriction: one live branch currently blocks every schema apply. The proposal lets main evolve while each branch keeps its own schema. A sequence number orders schema changes, including a rename followed by its reversal. Merges compare stable table and property identities across the base and both sides. They adopt a migrated table whole when safe and refuse unsupported combinations. This PR changes documentation only.
Correctness and design. The proposal addresses the contract at its owning boundaries. Schema apply owns main's evolution. Merge owns the compatibility decision. The manifest publisher owns the atomic contract, table pins and lineage update. Removing only the existing refusals would leave mismatched contracts and pins. The proposed design closes that class of failure without a separate schema synchronization operation.
The base signature matters as much as the two endpoint signatures. A property dropped and recreated with the same name has a different identity. Comparing values by name could hide a write even when both endpoints use the same schema. The sequence also solves a different problem: equal schema content does not imply the same occurrence in history. Keeping it beside the content hash preserves the existing archive and drift checks.
I checked the physical assumptions against the pinned Lance 11.0.0 implementation, not just current upstream prose. OmniGraph's schema staging uses Project and Merge over unchanged fragments. Lance's detached commit path writes a separate manifest. Its FTS fallback confirms why analyzer declarations must travel with the adopted pin. The current public open path checks main's format before reading branch state. Already-open older processes remain the operator obligation stated in the RFC.
Tradeoffs and liability. This adds real permanent cost: a stored sequence, a format fence, several accepted catalogs, branch-aware schema reads, and more merge states. The benefit is narrower refusal and safe schema evolution without rewriting every branch. Reusing the existing publisher, authority checks, retained history and whole-version adoption keeps that cost contained. Five more migration kinds can use the same signature and validation rules. They should not each add a synchronization API, a side record or a special merge path.
The first version is deliberately conservative. Even nullable additions can block when a branch changed that table. An index rebuild also changes its registration without changing rows. I left one optional inline comment to make that consequence explicit in the operator contract and test matrix. Retained branch creation versions also delay removal of dropped values. These are practical costs for long-lived review and agent branches.
The apply check adds work proportional to branch inventory and retained-history structures. I did not measure its requests, bytes, memory or latency. The 1/16/256-branch instrument and measured work budget must remain prerequisites. Retries must consume the same bounded work allowance. The local schema gate does not fence another process, so merge refusal remains necessary after a later branch write.
Alignment. I surveyed all open PRs. The material dependencies are #959, #964, #951, #884 and #677. The RFC correctly separates current-contract query typing from archived-contract change decoding. Both the catalog and live coverage view in #959 must follow the writing branch. Access restrictions in #951 must apply across branches or refuse. Validation-tightening changes in #884 must enter the compatibility rule, and restore must not lower the sequence. The branch authority work in #825 must retain coherent per-branch captures.
I also checked Lance 13.0.0, including its merge-columns field-ID and nested-schema repair changes. They do not remove the need for OmniGraph's stable identities or atomic graph publication. The upstream schema contract still warns that independently changed branches can assign the same field ID. Whole-pin adoption avoids a new fragment-grafting dependency. I did not run OmniGraph against Lance 13.
Validation and limits. Reviewed exact head f3885a4606871cead0532d436d4c079cfd420d77 in a clean isolated checkout. The comparison base was f53bf4fb756899b58df37f6047f42e01ba0c41ce. Only the RFC and its registry row differ.
Local checks passed:
python scripts/check-docs.py, using the existing documentation environment: 257 Markdown files.bash scripts/check-agents-md.sh: 54 links and 50 documents.typos docs/rfcs/2026-10-12-schema-apply-with-live-branches.md docs/rfcs/README.md.git diff --check f53bf4fb756899b58df37f6047f42e01ba0c41ce...HEAD.
I also ran this focused check with Rust 1.97.1 and the existing compatible build cache:
RUSTUP_TOOLCHAIN=1.97.1 CARGO_TARGET_DIR=/tmp/review-928/target \
RUSTFLAGS='--cfg tokio_unstable --cfg tokio_unstable' \
RUSTC_WRAPPER= RUST_MIN_STACK=16777216 \
cargo test --locked -p omnigraph-engine --test integration -- \
maintenance::full_text_rebuild_on_a_branch_stages_on_the_inherited_location_without_changing_main \
maintenance::full_text_rebuild_replaces \
changes::history_reads_are_the_same_after_a_merged_branch_is_deleted_and_recreatedTwo tests passed, with none failed or ignored. The branch rebuild test confirms a changed pin and preserved branch rows. The history test confirms that history remains available after branch deletion, while full snapshot access refuses the retired incarnation. The full_text_rebuild_replaces filter selected no test without the failpoints feature and is not counted as evidence.
The exact-head CI Gate passed. Test Workspace, the pinned DST suite and cloud integration jobs were skipped. The ordinary GQT job reported success through its no-input-change skip path. That is not a GQT test pass.
There is no implemented schema-with-branches fix in this PR, so no behavioral before-fail/after-pass proof exists here. I did not reproduce the author's temporary prototype or ordering model. The required matrix must prove the implemented behavior later. GQT currently lacks the schema-apply step, so the existing Rust schema/merge owners are appropriate until that operation is expressible. I made no source or test edits and left the checkout clean.
| "Differs from the base" uses the merge's existing `same_manifest_state` | ||
| comparison by stable identity: path, published version, native ref, row count | ||
| and version metadata. The registration clock and alias are ignored. |
There was a problem hiding this comment.
Optional: document maintenance-only refusals and add them to the proposed test matrix. This comparison also treats an index rebuild as a changed table. For example, create an untouched branch with an indexed Person table, then rebuild its full-text index. The existing writer publishes a new registration even though the rows do not change. The apply-time rule would then refuse a nullable property addition on main. If the rebuild happens after that apply, merge case 4 would refuse catch-up instead. The existing branch rebuild test confirms the changed pin. Please name this consequence in the user behavior and add before/after-apply rebuild cases to the test plan. It follows the draft's conservative whole-version rule, so I do not treat it as a blocking defect. Any future relaxation should prove logical equality generally, with registration equality as the cheap path, rather than add an index-specific exception.
There was a problem hiding this comment.
Covered by the row-based definition of "changed". An index rebuild on the branch, before or after the apply, no longer changes the outcome, and the schema_apply.rs row tests both orders.
…ons-active-branches-fb6101 # Conflicts: # docs/rfcs/README.md
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| buffered, else from one history record read. The merge capture path and | ||
| `pinned_graph_commit` are not reused: the first refuses differing contracts | ||
| and replaces a one-entry authority cache per branch, and the second opens a |
There was a problem hiding this comment.
P1: physical index maintenance decides whether apply and merge refuse. This is the case the deny-list names, but the Invariants section says "No deny-list shape is added."
"Changed the table" here (and in merge case 3/4 via same_manifest_state) means the registration differs: path, published version, native ref, row count, version metadata. Two existing branch-scoped operations change exactly that without changing any row:
ensure_indices_on(branch)publishes a new pin per indexed table (table_ops.rs#L456-L498:commit_staged_detached→published_dataset_version = expected + 1→commit_updates_on_branch_with_expected).rebuild_full_text_indices_on(branch)does the same (the case the existing thread at r4251845178 raises).
So whether schema apply succeeds on main, and whether a later catch-up merges or refuses schema_diverged, depends on whether someone built an index on the branch. Invariant 7 says "Missing coverage may change cost but not correctness". The deny-list rejects "a logical precondition based on physical index coverage … or staged layout". Today same_manifest_state is only a fast-path selector in merge (it changes cost, not outcome); this RFC promotes it to an admission predicate.
The refusal fails closed, so it isn't a correctness bug. But I'd treat it as blocking acceptance rather than optional: the RFC either needs a logical "changed" predicate or an explicit argued exception under invariant 7. A cheap, sound option already exists. Treat a branch registration as unchanged when its fragments' resolved data files, overlays and deletion files equal the base's. That machinery is already in merge (fragment_data_identical / resolved_data_file_key, merge.rs#L1896-L1990). Index-only commits keep every data and deletion file, optimize is main-only, and a mismatch still falls back to "changed", so the predicate stays conservative. Case 3's "take that side's version whole" also needs a rule for an index-only side: take the side with the newer contract.
There was a problem hiding this comment.
Addressed. "Changed" now means changed rows (Logical change): equal registrations first, then lineage discovery by file identity, which index-only commits keep, then a row comparison. Index rebuilds and compaction change cost, not the answer. The one result physical state can still reach is the apply check's budget refusal (cannot prove), stated under invariants 7 and 11. Head 9a3f6213.
| does not release its creation version. | ||
|
|
||
| ### Older binaries | ||
|
|
||
| The first publication by this release stamps a branch's `__manifest` 17. | ||
| Binaries that serve stamps up to 16 refuse a graph whose main is stamped 17, | ||
| before any read. `omnigraph upgrade` stamps every live branch at once for |
There was a problem hiding this comment.
P2: the fence is stronger than this paragraph says, and the stamp is applied earlier than soundness requires.
(a) It isn't only checked at open. guard_row_layout → refuse_if_stamp_unsupported runs on every manifest scan (state.rs#L1095). It also runs in overwrite_placed before every publish (commit.rs#L108) and on the publisher's retry reload (lib.rs#L1591). An older process that already has the graph open therefore fails closed at its next load or CAS of a stamped __manifest. It can't write a sequence-less contract row over main, and it can't merge into or out of main.
What stays unfenced is narrower: an older process working only on a lagging branch that is still stamped 16. Working under that branch's own contract is what this RFC allows anyway. Please name the exact residual risk, or drop the operator obligation if none remains, and add the in-flight older-writer case to the crossversion_upgrade.rs row.
(b) Consider stamping at the first contract write instead of the first publication. Soundness (L190-194) only needs main to be stamped by the first sequence-bearing contract write. As written, any write by the new binary stamps main 17 and forecloses rollback, even for operators who never apply while branches are live. 15 and 16 do set the precedent, but this fence guards against a hazard that exists only once contracts can diverge. If you keep first-publication stamping, record the narrower option under Alternatives with the reason it was rejected.
There was a problem hiding this comment.
Addressed. Older binaries now states that the stamp is checked on every scan and before every publication, and names the residual case: an older process on a branch still at stamp 16, under that branch's own contract. The operator obligation is gone. Stamping at the first contract write is under Alternatives, rejected because the writer would need two row layouts. The crossversion row covers an older process that already has the graph open.
| branch held the same contract. The first apply under this release publishes on | ||
| main and stamps main 17 in that publication, so no contract row with a | ||
| sequence can exist beside a pre-17 contract that differs from it. | ||
|
|
There was a problem hiding this comment.
P2: say whether the sequence enters SchemaContractDigest, which the cluster ledger persists. This list names same_graph_state, the authority token and the catalog cache key. It doesn't name SchemaContractDigest (prepared.rs#L6-L14). That type is #[serde(deny_unknown_fields)] and is stored durably in ledger v2's applied_revision.schema_contracts (execution.rs#L402). It is also what prepared-apply reconcile and settlement compare.
Adding a field would be a ledger protocol change. A binary without the field would refuse a ledger written by one with it, and that belongs under "Compatibility and reversibility". Keeping the digest content-only looks sufficient, because a prepared apply already binds base_manifest_version, the branch identifier and the graph head, so a reversed change (X→Y→X) can't satisfy a stale preparation. Whichever you choose, state it in the RFC and add a row for it to the cluster test plan.
There was a problem hiding this comment.
Addressed. SchemaContractDigest stays content-only, so the ledger format does not change. The reason is under Schema sequence: a prepared apply already binds the base version, branch identifier and head. A cluster test row checks that a sequence increment leaves the ledger digest unchanged.
| - Every other publication carries the branch's contract row unchanged, as | ||
| today. | ||
| - A contract row written before stamp 17 has no sequence and reads as 0. | ||
|
|
||
| Sequence 0 is sound for every pre-17 row. Before this release, schema apply |
There was a problem hiding this comment.
P3 (wording): "no contract row with a sequence can exist beside a pre-17 contract that differs from it" is false as written. Right after the first apply, main holds sequence 1 / X1 while a lagging stamp-16 branch holds a sequence-less X0. I think the claim you mean is: no two contract rows that both read sequence 0 differ. That follows because every pre-17 contract write required only main, so all live branches shared one contract until the first stamped write. That's also the property the "equal sequences must be byte-identical" check at L183-185 relies on.
It also helps to note that the base may be a sequence-0 commit whose content differs from a sequence-0 side; for example, main history from before an earlier only-main apply. That's fine, because identity comparison includes content, so it takes the classification path rather than the integrity error.
There was a problem hiding this comment.
Addressed. Schema sequence now says no two live branches' contract rows that both read 0 differ. A sequence-0 base with other content is expected, because the base is read by content, never by sequence.
| A read by snapshot or commit id uses the current contract of the branch that | ||
| wrote that commit, with tables rebound by stable identity as historical reads | ||
| already do. |
There was a problem hiding this comment.
P1: "the current contract of the branch that wrote that commit" is undefined when that branch no longer exists. The same rule appears at L373-381 for snapshot reads, the change-feed baseline and #959's binding.
Today point-in-time reads bind the handle's live contract and rebind tables by identity (omnigraph.rs#L2314-L2348). That is total: the handle always has a contract. The new rule is not total:
- Writing branch deleted. Its commits stay addressable by id: merged into main, or retained as merge bases or tags. With no current contract to bind, what does the read do?
- Writing branch deleted and recreated under the same name.
graph_branchis a logical name. Without an incarnation check, the read binds the new lifetime's contract. Invariant 6 says never infer identity from a branch name. The test table lists "branch delete and recreate during snapshot binding", but the user-behavior section doesn't say what the outcome is. - Unpublished
as commitproposals and unattached snapshots. These carrygraph_branchin their candidate property. State whether they follow the same rule.
Please make this total. One option: bind the writing branch's contract only while the incarnation that wrote the commit is live, and otherwise fall back to main's current contract (today's behavior), naming the fallback in the user section. Another: bind the commit's own archived contract (schema_content_hash plus the lineage row's sequence). Either way, add the deleted-branch case to the point_in_time.rs / changes.rs rows.
There was a problem hiding this comment.
Addressed. Reads by snapshot or commit id keep main's current contract with stable-identity binding (#959). That rule is total: deleted or recreated writing branches, unpublished commits and unattached snapshots all have an answer, and no identity comes from a branch name (Reads and maintenance; Alternatives). Only a live branch's change-feed baseline binds that branch's contract, and only while its captured incarnation is live.
| `omnigraph schema apply` on main succeeds while other branches exist, unless a | ||
| live branch changed, since its merge base with main, a table the apply | ||
| changes. The refusal names every such branch and table and gives two remedies: | ||
| merge the branch into main first, or delete it. Schema preview and plan report | ||
| the same list without refusing. A preview's list is advisory, including a |
There was a problem hiding this comment.
P2 (scope): state plainly that the most common migration on an active graph stays blocked. Motivation says long-lived agent and review branches block every schema change. Under this rule, adding a nullable property to a type any live branch has written still refuses: here at apply, and as schema_diverged at merge if the branch writes later. Agent branches typically write exactly the hot types.
So the RFC unblocks migrations on tables that no live branch has touched, plus type renames, enum widening, non-full-text @index and metadata edits. Future work's nullable-add conversion is what unblocks the motivating case. Please say this in the Summary and Motivation, so nobody reads the RFC as "migrations no longer wait for branches". Also consider whether nullable-add projection is cheap enough to bring into scope: it only needs null fill by stable property id inside the existing three-way cursors.
There was a problem hiding this comment.
Addressed, with a narrower scope than this comment suggested. On 10-17 we chose the lower-liability rule. A migration still waits for a branch that changed a table it touches, and a branch that writes such a table after the migration cannot merge until its work is redone. Summary and Motivation now say so. Nullable-add relief is Future work relaxations 1 and 2. Each only turns a refusal into a merge and needs no format change.
| and `GET /schema?branch=<name>` show it and require read access to that | ||
| branch. `branch list` and `branch show` report each branch's contract identity, | ||
| its schema sequence, and whether it is behind main's. |
There was a problem hiding this comment.
P3 (cost): branch list reads active refs today. Reporting each branch's contract identity, sequence and behind-main flag adds a __manifest open and a contract-row read per branch, which makes it O(branches × manifest open). Under invariant 11, either put these fields on branch show only (or behind a flag), or include branch list in the blocked_on cost instrument alongside the apply check. The OpenAPI regeneration for the new fields and the branch query parameter should also appear in the Surfaces list.
There was a problem hiding this comment.
Addressed. The fields are on branch show only, so branch list stays one ref read per branch. OpenAPI regeneration is listed under Surfaces, and a detailed listing such as #1012 may add the same fields at its measured cost.
| ## Design | ||
|
|
||
| ### Schema sequence | ||
|
|
||
| The `schema_contract` row and every graph lineage row gain `schema_sequence: | ||
| u64`, stored beside the contract head rather than inside it. A commit's | ||
| sequence is therefore available from its lineage row without opening its | ||
| snapshot. The archived schema object serializes the head with the source and |
There was a problem hiding this comment.
P2: specify how the sequence is stored in the two places "every graph lineage row" actually covers.
- Buffered rows use an inherit encoding. A settled row in the
__manifestbuffer can carryINHERITED_SCHEMA_IDENTITY_VERSIONand no head. It inherits the contract and content hash from the next newer commit (row.rs#L555-L604). So "available from its lineage row without opening its snapshot" holds only if the sequence follows the same inheritance and the same partial-identity refusal ("commit carries a partial schema contract identity"). A head plus a sequence plus an inherit marker needs one decode rule. __historyextents are immutable Lance files whose packedrecordis fixed per format (history.rs#L1-L22). Addingschema_sequencethere is a second format change beside the__manifestrow. It needs its own reader rule ("an extent without the child reads 0"), and since the packed struct can't hold nulls, apresentbit.
Neither changes the design, but the stamp-17 description should list both so the implementation and the crossversion_upgrade.rs row cover them.
There was a problem hiding this comment.
Addressed by dropping it. The sequence lives only in the schema_contract row. The merge orders the two sides by their live rows and reads the base by content, so lineage rows, the buffered-row inheritance and __history extents are unchanged.
Review: RFC "Schema apply with live branches"Verdict: request changes. The core design is sound, and I'd keep it: a stored schema sequence beside the contract head, a per-table stored signature over base/source/target, and taking a whole version only when one side changed it. There are two design gaps and a few spec gaps to close before this moves to I checked the RFC against current P1: fix before acceptance
P2: specify in the RFC
P3: nits
Claims verified against code
Checks run on the PR head
Summary: request changes. The design holds up. The main issues are that index maintenance on a branch can flip apply and merge outcomes (an invariant 7 / deny-list conflict), that snapshot reads have no binding rule once the writing branch is deleted or recreated, and that the ledger digest, lineage encoding and stamp-timing details are unspecified. |
Address PR review. Refusals now depend only on changed rows, not on registrations that index maintenance or compaction can move. A merge across contracts brings the base and both sides to the newer contract with schema apply's metadata-only evolution and runs today's merge, so nullable adds, renames and full-text declarations merge with ordinary conflicts and only drops of changed data refuse. The apply-time check runs the same rule. Snapshot reads keep main's contract, the sequence lives only in the schema_contract row, the ledger digest stays content-only, and the stamp-fence and branch-list wording is corrected.
| `branch merge --dry-run` runs the same capture and classification and reports | ||
| the same outcome, including `schema_diverged` and the contract the merge would | ||
| publish, without effects. |
There was a problem hiding this comment.
P1 (blocking): merge preparation is a durable write, but the RFC uses it in operations that are reads today.
Merge preparation commits detached Lance versions (L294-307). Logical change also prepares the base before it compares rows (L285-287). Three surfaces reach that code:
branch merge --dry-run. Today it is a read. It takes no schema permit, no branch gate and no merge-input tag. It "stages only to process-local scratch, and commits and publishes nothing". Transports authorize it as a read of both branches (merge.rs#L5009-L5015,#L5086-L5093).- Schema preview, plan and the stored deployment preview. Today they only plan (
schema_apply.rs#L186-L195). The deployment path captures its intent "without graph effects" (omnigraph.rs#L1240-L1242). L94-97 says these report the same branch list as the apply check. - The refusals. L260-261 refuses "before any effect", and L366-367 runs the apply check "before any table effect". A cross-contract Logical change can need a prepared base first, so a refusal can leave detached versions behind.
As written, a read-authorized actor can cause table commits on shared datasets, and "without effects" is not true.
Please choose one rule and state it:
- Comparison never commits. Then comparison needs a read-time mapping by stable property id. That reopens the alternative rejected at L564-567, because there would be two conversion paths. The other option is to show a public Lance 11 surface that reads an uncommitted
Project/Mergeresult. I did not find one:StagedWriteadds fragments to a scan of the committed schema, not a new schema (table_store.rs#L776-L789). - Comparison commits. Then name the staging witness that each prepared version records. State that dry run and preview write storage, and update their authorization and the dry-run contract. Add rows that prove no
__manifestversion names these versions and thatcleanupreclaims them.
It also helps to say when preparation is needed at all. The branch's contract was written by some main commit. That commit is a common ancestor of the branch and main. When the merge base descends from it, the base and the branch share a contract, because sequences never decrease along ancestry. In that case the apply check needs no preparation. Only a criss-cross base can differ.
There was a problem hiding this comment.
Addressed by removing preparation. The merge takes the newer side's version of a migrated table whole and compares rows only between versions of one shape. Dry runs, previews and the apply check therefore read committed versions only, and refusals happen before any effect. Read-time mapping is deferred to Future work relaxation 2.
| to it (Merge preparation). | ||
| 4. The merge runs today's classification, staging, validation and | ||
| publication against the merged catalog, matching tables across the three | ||
| versions by stable identity. | ||
|
|
There was a problem hiding this comment.
P1 (blocking): "today's merge unchanged" loses every merge fast path once versions are prepared.
Today's merge picks its route for each table by registration equality on the pins (merge.rs#L5338-L5352):
- source equals target, or base equals source: skip the table;
- base equals target: adopt the source's version (
classify_adopt, or the proven-insert path for Blob tables); - otherwise: the general three-way walk.
A prepared version is a new detached version, so its registration never equals another pin. If step 3 runs before step 4, every prepared table takes the general walk.
Example. Main adds a nullable column to Person. A branch wrote only Company. A catch-up merges main into the branch. The branch's Person pin equals the base's pin, so today's rule adopts main's Person version. With preparation first, the merge prepares the base and the target. Then it restages main's whole Person delta onto a branch-local version. The rows are correct, but the cost changes from O(1) to O(delta). The branch also stops sharing main's version and index coverage, so later merges lose the fast path too. A catch-up of an active branch is the motivating case, so this is the common path.
The lineage gate has the same problem. It requires one Lance schema across the pins (#L2414-L2428). Lance Field equality includes id (field.rs#L135-L138). plan_schema_evolution gives each new column an id above that version's own max_field_id (table_store.rs#L4605-L4607). Two lines can therefore give the same added column different ids. For example, after the branch forked, main added a column, wrote rows, and dropped it again. The gate then falls back to the ordered walk.
Suggested rule.
- Select the route from the unprepared pins.
- Prepare only the versions that route reads or publishes. Adopt then needs at most the source's preparation, and none when the source already holds the merged contract.
- After preparation, let the gate compare stored signatures, not Lance field ids. This is sound: a fragment that both versions share lists only ids at or below both lines'
max_field_id, so a new column is absent from it on both sides.
Add route assertions (adopt versus walk, ordered_cursor_scan_calls) to the merge_truth_table.rs and merge_fast_forward.rs rows. This also changes the "less merge-specific code" argument at L560-563, so please restate it.
There was a problem hiding this comment.
Addressed. Route selection uses the real registrations, as today. A migrated table resolves to a skip, or to adopting the newer side's version, so an untouched catch-up is a pointer change. The merge_fast_forward.rs row asserts the route and ordered_cursor_scan_calls. The field-id point no longer comes up, because no prepared version is compared.
| Whether a side changed a table since the base depends only on rows. The | ||
| existing cheap paths answer first: equal registrations, then the merge's | ||
| lineage candidate discovery, which compares the base's and the side's data, | ||
| overlay and deletion files by identity and reports no candidates when they | ||
| match. Index-only commits and schema evolution keep those files, so they cost | ||
| nothing here. | ||
|
|
||
| Only when candidates remain does the merge compare them. It brings the base's | ||
| version to the side's signature with the same preparation and runs the ordered | ||
| comparison over the candidates. Compaction therefore changes cost, not the | ||
| answer. The comparison runs under the merge's existing memory and spill | ||
| bounds, and only for a table where a lossy step meets a side whose files | ||
| differ from the base's. |
There was a problem hiding this comment.
P2: the cheap path does not run on every table, so a "cannot prove" refusal can still depend on physical work.
The lineage gate falls back to the full ordered walk for:
- Blob tables (
merge.rs#L2343-L2346); - pins at different table paths, for example pre-v11 table forks (
#L2350-L2361); - pins without stable row ids or the exact-id key (
#L2377-L2407); - pins whose Lance schemas differ (
#L2414-L2428). The comment there gives the reason: a metadata-only schema change keeps fragments identical while the row comparison sees every row as changed. So discovery cannot "answer first" when the base and the side differ in signature. Preparation has to come first, which leads to the effect question in the comment at L140.
On these tables, any registration change takes the walk. An index-only commit is enough, for example ensure_indices_on(branch) on a Blob table. The apply check has a per-branch budget on comparison work and refuses with "cannot prove" when the budget runs out (L388-392). So an index build on a branch can turn a successful apply into a refusal. L500-501 and L518-520 say refusals never depend on index coverage.
Please do one of these:
- extend the file-identity proof to these tables for this predicate; or
- qualify the statement: the answer depends only on rows, and only the budget refusal can depend on physical state.
Also add a schema_apply.rs row: a Blob table, an index build on the branch, then a drop on main, with the expected outcome.
There was a problem hiding this comment.
Addressed by qualifying the claim. The answer depends only on rows. The budget refusal (cannot prove, branch_check_unproven) is the one result physical state can reach, and it is named in the user section and under invariants 7 and 11. Logical change now also says that a discovery which does not complete falls through to the ordered comparison. The Blob table, index build and drop row is in the schema_apply.rs tests.
| sequence. Ordinary writers detect a contract change twice: write revalidation | ||
| compares the captured contract identity, and the exact graph-head | ||
| precondition fails because every contract change advances the head. Catalog |
There was a problem hiding this comment.
P3 (alignment with #785, optional): this sentence relies on the exact graph-head precondition as a second fence. The group commit draft composes entries staged against older pins onto a later pin (its blocked_on guard, group_commit_composes_inserts_staged_against_older_pins_onto_a_later_pin), so that precondition stops applying per entry. Under this RFC, a catch-up merge can change a branch's stored columns between an entry's capture and its batch's publication.
Please state that revalidation of the captured contract identity, sequence included, is the fence that must hold for each entry. A batch must refuse an entry captured under another contract. One sentence here and one in #785's admission rules is enough.
There was a problem hiding this comment.
Added under Merge publication: a batching publisher must refuse an entry captured under another contract identity, sequence included.
|
|
||
| | Owner | Proof | | ||
| |---|---| | ||
| | `tests/schema_apply.rs` | Apply with live branches leaves their pins and contracts; an add, rename or full-text `@index` never refuses; a drop refuses only for a branch that changed the table, naming it; an index rebuild or compaction on the branch, before or after the apply, never changes the outcome; preview lists affected branches; budget and walk-limit refusals; incarnation recheck | |
There was a problem hiding this comment.
P3 (nit): tests/merge_fast_forward.rs does not exist. The owner is crates/omnigraph/tests/integration/merge_fast_forward.rs (see the testing guide). All other owner paths in this table resolve at this head.
There was a problem hiding this comment.
Fixed: the path is tests/integration/merge_fast_forward.rs.
Review at
|
…ranches Address the second PR review. Preparing every version wrote to shared datasets inside dry runs, previews and the apply check, and defeated the merge's skip and adopt routes. A merge now takes the newer side's version of each migrated table whole and refuses when the older side changed it, comparing rows only between versions of one shape, so checks stay reads and catch-up keeps pointer adoption. Upgrading the older side, read-time mapping and narrower drops become Future work relaxations that only turn refusals into merges. Align with group commit, branch creation at a commit and branch details, and fix a test owner path.
…ons-active-branches-fb6101 # Conflicts: # docs/rfcs/README.md
Recommendation: request changesThis RFC lets The rule never combines rows written under two schemas. For a table whose stored columns or full-text declarations differ, the merge takes the newer side's version whole when the older side did not change that table's rows since the merge base. Otherwise the merge returns That removes the two blocking defects from the review of Earlier findings on
|
|
Alignment note from #1016 (branch create at a commit), following its review. The RFC already says the start refuses a commit whose contract differs from the source's current one. Three details that the relaxation will depend on:
One more consequence of the sequence joining the contract's identity: the start compares |
…y with live branches Address the maintainer review of 4fb1573: the apply check refuses exactly when the merge rule's condition 2 fails, including a table a branch can already not merge, and names cannot prove as the third result. Restore the keys-only validation delta for an adopted migrated table, state that a refusal refuses the whole merge, that an unfinished lineage discovery falls through to the ordered comparison, and that a shared version is checked against the merged signature. Describe branch creation at a commit as it landed, recheck the Lance facts on 13.0.0, and record the full-text refusal and constraint removal with a dropped property.
…ons-active-branches-fb6101
|
Reply to the review of Blocking: one success predicate. The user section and apply-check step 4 now share one rule. The apply refuses exactly when, for a table it changes, the merge rule's condition 2 fails for some live branch: the branch changed the table since its merge base, or the base holds the table in another shape. A table the branch could already not merge after an earlier apply fails condition 2 again, so it refuses every later apply that changes it. The remedies are to delete the branch or redo its work on a branch created from main. Optional points:
Also fixed while rechecking: the narrowed revision had dropped the keys-only validation delta. Today's adopt route compares the base's and the source's rows by column name, which is the cross-shape comparison the prototype's add and rename cases hit. For an adopted migrated table, the delta now reads only #1016. It is now described as it landed. A start compares against the contract at the captured head of the branch that wrote the commit, writes no contract row, and refuses a difference as The RFC records the narrower refusal this design permits: refuse only a differing contract without a sequence. A start with a sequenced older contract is a lagging branch unchanged since its merge base, the commit itself. I checked that against the merge rule and the apply check; both apply unchanged. The rename-and-back case is noted: it compares as a different contract, and the narrower refusal allows it. Main through
|
Recommendation: request changesWhy. One live branch blocks every schema apply. Long-lived branches block every migration. What. Main can apply a schema while other branches stay live. Each branch keeps its contract until a merge publishes a newer contract on that branch. How. For a table whose stored columns or full-text declarations differ, the merge takes the newer side's version whole. It refuses when the older side changed that table, or when the base holds another shape. This revision gives the apply that same refusal, and it builds the adopt delta from keys only. Impact. This pull request changes documentation only. Acceptance is a storage format change: stamp 17, with no in-place downgrade. The one success predicate from the review of BlockingThe keys-only delta can drop an edge that keeps its id and changes an endpoint.
That sentence has two readings.
The first reading can publish an edge to a missing node. Validation checks the target plus the delta. It does not read the adopted version as the merged edge set ( This shape is a supported write. An edge with no Example. The base has edge Write the second reading. A surviving id whose An edge whose Optional
Tradeoff and liabilityThe one predicate removes a split between the apply check and the later merge. The keys-only delta removes the cross-shape column compare that the add and rename cases hit. The first reading also removes the endpoint move from the validation input. That can publish a dangling edge. The repair stays in the existing delta and the existing evaluator. It adds no second compare path. Five later migration kinds can keep one signature test and one key delta. A later relaxation still only turns a refusal into a merge. Robustness stays ahead of the cost of a key compare. The AlignmentI read the open drafts #1015, #884, #951, #1017 and #1093.
The Lance 13.0.0 claims in this revision hold. Earlier findings
ValidationHead This pull request does not change runtime behavior. There is no GQT or Rust regression to run before and after. The keys-only rule is not implemented. A temporary logic test cannot fail on the old commit for this reason and pass on this head. The required proof is the Rust row named above, in the existing owner Code inspection at
Lance 13.0.0 source, unmodified crates.io package:
Local checks on this head, in a detached checkout:
CI on this exact head: run 39518878387 succeeded. Check AGENTS.md Links, the workflow pin check, the dependency guard, the PR title, and storage-upgrade compatibility passed. Test Workspace, clippy, and rustfmt were skipped. GQ Logic Tests run 39518878327 succeeded because "Skip when no GQT input changed" ran. The GQT test steps were skipped. That is not a logic-test run. Not done: no Inline comments (not posted)
Blocking: a keys-only delta of inserted and deleted ids drops a stable edge id whose endpoint moved.
Validation uses the target plus the delta (
Optional: the comparator reports a column missing on the right, relative to the left row.
|
What & why
Adds a draft RFC that allows
schema applyon main while other branches are live. Today apply refuses unless main is the only branch, so every long-lived branch blocks every migration.Under the proposal:
schema_divergedwhen the older side changed such a table since the merge base. Schema apply refuses when a live branch changed a table it would change.schema_contractrow, written by each branch's first publication. It fences older binaries on every scan and publication.Backing issue / RFC
Checklist
Local verification
python3 scripts/check-docs.py(docs venv) — Documentation OK (333 Markdown files checked)bash scripts/check-agents-md.sh— OKtyposon the changed files — cleanNotes for reviewers
branch listcost.9575b044were checked; none affects the design. Alignment notes cover fix(engine): historical queries bind properties by stable identity #959, feat(changes): graph diff, branch review and merge dry run #966/RFC 0030, rfc: polymorphic types with interfaces, unions and polymorphic edges #884, rfc: granular permissioning #951, rfc: branch operations v2 #677, feat(branch): start a branch at a graph commit #1016 and feat(server,cli,mcp): report each branch's head, start and times in the branch listing #1012.SchemaApplyin addition toBranchMerge.blocked_on: a checked-in cost instrument for the apply check.