feat: support schema evolution DDLs (ADD/DROP/RENAME/ALTER COLUMN) - #752
feat: support schema evolution DDLs (ADD/DROP/RENAME/ALTER COLUMN)#752puchengy wants to merge 8 commits into
Conversation
Closes lance-format#62. Extend `BaseLanceNamespaceSparkCatalog.alterTable` to handle Spark `TableChange.ColumnChange` requests, translating them into the corresponding Lance dataset operations via a new `LanceSchemaEvolution` helper: - `ALTER TABLE ADD COLUMN` -> `Dataset.addColumns` - `ALTER TABLE DROP COLUMN` -> `Dataset.dropColumns` - `ALTER TABLE RENAME COLUMN`-> `Dataset.alterColumns` (rename) - `ALTER TABLE ALTER COLUMN ... DROP/SET NOT NULL` -> `Dataset.alterColumns` (nullability) `ALTER COLUMN ... TYPE` is rejected with a clear `UnsupportedOperationException`: the current lance-core JNI drops the cast target type on the way to Rust (it parses `ArrowType.toString()` with `DataType::from_str` and swallows the failure), which would otherwise turn a type change into a silent no-op. Column-comment updates and positional (`FIRST`/`AFTER`) adds are likewise unsupported. Adds Java unit tests (directory + REST namespaces) and PySpark integration tests covering the happy path and the unsupported-type-change error, and documents the new operations in the ALTER TABLE docs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ce (#8417) ## Problem `Dataset.alterColumns(...)` with a `castTo(...)` alteration silently drops the cast: the commit lands (version bumps) but the stored column type is unchanged. Root cause is in the JNI `create_column_alteration` (`java/lance-jni/src/blocking_dataset.rs`). The cast target type was marshalled by calling the Java `ArrowType.toString()` (e.g. `"Int(64, true)"`, `"FloatingPoint(DOUBLE)"`) and parsing the string with `arrow_schema::DataType::from_str`, then discarding any parse error with `.ok()`: ```rust let data_type_str: String = env.get_string(&jstring)?.into(); DataType::from_str(&data_type_str) .map_err(|e| Error::input_error(e.to_string())) .ok() // parse failure -> None -> cast silently dropped ``` `DataType`'s `FromStr` grammar does not accept the `Debug`-style strings that `ArrowType.toString()` produces for parameterized types, so `data_type` became `None` for anything beyond the few types whose `toString()` happens to match (e.g. `Utf8`). The existing `DatasetTest.testAlterColumns` only asserted field *names* after a cast, so the dropped type went unnoticed. ## Fix Transfer the cast target type through the Arrow **C Data Interface**, mirroring the existing `addColumns(Schema)` path: - **Java** (`Dataset.alterColumns`): export one field per requested cast — in the same order as the alterations — into an `ArrowSchema`, and pass its memory address to the native method. - **JNI** (`inner_alter_columns`): import the schema via `FFI_ArrowSchema` and attach each imported `DataType` to the corresponding `ColumnAlteration`. Rename-only and nullability-only alterations are unaffected. Removes the now-unused `DataType` / `FromStr` imports. ## Test Adds `DatasetTest.testAlterColumnsCastType`: widens `id` from `Int32` to `Int64`, then does a combined rename+cast, asserting the resulting Arrow type (not just the field name). ## Context Surfaced while implementing schema-evolution DDLs in `lance-spark` (lance-format/lance-spark#752), where `ALTER COLUMN ... TYPE` had to be rejected because of this bug. With this fix released, lance-spark can enable type changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address review feedback on the schema-evolution DDL support: - Validate the entire ordered ALTER TABLE request against the current schema before mutating anything, so a rejected change never leaves the table partially mutated (Spark's alterTable "all-or-nothing" contract). - Honor DROP COLUMN IF EXISTS: a missing column is skipped instead of raising, while a plain DROP COLUMN on a missing column still fails. - Reject ADD COLUMN with a DEFAULT value rather than silently filling NULL (Lance's all-null add cannot backfill a default). - Reject ADD COLUMN on legacy-format (file_format_version='LEGACY') tables up front instead of surfacing a raw core error. Adds unit tests for atomic rejection, DROP COLUMN IF EXISTS, default rejection, and legacy-format rejection; a DROP COLUMN IF EXISTS integration test; and documents the new limitations. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address remaining review feedback: preserve Spark's all-or-nothing alterTable contract by committing an accepted request through exactly one Lance core operation instead of a sequence of per-change commits. - Same-kind column changes are batched into a single core call: ADD -> addColumns, DROP -> dropColumns, RENAME/nullability -> alterColumns (rename + nullability edits to the same column are merged into one ColumnAlteration). - Requests that would require more than one core mutation are rejected before the first write: mixing column additions, drops, and alterations in one statement, and combining any column change with TBLPROPERTIES changes (which commit separately). Adds tests for batched multi-column ADD, mixed-kind rejection, and column-change-plus-TBLPROPERTIES rejection; documents the single-mutation restriction. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address review feedback on the single-mutation compiler: the batched core operation applies to the current schema with no ordering between its entries, so validating against an evolving schema (or coalescing by path) accepted requests the core cannot represent. - Validate every referenced column against the current schema, not an evolving one, so a change that depends on an earlier change in the same request (e.g. altering a column by its just-assigned new name) is rejected before any write instead of failing mid-commit. - Reject a request that targets the same column more than once: order is lost when changes collapse into one batch, and intermediate validation would be skipped. - Reject nested (multi-part) column paths up front (top-level only), instead of truncating to the leading path element and sending a path the core rejects — this keeps DROP COLUMN IF EXISTS's no-error contract for nested paths. Adds tests for rename-dependent rejection, repeated-target rejection, nested-path rejection, and a distinct-column rename+nullability batch; documents the restrictions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address review feedback on the single-mutation compiler: - Simulate rename source-removal and destination-insertion in request order during validation, rejecting a rename whose destination name is already occupied at that step. The core alterColumns batch is unordered, so without this an ordered request like RENAME a->b, b->c would be wrongly accepted as simultaneous renames instead of failing on the a->b collision. - Pass top-level column names to dropColumns/alterColumns verbatim (the Lance path for a top-level field is the name itself), with a test for a special-character column name. Adds tests for ordered rename-to-occupied rejection and a special-character column drop. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Escape every DROP, RENAME, and nullability source column name into a canonical Lance field path (via FieldPathUtils.canonicalPath) before passing it to the core, so top-level names containing path syntax (dots, spaces, etc.) resolve to the intended field instead of being rejected or misparsed. Adds special-character-name drop and rename tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The current core drop API cannot resolve a column whose name contains a backtick under either the canonical (escaped) or raw representation. Detect such names during validation and reject the DROP up front with a clear message, instead of letting it fail mid-commit with a confusing "field not found". RENAME and nullability keep using canonical paths, which the core resolves correctly. Adds a test for the backtick-name DROP rejection. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolve column absence before applying the backtick-representability guard, so DROP COLUMN IF EXISTS on a missing column is a no-op regardless of how the absent name is spelled. The unsupported-backtick rejection now applies only to a column that actually exists and would reach dropColumns; a plain DROP of a missing column still errors. Adds a test for DROP COLUMN IF EXISTS on a missing backtick name. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
✅ Gate recommendation: approve.
The missing-name ordering fix now preserves DROP COLUMN IF EXISTS while rejecting only existing backtick-named fields that the current core cannot drop. The complete change keeps accepted schema evolution atomic and faithful to Spark’s ordered request semantics.
| * <li>nested (multi-part) column paths, since validation is top-level only. | ||
| * </ul> | ||
| */ | ||
| final class LanceSchemaEvolution { |
There was a problem hiding this comment.
Great implementation. I have a problem to discuss. LanceSchemaEvolution operates directly on Dataset . But LanceNamespace also has some interfaces for schema evolution, such as alterTableAddColumns . Do you think it is necessary to call the relevant interfaces of LanceNamespace?
There was a problem hiding this comment.
Thanks for the comment, let me take a look. Thanks!
There was a problem hiding this comment.
Thanks for the suggestion, I think it is the right call, I will implement that in lance-namespace. Thanks for calling this out!
There was a problem hiding this comment.
Hey @jackye1995, can you chime in here?
I looked into @fangbo's suggestion and it turns out reusing lance-namespace is a bigger change than I expected. Here's the gist of what we'd need first:
- Wire up the Java LanceNamespace alter-column methods to the Rust impls in the lance repo — the Rust side already does the work, but there's no bridge from Java to it yet.
- Extend the AddColumnsEntry spec in lance-namespace so a plain ADD COLUMN age INT can be expressed — right now it only supports computed/SQL-expression columns.
- Wait for lance and lance-namespace to cut releases, since this repo can only pick up the changes once they're published.
So it spans three repos and needs to land in order. There are two ways:
- Option 1: merge this PR as is and have a follow up to make above changes.
- Option 2: do the above changes to get things right in the first place.
Could you weigh in by sharing your preference? Since it touches multiple repos, I'd feel better having a maintainer back the approach before I start if we go by option 2. Thanks!
Closes #62.
Adds support for the schema-evolution DDLs requested in #62 by extending
BaseLanceNamespaceSparkCatalog.alterTableto handle SparkTableChange.ColumnChangerequests, delegating to the underlying Lance dataset schema-evolution API via a newLanceSchemaEvolutionhelper.Supported operations
ALTER TABLE ... ADD COLUMN[S]Dataset.addColumnsALTER TABLE ... DROP COLUMN [IF EXISTS]Dataset.dropColumnsALTER TABLE ... RENAME COLUMNDataset.alterColumns(rename)ALTER TABLE ... ALTER COLUMN ... DROP/SET NOT NULLDataset.alterColumns(nullability)Atomic, all-or-nothing semantics
To honor Spark's
alterTablecontract, an accepted request is validated in full against the current schema and then committed through exactly one Lance core operation (addColumns/dropColumns/alterColumns— each commits its whole batch atomically):ADD COLUMNS (a, b); rename + nullability edits to the same column are merged into oneColumnAlteration).TBLPROPERTIESchanges (which commit separately). This keeps a rejectedALTER TABLEfrom leaving the table partially mutated. Heterogeneous batching in one commit is not yet supported by the core.Deliberately unsupported (rejected before any mutation)
ALTER COLUMN ... TYPE— throwsUnsupportedOperationException. Thelance-core:10.0.0-rc.3JNIcreate_column_alterationreads the cast target from the JavaArrowType.toString()(e.g."Int(64, true)") and parses it with RustDataType::from_str, swallowing the parse failure with.ok()— so the cast target is dropped and the type change becomes a silent no-op (the commit lands but the stored type is unchanged; verified for both integer- and float-widening). This is fixed upstream in fix(java): carry alterColumns cast type across FFI via C Data Interface lance#8417 (carries the cast type across FFI via the Arrow C Data Interface), but that fix is not yet in a publishedorg.lance:lance-coreartifact — the newest published version (11.0.0-beta.3) predates it, and Lance does not publish nightly/snapshot Java builds. Once a lance release containing #8417 ships, this can be enabled by bumping<lance.version>and replacing the rejection with thecastTopath.DEFAULTvalue — Lance's all-null add cannot backfill a default, so it is rejected rather than silently fillingNULL.file_format_version='LEGACY') table — the core all-null add path rejects legacy datasets; rejected up front with a clear message instead of a raw core error.FIRST/AFTER) adds, nested-column adds, and column-comment updates.Tests
SparkLanceNamespaceTestBase(run against both directory and REST namespaces): ADD/DROP/RENAME COLUMN and ALTER COLUMN DROP NOT NULL happy paths; batched multi-column ADD; and rejection cases for mixed-kind requests, column-change-plus-TBLPROPERTIES,DROP COLUMN IF EXISTSon a missing column,DEFAULTvalues, legacy format, and unsupported type changes.TestDDLAlterTableColumns.TestSparkDirectoryNamespacefull suite passes locally (66/66);make lintclean.Docs updated in
docs/src/operations/ddl/alter-table.md.🤖 Generated with Claude Code