mirror of
https://github.com/lancedb/lancedb.git
synced 2026-08-18 03:58:26 +00:00
fix: make computed columns explicitly local-only
Both operations were advertised on remote tables and neither could work. A declaration reaches the wire as AllNulls, which RemoteTable::add_columns does not accept, and refresh_column fell through to the BaseTable default; the TypeScript wrapper reached the same surface. Callers got errors that named neither the feature nor the reason. The remote protocol has no representation for a stored expression, and what one should look like is not settled -- the server persists a declaration under a different vocabulary. So this states the boundary rather than guessing at a wire format: an explicit AllNulls arm, a default that names computed columns, and NotImplementedError raised in Python before the round trip.
This commit is contained in:
@@ -964,10 +964,14 @@ class RemoteTable(Table):
|
||||
*,
|
||||
computed: Dict[str, str] | None = None,
|
||||
) -> AddColumnsResult:
|
||||
return LOOP.run(self._table.add_columns(transforms, computed=computed))
|
||||
if computed:
|
||||
raise NotImplementedError(
|
||||
"computed columns are supported only on local tables"
|
||||
)
|
||||
return LOOP.run(self._table.add_columns(transforms))
|
||||
|
||||
def refresh_column(self, column: str):
|
||||
return LOOP.run(self._table.refresh_column(column))
|
||||
raise NotImplementedError("computed columns are supported only on local tables")
|
||||
|
||||
def alter_columns(
|
||||
self, *alterations: Iterable[Dict[str, str]]
|
||||
|
||||
@@ -2706,6 +2706,13 @@ impl<S: HttpSend> BaseTable for RemoteTable<S> {
|
||||
|
||||
Ok(result)
|
||||
}
|
||||
// A declaration reaches here as AllNulls, which the remote protocol
|
||||
// has no representation for.
|
||||
NewColumnTransform::AllNulls(_) => {
|
||||
return Err(Error::NotSupported {
|
||||
message: "computed columns are supported only on local tables".into(),
|
||||
});
|
||||
}
|
||||
_ => {
|
||||
return Err(Error::NotSupported {
|
||||
message: "Only SQL expressions are supported for adding columns".into(),
|
||||
@@ -6455,6 +6462,37 @@ mod tests {
|
||||
assert_eq!(result.version, if old_server { 0 } else { 43 });
|
||||
}
|
||||
|
||||
/// Computed columns are local-only. Both halves say so here rather than
|
||||
/// reaching the wire and failing somewhere less legible.
|
||||
#[tokio::test]
|
||||
async fn test_computed_columns_are_refused() {
|
||||
let table = Table::new_with_handler("my_table", |request| -> http::Response<String> {
|
||||
panic!("unexpected request: {}", request.url().path())
|
||||
});
|
||||
|
||||
let declared = Arc::new(Schema::new(vec![Field::new(
|
||||
"doubled",
|
||||
DataType::Int32,
|
||||
true,
|
||||
)]));
|
||||
let err = table
|
||||
.add_columns()
|
||||
.transform(NewColumnTransform::AllNulls(declared))
|
||||
.execute()
|
||||
.await
|
||||
.unwrap_err();
|
||||
assert!(
|
||||
matches!(&err, Error::NotSupported { message } if message.contains("local tables")),
|
||||
"{err:?}"
|
||||
);
|
||||
|
||||
let err = table.refresh_column("doubled").await.unwrap_err();
|
||||
assert!(
|
||||
matches!(&err, Error::NotSupported { message } if message.contains("local tables")),
|
||||
"{err:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_prewarm_index() {
|
||||
let table = Table::new_with_handler("my_table", |request| {
|
||||
|
||||
@@ -793,7 +793,7 @@ pub trait BaseTable: std::fmt::Display + std::fmt::Debug + Send + Sync {
|
||||
/// The default returns `NotSupported`; Lance-backed tables override it.
|
||||
async fn refresh_column(&self, _column: &str) -> Result<RefreshColumnResult> {
|
||||
Err(Error::NotSupported {
|
||||
message: "refresh_column is not supported on this table type".into(),
|
||||
message: "computed columns are supported only on local tables".into(),
|
||||
})
|
||||
}
|
||||
/// Alter columns in the table.
|
||||
@@ -1688,7 +1688,25 @@ impl Table {
|
||||
AddColumnsBuilder::new(self.inner.clone())
|
||||
}
|
||||
|
||||
/// Compute and store values for a computed column's unfilled rows.
|
||||
/// Fill the fragments of a computed column that hold no values yet.
|
||||
///
|
||||
/// Declared with
|
||||
/// [`AddColumnsBuilder::computed`](add_columns::AddColumnsBuilder::computed),
|
||||
/// a column starts empty and gets its values here. Fragments appended
|
||||
/// since the last refresh are filled by the next one; fragments already
|
||||
/// filled are left as they are, so the call is idempotent and does not
|
||||
/// observe a mutated input.
|
||||
///
|
||||
/// Local tables only.
|
||||
///
|
||||
/// ```
|
||||
/// # use lancedb::Table;
|
||||
/// # async fn refresh(table: &Table) -> Result<(), Box<dyn std::error::Error>> {
|
||||
/// let result = table.refresh_column("doubled").await?;
|
||||
/// println!("filled {} rows at version {}", result.rows_filled, result.version);
|
||||
/// # Ok(())
|
||||
/// # }
|
||||
/// ```
|
||||
pub async fn refresh_column(&self, column: impl AsRef<str>) -> Result<RefreshColumnResult> {
|
||||
self.inner.refresh_column(column.as_ref()).await
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user