From 8885af23a3536de7237c77d8e13eed4fd5c72b5a Mon Sep 17 00:00:00 2001 From: Jake Boone Date: Thu, 20 Aug 2026 16:56:35 -0700 Subject: [PATCH 1/3] Add support for anonymous traversals in Gremlin queries --- .../src/query/gremlin/lexer.rs | 46 ++++++- .../src/query/gremlin/parser.rs | 99 ++++++++++++++- .../src/query/translators/gremlin.rs | 117 +++++++++++------- crates/grafeo-engine/tests/gremlin.rs | 111 +++++++++++++++++ .../lpg/gremlin/anonymous_traversal.gtest | 115 +++++++++++++++++ 5 files changed, 437 insertions(+), 51 deletions(-) create mode 100644 tests/spec/lpg/gremlin/anonymous_traversal.gtest diff --git a/crates/grafeo-adapters/src/query/gremlin/lexer.rs b/crates/grafeo-adapters/src/query/gremlin/lexer.rs index 0f66b5f24..d41c8ec72 100644 --- a/crates/grafeo-adapters/src/query/gremlin/lexer.rs +++ b/crates/grafeo-adapters/src/query/gremlin/lexer.rs @@ -249,6 +249,8 @@ pub enum TokenKind { RBracket, /// Underscore (`_`) token. Underscore, + /// Anonymous traversal (`__`). + Anon, /// A `$name` parameter reference. Parameter(String), @@ -307,7 +309,9 @@ impl<'a> Lexer<'a> { Some(')') => TokenKind::RParen, Some('[') => TokenKind::LBracket, Some(']') => TokenKind::RBracket, - Some('_') if self.peek_is(|c| !c.is_alphanumeric()) => TokenKind::Underscore, + Some('_') if self.peek_is(|c| !c.is_alphanumeric() && c != '_') => { + TokenKind::Underscore + } Some('"') => self.read_string('"'), Some('\'') => self.read_string('\''), @@ -467,6 +471,7 @@ impl<'a> Lexer<'a> { // Match keywords match value.as_str() { + "__" => TokenKind::Anon, "g" => TokenKind::G, "V" => TokenKind::V, "E" => TokenKind::E, @@ -616,4 +621,43 @@ mod tests { assert_eq!(tokens[1].kind, TokenKind::Dot); assert_eq!(tokens[2].kind, TokenKind::Gt); } + + #[test] + fn test_anon_token() { + let tokens = Lexer::new("__").tokenize(); + assert_eq!(tokens[0].kind, TokenKind::Anon); + assert_eq!(tokens[1].kind, TokenKind::Eof); + } + + #[test] + fn test_single_underscore_still_underscore() { + // Regression guard for the tightened `_` lookahead. (A bare `_` at EOF has always + // lexed as an identifier, since `peek_is` is false on `None`.) + let tokens = Lexer::new("_ ").tokenize(); + assert_eq!(tokens[0].kind, TokenKind::Underscore); + assert_eq!(tokens[1].kind, TokenKind::Eof); + + let tokens = Lexer::new("_.has('a', 1)").tokenize(); + assert_eq!(tokens[0].kind, TokenKind::Underscore); + assert_eq!(tokens[1].kind, TokenKind::Dot); + } + + #[test] + fn test_double_underscore_prefixed_identifier() { + let tokens = Lexer::new("__foo").tokenize(); + assert_eq!(tokens[0].kind, TokenKind::Identifier("__foo".to_string())); + assert_eq!(tokens[1].kind, TokenKind::Eof); + + let tokens = Lexer::new("___").tokenize(); + assert_eq!(tokens[0].kind, TokenKind::Identifier("___".to_string())); + } + + #[test] + fn test_anon_traversal_prefix() { + let tokens = Lexer::new("__.has('a', 1)").tokenize(); + assert_eq!(tokens[0].kind, TokenKind::Anon); + assert_eq!(tokens[1].kind, TokenKind::Dot); + assert_eq!(tokens[2].kind, TokenKind::Has); + assert_eq!(tokens[3].kind, TokenKind::LParen); + } } diff --git a/crates/grafeo-adapters/src/query/gremlin/parser.rs b/crates/grafeo-adapters/src/query/gremlin/parser.rs index b83a4efbd..5540cfce3 100644 --- a/crates/grafeo-adapters/src/query/gremlin/parser.rs +++ b/crates/grafeo-adapters/src/query/gremlin/parser.rs @@ -1009,8 +1009,8 @@ impl<'a> Parser<'a> { return Ok(FromTo::Traversal(steps)); } - // Check for bare V()/E() traversal (without 'g.' prefix) - if self.check(TokenKind::V) || self.check(TokenKind::E) { + // Check for bare V()/E() traversal (without 'g.' prefix), optionally `__.`-prefixed + if self.check(TokenKind::V) || self.check(TokenKind::E) || self.check(TokenKind::Anon) { let steps = self.parse_bare_traversal()?; return Ok(FromTo::Traversal(steps)); } @@ -1023,6 +1023,7 @@ impl<'a> Parser<'a> { /// responsible for consuming the outer delimiters. fn parse_inner_steps(&mut self) -> Result> { self.enter_nesting()?; + self.skip_anon_prefix()?; let mut steps = Vec::new(); // Parse first step let step = self.parse_step()?; @@ -1053,6 +1054,7 @@ impl<'a> Parser<'a> { /// Used inside from()/to() arguments, e.g. `from(V().has('name', 'Gus'))`. fn parse_bare_traversal(&mut self) -> Result> { self.enter_nesting()?; + self.skip_anon_prefix()?; // Parse source (V, E, etc.) and convert to a step let source = self.parse_source()?; @@ -1195,6 +1197,15 @@ impl<'a> Parser<'a> { } } + /// Consume the optional `__.` anonymous-traversal prefix. + fn skip_anon_prefix(&mut self) -> Result<()> { + if self.check(TokenKind::Anon) { + self.advance(); + self.expect(TokenKind::Dot)?; + } + Ok(()) + } + fn check(&self, kind: TokenKind) -> bool { self.current_kind() == Some(&kind) } @@ -1941,4 +1952,88 @@ mod tests { "300 levels of or() should error, not stack overflow" ); } + + // ------------------------------------------------------------------------- + // Anonymous traversal (`__`) prefix + // ------------------------------------------------------------------------- + + fn parse_ok(query: &str) -> Statement { + Parser::new(query) + .parse() + .unwrap_or_else(|e| panic!("{query} should parse: {e}")) + } + + #[test] + fn test_anon_prefix_in_or() { + let stmt = parse_ok("g.V().or(__.has('a', 1), __.has('b', 2))"); + let Step::Or(traversals) = &stmt.steps[0] else { + panic!("expected Step::Or, got {:?}", stmt.steps[0]); + }; + assert_eq!(traversals.len(), 2); + assert!(matches!(traversals[0][0], Step::Has(_))); + assert!(matches!(traversals[1][0], Step::Has(_))); + } + + #[test] + fn test_anon_prefix_is_optional_and_mixable() { + let prefixed = parse_ok("g.V().or(__.has('a', 1), __.has('b', 2))"); + let mixed = parse_ok("g.V().or(__.has('a', 1), has('b', 2))"); + let bare = parse_ok("g.V().or(has('a', 1), has('b', 2))"); + assert_eq!( + format!("{:?}", prefixed.steps), + format!("{:?}", mixed.steps) + ); + assert_eq!(format!("{:?}", prefixed.steps), format!("{:?}", bare.steps)); + } + + #[test] + fn test_anon_prefix_nested_compound() { + let stmt = parse_ok("g.V().or(__.and(__.has('a', 1), __.has('b', 2)), __.has('c', 3))"); + let Step::Or(traversals) = &stmt.steps[0] else { + panic!("expected Step::Or, got {:?}", stmt.steps[0]); + }; + assert_eq!(traversals.len(), 2); + let Step::And(inner) = &traversals[0][0] else { + panic!("expected nested Step::And, got {:?}", traversals[0][0]); + }; + assert_eq!(inner.len(), 2); + assert!(matches!(traversals[1][0], Step::Has(_))); + } + + #[test] + fn test_anon_prefix_in_not() { + let stmt = parse_ok("g.V().not(__.has('a', 1))"); + let Step::Not(steps) = &stmt.steps[0] else { + panic!("expected Step::Not, got {:?}", stmt.steps[0]); + }; + assert_eq!(steps.len(), 1); + } + + #[test] + fn test_multi_arg_not_still_rejected() { + // TinkerPop's not() takes exactly one traversal. + assert!( + Parser::new("g.V().not(__.has('a', 1), __.has('b', 2))") + .parse() + .is_err() + ); + } + + #[test] + fn test_anon_prefix_in_where() { + let stmt = parse_ok("g.V().where(__.out('knows'))"); + let Step::Where(WhereClause::Traversal(steps)) = &stmt.steps[0] else { + panic!("expected Step::Where(Traversal), got {:?}", stmt.steps[0]); + }; + assert!(matches!(steps[0], Step::Out(_))); + } + + #[test] + fn test_anon_prefix_chained_inner_steps() { + let stmt = parse_ok("g.V().where(__.out('knows').has('name', 'Gus'))"); + let Step::Where(WhereClause::Traversal(steps)) = &stmt.steps[0] else { + panic!("expected Step::Where(Traversal), got {:?}", stmt.steps[0]); + }; + assert_eq!(steps.len(), 2); + } } diff --git a/crates/grafeo-engine/src/query/translators/gremlin.rs b/crates/grafeo-engine/src/query/translators/gremlin.rs index ac0757924..8d4a691b9 100644 --- a/crates/grafeo-engine/src/query/translators/gremlin.rs +++ b/crates/grafeo-engine/src/query/translators/gremlin.rs @@ -79,6 +79,38 @@ struct EdgeContext { direction: ExpandDirection, } +/// Fold `preds` into one expression joined by `op`. Returns `None` when empty. +/// +/// Right-fold: last element is the innermost right operand, matching the shape the +/// `and()`/`or()` arms have always produced. +fn fold_predicates(mut preds: Vec, op: BinaryOp) -> Option { + let mut combined = preds.pop()?; + for pred in preds { + combined = LogicalExpression::Binary { + left: Box::new(pred), + op, + right: Box::new(combined), + }; + } + Some(combined) +} + +/// TinkerPop `not()` semantics: a traverser missing the property is *retained*. +/// SQL `NOT NULL` is `NULL` (row dropped), so widen to `NOT(p) OR p IS NULL`. +fn negate_null_safe(pred: LogicalExpression) -> LogicalExpression { + LogicalExpression::Binary { + left: Box::new(LogicalExpression::Unary { + op: UnaryOp::Not, + operand: Box::new(pred.clone()), + }), + op: BinaryOp::Or, + right: Box::new(LogicalExpression::Unary { + op: UnaryOp::IsNull, + operand: Box::new(pred), + }), + } +} + impl GremlinTranslator { fn new() -> Self { Self { @@ -1422,21 +1454,10 @@ impl GremlinTranslator { predicates.push(pred); } } - if predicates.is_empty() { - return Ok((input, None)); - } - let mut combined = predicates - .pop() - .expect("predicates non-empty after is_empty check"); - for pred in predicates { - combined = LogicalExpression::Binary { - left: Box::new(pred), - op: BinaryOp::And, - right: Box::new(combined), - }; + match fold_predicates(predicates, BinaryOp::And) { + Some(combined) => Ok((wrap_filter(input, combined), None)), + None => Ok((input, None)), } - let plan = wrap_filter(input, combined); - Ok((plan, None)) } // or() filter: at least one sub-traversal must produce results @@ -1447,34 +1468,16 @@ impl GremlinTranslator { predicates.push(pred); } } - if predicates.is_empty() { - return Ok((input, None)); - } - let mut combined = predicates - .pop() - .expect("predicates non-empty after is_empty check"); - for pred in predicates { - combined = LogicalExpression::Binary { - left: Box::new(pred), - op: BinaryOp::Or, - right: Box::new(combined), - }; + match fold_predicates(predicates, BinaryOp::Or) { + Some(combined) => Ok((wrap_filter(input, combined), None)), + None => Ok((input, None)), } - let plan = wrap_filter(input, combined); - Ok((plan, None)) } - // not() filter: negate a sub-traversal filter + // not() filter: negate a sub-traversal filter (null-safe, per TinkerPop) ast::Step::Not(steps) => { if let Some(pred) = self.steps_to_predicate(steps, current_var)? { - let plan = wrap_filter( - input, - LogicalExpression::Unary { - op: UnaryOp::Not, - operand: Box::new(pred), - }, - ); - Ok((plan, None)) + Ok((wrap_filter(input, negate_null_safe(pred)), None)) } else { Ok((input, None)) } @@ -2290,23 +2293,41 @@ impl GremlinTranslator { }); predicates.push(LogicalExpression::ExistsSubquery(Box::new(expand))); } + // Nested compound groups: recurse so `.or(__.and(..), __.has(..))` is not + // silently dropped by the fallthrough below. + ast::Step::And(traversals) | ast::Step::Or(traversals) => { + let op = if matches!(step, ast::Step::And(_)) { + BinaryOp::And + } else { + BinaryOp::Or + }; + let mut inner: Vec = Vec::new(); + for sub in traversals { + if let Some(pred) = self.steps_to_predicate(sub, current_var)? { + inner.push(pred); + } + } + if let Some(pred) = fold_predicates(inner, op) { + predicates.push(pred); + } + } + ast::Step::Not(inner_steps) => { + if let Some(pred) = self.steps_to_predicate(inner_steps, current_var)? { + predicates.push(negate_null_safe(pred)); + } + } + ast::Step::Where(ast::WhereClause::Traversal(inner_steps)) => { + if let Some(pred) = self.steps_to_predicate(inner_steps, current_var)? { + predicates.push(pred); + } + } _ => {} } } if predicates.is_empty() { return Ok(None); } - let mut result = predicates - .pop() - .expect("predicates non-empty after is_empty check"); - for pred in predicates { - result = LogicalExpression::Binary { - left: Box::new(pred), - op: BinaryOp::And, - right: Box::new(result), - }; - } - Ok(Some(result)) + Ok(fold_predicates(predicates, BinaryOp::And)) } fn build_id_filter(&self, var: &str, ids: &[Value]) -> LogicalExpression { diff --git a/crates/grafeo-engine/tests/gremlin.rs b/crates/grafeo-engine/tests/gremlin.rs index 8be4ea895..413338668 100644 --- a/crates/grafeo-engine/tests/gremlin.rs +++ b/crates/grafeo-engine/tests/gremlin.rs @@ -1096,6 +1096,117 @@ fn test_step_not_excludes_matching() { ); } +// ============================================================================ +// Anonymous Traversals (`__`) +// ============================================================================ + +#[test] +fn test_anon_and_matches_bare_form() { + let db = create_social_network(); + let prefixed = db + .execute_gremlin("g.V().and(__.has('age', gt(25)), __.has('city', 'Amsterdam'))") + .unwrap(); + assert_eq!(prefixed.row_count(), 1, "Only Alix matches"); +} + +#[test] +fn test_anon_or_matches_bare_form() { + let db = create_social_network(); + let prefixed = db + .execute_gremlin("g.V().hasLabel('Person').or(__.has('age', 25), __.has('age', 35))") + .unwrap(); + assert_eq!(prefixed.row_count(), 2, "Gus (25) and Vincent (35)"); +} + +#[test] +fn test_anon_not_matches_bare_form() { + let db = create_social_network(); + let prefixed = db + .execute_gremlin("g.V().hasLabel('Person').not(__.has('city', 'Berlin'))") + .unwrap(); + assert_eq!(prefixed.row_count(), 2, "Alix and Vincent"); +} + +#[test] +fn test_anon_mixed_prefixed_and_bare_args() { + let db = create_social_network(); + let result = db + .execute_gremlin("g.V().or(__.has('city', 'Berlin'), has('city', 'Paris'))") + .unwrap(); + assert_eq!(result.row_count(), 2, "Gus (Berlin) and Vincent (Paris)"); +} + +#[test] +fn test_anon_where_traversal() { + let db = create_social_network(); + let result = db + .execute_gremlin("g.V().hasLabel('Person').where(__.out('KNOWS'))") + .unwrap(); + assert_eq!(result.row_count(), 2, "Alix and Gus have outgoing KNOWS"); +} + +#[test] +fn test_anon_nested_compound_group() { + // Regression: nested and()/or() inside a compound group used to be dropped + // silently, yielding only the last sub-traversal's predicate. + let db = create_social_network(); + let result = db + .execute_gremlin( + "g.V().hasLabel('Person')\ + .or(__.and(__.has('age', gt(25)), __.has('city', 'Amsterdam')), \ + __.has('city', 'Paris'))", + ) + .unwrap(); + assert_eq!( + result.row_count(), + 2, + "Alix (age>25 AND Amsterdam) and Vincent (Paris); Gus excluded" + ); +} + +#[test] +fn test_anon_nested_compound_group_and_of_ors() { + let db = create_social_network(); + let result = db + .execute_gremlin( + "g.V().hasLabel('Person')\ + .and(__.or(__.has('city', 'Berlin'), __.has('city', 'Paris')), \ + __.has('age', gt(30)))", + ) + .unwrap(); + assert_eq!( + result.row_count(), + 1, + "Only Vincent is (Berlin OR Paris) AND age > 30" + ); +} + +#[test] +fn test_not_retains_vertices_missing_the_property() { + // TinkerPop: not() keeps traversers where the inner traversal yields nothing, + // including vertices that simply lack the property. Acme (Company) has no `city`. + let db = create_social_network(); + let result = db + .execute_gremlin("g.V().not(__.has('city', 'Berlin'))") + .unwrap(); + assert_eq!( + result.row_count(), + 3, + "Alix, Vincent, and Acme (no `city` property) are retained; Gus excluded" + ); +} + +#[test] +fn test_nested_not_is_null_safe_too() { + let db = create_social_network(); + let result = db + .execute_gremlin( + "g.V().and(__.not(__.has('city', 'Berlin')), __.has('name', neq('Nobody')))", + ) + .unwrap(); + assert_eq!(result.row_count(), 3, "Alix, Vincent, Acme"); +} + // ============================================================================ // Compound Predicates via Equivalent Built-in Predicates // diff --git a/tests/spec/lpg/gremlin/anonymous_traversal.gtest b/tests/spec/lpg/gremlin/anonymous_traversal.gtest new file mode 100644 index 000000000..42b0feda8 --- /dev/null +++ b/tests/spec/lpg/gremlin/anonymous_traversal.gtest @@ -0,0 +1,115 @@ +# Gremlin anonymous traversals: the `__.` prefix inside sub-traversal arguments. +# +# Covers: `__` in and()/or()/not()/where(), mixing prefixed and bare arguments, +# nested compound groups, and TinkerPop's null-retaining not() semantics. +# +# Social network topology (from dataset): +# Persons: Alix(30, Amsterdam), Gus(25, Berlin), Vincent(35, Paris) +# Company: Grafeo OSS (founded 2025) - has no `age` or `city` property +# Edges: Alix-KNOWS->Gus, Gus-KNOWS->Vincent, Alix-WORKS_AT->Grafeo OSS + +meta: + language: gremlin + model: lpg + section: "anonymous-traversal" + title: Anonymous Traversals + dataset: social_network + +tests: + + # --------------------------------------------------------------------------- + # `__` inside and() / or() / not() + # --------------------------------------------------------------------------- + + - name: anon_in_and + query: "g.V().and(__.hasLabel('Person'), __.has('age', gt(25))).values('name')" + expect: + rows: + - [Alix] + - [Vincent] + + - name: anon_in_or + query: "g.V().hasLabel('Person').or(__.has('city', 'Berlin'), __.has('city', 'Paris')).values('name')" + expect: + rows: + - [Gus] + - [Vincent] + + - name: anon_in_not + query: "g.V().hasLabel('Person').not(__.has('city', 'Berlin')).values('name')" + expect: + rows: + - [Alix] + - [Vincent] + + # --------------------------------------------------------------------------- + # The prefix is optional and may be mixed with bare arguments + # --------------------------------------------------------------------------- + + - name: anon_mixed_with_bare_args + query: "g.V().or(__.has('city', 'Berlin'), has('city', 'Paris')).values('name')" + expect: + rows: + - [Gus] + - [Vincent] + + - name: anon_chained_inner_steps + skip: "pre-existing, unrelated to `__`: steps_to_predicate applies the chained has() to the outer vertex instead of the out() target, so this returns Gus" + query: "g.V().hasLabel('Person').where(__.out('KNOWS').has('name', 'Gus')).values('name')" + expect: + rows: + - [Alix] + + # --------------------------------------------------------------------------- + # Nested compound groups (previously dropped silently) + # --------------------------------------------------------------------------- + + - name: anon_nested_or_of_and + query: "g.V().hasLabel('Person').or(__.and(__.has('age', gt(25)), __.has('city', 'Amsterdam')), __.has('city', 'Paris')).values('name')" + expect: + rows: + - [Alix] + - [Vincent] + + - name: anon_nested_and_of_or + query: "g.V().hasLabel('Person').and(__.or(__.has('city', 'Berlin'), __.has('city', 'Paris')), __.has('age', gt(30))).values('name')" + expect: + rows: + - [Vincent] + + - name: anon_nested_not_inside_and + query: "g.V().hasLabel('Person').and(__.not(__.has('city', 'Berlin')), __.has('age', lt(35))).values('name')" + expect: + rows: + - [Alix] + + # --------------------------------------------------------------------------- + # where(traversal) + # --------------------------------------------------------------------------- + + - name: anon_in_where_out + query: "g.V().hasLabel('Person').where(__.out('KNOWS')).values('name')" + expect: + rows: + - [Alix] + - [Gus] + + # --------------------------------------------------------------------------- + # not() null retention: TinkerPop keeps traversers whose inner traversal is + # empty, including vertices that simply lack the property. + # --------------------------------------------------------------------------- + + - name: not_retains_vertices_missing_property + # Grafeo OSS (Company) has no `city`, so it survives not(has('city', ...)). + query: "g.V().not(__.has('city', 'Berlin')).values('name')" + expect: + rows: + - [Alix] + - [Vincent] + - ["Grafeo OSS"] + + - name: not_retains_vertices_missing_property_count + query: "g.V().not(__.has('age', lt(100))).count()" + expect: + rows: + - [1] From 048f48f6c39de1fd9550efacf2159028889affe8 Mon Sep 17 00:00:00 2001 From: Jake Boone Date: Thu, 20 Aug 2026 17:17:28 -0700 Subject: [PATCH 2/3] Refactor assertions to use assert_eq! for consistency in tests --- crates/grafeo-common/src/types/value.rs | 2 +- crates/grafeo-core/src/codec/delta.rs | 2 +- crates/grafeo-core/src/codec/selector.rs | 8 ++----- .../src/execution/operators/filter.rs | 20 ++++++---------- crates/grafeo-core/src/graph/projection.rs | 4 ++-- crates/grafeo-core/src/index/adjacency.rs | 2 +- crates/grafeo-core/src/index/vector/hnsw.rs | 2 +- .../grafeo-core/src/index/vector/storage.rs | 12 ++++------ crates/grafeo-engine/src/query/plan.rs | 2 +- .../src/query/planner/lpg/filter_hybrid.rs | 4 ++-- .../src/query/planner/rdf/mod.rs | 2 +- crates/grafeo-engine/src/query/processor.rs | 2 +- .../src/query/translators/gremlin.rs | 24 +++++++++++++++---- crates/grafeo-engine/tests/sql_pgq.rs | 4 ++-- .../lpg/gremlin/anonymous_traversal.gtest | 1 - 15 files changed, 45 insertions(+), 46 deletions(-) diff --git a/crates/grafeo-common/src/types/value.rs b/crates/grafeo-common/src/types/value.rs index 6eddfa164..578128a32 100644 --- a/crates/grafeo-common/src/types/value.rs +++ b/crates/grafeo-common/src/types/value.rs @@ -1384,7 +1384,7 @@ mod tests { assert!(v1 < v2); assert!(v2 < v_inf); assert!(v_inf < v_nan); // NaN is greater than everything - assert!(v_nan == v_nan); // NaN equals itself for total ordering + assert_eq!(v_nan, v_nan); // NaN equals itself for total ordering } #[test] diff --git a/crates/grafeo-core/src/codec/delta.rs b/crates/grafeo-core/src/codec/delta.rs index 04e2cd56f..348f11b5f 100644 --- a/crates/grafeo-core/src/codec/delta.rs +++ b/crates/grafeo-core/src/codec/delta.rs @@ -370,6 +370,6 @@ mod tests { let sequential: Vec = (0..1000).collect(); let encoded = DeltaEncoding::encode(&sequential); // Each delta is 1, so compression is minimal but base + deltas is stored - assert!(encoded.len() == 1000); + assert_eq!(encoded.len(), 1000); } } diff --git a/crates/grafeo-core/src/codec/selector.rs b/crates/grafeo-core/src/codec/selector.rs index ad5df44e6..96890a4f1 100644 --- a/crates/grafeo-core/src/codec/selector.rs +++ b/crates/grafeo-core/src/codec/selector.rs @@ -374,12 +374,8 @@ impl TypeSpecificCompressor { match data.codec { CompressionCodec::None => { let mut values = Vec::with_capacity(data.data.len() / 8); - for chunk in data.data.chunks_exact(8) { - values.push(u64::from_le_bytes( - chunk - .try_into() - .map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e))?, - )); + for chunk in data.data.as_chunks::<8>().0 { + values.push(u64::from_le_bytes(*chunk)); } Ok(values) } diff --git a/crates/grafeo-core/src/execution/operators/filter.rs b/crates/grafeo-core/src/execution/operators/filter.rs index 5cd43d315..46d42db88 100644 --- a/crates/grafeo-core/src/execution/operators/filter.rs +++ b/crates/grafeo-core/src/execution/operators/filter.rs @@ -926,18 +926,15 @@ impl ExpressionPredicate { } => { let list_val = self.eval_expr(list_expr, chunk, row)?; // Accept both List and Vector as iterable sequences - let items: Vec<&Value>; let vec_items: Vec; - match &list_val { - Value::List(list) => { - items = list.iter().collect(); - } + let items: Vec<&Value> = match &list_val { + Value::List(list) => list.iter().collect(), Value::Vector(vec) => { vec_items = vec.iter().map(|&f| Value::Float64(f64::from(f))).collect(); - items = vec_items.iter().collect(); + vec_items.iter().collect() } _ => return None, - } + }; let mut match_count: u32 = 0; for item in &items { @@ -2285,10 +2282,9 @@ impl ExpressionPredicate { let col = chunk.column(col_idx)?; if let Some(nid) = col.get_node_id(row) { ids.push(nid.0); - } else if let Some(eid) = col.get_edge_id(row) { - ids.push(eid.0); } else { - return None; + let eid = col.get_edge_id(row)?; + ids.push(eid.0); } } else { return None; @@ -2330,10 +2326,8 @@ impl ExpressionPredicate { let col = chunk.column(col_idx)?; let current_id = if let Some(nid) = col.get_node_id(row) { nid.0 - } else if let Some(eid) = col.get_edge_id(row) { - eid.0 } else { - return None; + col.get_edge_id(row)?.0 }; match first_id { None => first_id = Some(current_id), diff --git a/crates/grafeo-core/src/graph/projection.rs b/crates/grafeo-core/src/graph/projection.rs index e97c7f1a7..eb6ccfbc5 100644 --- a/crates/grafeo-core/src/graph/projection.rs +++ b/crates/grafeo-core/src/graph/projection.rs @@ -657,8 +657,8 @@ mod tests { .with_edge_types(["KNOWS"]); let proj = GraphProjection::new(store, spec); - assert!(proj.estimate_label_cardinality("City") == 0.0); - assert!(proj.estimate_avg_degree("LIVES_IN", true) == 0.0); + assert_eq!(proj.estimate_label_cardinality("City"), 0.0); + assert_eq!(proj.estimate_avg_degree("LIVES_IN", true), 0.0); } #[test] diff --git a/crates/grafeo-core/src/index/adjacency.rs b/crates/grafeo-core/src/index/adjacency.rs index 7d2fdf956..d02684003 100644 --- a/crates/grafeo-core/src/index/adjacency.rs +++ b/crates/grafeo-core/src/index/adjacency.rs @@ -1153,7 +1153,7 @@ mod tests { // Verify only odd-numbered destinations remain for neighbor in neighbors { - assert!(neighbor.as_u64() % 2 == 0); // Original IDs were i+1, so even means odd i + assert_eq!(neighbor.as_u64() % 2, 0); // Original IDs were i+1, so even means odd i } } diff --git a/crates/grafeo-core/src/index/vector/hnsw.rs b/crates/grafeo-core/src/index/vector/hnsw.rs index 26e31cb44..58a97d5a1 100644 --- a/crates/grafeo-core/src/index/vector/hnsw.rs +++ b/crates/grafeo-core/src/index/vector/hnsw.rs @@ -753,7 +753,7 @@ impl HnswIndex { } // Remove bidirectional links - for (_, node) in nodes_map.iter_mut() { + for node in nodes_map.values_mut() { for neighbors in &mut node.neighbors { neighbors.retain(|&n| n != id); } diff --git a/crates/grafeo-core/src/index/vector/storage.rs b/crates/grafeo-core/src/index/vector/storage.rs index e2fdad2f8..a19fd5aa6 100644 --- a/crates/grafeo-core/src/index/vector/storage.rs +++ b/crates/grafeo-core/src/index/vector/storage.rs @@ -474,14 +474,10 @@ impl VectorStorage for MmapStorage { // Convert bytes to f32 let vector: Vec = bytes - .chunks_exact(4) - .map(|chunk| { - f32::from_le_bytes( - chunk - .try_into() - .expect("chunks_exact(4) yields 4-byte slices"), - ) - }) + .as_chunks::<4>() + .0 + .iter() + .map(|chunk| f32::from_le_bytes(*chunk)) .collect(); let arc: Arc<[f32]> = vector.into(); diff --git a/crates/grafeo-engine/src/query/plan.rs b/crates/grafeo-engine/src/query/plan.rs index 6028d3d06..73289ac93 100644 --- a/crates/grafeo-engine/src/query/plan.rs +++ b/crates/grafeo-engine/src/query/plan.rs @@ -3029,7 +3029,7 @@ mod tests { // Display/Equality sanity assert_eq!(format!("{literal}"), "42"); assert_eq!(format!("{param}"), "$limit"); - assert!(literal == 42usize); + assert_eq!(literal, 42usize); } // ==================== CountExpr ==================== diff --git a/crates/grafeo-engine/src/query/planner/lpg/filter_hybrid.rs b/crates/grafeo-engine/src/query/planner/lpg/filter_hybrid.rs index ff871132f..c8c2e549d 100644 --- a/crates/grafeo-engine/src/query/planner/lpg/filter_hybrid.rs +++ b/crates/grafeo-engine/src/query/planner/lpg/filter_hybrid.rs @@ -247,10 +247,10 @@ impl super::Planner { // Try vector-left + text-right, then vector-right + text-left. let result = self .extract_vector_predicate(left) - .and_then(|v| self.extract_text_predicate(right).map(|t| (v, t))) + .zip(self.extract_text_predicate(right)) .or_else(|| { self.extract_vector_predicate(right) - .and_then(|v| self.extract_text_predicate(left).map(|t| (v, t))) + .zip(self.extract_text_predicate(left)) }); let Some((v, t)) = result else { return Ok(None); diff --git a/crates/grafeo-engine/src/query/planner/rdf/mod.rs b/crates/grafeo-engine/src/query/planner/rdf/mod.rs index c622536a2..ca15546eb 100644 --- a/crates/grafeo-engine/src/query/planner/rdf/mod.rs +++ b/crates/grafeo-engine/src/query/planner/rdf/mod.rs @@ -4190,7 +4190,7 @@ impl RdfExpressionPredicate { { // Regex-based replace let regex_pattern = if flags.contains('i') { - format!("(?i){}", &pattern) + format!("(?i){}", pattern) } else { pattern.clone() }; diff --git a/crates/grafeo-engine/src/query/processor.rs b/crates/grafeo-engine/src/query/processor.rs index 7d345c6e7..16c800186 100644 --- a/crates/grafeo-engine/src/query/processor.rs +++ b/crates/grafeo-engine/src/query/processor.rs @@ -1164,7 +1164,7 @@ mod tests { fn test_processor_creation() { let store = Arc::new(LpgStore::new().unwrap()); let processor = QueryProcessor::for_lpg(store); - assert!(processor.lpg_store().node_count() == 0); + assert_eq!(processor.lpg_store().node_count(), 0); } #[cfg(feature = "gql")] diff --git a/crates/grafeo-engine/src/query/translators/gremlin.rs b/crates/grafeo-engine/src/query/translators/gremlin.rs index 8d4a691b9..57b0bc205 100644 --- a/crates/grafeo-engine/src/query/translators/gremlin.rs +++ b/crates/grafeo-engine/src/query/translators/gremlin.rs @@ -5,7 +5,7 @@ use super::common::{VarGen, wrap_filter, wrap_limit, wrap_return, wrap_skip, wrap_sort}; use crate::query::plan::{ AggregateExpr, AggregateFunction, AggregateOp, BinaryOp, CreateEdgeOp, CreateNodeOp, - DeleteNodeOp, DistinctOp, ExpandDirection, ExpandOp, JoinOp, JoinType, LeftJoinOp, + DeleteNodeOp, DistinctOp, ExpandDirection, ExpandOp, FilterOp, JoinOp, JoinType, LeftJoinOp, LogicalExpression, LogicalOperator, LogicalPlan, MapCollectOp, NodeScanOp, OtherwiseOp, PathMode, ProjectOp, Projection, ReturnItem, SetPropertyOp, SortKey, SortOrder, UnaryOp, UnionOp, UnwindOp, @@ -2213,7 +2213,7 @@ impl GremlinTranslator { current_var: &str, ) -> Result> { let mut predicates: Vec = Vec::new(); - for step in steps { + for (idx, step) in steps.iter().enumerate() { match step { ast::Step::Has(has_step) => { predicates.push(self.translate_has_step(has_step, current_var)?); @@ -2265,7 +2265,9 @@ impl GremlinTranslator { predicates.push(self.build_id_filter(current_var, ids)); } // For navigation steps like out('knows') in where(), check if - // expanding produces any results (existence check). + // expanding produces any results (existence check). Steps that + // follow the navigation apply to the expansion target, not the + // outer variable, so they become a filter inside the subquery. ast::Step::Out(labels) | ast::Step::In(labels) | ast::Step::Both(labels) => { let direction = match step { ast::Step::Out(_) => ExpandDirection::Outgoing, @@ -2274,6 +2276,8 @@ impl GremlinTranslator { }; let edge_types = labels.clone(); let target_var = self.var_gen.next(); + // Remaining steps are evaluated against the expansion target. + let inner = self.steps_to_predicate(&steps[idx + 1..], &target_var)?; // Create an existence subquery via Expand + count > 0 let expand = LogicalOperator::Expand(ExpandOp { from_variable: current_var.to_string(), @@ -2291,7 +2295,17 @@ impl GremlinTranslator { path_alias: None, path_mode: PathMode::Walk, }); - predicates.push(LogicalExpression::ExistsSubquery(Box::new(expand))); + let subquery = match inner { + Some(predicate) => LogicalOperator::Filter(FilterOp { + predicate, + input: Box::new(expand), + pushdown_hint: None, + }), + None => expand, + }; + predicates.push(LogicalExpression::ExistsSubquery(Box::new(subquery))); + // Trailing steps consumed by the subquery above. + break; } // Nested compound groups: recurse so `.or(__.and(..), __.has(..))` is not // silently dropped by the fallthrough below. @@ -2386,7 +2400,7 @@ impl GremlinTranslator { #[cfg(test)] mod tests { use super::*; - use crate::query::plan::{FilterOp, LimitOp, SkipOp, SortOp}; + use crate::query::plan::{LimitOp, SkipOp, SortOp}; // === Basic Traversal Tests === diff --git a/crates/grafeo-engine/tests/sql_pgq.rs b/crates/grafeo-engine/tests/sql_pgq.rs index 7c0f0d6a8..812095f9c 100644 --- a/crates/grafeo-engine/tests/sql_pgq.rs +++ b/crates/grafeo-engine/tests/sql_pgq.rs @@ -364,7 +364,7 @@ fn test_nodes_path_function() { assert!( matches!(&row[nodes_col], Value::List(_)), "path_nodes should be a list, got: {:?}", - &row[nodes_col] + row[nodes_col] ); } } @@ -396,7 +396,7 @@ fn test_edges_path_function() { assert!( matches!(&row[edges_col], Value::List(_)), "path_edges should be a list, got: {:?}", - &row[edges_col] + row[edges_col] ); } } diff --git a/tests/spec/lpg/gremlin/anonymous_traversal.gtest b/tests/spec/lpg/gremlin/anonymous_traversal.gtest index 42b0feda8..e10952b3b 100644 --- a/tests/spec/lpg/gremlin/anonymous_traversal.gtest +++ b/tests/spec/lpg/gremlin/anonymous_traversal.gtest @@ -54,7 +54,6 @@ tests: - [Vincent] - name: anon_chained_inner_steps - skip: "pre-existing, unrelated to `__`: steps_to_predicate applies the chained has() to the outer vertex instead of the out() target, so this returns Gus" query: "g.V().hasLabel('Person').where(__.out('KNOWS').has('name', 'Gus')).values('name')" expect: rows: From f7ee5206ecbe52da7fa2629ab437b63675a056ae Mon Sep 17 00:00:00 2001 From: Jake Boone Date: Thu, 20 Aug 2026 17:27:29 -0700 Subject: [PATCH 3/3] Update CI configuration to install Rust toolchain conditionally and add rust-toolchain.toml --- .github/workflows/ci.yml | 8 +++----- rust-toolchain.toml | 3 +++ 2 files changed, 6 insertions(+), 5 deletions(-) create mode 100644 rust-toolchain.toml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1de73a0a1..e40c279ed 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,9 +18,7 @@ jobs: - uses: actions/checkout@v4 - name: Install Rust toolchain - uses: dtolnay/rust-toolchain@stable - with: - components: rustfmt, clippy + run: rustup show active-toolchain || rustup toolchain install - name: Cache cargo registry uses: actions/cache@v4 @@ -47,7 +45,7 @@ jobs: - uses: actions/checkout@v4 - name: Install Rust toolchain - uses: dtolnay/rust-toolchain@stable + run: rustup show active-toolchain || rustup toolchain install - name: Cache cargo registry uses: actions/cache@v4 @@ -76,7 +74,7 @@ jobs: node-version: '24' - name: Install Rust toolchain - uses: dtolnay/rust-toolchain@stable + run: rustup show active-toolchain || rustup toolchain install - name: Cache cargo registry uses: actions/cache@v4 diff --git a/rust-toolchain.toml b/rust-toolchain.toml new file mode 100644 index 000000000..8565ca6bc --- /dev/null +++ b/rust-toolchain.toml @@ -0,0 +1,3 @@ +[toolchain] +channel = "1.97.1" +components = ["rustfmt", "clippy"]