Skip to content

fix(java): carry alterColumns cast type across FFI via C Data Interface - #8417

Merged
westonpace merged 2 commits into
lance-format:mainfrom
puchengy:fix-alter-column-cast-type
Aug 7, 2026
Merged

fix(java): carry alterColumns cast type across FFI via C Data Interface#8417
westonpace merged 2 commits into
lance-format:mainfrom
puchengy:fix-alter-column-cast-type

Conversation

@puchengy

@puchengy puchengy commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

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():

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

`Dataset.alterColumns(...castTo...)` silently dropped the cast target
type. The JNI `create_column_alteration` marshalled it by calling the
Java `ArrowType.toString()` (e.g. `"Int(64, true)"`,
`"FloatingPoint(DOUBLE)"`) and parsing the result with
`arrow_schema::DataType::from_str`, then swallowing the parse failure
with `.ok()`. Parameterized types do not round-trip through that
grammar, so `data_type` became `None` and the cast was a no-op: the
commit landed but the stored column type was unchanged.

Transfer the cast target type through the Arrow C Data Interface
instead, mirroring `addColumns(Schema)`: the Java side exports one field
per requested cast (in alteration order) into an `ArrowSchema`, and the
JNI imports it via `FFI_ArrowSchema` and attaches each type to the
corresponding `ColumnAlteration`. Rename and nullability-only
alterations are unaffected. Removes the now-unused `DataType`/`FromStr`
imports.

Adds `DatasetTest.testAlterColumnsCastType` covering an Int32->Int64
widen and a combined rename+cast, asserting the resulting Arrow type
(the existing `testAlterColumns` only checked field names, so the
dropped cast went unnoticed).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added A-java Java bindings + JNI bug Something isn't working labels Aug 7, 2026
…ent)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@lance-gatekeeper lance-gatekeeper Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gate recommendation: approve.

The typed Arrow C Data transfer fixes the lossy Java-to-Rust boundary without extending display-string parsing, while preserving rename-only and nullability-only alterations. The regression coverage exercises a parameterized cast and a combined rename/cast.

@westonpace westonpace left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do have String<->DataType somewhere I think but C FFI seems reasonable to me as well. No change to public APIs that I can see.

@westonpace
westonpace merged commit 855154f into lance-format:main Aug 7, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-java Java bindings + JNI bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants