feat(sql): add REPLACE ... WHERE for atomic predicate-scoped overwrite - #756
feat(sql): add REPLACE ... WHERE for atomic predicate-scoped overwrite#756puchengy wants to merge 6 commits into
Conversation
Adds a `REPLACE <table> WHERE <predicate> AS <query>` SQL command that
atomically replaces the rows matching the predicate with the result of the
query, in a single Lance `Update` commit (one table version). This is the
predicate-scoped analogue of Iceberg's `INSERT OVERWRITE ... PARTITION(...)`:
Lance has no partition spec, so the region to replace is chosen by a row
filter rather than a declared partition.
The delete of matching rows and the append of new rows land in one
`Operation.Update{removedFragmentIds, updatedFragments, newFragments}`, so
readers never see a deleted-but-not-reinserted state and a crash cannot leave
the region half-written. Fragments fully covered by the predicate are dropped;
fragments that only partially match keep their non-matching rows via a
deletion vector.
Implementation reuses the existing distributed write pipeline: the predicate
rides through as an internal write option and `LanceBatchWrite.commit()`
branches to build the atomic `Update`. Adds the grammar rule + per-version
AST builders, the `ReplaceWhere` logical plan, strategy mapping, and
`ReplaceWhereExec`, plus unit tests and docs.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses lance-gatekeeper review feedback on the REPLACE ... WHERE command: 1. Predicate/query split: the grammar previously delimited on the first AS token, which broke predicates containing AS (e.g. CAST(dt AS STRING)). The grammar now captures everything after WHERE as one raw region, and the AST builder splits it at the first *top-level* AS via ParserUtils.splitReplaceBody (honoring parentheses, string/backtick literals, and line/block comments). Adds a CAST-in-predicate regression test. 2. Deletion planning memory: LanceBatchWrite collected a boxed Integer per matched row across all fragments, so a partition-scale replace could OOM the driver. Deletions are now accumulated as a compressed RoaringBitmap per fragment and materialized to a row-index list one fragment at a time, keeping driver memory bounded regardless of total matched row count. This mirrors the existing SparkPositionDeltaWrite pattern. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Pushed 498fdef addressing both gate-keeper blockers:
Local: spotless + checkstyle clean; |
Second round of lance-gatekeeper feedback on REPLACE ... WHERE: 1. Nested block comments: splitReplaceBody's block-comment skip stopped at the first `*/`, so an `AS` inside an outer comment (e.g. `/* outer /* inner */ AS ... */`) was mistaken for the separator. Comment skipping is now depth-aware, matching Spark's nested-comment behavior. Adds a nested-block-comment regression test. 2. Per-fragment deletion memory: a fully-matched fragment is now dropped without materializing any per-row list — matched cardinality is compared against the fragment's live row count and, on equality, the fragment id goes straight to removedFragmentIds. This covers the common whole-partition-per-fragment case with memory independent of fragment size. Only genuinely partial fragments materialize a list, bounded by a single fragment's row count (the native deleteRows(List<Integer>) API's inherent limit). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Pushed a79e7b6 for the second-round findings:
Local: spotless + checkstyle clean; |
Third round of lance-gatekeeper feedback on REPLACE ... WHERE: - splitReplaceBody's line-comment skip only stopped at \n, but Spark ends a line comment at \r too, so a predicate ending in `-- ...\r` swallowed the AS separator. Line comments now end at either \r or \n. Adds a regression test. - Use RoaringBitmap.getLongCardinality() when comparing against a fragment's live row count in the full-match fast path, avoiding int truncation for very large fragments. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Pushed 4ce419b. Line comments (fixed): Partial-fragment materialization (won't fix in this PR): I don't think this is a real concern, and I'd like to resolve it as accepted rather than block on it. The reasoning:
Proposal: land this connector PR with the full-match fast path (covers the intended partition-aligned workload with bounded memory), and track a bitmap-accepting |
|
ACTION NEEDED The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification. For details on the error please inspect the "PR Title Check" action. |
|
The one red check ( |
|
Tracking issue for this feature: #762 (support partition-scoped INSERT OVERWRITE semantics via SQL). It captures the motivation (Hive/Iceberg migration relying on partition-scoped |
Adds a regression test verifying REPLACE ... WHERE is correct when the target fragment already carries a deletion vector: the full-fragment drop compares matched live rows against getNumRows() (physical − deletions) and a filtered scan enumerates only live rows, so matching all remaining live rows drops the fragment cleanly rather than mis-counting against the physical total. Verified against lance-core semantics (getNumRows is logical; deleteRows takes physical offsets = _rowaddr low 32 bits; filtered scans exclude deleted rows). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…olumns For a REPLACE ... WHERE whose predicate is a pure conjunction of `column = literal` equalities on columns that have a zonemap index, fragments that the zonemap proves are fully covered (every zone pinned to the required value, no nulls) are now dropped by id without scanning their rows. The remaining (unproven) fragments are scanned exactly as before, restricted to their ids. When every matching fragment is proven covered, the delete-planning scan is skipped entirely — replacing a partition that occupies its own fragments becomes metadata-only (O(#fragments) instead of O(rows)). Mechanics: - ReplaceWhereExec parses the predicate with Spark's parser and, only when it is a conjunction of equalities on string/integral columns, encodes the terms as JSON in a new internal write option (else the option is omitted). - ReplaceCoverage proves per-fragment coverage from getZonemapStats: a fragment qualifies only if, for every equality column, all its zones have min == max == value with zero nulls. Multiple single-column zonemaps combine by intersection for multi-column predicates. - Correctness is fail-safe: any missing zonemap, unproven zone, non-equality predicate, or value/format mismatch excludes the fragment and defers to the exact scan, so the optimization never changes which rows are replaced. Tests: zonemap-covered single-partition drop, multi-column zonemap, and a range-predicate fallback, each asserting results identical to the scan path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Pushed 497011c implementing the metadata-only fragment removal follow-up (previously noted on #762). For a
New tests cover the covered-partition drop, multi-column zonemap, and range-predicate fallback (each asserting results identical to the scan path). 12 |
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
The new zonemap optimization regresses case-insensitive equality predicates before the exact fallback. Restore fail-safe behavior by resolving target columns canonically or treating an ineligible metadata lookup as a scan fallback.
The product contract from the prior review also remains open. Maintainers still need to choose general Lance predicate replacement versus Spark overwrite-by-filter or partition overwrite, and define inserted-row constraints plus Spark-versus-Lance predicate semantics. The single-version Update direction and the previously accepted large-partial-fragment driver-memory risk are otherwise unchanged.
| * zonemap index. | ||
| */ | ||
| private static Set<Integer> fragmentsPinnedToValue(Dataset ds, String column, String value) { | ||
| List<ZoneStats> zones = ds.getZonemapStats(column); |
There was a problem hiding this comment.
This metadata lookup is not fail-safe for case-insensitive column references. ReplaceWhereExec serializes the raw UnresolvedAttribute spelling, but Lance's filter planner resolves ordinary names against the schema case-insensitively while getZonemapStats requires the exact schema name. Consequently, WHERE DT = ... on a dt column now aborts here before the exact scan, even when no zonemap exists. Canonicalize the equality column against the target schema before probing, or treat lookup failure as ineligible for the optimization and fall back to the exact scan; retain a mixed-case regression.
Reproducer
I added and ran this test against 497011c:
@Test
public void testReplacePredicateColumnIsCaseInsensitive() {
TableOperator op = new TableOperator(spark, catalogName);
op.create();
op.insert(Arrays.asList(Row.of(1, "2026-08-01", 100)));
op.replace(
"DT = '2026-08-01'",
"SELECT 2 AS id, '2026-08-01' AS dt, 200 AS value");
op.check(Arrays.asList(Row.of(2, "2026-08-01", 200)));
}./mvnw -pl lance-spark-3.5_2.12 -Dtest=ReplaceWhereTest#testReplacePredicateColumnIsCaseInsensitive -Djava.io.tmpdir=/home/repo/.gate-tmp.PKyukZ test
Expected: the case-insensitive predicate replaces the row. Observed: IllegalArgumentException: Column 'DT' not found in dataset schema from this call; the submitted 12-test suite passes because every equality uses exact-case column names.
What
Adds a
REPLACE <table> WHERE <predicate> AS <query>SQL command that atomically replaces the rows of a Lance table matching<predicate>with the result of<query>, in a single table version.Why
This is the predicate-scoped analogue of Iceberg's
INSERT OVERWRITE ... PARTITION(...). Lance has no partition spec, so today the only ways to replace part of a table are:DELETEthenINSERT— two separate commits, so a reader can observe the region deleted-but-not-reinserted, and a crash between them leaves it permanently missing.INSERT OVERWRITE— atomic but replaces the entire table.MERGE INTO— atomic, but matching on non-unique columns (e.g. a date) is a per-partition cartesian join.REPLACE ... WHEREfills the gap: a single atomic delete + append scoped by an arbitrary predicate.How
The delete of matching rows and the append of new rows are committed as one Lance
Operation.Update{removedFragmentIds, updatedFragments, newFragments}— so readers never see a half-applied state, and a failure cannot leave the region half-written.removedFragmentIds).updatedFragments), so a fragment holding multipledtvalues is handled correctly.Implementation reuses the existing distributed write pipeline. The predicate rides through as an internal write option and
LanceBatchWrite.commit()branches to assemble the atomicUpdate(the same commit primitive already used by the row-level position-delta path). Grammar captures the predicate and query as raw text and re-parses the query with Spark's own parser via the delegate.Files
LanceSqlExtensions.g4(newREPLACE ... WHERE ... ASrule; adds aWSskip rule + catch-all token so the raw predicate/query regions tokenize cleanly).visitReplaceWhereadded to the AST builder in all 5 per-version modules (3.4 / 3.5 / 4.0 / 4.1 / 4.2).ReplaceWherelogical plan,LanceDataSourceV2Strategycase,ReplaceWhereExec.LanceBatchWrite.buildReplaceOperation+ areplaceWhereoption onLanceSparkWriteOptions(excluded fromtoWriteParams, never forwarded to native).docs/src/operations/dml/replace.md.Testing
New
ReplaceWhereTest(base test inlance-spark-base, run under 3.5): single-partition replace, other-partitions-untouched, partially-matching fragment (deletion vector), replace of a non-existing partition (append), and single-atomic-commit (exactly one version bump). Ranspotless:check+checkstyle:check(clean) and the full base + 3.5 suite locally; all pass except pre-existing environmental OOMs inAddIndexTest(external-sort memory limit in the sandbox), which are unrelated to this change.🤖 Generated with Claude Code