diff --git a/nodejs/__test__/materialized_view.test.ts b/nodejs/__test__/materialized_view.test.ts index 2e7b2ec4d..b9d8ec911 100644 --- a/nodejs/__test__/materialized_view.test.ts +++ b/nodejs/__test__/materialized_view.test.ts @@ -48,6 +48,28 @@ describe("materialized views", () => { expect(definitionFromMetadata(safe, "v").limit).toBe(42); }); + it("reads the namespaced select kind and refuses unknown kinds", () => { + // "namespaced_select" is the namespaced form of "select": same shape, a + // separate kind so readers that predate it refuse instead of resolving + // the source at the root. + const namespaced = new Map([ + [ + DEFINITION_META_KEY, + '{"kind":"namespaced_select","source_table":"people","source_namespace":["ns"]}', + ], + ]); + const definition = definitionFromMetadata(namespaced, "v"); + expect(definition.sourceTable).toBe("people"); + expect(definition.sourceNamespace).toEqual(["ns"]); + + const unknown = new Map([ + [DEFINITION_META_KEY, '{"kind":"select_v3","source_table":"people"}'], + ]); + expect(() => definitionFromMetadata(unknown, "v")).toThrow( + /cannot refresh/, + ); + }); + it("creates, refreshes and queries a view", async () => { const view = await db.createMaterializedView("adults", "people", { select: ["name", ["shout", "upper(name)"]], diff --git a/nodejs/lancedb/materialized_view.ts b/nodejs/lancedb/materialized_view.ts index b26dee59e..a7cd8b7a7 100644 --- a/nodejs/lancedb/materialized_view.ts +++ b/nodejs/lancedb/materialized_view.ts @@ -19,6 +19,8 @@ export interface MaterializedViewDefinition { limit?: number; /** Source columns the projections and filter read. */ inputs: string[]; + /** Namespace holding the source table; empty is the root namespace. */ + sourceNamespace: string[]; } /** @@ -78,7 +80,10 @@ export function definitionFromMetadata( } // biome-ignore lint/suspicious/noExplicitAny: raw JSON const value: any = JSON.parse(raw); - if (value.kind !== "select") { + // "select" is the root-namespace form; "namespaced_select" carries a + // source namespace, a separate kind so older readers refuse it rather + // than silently resolving the source at the root. + if (value.kind !== "select" && value.kind !== "namespaced_select") { throw new Error( `materialized view '${name}' is defined by '${value.kind}', which this ` + "version of lancedb cannot refresh", @@ -103,6 +108,7 @@ export function definitionFromMetadata( filter: value.filter ?? undefined, limit, inputs: value.inputs ?? [], + sourceNamespace: value.source_namespace ?? [], }; } diff --git a/python/python/lancedb/materialized_view.py b/python/python/lancedb/materialized_view.py index 5abb44dc0..dc0974726 100644 --- a/python/python/lancedb/materialized_view.py +++ b/python/python/lancedb/materialized_view.py @@ -42,6 +42,8 @@ class MaterializedViewDefinition: """Cap on the number of rows the view holds.""" inputs: List[str] = field(default_factory=list) """Source columns the projections and filter read.""" + source_namespace: List[str] = field(default_factory=list) + """Namespace holding the source table; empty is the root namespace.""" def _definition_from_schema( @@ -53,7 +55,10 @@ def _definition_from_schema( raise ValueError(f"Table '{name}' is not a materialized view") value = json.loads(raw) kind = value.get("kind") - if kind != "select": + # "select" is the root-namespace form; "namespaced_select" carries a + # source namespace, a separate kind so older readers refuse it rather + # than silently resolving the source at the root. + if kind not in ("select", "namespaced_select"): raise NotImplementedError( f"materialized view '{name}' is defined by '{kind}', which this " "version of lancedb cannot refresh" @@ -66,6 +71,7 @@ def _definition_from_schema( filter=value.get("filter"), limit=value.get("limit"), inputs=value.get("inputs", []), + source_namespace=value.get("source_namespace", []), ) diff --git a/python/python/tests/test_materialized_views.py b/python/python/tests/test_materialized_views.py index 5fa3aa4fb..5cd7b23d6 100644 --- a/python/python/tests/test_materialized_views.py +++ b/python/python/tests/test_materialized_views.py @@ -266,3 +266,38 @@ async def test_async_namespace_connection_materialized_views(tmp_path): handle._route_pushdown_to_rust == through_namespace._route_pushdown_to_rust ) assert handle._namespace_path == through_namespace._namespace_path + + +def test_namespaced_select_kind_is_read_and_unknown_kinds_are_refused(): + import json + + import pyarrow as pa + + from lancedb.materialized_view import _definition_from_schema + + def schema_with(definition: dict) -> pa.Schema: + return pa.schema([pa.field("id", pa.int32())]).with_metadata( + {b"mv.definition": json.dumps(definition).encode()} + ) + + # "namespaced_select" is the namespaced form of "select": same shape, + # a separate kind so readers that predate it refuse instead of + # resolving the source at the root. + definition = _definition_from_schema( + schema_with( + { + "kind": "namespaced_select", + "source_table": "people", + "source_namespace": ["ns"], + "projections": [{"output": "name", "expression": "name"}], + } + ), + "v", + ) + assert definition.source_table == "people" + assert definition.source_namespace == ["ns"] + + with pytest.raises(NotImplementedError, match="cannot refresh"): + _definition_from_schema( + schema_with({"kind": "select_v3", "source_table": "people"}), "v" + ) diff --git a/rust/lancedb/src/materialized_view.rs b/rust/lancedb/src/materialized_view.rs index 95480098d..161a35589 100644 --- a/rust/lancedb/src/materialized_view.rs +++ b/rust/lancedb/src/materialized_view.rs @@ -73,9 +73,19 @@ const EMBEDDING_FUNCTIONS_META_KEY: &str = "embedding_functions"; /// produces, which is what lets a query embed its own text. const COLUMN_DEFINITIONS_META_KEY: &str = "lancedb::column_definitions"; -/// Value of the definition's `kind` tag for the projected `select` form. +/// Value of the definition's `kind` tag for the projected `select` form +/// over a root-namespace source. Reserved for root: releases that predate +/// [`NAMESPACED_SELECT_KIND`] parse this kind and resolve its source at the +/// root, so a `select` definition must never carry a namespace. pub const SELECT_KIND: &str = "select"; +/// The `select` form over a namespaced source. A separate kind because it is +/// a version boundary: readers that predate it drop unknown fields, so under +/// [`SELECT_KIND`] they would silently resolve the source at the root and +/// refresh from the wrong table. This kind routes them to the +/// [`MaterializedViewKind::Unrecognized`] refusal instead. +pub const NAMESPACED_SELECT_KIND: &str = "namespaced_select"; + /// Which view outputs each source column is projected to directly. A column /// may be projected more than once, so each carries every name the view gives /// it, in projection order. @@ -133,7 +143,12 @@ pub(crate) fn definition_to_metadata(definition: &MaterializedViewDefinition) -> let mut value = serde_json::to_value(definition).map_err(|e| Error::Runtime { message: format!("failed to serialize view definition: {e}"), })?; - value["kind"] = serde_json::Value::String(SELECT_KIND.to_string()); + let kind = if definition.source_namespace.is_empty() { + SELECT_KIND + } else { + NAMESPACED_SELECT_KIND + }; + value["kind"] = serde_json::Value::String(kind.to_string()); Ok(value.to_string()) } @@ -154,12 +169,23 @@ pub fn materialized_view_kind( .get("kind") .and_then(|k| k.as_str()) .ok_or_else(|| unreadable(&"missing kind tag"))?; - if kind != SELECT_KIND { + if kind != SELECT_KIND && kind != NAMESPACED_SELECT_KIND { return Ok(Some(MaterializedViewKind::Unrecognized { kind: kind.to_string(), })); } - let definition = serde_json::from_value(value).map_err(|e| unreadable(&e))?; + let kind = kind.to_string(); + let definition: MaterializedViewDefinition = + serde_json::from_value(value).map_err(|e| unreadable(&e))?; + // The kind states where the source lives; a mismatch is a definition no + // correct writer produces, and under `select` one that pre-namespace + // readers would resolve at the root. + if (kind == SELECT_KIND) != definition.source_namespace.is_empty() { + return Err(unreadable(&format!( + "kind '{kind}' does not match its source namespace {:?}", + definition.source_namespace + ))); + } Ok(Some(MaterializedViewKind::Select(definition))) } @@ -2191,4 +2217,72 @@ mod tests { let definition: MaterializedViewDefinition = serde_json::from_str(stored).unwrap(); assert!(definition.source_namespace.is_empty()); } + + fn definition(source_namespace: Vec) -> MaterializedViewDefinition { + MaterializedViewDefinition { + source_table: "people".to_string(), + source_namespace, + projections: vec![ViewProjection { + output: "name".to_string(), + expression: "name".to_string(), + }], + filter: None, + limit: None, + inputs: vec!["name".to_string()], + } + } + + /// The stored kind is a version boundary. A root definition keeps the + /// pre-namespace `select` form with no namespace key, so older releases + /// read it unchanged. A namespaced one moves to `namespaced_select`: + /// their readers drop unknown fields and resolve `select` sources at the + /// root, so keeping the kind would refresh from the wrong table -- + /// instead the unfamiliar kind routes them to the `Unrecognized` refusal + /// (`test_unrecognized_kind_is_refused_by_name` is that path). + #[test] + fn a_namespaced_definition_is_refused_by_the_pre_namespace_reader() { + let root = definition_to_metadata(&definition(Vec::new())).unwrap(); + let root: serde_json::Value = serde_json::from_str(&root).unwrap(); + assert_eq!(root["kind"], "select"); + assert!( + root.get("source_namespace").is_none(), + "a root definition must not grow new keys: {root}" + ); + + let stored = definition_to_metadata(&definition(vec!["ns".to_string()])).unwrap(); + let value: serde_json::Value = serde_json::from_str(&stored).unwrap(); + // The pre-namespace discriminator is `kind == "select"`; anything + // else lands in its Unrecognized refusal rather than in a root open. + assert_eq!(value["kind"], "namespaced_select"); + + // The current reader round-trips the coordinate. + let metadata = HashMap::from([(DEFINITION_META_KEY.to_string(), stored)]); + match materialized_view_kind(&metadata).unwrap() { + Some(MaterializedViewKind::Select(read)) => { + assert_eq!(read.source_namespace, vec!["ns".to_string()]) + } + other => panic!("expected the namespaced select form, got {other:?}"), + } + } + + /// A kind that disagrees with its namespace is a definition no correct + /// writer produces; under `select` it is exactly the shape pre-namespace + /// readers would resolve at the root, so it is an error, not a view. + #[test] + fn a_kind_namespace_mismatch_is_refused() { + for (kind, namespace) in [ + (SELECT_KIND, vec!["ns".to_string()]), + (NAMESPACED_SELECT_KIND, Vec::new()), + ] { + let mut value = serde_json::to_value(definition(namespace)).unwrap(); + value["kind"] = serde_json::Value::String(kind.to_string()); + let metadata = HashMap::from([(DEFINITION_META_KEY.to_string(), value.to_string())]); + let err = materialized_view_kind(&metadata).unwrap_err(); + assert!( + err.to_string() + .contains("does not match its source namespace"), + "kind '{kind}': {err}" + ); + } + } }