mirror of
https://github.com/lancedb/lancedb.git
synced 2026-09-04 04:28:44 +00:00
fix: store namespaced view definitions under their own kind
Keeping `kind: "select"` for a namespaced definition was a silent downgrade hazard: released readers drop unknown fields and resolve a select source at the root, so a rolled-back worker could refresh a view from a same-name root table and certify the wrong rows. Root definitions keep the `select` form byte-for-byte; a namespaced source is stored as `namespaced_select`, which older readers route to their existing unrecognized-kind refusal. The reader also refuses a kind that disagrees with its namespace, since no correct writer produces one. The Python and Node definition parsers learn the new kind alongside the Rust core, so every same-version reader agrees.
This commit is contained in:
@@ -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)"]],
|
||||
|
||||
@@ -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 ?? [],
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -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", []),
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -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"
|
||||
)
|
||||
|
||||
@@ -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<String>) -> 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}"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user