perf(schema): apply property changes as metadata-only commits; optimize erases dropped columns

[ragnorc]

What & why

Schema apply rewrote every table whose properties changed. It scanned the whole table into one batch and read every managed Blob value into memory (batch_for_schema_apply_rewrite, schema apply's rebuild_blob_column). Peak memory grew with the table's Blob bytes. Schema deployments now run inside the live server (#861), so one large Blob table could exhaust the server. That breaks invariant 11. The rewrite also discarded index coverage and refused ranged external Blob references.

The planner never allows a property change that needs a data rewrite: required adds (OG-MF-103) and type changes (OG-MF-106) are refused. And since --allow-data-loss is gone (#810), every drop behaves the same. So the rewrite is deleted rather than made streaming.

Schema apply is metadata-only.

• Adding a nullable property, renaming one, and dropping one are now detached Lance commits that keep every data file:
  • Operation::Project handles renames and drops;
  • an all-null Operation::Merge handles adds, with missing columns read as null.
• They are published once with the accepted schema contract, through the same door as before.
• Lance refuses a field-id rename inside a Merge (merge_schema_valid, also on detached commits). A table that both renames or drops and adds therefore gets two detached commits, Project then Merge, and the pin names the last.
• TableStore::plan_schema_evolution reads only the manifest and refuses conflicts before any effect. stage_schema_evolution is the new sealed primitive, registered in forbidden_apis.
• Field ids, stable property markers, the primary key and Blob-v2 metadata are preserved. New ids sit above max_field_id, and the evolved schema keeps catalog column order, which Lance merge-insert's full-schema path requires.

Measured peak (instrument in compaction_memory.rs, since moved by #894 to benchmarks/deferred/ and no longer run):

Blob bytes in table	Before	After
64 MiB	72.1 MiB	5.1 MiB
128 MiB	136.1 MiB	5.1 MiB

Explicit erasure of dropped properties. A drop is logical at apply: the column's bytes stay in current data files.

• optimize now always rewrites every fragment that still stores a dropped field id, alongside Lance's own size- and deletion-based selection. Detection reads manifest metadata only; each such fragment becomes a single-fragment task added to Lance's own plan, using the public CompactionPlan/TaskData, with the existing per-row Blob batch bound.
• cleanup then reclaims the old files.
• Index coverage carries over: with stable row ids, Lance's Rewrite moves every row-id-domain index's fragment bitmap onto the new fragments, so erasure costs no index rebuild.
• plan_table_compaction refuses options that set a Lance per-run source budget (max_source_*), which Lance would apply to its own tasks only. The engine sets none.
• So the contract is: drop, then optimize, then cleanup erases the values. A test asserts that no file under the graph root holds the dropped bytes afterwards.

Compatibility and behaviour changes

• Existing indexes keep covering their rows across these steps; a renamed column's index is looked up by field id.
• Ranged external Blob references now survive schema changes, which previously refused them.
• A dropped property's values are no longer freed by cleanup alone; they need optimize first. The merged, unreleased allow-data-loss-removed fragment and the schema guide are corrected to say so.
• Fragments: schema-apply-metadata-only.performance, dropped-property-storage.changed.

Testing

• lance_surface_guards: detached Project/Merge on a Blob-v2 table keeps data files, payloads, null/empty, ids and metadata; one Merge can't carry a rename; compacting a chosen fragment drops the dropped column's bytes, including Blob sidecars.
• schema_apply: add, rename and drop on Blob and plain tables — data files unchanged, zero payload reads, index coverage, markers, a ranged external reference kept, historical shape readable, one publication, then keyed writes, queries, optimize and export.
• maintenance: drop a String and a Blob property (inline, packed, dedicated) on one large fragment with no deletions. optimize rewrites it in one commit and a second optimize is a no-op. After cleanup --keep 1 no file contains the dropped bytes. A table with nothing to do isn't scanned. The test fails with the new selection disabled.
• changes: no spurious row changes across a metadata-only schema change.
• detached_commit_matrix: schema-apply cells, including a window between the two chained commits; all 78 cells pass with OMNIGRAPH_MATRIX=full.
• Suites: engine lib 388, schema_apply 39, lance_surface_guards 63, maintenance 53, changes 50, writes 56, failpoints 118; GQT 255; compiler 434; CLI cli_cluster_e2e 11 and cli_schema_config 27. Clippy (both graphs), fmt, docs and release-note checks all pass.
• Not run: the DST suite; each commit built individually.

Decisions

The six design questions for @azimafroozeh are decided, with the reasoning in the decisions comment:

1. A drop is logical at apply; optimize then cleanup erase the bytes.
2. Erasure is always on in optimize, with no flag.
3. A table that renames or drops and also adds gets two chained detached commits, Lance's own Project and Merge shapes.
4. Lance per-run source budgets are refused in code rather than silently bypassed.
5. Planner refusals are manifest_internal.
6. Erasure runs on main only; branches and tags are retained references.

@aaltshuler (runtime / deployments): schema apply's peak no longer scales with table size, so a deployment needn't reserve table-sized memory for it. I didn't amend the server-runtime or settlement RFCs; tell me if they should mention it.

[ragnorc]

Recommendation: request changes for the erasure instructions below. I found no blocking defect in the new storage path.

This PR makes schema changes practical on large graphs, especially graphs with large Blob values. The old path read and rebuilt each affected table. The new path changes Lance metadata and preserves existing files, row IDs, and surviving indexes. Added properties read as null. A drop hides the property immediately, but optimize must rewrite its files before cleanup can erase the stored values.

Finding — P2, correct the branch retention instructions.

Merging a branch does not release its old table pin. A branch created after a property drop can still retain files containing that property's bytes. Merging it, optimizing main, and running cleanup does not erase those bytes while that branch remains. The inline comment gives the reproduction and required correction.

Contract, workload, and tradeoffs

• The target workload has many versioned tables, frequent small writes, and potentially large Blob values. Removing payload work from schema apply addresses the cause of its memory growth. The supported deployment still has one writer process. Schema apply still requires main to be the only live branch. This PR adds no distributed writer guarantee.
• Renames and drops use Project. Additions use Merge with fresh field IDs. A combined change needs two detached commits because Lance rejects field-path changes inside Merge. This adds one metadata commit and a failure window. Graph readers see neither commit until the existing graph publication installs all pins and the schema together. See the publication loop.
• Erasure now costs a rewrite during optimize, even for a large fragment with no deletions. A single-fragment task fits Lance's existing rewrite model. It avoids another maintenance API or durable queue. It can copy large surviving Blob values and remove vector-index coverage, which can trigger a whole index rebuild in the same optimize.
• The extra erasure tasks bypass Lance's source budgets. Current engine callers use default options without those budgets. This is acceptable within that boundary, but future budget controls must include these tasks. See the shared compaction planner. Erasure also needs enough temporary storage for old and new files until retention releases the old files.

Liability

The direction reduces liability despite the positive total line count. It removes the whole-table batch, Blob reconstruction, and schema-specific external-range refusal. It reuses Lance's existing operations, the existing detached-commit path, and the existing collector. The added obligations are field-ID preservation, commit ordering, and explicit dropped-column detection. The schema planner centralizes the first two. The new substrate guards cover those obligations. Five similar changes should extend these shared paths, rather than add five independent rewrite systems.

Validation and limits

Reviewed head: 41ade515db21dead5cbe45985ded2d0e957eb29c, in a clean isolated checkout.

• Local baseline, Rust 1.97.1: 63 Lance guards passed, with two ignored. All 51 maintenance tests and 31 schema-apply tests passed.
• With failpoints enabled: all 39 schema-apply tests passed. All 78 schema-apply recovery cells passed, including failures between the two detached commits.
• All 50 change-feed tests passed. Formatting, documentation checks, and AGENTS link checks passed.
• The temporary retention probe failed after a real merge, optimize, and cleanup. The control passed after branch deletion and another cleanup. All temporary source edits are restored.
• CI passed on merge commit 97d37b591454a8f3326943838d6b4b6ef350deed. Its Git tree exactly matches the reviewed head. Local checks used file storage. I did not repeat cloud-backend or full-workspace tests locally.

I checked the pinned Lance 11.0.0 implementation at ab6b5bbe. The relevant code covers all-null additions, Project/Merge validation, missing-field reads, index retention, and compaction. Current upstream guides differ from this pin in places, so the source and tests control the conclusion. I did not independently measure the PR's heap figures or production latency.

Optional documentation improvement: qualify the claim that apply time and memory do not grow with table size. Payload size no longer drives them, but fragment manifests and index metadata still do. The new Merge clones the fragment inventory (source). That distinction matters for graphs built through many small commits.

[ragnorc]

Thanks. All findings are addressed in five commits on top, rebased onto main (v0.12.0):

• P2, branch retention: fixed as in the inline reply. The erasure test now includes the branch case.
• Cost wording: the schema guide, docs/dev/writes.md and the performance note now say apply cost no longer grows with stored rows or payload bytes, and still grows with fragment, column and index metadata.
• A second review (gpt-6-astra) found a real bug. After a nullable Blob property is added, older fragments have no data file for it, and Lance reads the column as null. row_compare looked up the field's data file before classifying the descriptor. Diffs, the change feed and three-way merges over rows in old fragments therefore failed with fragment … has no data file for blob field ….
  • Fix (b130f793): classify first, and resolve the data file only for managed values. A managed value with no backing file is still BlobIntegrity.
  • Tests (21dd2790): the schema-boundary change-feed test now adds attachment: Blob? and updates rows in the old fragment. A new three-way merge test, branch_merge_compares_rows_written_before_a_blob_property_was_added, covers merges. Both fail without the fix.
  • The other Blob paths were swept: single-cell reads, export, update/merge carry, merge and compaction sizing, the collector, and upgrade. They already classify first or don't depend on per-field files.
• Release notes: v0.12.0 shipped, so the earlier edit to allow-data-loss-removed.breaking.md is reverted; it's byte-identical to the published note, and release_notes.py verify --version v0.12.0 passes. The drop change is stated only in this PR's dropped-property-storage.changed.md, relative to v0.12. release.json is moved to v0.12.1, identical to feat(cluster): simplify live deployment and delete managed graphs #878, so check-docs passes; whichever PR merges first, the other merges cleanly.

Engine suites: 1,012 passed. Also CLI cluster/schema tests, GQT 255, both clippy graphs, and the docs and release-note checks.

[ragnorc]

@azimafroozeh I need your call on six decisions in schema_apply and table_store. Each has my recommendation, so a plain "agree" works. CI is green, apart from the still-running Storage Upgrade Compatibility check, and the review findings so far are addressed.

1. Dropped-property storage. A drop is now logical at apply. Its values stay in the current data files until optimize rewrites those fragments; then cleanup reclaims the old files. In v0.12 the drop's rewrite did this at apply, at the cost of holding the table in memory. Recommend: accept. The alternative is to keep a physical rewrite for drops only, which brings back the unbounded rewrite this PR removes.
2. Erasure in optimize: always on, or behind a flag. plan_table_compaction always adds a single-fragment task for every fragment that still lists a dropped field id. That costs one rewrite per affected fragment, once per drop. On a vector-indexed table, a rewritten large fragment loses vector-index coverage, so the same optimize rebuilds that index. Recommend: always on. Erasure shouldn't depend on an operator remembering a flag, and the cost is paid once. If you'd rather keep optimize's cost predictable, the alternative is optimize --erase-dropped, with docs saying erasure needs it.
3. Two detached commits when a table renames or drops and also adds. Lance's merge_schema_valid refuses a field-id rename inside a Merge, including on detached commits. So the table gets Project (renames and drops), then Merge (adds) chained on it. Both are published once, and the matrix covers the window between them. A drop plus an add alone could go in one Merge, but I kept one shape. Recommend: keep.
4. Erasure tasks and Lance's budgets. The added tasks are single-fragment, so adjacent small fragments holding dropped data are rewritten but not coalesced. They also aren't counted against max_source_fragments / max_source_rows / max_source_bytes. The engine sets none today, and the function's comment says any future budget must count them. Recommend: accept for now.
5. Error type for storage-planner refusals. plan_schema_evolution refuses type, nullability or stable-marker conflicts as OmniError::Manifest, mirroring the system-column upgrade preflight. The compiler never emits such steps, so reaching that refusal means internal inconsistency. Recommend: manifest_internal, unless you want them typed for callers.
6. Branches and tags. optimize erases on main only. A branch or tag created after the drop keeps the old files until it's deleted; merging releases nothing, as the earlier review showed. This is documented. Recommend: accept. Running erasure per branch can be a follow-up if needed.

I'll mark the PR ready once you've answered these and the running re-review is clean.

[ragnorc]

The red Storage Upgrade Compatibility check here is not caused by this PR. main has failed the same three crossversion journeys since #876: the same errors appear on its merge-queue run, on main, on the v0.12.0 tag and on #878, and this PR touches neither the CLI tests nor the upgrade code. Diagnosis and a question for @azimafroozeh are on #876. Everything else is green, and the gpt-6-astra re-review of the fixes approves with no new findings.

[ragnorc]

The red GQ Logic Tests check here is not caused by this PR. GQT (ordinary) was cancelled at its 60-minute budget. Every step that finished passed; the job ran out of time in the benchmark-parity step.

Since #894, that job builds the engine three times, because omnigraph-gqt, omnigraph-gqt-core and omnigraph-bench each resolve their own feature graph. On main this costs about 20 minutes per extra graph even with a warm cache.

This run also restored no cache. Its runner image ships Rust 1.98.1, main's cache was saved under 1.99.0, and rust-cache includes the runner's preinstalled toolchains in its key. With no cache the job needs about 70 minutes. bench/concurrent-merges timed out the same way this morning, so the rerun I started will probably time out as well.

The fix is #896: one package selection, so one engine build, which matches the cache rule in docs/dev/ci.md. Once it merges I will merge main into this branch.

[ragnorc]

Decisions on the six open questions

I re-examined each one against the invariants and the Lance 11 source and decided them, so the PR no longer waits on them. @azimafroozeh, flag any you disagree with; each is cheap to change.

Throughout: the logical schema is the contract. Bytes in data files are physical state, which maintenance reclaims and retention protects.

1. A drop is logical at apply; optimize then cleanup erase the bytes. Decided: accept.

• Physical erasure has always needed cleanup. Under the old apply-time rewrite, earlier __manifest versions still named the old files until cleanup released them, and so did any branch, tag or kept version. So the rewrite never gave a stronger guarantee; it only folded one step into apply.
• The price of that rewrite was memory proportional to the table's Blob bytes (136 MiB at 128 MiB) inside the live server, which breaks invariant 11.
• The new procedure, drop then optimize then cleanup, is deterministic and bounded.

2. Erasure is always on in optimize, with no flag. Decided: keep.

• Lance itself rewrites a fragment once its deletions pass 10% of its rows (materialize_deletions_threshold). A dropped column deletes 100% of that column's cells in every fragment, so the same rule makes every such fragment a candidate.
• A flag would leave dropped data in place indefinitely for any operator who doesn't know about it.
• The cost is paid once per drop: one rewrite of each fragment that still holds the column, with the existing per-task Blob batch bound.
• Correction to my earlier note: this costs no index rebuild. With stable row ids, Lance's Rewrite recalculates every row-id-domain index's fragment bitmap onto the new fragments (recalculate_fragment_bitmap). Only ZoneMap and Bloom indexes, the address-domain ones, lose coverage, and the engine builds neither. A single-fragment task always meets that function's all-or-nothing group rule.

3. Two detached commits for a table that renames or drops and also adds. Decided: keep.

• They are exactly Lance's own commit shapes. alter_columns (rename) and drop_columns commit Operation::Project; the all-null add_columns commits Operation::Merge.
• merge_schema_valid refuses a field-id rename inside a Merge, and putting additions into a Project would rely on behaviour Lance never uses itself.
• Atomicity is unaffected. The intermediate version is unreachable until the single publication, the collector follows the chain through read_version, and the matrix covers the window between the two commits.

4. Erasure tasks and Lance's compaction budgets. Decided: refuse budgets in code, not just in a comment.

• The coalescing concern was wrong. Lance's planner already groups adjacent small fragments into one task (CompactWithNeighbors), whether or not they hold dropped data. The engine adds a task only for a fragment no Lance task includes: one at target size, or a lone small one.
• The budget concern was real but latent. limit_tasks_to_source_budget runs inside Lance's planner, so appended tasks would bypass max_source_*.
• The engine sets no budget: it always passes CompactionOptions::default(), and dataset-config overrides apply only through from_dataset_config, which the engine doesn't call.
• So plan_table_compaction now refuses options that set any budget, as manifest_internal. A future budget then fails loudly until it counts these tasks, instead of being silently exceeded. Test: staged_tests::table_compaction_planning_refuses_source_budgets, which fails with the check removed. Commit 3c88c18; the docs/dev/lance.md row records it.

5. Error type for planner refusals. Decided: manifest_internal, already implemented.

• The compiler never emits a type change, a nullability change or a marker conflict as a schema-evolution step, so reaching the refusal means internal inconsistency. That is a defect class, not a user error.
• It also matches the compaction refusal above and the address-style rewrite check in stage_compaction.

6. Erasure on main only. Decided: accept.

• optimize compacts only main for everything (optimize_all_datasets), not just erasure. Schema apply already requires that main is the only live branch, so the only branches that can hold dropped bytes are ones created after the drop.
• A branch or tag is a retained reference by design. Erasing past it would mean rewriting the history it exists to keep.
• So the contract stays as documented in the fragment and the schema guide: delete those branches and tags, then run optimize and cleanup (e.g. --keep 1). Per-branch compaction would be a separate feature for all of optimize.

Also corrected in the PR description: the vector-index cost (see 2) and the coalescing claim (see 4).

[ragnorc]

Recommendation: approve, subject to the remaining required CI checks. This follow-up reviews cf3109fb5077f8785da36de3896500d1ef48464b. The earlier blocking documentation finding is fixed. I found no new blocking defect.

This PR makes schema changes cheaper for graphs with large tables or Blob values. It changes Lance metadata instead of reading and rebuilding all stored values. A dropped property disappears from the schema immediately. Physical erasure requires optimize, release of retained references, and cleanup.

The new commit closes a maintenance contract gap. Lance limits the tasks its planner returns. OmniGraph then adds tasks to remove dropped columns. Those added tasks could exceed a caller's limit. The shared planner now rejects all three source limits before calling Lance.

Contract and tradeoffs

• Both planning and execution use default options. They set no source limits. Thus current CLI, server, and embedded optimize calls keep their behavior. The check applies before storage-backend work and adds no cloud-specific path.
• The planner rejects limits even when a table has no dropped columns. This gives every graph table one supported contract. The cost is loss of optional incremental-compaction controls at this internal boundary. A future bounded-maintenance feature must budget the complete plan, including erasure tasks.
• These limits count source fragments, rows, or file bytes. They are not process-memory limits. Lance's byte limit also excludes Blob sidecars. This change does not bound total optimize memory, duration, or I/O. Large surviving Blob values still make erasure expensive.
• The schema guide now explains that merging a branch releases no files. It names branch deletion, tags, and retained history as erasure conditions. It also qualifies metadata cost for highly fragmented tables. These changes address the earlier review.

Liability

The incremental fix adds 56 lines and removes five, mainly documentation and a regression test. It reduces liability by turning an unsupported combination into an explicit error. It adds no queue, durable state, public API, or second budget implementation. Five similar maintenance changes should share this planner and its admission rules. They should not add separate task schedulers. The broader PR still trades cheaper schema changes for explicit erasure work during maintenance.

Validation and limits

• Inspected the exact head in a clean isolated checkout. The new merges contain no manual resolutions. The only engine changes since the previously assessed head are this check and its test.
• Checked pinned Lance 11.0.0 at ab6b5bbe46009ed78746b444df8db59a8bc5d842. Its budget function limits only its own task list. Its public planner does not load another budget from dataset configuration.
• Local checks: formatting of both changed Rust files, documentation checks (219 Markdown files), and AGENTS links passed. The checkout remains clean. I did not complete a fresh local Rust test build. A cached binary failed the refusal test, but its source provenance was unverified, so I excluded that result. The Rust test conclusion below relies on exact-head CI.
• Exact-head CI passes the workspace tests. Its log includes the new budget-refusal test, both relevant Lance guards, and the optimize/cleanup erasure test. RustFS S3 tests also pass. GQT ordinary, GQT DST, and the pinned DST suite also pass. Azurite and storage-upgrade checks remain in progress.
• I did not repeat cloud tests or heap benchmarks locally. The performance figures in the PR remain author measurements. The historical memory instrument is now deferred.

This is a COMMENT review because the authenticated account is the PR author. No new inline finding is required.