From 3ae1e8cb404ecb242c0255e5d7c781bdde8932ad Mon Sep 17 00:00:00 2001 From: OceanLi <122793010+ohdearquant@users.noreply.github.com> Date: Mon, 3 Aug 2026 13:10:38 -0400 Subject: [PATCH 1/2] fix(filter): make logical variants struct-shaped so the crate compiles `FilterExpression` is internally tagged (`#[serde(tag = "type")]`) and recursive: `And`/`Or`/`Not` carried `Vec` and `Box` as newtype variants. serde's derive routes an internally tagged newtype variant through `TaggedSerializer`, so a self-referential newtype variant asks the compiler for `TaggedSerializer>` with no bound. Building the crate's test target spins in type inference rather than failing fast; the `recursion_limit` escalations (2048, then 4096) only moved the wall further out. The same shape is a runtime error independent of the build: serde cannot serialize an internally tagged newtype variant whose payload is a sequence, so `And` and `Or` values could never be encoded ("cannot serialize tagged newtype variant ... containing a sequence"). Convert the three logical variants to struct variants (`And { exprs }`, `Or { exprs }`, `Not { expr }`). Struct variants serialize their fields directly through the outer serializer, so no wrapper type nests and the encoding of the comparison variants is unchanged. The `and`/`or`/`not` builders keep their signatures, so callers constructing through them are unaffected. The crate now builds at the default recursion limit, so the explicit `recursion_limit` attribute is dropped. Adds a round-trip test over a nested and/not/or expression, which previously failed at serialization time. --- crates/ruvector-filter/src/evaluator.rs | 12 +++---- crates/ruvector-filter/src/expression.rs | 44 +++++++++++++++++++----- crates/ruvector-filter/src/lib.rs | 2 -- 3 files changed, 41 insertions(+), 17 deletions(-) diff --git a/crates/ruvector-filter/src/evaluator.rs b/crates/ruvector-filter/src/evaluator.rs index 9af3486460..92c8ac4071 100644 --- a/crates/ruvector-filter/src/evaluator.rs +++ b/crates/ruvector-filter/src/evaluator.rs @@ -41,9 +41,9 @@ impl<'a> FilterEvaluator<'a> { top_left, bottom_right, } => self.evaluate_geo_bbox(field, *top_left, *bottom_right), - FilterExpression::And(filters) => self.evaluate_and(filters), - FilterExpression::Or(filters) => self.evaluate_or(filters), - FilterExpression::Not(filter) => self.evaluate_not(filter), + FilterExpression::And { exprs } => self.evaluate_and(exprs), + FilterExpression::Or { exprs } => self.evaluate_or(exprs), + FilterExpression::Not { expr } => self.evaluate_not(expr), FilterExpression::Exists { field } => self.evaluate_exists(field), FilterExpression::IsNull { field } => self.evaluate_is_null(field), } @@ -103,9 +103,9 @@ impl<'a> FilterEvaluator<'a> { FilterExpression::Match { field, text } => Self::get_field_value(payload, field) .and_then(|v| v.as_str()) .is_some_and(|s| s.to_lowercase().contains(&text.to_lowercase())), - FilterExpression::And(filters) => filters.iter().all(|f| self.matches(payload, f)), - FilterExpression::Or(filters) => filters.iter().any(|f| self.matches(payload, f)), - FilterExpression::Not(filter) => !self.matches(payload, filter), + FilterExpression::And { exprs } => exprs.iter().all(|f| self.matches(payload, f)), + FilterExpression::Or { exprs } => exprs.iter().any(|f| self.matches(payload, f)), + FilterExpression::Not { expr } => !self.matches(payload, expr), FilterExpression::Exists { field } => Self::get_field_value(payload, field).is_some(), FilterExpression::IsNull { field } => { Self::get_field_value(payload, field).is_none_or(|v| v.is_null()) diff --git a/crates/ruvector-filter/src/expression.rs b/crates/ruvector-filter/src/expression.rs index c19257f24c..eb2ea61499 100644 --- a/crates/ruvector-filter/src/expression.rs +++ b/crates/ruvector-filter/src/expression.rs @@ -64,9 +64,15 @@ pub enum FilterExpression { }, // Logical operators - And(Vec), - Or(Vec), - Not(Box), + And { + exprs: Vec, + }, + Or { + exprs: Vec, + }, + Not { + expr: Box, + }, // Existence check Exists { @@ -176,19 +182,21 @@ impl FilterExpression { /// Create an AND filter pub fn and(filters: Vec) -> Self { - Self::And(filters) + Self::And { exprs: filters } } /// Create an OR filter pub fn or(filters: Vec) -> Self { - Self::Or(filters) + Self::Or { exprs: filters } } /// Create a NOT filter // Public API constructor mirrors `and`/`or`; not the `std::ops::Not` trait. #[allow(clippy::should_implement_trait)] pub fn not(filter: FilterExpression) -> Self { - Self::Not(Box::new(filter)) + Self::Not { + expr: Box::new(filter), + } } /// Create an EXISTS filter @@ -231,12 +239,12 @@ impl FilterExpression { | Self::IsNull { field } => { fields.push(field.clone()); } - Self::And(exprs) | Self::Or(exprs) => { + Self::And { exprs } | Self::Or { exprs } => { for expr in exprs { expr.collect_fields(fields); } } - Self::Not(expr) => { + Self::Not { expr } => { expr.collect_fields(fields); } } @@ -257,7 +265,7 @@ mod tests { FilterExpression::eq("status", json!("active")), FilterExpression::gte("age", json!(18)), ]); - assert!(matches!(filter, FilterExpression::And(_))); + assert!(matches!(filter, FilterExpression::And { .. })); } #[test] @@ -281,4 +289,22 @@ mod tests { let deserialized: FilterExpression = serde_json::from_str(&json).unwrap(); assert!(matches!(deserialized, FilterExpression::Eq { .. })); } + + #[test] + fn test_logical_operator_round_trip() { + let filter = FilterExpression::and(vec![ + FilterExpression::eq("status", json!("active")), + FilterExpression::not(FilterExpression::or(vec![ + FilterExpression::lt("score", json!(10)), + FilterExpression::exists("banned"), + ])), + ]); + + let encoded = serde_json::to_string(&filter).unwrap(); + assert!(encoded.contains("\"type\":\"and\"")); + + let decoded: FilterExpression = serde_json::from_str(&encoded).unwrap(); + assert_eq!(decoded.get_fields(), filter.get_fields()); + assert!(matches!(decoded, FilterExpression::And { .. })); + } } diff --git a/crates/ruvector-filter/src/lib.rs b/crates/ruvector-filter/src/lib.rs index 5da9ac64fc..8ef8f65ee5 100644 --- a/crates/ruvector-filter/src/lib.rs +++ b/crates/ruvector-filter/src/lib.rs @@ -1,5 +1,3 @@ -#![recursion_limit = "4096"] - //! # rUvector Filter //! //! Advanced payload indexing and filtering for rUvector. From 2068dac7f708d49a22a795ac20900fa0fd4a6e0b Mon Sep 17 00:00:00 2001 From: OceanLi <122793010+ohdearquant@users.noreply.github.com> Date: Mon, 3 Aug 2026 13:33:32 -0400 Subject: [PATCH 2/2] test(filter): pin the logical-variant wire shape; refresh the stack-size note `test_logical_operator_round_trip` asserted only the root tag and the field set, so it would still have passed with a nested variant encoded under the wrong field name. It now compares `serde_json::to_value` against the complete expected nested object and re-checks that the decoded value re-encodes to the same object, which pins `{"type":"and","exprs":[...]}`, `{"type":"not","expr":{...}}` and `{"type":"or","exprs":[...]}`. The workspace `RUST_MIN_STACK` comment justified itself with `ruvector-filter`'s `#![recursion_limit]` and the trait-resolution recursion behind it, both of which this branch removes. The comment now records that, and records the measurement: `cargo test -p ruvector-filter` builds and passes with `RUST_MIN_STACK=2097152`, well under the 8 MB default. The setting itself is left in place, since nothing here measures whether the rest of the workspace still needs it. --- .cargo/config.toml | 14 ++++++------ crates/ruvector-filter/src/expression.rs | 27 ++++++++++++++++++------ 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/.cargo/config.toml b/.cargo/config.toml index 09f70a0241..618d8ba332 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -4,12 +4,14 @@ git-fetch-with-cli = true [registries.crates-io] protocol = "git" -# `ruvector-filter` carries `#![recursion_limit = "4096"]` (PR #389) to -# survive trait-resolution overflow when serde_json's `Serializer` blanket -# impl recurses through the crate's expression types. The deeper resolution -# in turn drives rustc's own process stack past the default 8 MB on x86_64 -# Linux, so cargo test/cargo check must run with a larger thread stack. -# 16 MB is rustc's documented suggested value when this happens. +# `ruvector-filter` no longer needs a raised stack: its `FilterExpression` +# logical variants (`And`/`Or`/`Not`) are struct-shaped now, which removed +# the trait-resolution recursion that used to overflow rustc's process +# stack, and `cargo test -p ruvector-filter` builds and runs clean even at +# a 2 MB `RUST_MIN_STACK` (well under the 8 MB default). This setting is +# kept anyway because no workspace-wide measurement has been done to show +# every other crate is fine without it — treat it as still load-bearing +# for the workspace as a whole until that measurement happens. [env] RUST_MIN_STACK = "16777216" diff --git a/crates/ruvector-filter/src/expression.rs b/crates/ruvector-filter/src/expression.rs index eb2ea61499..5c6d3ddd03 100644 --- a/crates/ruvector-filter/src/expression.rs +++ b/crates/ruvector-filter/src/expression.rs @@ -300,11 +300,26 @@ mod tests { ])), ]); - let encoded = serde_json::to_string(&filter).unwrap(); - assert!(encoded.contains("\"type\":\"and\"")); - - let decoded: FilterExpression = serde_json::from_str(&encoded).unwrap(); - assert_eq!(decoded.get_fields(), filter.get_fields()); - assert!(matches!(decoded, FilterExpression::And { .. })); + // Exact wire shape, per the `#[serde(tag = "type", rename_all = "snake_case")]` + // enum definition: each variant is `{"type": , ...struct fields}`. + let expected = json!({ + "type": "and", + "exprs": [ + {"type": "eq", "field": "status", "value": "active"}, + {"type": "not", "expr": { + "type": "or", + "exprs": [ + {"type": "lt", "field": "score", "value": 10}, + {"type": "exists", "field": "banned"}, + ] + }}, + ] + }); + + let actual = serde_json::to_value(&filter).unwrap(); + assert_eq!(actual, expected); + + let decoded: FilterExpression = serde_json::from_value(actual).unwrap(); + assert_eq!(serde_json::to_value(&decoded).unwrap(), expected); } }