From 837ba24ab2c98477fc16bc97c4d81c0ca05df74b Mon Sep 17 00:00:00 2001 From: Jack Ye Date: Fri, 2 Oct 2026 10:32:26 -0500 Subject: [PATCH] fix: preserve arbitrary remote MV definitions --- nodejs/src/table.rs | 8 ++--- python/src/table.rs | 8 ++--- rust/lancedb/src/materialized_view.rs | 43 ++++++++++++++++++++------- rust/lancedb/src/remote/table.rs | 19 +++++++----- 4 files changed, 49 insertions(+), 29 deletions(-) diff --git a/nodejs/src/table.rs b/nodejs/src/table.rs index 3a20e85a8..79b335c37 100644 --- a/nodejs/src/table.rs +++ b/nodejs/src/table.rs @@ -491,11 +491,9 @@ impl Table { let view = lancedb::MaterializedView::from_table(inner) .await .default_error()?; - view.definition().to_json().map_err(|err| { - napi::Error::from_reason(format!( - "failed to serialize materialized-view definition: {err}" - )) - }) + Ok(lancedb::materialized_view::definition_metadata_from_sql( + view.definition_sql(), + )) } #[napi(catch_unwind)] diff --git a/python/src/table.rs b/python/src/table.rs index 6c215946f..6fd93a65f 100644 --- a/python/src/table.rs +++ b/python/src/table.rs @@ -1859,11 +1859,9 @@ impl Table { let view = lancedb::MaterializedView::from_table(inner) .await .infer_error()?; - view.definition().to_json().map_err(|err| { - PyRuntimeError::new_err(format!( - "failed to serialize materialized-view definition: {err}" - )) - }) + Ok(lancedb::materialized_view::definition_metadata_from_sql( + view.definition_sql(), + )) }) } diff --git a/rust/lancedb/src/materialized_view.rs b/rust/lancedb/src/materialized_view.rs index 204129918..fd0a2dac0 100644 --- a/rust/lancedb/src/materialized_view.rs +++ b/rust/lancedb/src/materialized_view.rs @@ -267,8 +267,10 @@ pub enum StoredDefinition { #[doc(hidden)] #[derive(Debug, Clone, PartialEq, Eq)] pub struct MaterializedViewInfo { - /// The parsed view definition. - pub definition: MaterializedViewDefinition, + /// The defining SQL as stored. + pub definition_sql: String, + /// The definition parsed into the local refresh grammar, when supported. + pub parsed_definition: Option, } /// The backend-independent request used to create a remote materialized view. @@ -454,7 +456,10 @@ pub(crate) fn materialized_view_info_from_metadata( metadata: &HashMap, ) -> Result { match read_definition(metadata)? { - Some(StoredDefinition::Query(definition)) => Ok(MaterializedViewInfo { definition }), + Some(StoredDefinition::Query(definition)) => Ok(MaterializedViewInfo { + definition_sql: definition.to_sql(), + parsed_definition: Some(definition), + }), Some(StoredDefinition::Newer { format }) => Err(Error::NotSupported { message: format!( "materialized view '{name}' is stored in format {format}, which this version \ @@ -1651,22 +1656,24 @@ impl CreateMaterializedViewBuilder { } } -/// A handle on a materialized view: the view table plus its parsed definition. +/// A handle on a materialized view: the view table plus its defining SQL. #[derive(Debug, Clone)] pub struct MaterializedView { table: Table, - definition: MaterializedViewDefinition, + definition_sql: String, + parsed_definition: Option, } impl MaterializedView { /// Interpret `table` as a materialized view: [`Error::NotAMaterializedView`] - /// for a plain table, [`Error::NotSupported`] for a query this version - /// cannot refresh. + /// for a plain table. A remote definition broader than the local refresh + /// grammar remains available through [`Self::definition_sql`]. pub async fn from_table(table: Table) -> Result { let info = table.base_table().materialized_view_info().await?; Ok(Self { table, - definition: info.definition, + definition_sql: info.definition_sql, + parsed_definition: info.parsed_definition, }) } @@ -1680,9 +1687,23 @@ impl MaterializedView { self.table.name() } - /// The query that defines the view. - pub fn definition(&self) -> &MaterializedViewDefinition { - &self.definition + /// The query parsed into the local refresh grammar. + /// + /// Sophon may store SQL broader than the local grammar; use + /// [`Self::definition_sql`] when the original SQL is sufficient. + pub fn definition(&self) -> Result<&MaterializedViewDefinition> { + self.parsed_definition + .as_ref() + .ok_or_else(|| Error::NotSupported { + message: + "this materialized-view definition is supported only by the remote SQL engine" + .to_string(), + }) + } + + /// The complete SQL query that defines the view. + pub fn definition_sql(&self) -> &str { + &self.definition_sql } /// Recompute the view from its source. diff --git a/rust/lancedb/src/remote/table.rs b/rust/lancedb/src/remote/table.rs index 7192f8f9b..96f324e18 100644 --- a/rust/lancedb/src/remote/table.rs +++ b/rust/lancedb/src/remote/table.rs @@ -2125,8 +2125,11 @@ impl BaseTable for RemoteTable { let response = self.check_table_response(&request_id, response).await?; let response: DescribeMaterializedViewResponse = response.json().await.err_to_http(request_id)?; - let definition = MaterializedViewDefinition::from_sql(&response.query)?; - Ok(MaterializedViewInfo { definition }) + let parsed_definition = MaterializedViewDefinition::from_sql(&response.query).ok(); + Ok(MaterializedViewInfo { + definition_sql: response.query, + parsed_definition, + }) } async fn refresh_materialized_view_async( @@ -12730,24 +12733,24 @@ mod tests { #[tokio::test] async fn test_materialized_view_describe_and_refresh_requires_sql() { - let table = Table::new_with_handler("my_table", |request| { - match request.url().path() { + const QUERY: &str = + "SELECT s.x, d.label FROM analytics.source s JOIN analytics.dim d ON s.id = d.id"; + let table = Table::new_with_handler("my_table", |request| match request.url().path() { "/v1/materialized_view/my_table/describe" => http::Response::builder() .status(200) .body( json!({ "name": "my_table", - "query": "SELECT x * 2 AS double_x FROM analytics.source WHERE x > 0 LIMIT 10" + "query": QUERY }) .to_string(), ) .unwrap(), path => panic!("unexpected request: {path}"), - } }); let view = crate::MaterializedView::from_table(table).await.unwrap(); - assert_eq!(view.definition().source_table, "source"); - assert_eq!(view.definition().source_namespace, ["analytics"]); + assert_eq!(view.definition_sql(), QUERY); + assert!(view.definition().is_err()); let error = view.refresh().execute().await.unwrap_err(); assert!(error.to_string().contains("SQL is unavailable")); }