Repository navigation
Conversation
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
The composite-key model and transport change is sound, but the dependency sequence must keep the workspace publishable. Land lance-namespace#363, publish its 0.12 crates, and then replace the temporary Git dependency with that released version before this PR merges.
| # TODO: switch back to a crates.io version once lance-namespace 0.12 is released. | ||
| # Pinned to https://github.com/lance-format/lance-namespace/pull/363, which makes | ||
| # `MergeInsertIntoTableRequest.on` a list. | ||
| lance-namespace-reqwest-client = { git = "https://github.com/wjones127/lance-namespace", branch = "will/ent-2084-merge-insert-multi-column-on" } |
There was a problem hiding this comment.
This Git-only dependency makes lance-namespace fail Cargo packaging, so the workspace no longer satisfies the explicit “Check crates are publishable” contract in .github/workflows/rust.yml and cannot follow the configured publish workflow. The upstream schema change is still open and no v0.12 tag exists. Please land and release lance-namespace#363, then use the released version here.
Reproducer
$ CARGO_TARGET_DIR=/home/agent/tmp/gate-8915-package cargo package -p lance-namespace --no-verify --locked
error: failed to verify manifest at `/home/repo/rust/lance-namespace/Cargo.toml`
Caused by:
all dependencies must have a version requirement specified when packaging.
dependency `lance-namespace-reqwest-client` does not specify a version…4102) Merge insert has always taken a list of columns to match on, and local tables have always joined on all of them. Remote tables did not: any list longer than one was rejected with `MergeInsertBuilder only supports a single 'on' column`, so a composite-key upsert was impossible against LanceDB Cloud and Enterprise from Rust, Python or TypeScript. The remote request now carries `on` as a list and sends it as one repeated query parameter per column — `?on=shard_key&on=id`. That is how the lance-namespace spec encodes an array-valued `on`, so the server receives a composite key in the shape it expects. A single column still serializes to `?on=id`, exactly what clients sent before, so existing callers are unaffected. A column repeated within `on` is now rejected client-side rather than sent for the server to reject with a 400. No binding changes were needed: `Table.merge_insert` in Python and `Table.mergeInsert` in TypeScript already accepted a list, it just could not reach a remote table. Both gain a test for composite keys, and the doc comments now say what passing several columns means. Part of [ENT-2084](https://linear.app/lancedb/issue/ENT-2084/mergeinsertintotablerequest-support-multiple-columns-for-the). ## Example ```python table.merge_insert(["shard_key", "id"]) \ .when_matched_update_all() \ .when_not_matched_insert_all() \ .execute(new_data) ``` A row whose `id` matches an existing row but whose `shard_key` differs is an insert, not an update. ## Not included Java. Java callers reach merge insert through `org.lance.namespace.LanceNamespace`, whose `MergeInsertIntoTableRequest.on` is a single string until lance-namespace 0.12 ([lance-namespace#363](lance-format/lance-namespace#363), [lance#8915](lance-format/lance#8915)). There is nothing in this repo's Java SDK to change until the `lance-core` pin can move. Sending more than one column requires a server that accepts the repeated parameter ([sophon#7571](lancedb/sophon#7571)); an older server returns a 400 rather than silently merging on one column. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Merge insert through the namespace API only accepted a single column as the match key, so composite-key upserts were impossible even though `MergeInsertBuilder::try_new` has always taken a list of join columns. `MergeInsertIntoTableRequest.on` is now a list, and the directory and REST implementations pass it straight through to the builder. The REST client repeats the `on` query parameter once per column; the adapter collects the repeated values from the raw query string, since `serde_urlencoded` (which backs axum's `Query`) cannot deserialize a sequence. Requests crossing JNI and pyo3 are deserialized through `LenientMergeInsertIntoTableRequest`, which promotes a scalar `on` to a one-element list so a Java or Python SDK build predating this change keeps working against current native code. `lance-namespace-reqwest-client` is pinned to the spec branch until 0.12 is released. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…unction `LenientMergeInsertIntoTableRequest` named the serde quirk rather than the reason the shim exists. Replace it with `compat::merge_insert_request_from_json`, which takes a `serde_json::Value` and returns the model. The bridge sites deserialize to `Value` and convert, which reads shorter at all four and keeps the compat framing visible in the path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Core switches NULL join semantics on the arity of the match key: a single-column key treats NULL as equal to NULL, a composite key uses standard SQL equality where it is not. Composite keys are reachable through the namespace API for the first time here, so pin the behavior down. Makes `region` nullable in the shared test data helper and pulls the row readback into `read_composite_key_rows`, now that two tests scan the result. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replace the temporary git-branch pin now that the spec change is released, and refresh all three lockfiles. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1d3ba9f to
09eee13
Compare
The Rust `lance-namespace-reqwest-client` dependency was already bumped to 0.12.0 in #8915, which changed `MergeInsertIntoTableRequest.on` from a single string to a repeated field so `merge_insert` can express a composite key. The Java `lance-namespace-core`/`lance-namespace-apache-client` dependencies in `java/pom.xml` were left behind at 0.11.1, where the generated model still only has a single `on` string. This PR bumps those two Java dependencies to 0.12.0 to match, so `org.lance:lance-core` can send composite `on` keys as a real `List<String>` instead of relying on the legacy comma-joined string encoding. ## Not included Wiring an actual `mergeInsert` Java client method that uses the new `List<String>` `on` field is separate downstream work (in `lancedb`'s Java bindings), not part of this dependency bump. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Merge insert (upsert) through the namespace API only accepted a single column as the match key, so there was no way to upsert on a composite key — even though Lance core has always supported it (
MergeInsertBuilder::try_newtakes a list of join columns). The limitation was purely in the request and transport layer.MergeInsertIntoTableRequest.onis now a list of Lance field paths, and both implementations pass it straight through to the builder. An empty list or a repeated column is rejected withInvalidInput.This is the implementation half of lance-format/lance-namespace#363, which made the same change in the spec and the generated clients.
Example
Breaking changes
MergeInsertIntoTableRequest.onchanges fromOption<String>toOption<Vec<String>>. Rust callers passing a single column need to wrap it in a list.The HTTP wire format is unchanged for single-column callers: the query parameter uses
style: form, explode: true, so a one-element list still serializes to?on=id. An older client keeps working against a server built from this change.Moving from one match column to several also changes how NULL keys behave, because core switches NULL join semantics on the arity of the key: a single-column key treats NULL as equal to NULL, while a composite key uses standard SQL equality, under which a NULL key matches nothing — not even a byte-identical NULL. That is pre-existing core behavior, but composite keys are reachable through the namespace API for the first time here, so it is newly visible.
test_merge_insert_composite_key_never_matches_a_null_key_columnpins it down.Java and Python SDK requests reach the Rust implementations as JSON across JNI and pyo3, so a jar or wheel predating this change sends
"on": "id"where the model now expects"on": ["id"]. Those four bridge sites deserialize throughLenientMergeInsertIntoTableRequest, which promotes a scalar to a one-element list, so mismatched SDK builds keep working. This is inbound only — a Java namespace implementation called from Rust still needs a matching jar.Not included
The Java and Python
lance-namespacepins stay on 0.11, so their generated models still sendonas a bare string and rely on the promotion described above. Moving those pins to 0.12 and adding binding-level merge-insert coverage is a follow-up.The LanceDB Enterprise namespace server needs to accept the repeated
onquery parameter separately; that change is backward compatible on its own and does not depend on this one.Part of ENT-2084.