Skip to content

Commit 68da0b3

Browse files
authored
feat: add lance_dataset_alter_columns for rename / nullability / type changes (#44)
## Summary Second of three PRs against #41. Exposes upstream's `Dataset::alter_columns` — rename a column, change its nullability, or change its data type, committing a new manifest. Rename and nullability-only changes are zero-copy and preserve indices on the affected column; a type change rewrites the column's data files and drops any associated indices, mirroring upstream behavior. Mutates the dataset in place under an exclusive write lock; scanners already in flight against it keep their pre-alteration view via the existing Arc clone-on-write, same as `_delete` (#31) / `_drop_columns` (#42). ## Surface ```c typedef enum { LANCE_COLUMN_NULLABLE_UNCHANGED = 0, LANCE_COLUMN_NULLABLE_TRUE = 1, LANCE_COLUMN_NULLABLE_FALSE = 2, } LanceColumnNullableMode; typedef struct LanceColumnAlteration { const char* path; /* required */ const char* rename; /* NULL = keep */ int32_t nullable_mode; /* LanceColumnNullableMode discriminant */ const struct ArrowSchema* data_type; /* NULL = keep */ } LanceColumnAlteration; int32_t lance_dataset_alter_columns( LanceDataset* dataset, const LanceColumnAlteration* alterations, size_t num_alterations ); ``` Per-alteration validation runs up front with index-tagged error messages. The struct uses sentinels for the three optional fields (`rename = NULL`, `nullable_mode = UNCHANGED`, `data_type = NULL`); at least one must request a change, so a zero-init struct with only `path` set is rejected as a no-op rather than silently consuming a manifest version. Two design choices worth calling out: - **`nullable_mode` is `int32_t`, not the enum directly.** The struct is read across the FFI boundary, and Rust treats a `#[repr(C)]` enum read from C with an out-of-range discriminant as UB. So the field is `int32_t` and a `LanceColumnNullableMode::from_raw(i32)` helper converts and returns `INVALID_ARGUMENT` for unknown values — same pattern as `merge_insert`'s `WhenMatched::from_raw`. - **`data_type` borrows an Arrow `ArrowSchema`.** The wrapper never calls its `release` callback. Before handing the pointer to arrow-rs, the wrapper checks both `release == NULL` (the Arrow CADI "released" sentinel) and `format == NULL` (catches `FFI_ArrowSchema::empty()` and other half-built structs that would otherwise hit an `assert!` in `DataType::try_from` and abort the host process under `panic = "abort"`). The C++ wrapper takes `const std::vector<lance::ColumnAlteration>&` and uses the same direct-pass convention as `update`/`merge_insert` siblings — `raw.data()` unconditionally; an empty vector flows through the Rust-side `num_alterations == 0` guard so the error message is precise. ## Tests Nineteen new Rust integration tests cover the positive paths (rename, relax / tighten nullability, Int32→Int64 upcast with value round-trip, combined rename+relax, multi-alteration per call, version bump) and the full rejection surface (NULL dataset / NULL array / zero count / NULL path / empty path / empty rename / unknown column / incompatible cast / no-op alteration with schema-unchanged assertion / invalid `nullable_mode` discriminant / uninitialised `FFI_ArrowSchema` / tightening nullability when existing rows hold NULLs). C and C++ smoke tests slot in before `test_drop_columns`, relax `id` to nullable, and verify via `ArrowSchema.flags & ARROW_FLAG_NULLABLE`. Both also exercise the NULL / zero / no-op / bad-discriminant negative paths. `cargo test` and `cargo test --test compile_and_run_test -- --ignored` both green. ## Follow-up - `lance_dataset_add_columns` — SQL expressions / AllNulls / ArrowArrayStream The README roadmap entry stays unticked until that lands.
1 parent d5133a8 commit 68da0b3

7 files changed

Lines changed: 1077 additions & 0 deletions

File tree

include/lance/lance.h

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,81 @@ int32_t lance_dataset_drop_columns(
484484
size_t num_columns
485485
);
486486

487+
/* ─── lance_dataset_alter_columns ─────────────────────────────────────────── */
488+
489+
/**
490+
* Tri-state nullability override for `LanceColumnAlteration`. The
491+
* `UNCHANGED` discriminant is zero so a zero-initialised
492+
* `LanceColumnAlteration` leaves nullability alone by default.
493+
*
494+
* Discriminants are pinned for ABI stability. Out-of-range values are
495+
* rejected with `LANCE_ERR_INVALID_ARGUMENT` — that's why the field on
496+
* `LanceColumnAlteration` is `int32_t`, not this enum directly.
497+
*/
498+
typedef enum {
499+
/* Do not touch the column's existing nullability. */
500+
LANCE_COLUMN_NULLABLE_UNCHANGED = 0,
501+
/* Set the column to nullable. */
502+
LANCE_COLUMN_NULLABLE_TRUE = 1,
503+
/* Set the column to non-nullable. Upstream verifies via a scan that no
504+
row holds a NULL — the call fails if any do. */
505+
LANCE_COLUMN_NULLABLE_FALSE = 2,
506+
} LanceColumnNullableMode;
507+
508+
/**
509+
* A single alteration applied to one column. Every non-`path` field is
510+
* optional via a sentinel:
511+
*
512+
* - `rename = NULL` keeps the current name.
513+
* - `nullable_mode = LANCE_COLUMN_NULLABLE_UNCHANGED` keeps current nullability.
514+
* - `data_type = NULL` keeps the current data type.
515+
*
516+
* At least one of `rename`, `nullable_mode`, or `data_type` must request a
517+
* change; an alteration that touches nothing is rejected at the FFI boundary.
518+
*
519+
* `data_type`, when non-NULL, borrows an Arrow C Data Interface `ArrowSchema`
520+
* describing the target type. The struct is read by shared reference for the
521+
* duration of the call; its `release` callback is never invoked.
522+
*/
523+
typedef struct LanceColumnAlteration {
524+
/* Path to the existing column. Required, non-empty UTF-8. */
525+
const char* path;
526+
/* New column name, or NULL to keep the current name. */
527+
const char* rename;
528+
/* LanceColumnNullableMode discriminant. */
529+
int32_t nullable_mode;
530+
/* New data type, or NULL to keep the current type. */
531+
const struct ArrowSchema* data_type;
532+
} LanceColumnAlteration;
533+
534+
/**
535+
* Apply one or more column alterations and commit a new manifest. Rename and
536+
* nullability-only changes are zero-copy and preserve any indices on the
537+
* affected columns. A type change rewrites the column's data files and drops
538+
* any indices that referenced it, mirroring upstream behaviour.
539+
*
540+
* Mutates `dataset` in place — the same handle remains valid afterward and
541+
* sees the new version. Scanners already in flight against this dataset keep
542+
* their pre-alteration view.
543+
*
544+
* @param dataset Open dataset (not consumed). Mutated in place to
545+
* see the new version. Must not be NULL.
546+
* @param alterations Array of `LanceColumnAlteration`. Must not be NULL.
547+
* @param num_alterations Length of `alterations`. Must be > 0.
548+
* @return 0 on success, -1 on error. Error codes:
549+
* LANCE_ERR_INVALID_ARGUMENT for NULL/empty inputs, NULL or empty
550+
* `path`, non-UTF-8 strings, no-op alterations (all three optional
551+
* fields left at their sentinels), invalid `nullable_mode`
552+
* discriminant, unknown columns, type changes that aren't a valid
553+
* cast, or tightening nullability when existing rows hold NULLs;
554+
* LANCE_ERR_COMMIT_CONFLICT for a concurrent writer.
555+
*/
556+
int32_t lance_dataset_alter_columns(
557+
LanceDataset* dataset,
558+
const LanceColumnAlteration* alterations,
559+
size_t num_alterations
560+
);
561+
487562
/**
488563
* Export the dataset schema via Arrow C Data Interface.
489564
* @param out Pointer to caller-allocated ArrowSchema struct

include/lance/lance.hpp

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,21 @@ struct WriteParams {
121121
bool enable_stable_row_ids = false;
122122
};
123123

124+
// ─── Column alteration ───────────────────────────────────────────────────────
125+
126+
/// A single alteration applied to one column by `Dataset::alter_columns`.
127+
/// Every non-`path` field is optional; at least one must request a change.
128+
///
129+
/// `data_type`, when non-null, borrows an Arrow C Data Interface `ArrowSchema`
130+
/// describing the target type. The caller owns it and must keep it alive for
131+
/// the duration of the `alter_columns` call; the wrapper does not release it.
132+
struct ColumnAlteration {
133+
std::string path;
134+
std::optional<std::string> rename;
135+
LanceColumnNullableMode nullable_mode = LANCE_COLUMN_NULLABLE_UNCHANGED;
136+
const ArrowSchema* data_type = nullptr;
137+
};
138+
124139
// ─── Dataset ─────────────────────────────────────────────────────────────────
125140

126141
class Dataset {
@@ -463,6 +478,39 @@ class Dataset {
463478
}
464479
}
465480

481+
/// Apply one or more column alterations (rename / nullability / type
482+
/// change) and commit a new manifest. Rename and nullability-only
483+
/// changes are zero-copy and preserve any indices on the affected
484+
/// columns; a type change rewrites the column's data files and drops
485+
/// any indices that referenced it.
486+
///
487+
/// `alterations` must be non-empty and each entry must request at least
488+
/// one change. Any `data_type` pointer must remain valid for the
489+
/// duration of this call. Throws lance::Error on failure (empty list,
490+
/// no-op alteration, unknown column, incompatible cast, tightening
491+
/// nullability when NULLs exist, commit conflict, ...).
492+
void alter_columns(const std::vector<ColumnAlteration>& alterations) {
493+
// The C strings we install in each entry borrow from `alterations`
494+
// (the caller's std::strings), which outlive this call. The entries
495+
// themselves are copied by value into `raw`, so any reallocation
496+
// during push_back just moves the raw bytes — pointer values are
497+
// preserved. `reserve` is a performance hint, not a lifetime guard.
498+
std::vector<LanceColumnAlteration> raw;
499+
raw.reserve(alterations.size());
500+
for (const auto& a : alterations) {
501+
LanceColumnAlteration entry{};
502+
entry.path = a.path.c_str();
503+
entry.rename = a.rename ? a.rename->c_str() : nullptr;
504+
entry.nullable_mode = static_cast<int32_t>(a.nullable_mode);
505+
entry.data_type = a.data_type;
506+
raw.push_back(entry);
507+
}
508+
if (lance_dataset_alter_columns(
509+
handle_.get(), raw.data(), raw.size()) != 0) {
510+
check_error();
511+
}
512+
}
513+
466514
/// Export the schema as an Arrow C Data Interface struct.
467515
void schema(ArrowSchema* out) const {
468516
if (lance_dataset_schema(handle_.get(), out) != 0) {

src/alter_columns.rs

Lines changed: 254 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,254 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
// SPDX-FileCopyrightText: Copyright The Lance Authors
3+
4+
//! Alter columns C API: rename a column, change its nullability, or change its
5+
//! data type, committing a new manifest. Rename and nullability-only changes
6+
//! are zero-copy and preserve indices; a type change rewrites the column's
7+
//! data files and drops any associated indices, mirroring upstream behaviour.
8+
//!
9+
//! Mutates the dataset in place under an exclusive write lock; existing
10+
//! scanners that already cloned the inner Arc keep their pre-alteration view.
11+
12+
use std::ffi::c_char;
13+
14+
use arrow::ffi::FFI_ArrowSchema;
15+
use arrow_schema::DataType;
16+
use lance::dataset::ColumnAlteration;
17+
use lance_core::Result;
18+
use snafu::location;
19+
20+
use crate::dataset::LanceDataset;
21+
use crate::error::ffi_try;
22+
use crate::helpers;
23+
use crate::runtime::block_on;
24+
25+
/// Tri-state nullability override for `LanceColumnAlteration`. The `Unchanged`
26+
/// discriminant is zero so a zero-initialised struct leaves nullability alone.
27+
///
28+
/// Discriminants are pinned for ABI stability. Out-of-range values stored on
29+
/// the FFI side are rejected with `LANCE_ERR_INVALID_ARGUMENT` rather than
30+
/// being transmuted into a `repr(C)` enum (which would be UB) — that's why
31+
/// the corresponding field on `LanceColumnAlteration` is `i32`, not this
32+
/// enum directly.
33+
#[repr(C)]
34+
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
35+
pub enum LanceColumnNullableMode {
36+
/// Do not touch the column's existing nullability.
37+
Unchanged = 0,
38+
/// Set the column to nullable.
39+
True = 1,
40+
/// Set the column to non-nullable. Upstream verifies via a scan that no
41+
/// existing rows hold a NULL — the call fails if any do.
42+
False = 2,
43+
}
44+
45+
impl LanceColumnNullableMode {
46+
fn from_raw(raw: i32) -> Result<Self> {
47+
match raw {
48+
0 => Ok(Self::Unchanged),
49+
1 => Ok(Self::True),
50+
2 => Ok(Self::False),
51+
other => Err(lance_core::Error::InvalidInput {
52+
source: format!(
53+
"invalid nullable_mode {other}; expected 0..=2 (see LanceColumnNullableMode)"
54+
)
55+
.into(),
56+
location: location!(),
57+
}),
58+
}
59+
}
60+
}
61+
62+
/// A single alteration applied to one column. Each non-`path` field is
63+
/// optional via a sentinel:
64+
///
65+
/// * `rename = NULL` keeps the current name.
66+
/// * `nullable_mode = LANCE_COLUMN_NULLABLE_UNCHANGED` keeps the current
67+
/// nullability.
68+
/// * `data_type = NULL` keeps the current data type.
69+
///
70+
/// At least one of `rename`, `nullable_mode`, or `data_type` must request a
71+
/// change; an alteration that touches nothing is rejected at the FFI
72+
/// boundary.
73+
///
74+
/// `data_type` borrows an Arrow C Data Interface `ArrowSchema` describing the
75+
/// target type for the duration of the call. The struct is read by shared
76+
/// reference; we never call its `release` callback.
77+
#[repr(C)]
78+
pub struct LanceColumnAlteration {
79+
/// Path to the existing column to alter. Required, non-empty UTF-8.
80+
pub path: *const c_char,
81+
/// New column name, or NULL to keep the current name.
82+
pub rename: *const c_char,
83+
/// `LanceColumnNullableMode` discriminant; carried as `i32` so an
84+
/// invalid value coming in from C is rejected at the FFI boundary
85+
/// instead of being transmuted into an enum (which would be UB).
86+
pub nullable_mode: i32,
87+
/// New data type, or NULL to keep the current type.
88+
pub data_type: *const FFI_ArrowSchema,
89+
}
90+
91+
/// Apply one or more `LanceColumnAlteration`s and commit a new manifest.
92+
/// Rename and nullability-only changes are zero-copy and preserve any indices
93+
/// on the affected columns. A type change rewrites the column's data files
94+
/// and drops any indices that referenced it.
95+
///
96+
/// - `dataset`: Open dataset (mutated; same handle remains valid afterward).
97+
/// Must not be NULL.
98+
/// - `alterations`: Pointer to an array of `LanceColumnAlteration`. Must not
99+
/// be NULL.
100+
/// - `num_alterations`: Length of the `alterations` array. Must be non-zero.
101+
///
102+
/// Returns 0 on success, -1 on error. Error codes:
103+
/// `LANCE_ERR_INVALID_ARGUMENT` for NULL/empty args, NULL or empty `path`,
104+
/// non-UTF-8 strings, no-op alterations (all three optional fields left at
105+
/// their sentinels), invalid `nullable_mode` discriminant, unknown columns,
106+
/// type changes that aren't a valid cast, or tightening nullability when
107+
/// existing rows hold NULLs. `LANCE_ERR_COMMIT_CONFLICT` for a concurrent
108+
/// writer.
109+
#[unsafe(no_mangle)]
110+
pub unsafe extern "C" fn lance_dataset_alter_columns(
111+
dataset: *mut LanceDataset,
112+
alterations: *const LanceColumnAlteration,
113+
num_alterations: usize,
114+
) -> i32 {
115+
ffi_try!(
116+
unsafe { alter_columns_inner(dataset, alterations, num_alterations) },
117+
neg
118+
)
119+
}
120+
121+
unsafe fn alter_columns_inner(
122+
dataset: *mut LanceDataset,
123+
alterations: *const LanceColumnAlteration,
124+
num_alterations: usize,
125+
) -> Result<i32> {
126+
if dataset.is_null() {
127+
return Err(lance_core::Error::InvalidInput {
128+
source: "dataset must not be NULL".into(),
129+
location: location!(),
130+
});
131+
}
132+
if alterations.is_null() {
133+
return Err(lance_core::Error::InvalidInput {
134+
source: "alterations must not be NULL".into(),
135+
location: location!(),
136+
});
137+
}
138+
if num_alterations == 0 {
139+
return Err(lance_core::Error::InvalidInput {
140+
source: "num_alterations must be > 0".into(),
141+
location: location!(),
142+
});
143+
}
144+
145+
// Materialize alterations up front so any per-index validation error
146+
// fires before the dataset's write lock is taken — matches the pre-lock
147+
// validation pattern used by `update.rs` and `drop_columns.rs`.
148+
let mut owned: Vec<ColumnAlteration> = Vec::with_capacity(num_alterations);
149+
for i in 0..num_alterations {
150+
// SAFETY: `alterations` is non-NULL (checked above) and the caller
151+
// guarantees the array has at least `num_alterations` initialised
152+
// entries valid for the duration of this call. `parse_alteration`
153+
// is `unsafe` solely because it dereferences each entry's `path` /
154+
// `rename` / `data_type` pointers — same caller-side guarantee.
155+
let entry = unsafe { &*alterations.add(i) };
156+
owned.push(unsafe { parse_alteration(i, entry)? });
157+
}
158+
159+
// SAFETY: `dataset` is non-NULL (checked above) and the caller guarantees
160+
// it points to a live `LanceDataset`. `with_mut` takes an exclusive
161+
// write lock on the inner `Arc<Dataset>` before yielding `&mut Dataset`,
162+
// so a shared `&*dataset` borrow here is sound — interior mutability is
163+
// the synchronization point.
164+
let ds = unsafe { &*dataset };
165+
ds.with_mut(|d| block_on(d.alter_columns(&owned)))?;
166+
Ok(0)
167+
}
168+
169+
/// Translate one C-side `LanceColumnAlteration` into the upstream owned form,
170+
/// validating every field at the FFI boundary so the caller sees precise
171+
/// per-index errors rather than upstream-internal ones.
172+
unsafe fn parse_alteration(
173+
index: usize,
174+
entry: &LanceColumnAlteration,
175+
) -> Result<ColumnAlteration> {
176+
// SAFETY: `entry.path` is either NULL (rejected below) or a NUL-terminated
177+
// C string the caller keeps alive for this call.
178+
let path = unsafe { helpers::parse_c_string(entry.path)? }
179+
.filter(|s| !s.is_empty())
180+
.ok_or_else(|| lance_core::Error::InvalidInput {
181+
source: format!("alterations[{index}].path must not be NULL or empty").into(),
182+
location: location!(),
183+
})?
184+
.to_string();
185+
186+
// SAFETY: same shape as `path`; NULL is a documented sentinel for "keep
187+
// the current name", not an error.
188+
let rename = unsafe { helpers::parse_c_string(entry.rename)? }
189+
.map(|s| {
190+
if s.is_empty() {
191+
Err(lance_core::Error::InvalidInput {
192+
source: format!("alterations[{index}].rename must not be empty").into(),
193+
location: location!(),
194+
})
195+
} else {
196+
Ok(s.to_string())
197+
}
198+
})
199+
.transpose()?;
200+
201+
let nullable = match LanceColumnNullableMode::from_raw(entry.nullable_mode)? {
202+
LanceColumnNullableMode::Unchanged => None,
203+
LanceColumnNullableMode::True => Some(true),
204+
LanceColumnNullableMode::False => Some(false),
205+
};
206+
207+
let data_type = if entry.data_type.is_null() {
208+
None
209+
} else {
210+
// SAFETY: caller guarantees `data_type` (when non-NULL) points to a
211+
// valid `FFI_ArrowSchema` for the duration of this call. We read by
212+
// shared reference and never invoke the struct's release callback.
213+
let ffi_schema = unsafe { &*entry.data_type };
214+
// Reject an already-released or never-initialised schema before
215+
// handing it to arrow-rs, which would otherwise `assert!` on the
216+
// NULL `format` field and abort the host process under our
217+
// `panic = "abort"` profile. Both checks are intentional:
218+
// - `release == NULL`: the canonical Arrow CADI "released" sentinel.
219+
// - `format == NULL`: catches a zero-initialised or otherwise
220+
// half-built struct that would slip past the release check.
221+
if ffi_schema.release.is_none() || ffi_schema.format.is_null() {
222+
return Err(lance_core::Error::InvalidInput {
223+
source: format!(
224+
"alterations[{index}].data_type is uninitialised or already released"
225+
)
226+
.into(),
227+
location: location!(),
228+
});
229+
}
230+
let dt = DataType::try_from(ffi_schema).map_err(|e| lance_core::Error::InvalidInput {
231+
source: format!("alterations[{index}].data_type is not a valid Arrow type: {e}").into(),
232+
location: location!(),
233+
})?;
234+
Some(dt)
235+
};
236+
237+
if rename.is_none() && nullable.is_none() && data_type.is_none() {
238+
return Err(lance_core::Error::InvalidInput {
239+
source: format!(
240+
"alterations[{index}] is a no-op: \
241+
set rename, nullable_mode, or data_type to request a change"
242+
)
243+
.into(),
244+
location: location!(),
245+
});
246+
}
247+
248+
Ok(ColumnAlteration {
249+
path,
250+
rename,
251+
nullable,
252+
data_type,
253+
})
254+
}

0 commit comments

Comments
 (0)