From 1939b6948792ed84eecd0914ff413da792ac2875 Mon Sep 17 00:00:00 2001 From: David Hughes Date: Sun, 25 Oct 2026 10:02:41 -0700 Subject: [PATCH] feat(cypher): validate known comparison and arithmetic types Use catalog schemas and scoped aliases to diagnose incompatible operands before planning. Preserve numeric and temporal coercions and defer unknown types to DataFusion. Add Rust semantic and Python execution coverage. Part of #23. Implemented with assistance from Codex; runtime tests queued and benchmark validation pending. --- crates/lance-graph/src/query.rs | 2 +- crates/lance-graph/src/semantic.rs | 108 +++++-- crates/lance-graph/src/semantic/types.rs | 377 +++++++++++++++++++++++ docs/api-reference.md | 2 +- docs/cypher.md | 7 +- python/python/tests/test_engine_types.py | 97 ++++++ 6 files changed, 560 insertions(+), 33 deletions(-) create mode 100644 crates/lance-graph/src/semantic/types.rs create mode 100644 python/python/tests/test_engine_types.py diff --git a/crates/lance-graph/src/query.rs b/crates/lance-graph/src/query.rs index d7f890e..5fb7b37 100644 --- a/crates/lance-graph/src/query.rs +++ b/crates/lance-graph/src/query.rs @@ -883,7 +883,7 @@ impl CypherQuery { let config = self.require_config()?; // Phase 1: Semantic Analysis - let mut analyzer = SemanticAnalyzer::new(config.clone()); + let mut analyzer = SemanticAnalyzer::new(config.clone()).with_catalog(catalog.clone()); let semantic = analyzer.analyze(&self.ast, &self.parameters)?; if !semantic.errors.is_empty() { return Err(GraphError::PlanError { diff --git a/crates/lance-graph/src/semantic.rs b/crates/lance-graph/src/semantic.rs index b8cf649..61679ea 100644 --- a/crates/lance-graph/src/semantic.rs +++ b/crates/lance-graph/src/semantic.rs @@ -13,6 +13,10 @@ use crate::case_insensitive::CaseInsensitiveLookup; use crate::config::GraphConfig; use crate::error::{GraphError, Result}; use std::collections::{HashMap, HashSet}; +use std::sync::Arc; + +mod types; +use types::{arithmetic_operator, comparison_operator, ExpressionType}; /// Semantic analyzer - validates and enriches the AST pub struct SemanticAnalyzer { @@ -21,6 +25,8 @@ pub struct SemanticAnalyzer { /// Variables whose scope a WITH ended; still type-checked at the end. retired: Vec, current_scope: ScopeType, + catalog: Option>, + expression_types: HashMap, } /// Information about a variable in the query @@ -70,9 +76,20 @@ impl SemanticAnalyzer { variables: HashMap::new(), retired: Vec::new(), current_scope: ScopeType::Match, + catalog: None, + expression_types: HashMap::new(), } } + /// Use the execution snapshot's schemas for checks that configuration alone cannot make. + pub(crate) fn with_catalog( + mut self, + catalog: Arc, + ) -> Self { + self.catalog = Some(catalog); + self + } + /// Analyze a Cypher query AST pub fn analyze( &mut self, @@ -214,6 +231,7 @@ impl SemanticAnalyzer { /// Analyze UNWIND clause and register variables fn analyze_unwind_clause(&mut self, unwind_clause: &UnwindClause) -> Result<()> { self.analyze_value_expression(&unwind_clause.expression)?; + let element_type = self.expression_type(&unwind_clause.expression).element(); // Register the aliased variable (normalize to lowercase for case-insensitive behavior) let var_name = &unwind_clause.alias; @@ -236,8 +254,9 @@ impl SemanticAnalyzer { properties: HashSet::new(), defined_in: self.current_scope.clone(), }; - self.variables.insert(var_name_lower, var_info); + self.variables.insert(var_name_lower.clone(), var_info); } + self.expression_types.insert(var_name_lower, element_type); Ok(()) } @@ -308,6 +327,19 @@ impl SemanticAnalyzer { }; self.variables.insert(var_name_lower, var_info); } + // MATCH binds an entity even when the name previously came from UNWIND. + let name = var_name.to_lowercase(); + let info = &self.variables[&name]; + let mut labels = info.labels.clone(); + if let Some(ExpressionType::Entity(_, inferred)) = self.expression_types.get(&name) { + for label in inferred { + if !labels.contains(label) { + labels.push(label.clone()); + } + } + } + self.expression_types + .insert(name, ExpressionType::Entity(VariableType::Node, labels)); } Ok(()) } @@ -356,9 +388,14 @@ impl SemanticAnalyzer { /// Analyze boolean expression and check variable references fn analyze_boolean_expression(&mut self, expr: &BooleanExpression) -> Result<()> { match expr { - BooleanExpression::Comparison { left, right, .. } => { + BooleanExpression::Comparison { + left, + operator, + right, + } => { self.analyze_value_expression(left)?; self.analyze_value_expression(right)?; + self.validate_binary(left, comparison_operator(operator), right)?; } BooleanExpression::And(left, right) | BooleanExpression::Or(left, right) => { self.analyze_boolean_expression(left)?; @@ -374,12 +411,20 @@ impl SemanticAnalyzer { patterns, where_clause, } => { - for pattern in patterns { - self.analyze_graph_pattern(pattern)?; - } - if let Some(w) = where_clause { - self.analyze_boolean_expression(w)?; - } + let before = self.variables.clone(); + let before_types = self.expression_types.clone(); + let result = (|| -> Result<()> { + for pattern in patterns { + self.analyze_graph_pattern(pattern)?; + } + if let Some(w) = where_clause { + self.analyze_boolean_expression(w)?; + } + Ok(()) + })(); + self.variables = before; + self.expression_types = before_types; + result?; } BooleanExpression::In { expression, list } => { self.analyze_value_expression(expression)?; @@ -623,26 +668,15 @@ impl SemanticAnalyzer { self.analyze_value_expression(otherwise)?; } } - ValueExpression::Arithmetic { left, right, .. } => { + ValueExpression::Arithmetic { + left, + operator, + right, + } => { // Validate arithmetic operands recursively self.analyze_value_expression(left)?; self.analyze_value_expression(right)?; - - // If both sides are literals, ensure they are numeric - let is_numeric_literal = |pv: &PropertyValue| { - matches!(pv, PropertyValue::Integer(_) | PropertyValue::Float(_)) - }; - - if let (ValueExpression::Literal(l1), ValueExpression::Literal(l2)) = - (&**left, &**right) - { - if !(is_numeric_literal(l1) && is_numeric_literal(l2)) { - return Err(GraphError::PlanError { - message: "Arithmetic requires numeric literal operands".to_string(), - location: snafu::Location::new(file!(), line!(), column!()), - }); - } - } + self.validate_binary(left, arithmetic_operator(operator), right)?; } ValueExpression::VectorDistance { left, right, .. } => { // Validate vector distance function arguments @@ -705,6 +739,7 @@ impl SemanticAnalyzer { } => { // New variables of the pattern are local to the comprehension. let before = self.variables.clone(); + let before_types = self.expression_types.clone(); let result = (|| -> Result<()> { self.analyze_graph_pattern(&GraphPattern::Path((**pattern).clone()))?; if let Some(b) = filter { @@ -713,6 +748,7 @@ impl SemanticAnalyzer { self.analyze_value_expression(map) })(); self.variables = before; + self.expression_types = before_types; result?; } ValueExpression::ListComprehension { @@ -724,7 +760,11 @@ impl SemanticAnalyzer { self.analyze_value_expression(list)?; // The variable is local to the comprehension. let before = self.variables.clone(); + let before_types = self.expression_types.clone(); + let element_type = self.expression_type(list).element(); self.register_projection_alias(variable); + self.expression_types + .insert(variable.to_lowercase(), element_type); let result = (|| -> Result<()> { if let Some(b) = filter { self.analyze_boolean_expression(b)?; @@ -735,6 +775,7 @@ impl SemanticAnalyzer { Ok(()) })(); self.variables = before; + self.expression_types = before_types; result?; } } @@ -774,7 +815,10 @@ impl SemanticAnalyzer { for item in &return_clause.items { self.analyze_value_expression(&item.expression)?; if let Some(alias) = &item.alias { + let value_type = self.expression_type(&item.expression); self.register_projection_alias(alias); + self.expression_types + .insert(alias.to_lowercase(), value_type); } } Ok(()) @@ -788,6 +832,7 @@ impl SemanticAnalyzer { } // WITH ends the scope of everything it does not carry; `WITH b AS x` renames b. let mut kept = HashMap::new(); + let mut kept_types = HashMap::new(); for item in &with_clause.items { let carried = match &item.expression { ValueExpression::Variable(v) => self.variables.get(&v.to_lowercase()).cloned(), @@ -801,10 +846,15 @@ impl SemanticAnalyzer { if let Some(info) = self.variables.get(&p.variable.to_lowercase()) { kept.insert(p.variable.to_lowercase(), info.clone()); } + if let Some(value_type) = self.expression_types.get(&p.variable.to_lowercase()) + { + kept_types.insert(p.variable.to_lowercase(), value_type.clone()); + } continue; } (None, _) => continue, }; + kept_types.insert(name.to_lowercase(), self.expression_type(&item.expression)); let info = carried.unwrap_or_else(|| VariableInfo { name: name.clone(), variable_type: VariableType::Property, @@ -817,6 +867,7 @@ impl SemanticAnalyzer { self.retired .extend(std::mem::take(&mut self.variables).into_values()); self.variables = kept; + self.expression_types = kept_types; // Validate ORDER BY within WITH if present if let Some(order_by) = &with_clause.order_by { for item in &order_by.items { @@ -865,10 +916,7 @@ impl SemanticAnalyzer { /// Validate types and operations fn validate_types(&self, errors: &mut Vec) { - // TODO: Implement type checking - // - Check that properties exist on nodes/relationships - // - Check that comparison operations are valid for data types - // - Check that arithmetic operations are valid + // Expression operations are checked during traversal, while their scope is available. // Check that properties referenced in patterns exist in schema when property fields are defined for var_info in self.variables.values().chain(&self.retired) { @@ -1815,7 +1863,7 @@ mod tests { assert!(result .errors .iter() - .any(|e| e.contains("Arithmetic requires numeric literal operands"))); + .any(|e| e.contains("Type mismatch") && e.contains("Utf8") && e.contains("Int64"))); } #[test] diff --git a/crates/lance-graph/src/semantic/types.rs b/crates/lance-graph/src/semantic/types.rs new file mode 100644 index 0000000..7331815 --- /dev/null +++ b/crates/lance-graph/src/semantic/types.rs @@ -0,0 +1,377 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright The lance-csr Authors + +//! Conservative expression checks against the current catalog and lexical scope. + +use super::*; +use arrow_schema::{DataType, TimeUnit}; +use datafusion_expr::binary::BinaryTypeCoercer; +use datafusion_expr::Operator; + +#[derive(Clone, Debug, PartialEq)] +pub(super) enum ExpressionType { + Unknown, + Scalar(DataType), + Entity(VariableType, Vec), + List(Box), + Map(Vec<(String, Self)>), +} + +impl ExpressionType { + pub(super) fn element(self) -> Self { + match self { + Self::List(element) => *element, + Self::Scalar( + DataType::List(field) + | DataType::LargeList(field) + | DataType::FixedSizeList(field, _), + ) => Self::Scalar(field.data_type().clone()), + _ => Self::Unknown, + } + } + + fn known(&self) -> bool { + !matches!(self, Self::Unknown | Self::Scalar(DataType::Null)) + } +} + +impl SemanticAnalyzer { + fn variable_expression_type(&self, name: &str) -> ExpressionType { + if let Some(value_type) = self.expression_types.get(&name.to_lowercase()) { + return value_type.clone(); + } + match self.variables.get_ci(name) { + Some(info) if info.variable_type != VariableType::Property => { + ExpressionType::Entity(info.variable_type.clone(), info.labels.clone()) + } + _ => ExpressionType::Unknown, + } + } + + fn property_expression_type(&self, base: ExpressionType, property: &str) -> ExpressionType { + match base { + ExpressionType::Entity(kind, labels) => { + let Some(catalog) = &self.catalog else { + return ExpressionType::Unknown; + }; + let types = labels + .iter() + .map(|label| { + let source = match kind { + VariableType::Node => catalog.node_source(label), + VariableType::Relationship => catalog.relationship_source(label), + _ => None, + }?; + source + .schema() + .fields() + .iter() + .find(|field| field.name().eq_ignore_ascii_case(property)) + .map(|field| ExpressionType::Scalar(field.data_type().clone())) + }) + .collect::>>(); + types.map(common_type).unwrap_or(ExpressionType::Unknown) + } + ExpressionType::Map(fields) => fields + .into_iter() + .find(|(name, _)| name.eq_ignore_ascii_case(property)) + .map(|(_, value_type)| value_type) + .unwrap_or(ExpressionType::Unknown), + ExpressionType::Scalar(DataType::Struct(fields)) => fields + .iter() + .find(|field| field.name().eq_ignore_ascii_case(property)) + .map(|field| ExpressionType::Scalar(field.data_type().clone())) + .unwrap_or(ExpressionType::Unknown), + // Date component access and function-specific fields remain with DataFusion. + _ => ExpressionType::Unknown, + } + } + + pub(super) fn expression_type(&self, expression: &ValueExpression) -> ExpressionType { + use ExpressionType::{List, Map, Scalar, Unknown}; + match expression { + ValueExpression::Variable(name) => self.variable_expression_type(name), + ValueExpression::Property(property) + | ValueExpression::Literal(PropertyValue::Property(property)) => self + .property_expression_type( + self.variable_expression_type(&property.variable), + &property.property, + ), + ValueExpression::Literal(value) => match value { + PropertyValue::Integer(_) => Scalar(DataType::Int64), + PropertyValue::Float(_) => Scalar(DataType::Float64), + PropertyValue::String(_) => Scalar(DataType::Utf8), + PropertyValue::Boolean(_) => Scalar(DataType::Boolean), + PropertyValue::Null => Scalar(DataType::Null), + PropertyValue::Variable(name) => self.variable_expression_type(name), + _ => Unknown, + }, + ValueExpression::Arithmetic { + left, + operator, + right, + } => match (self.expression_type(left), self.expression_type(right)) { + (Scalar(left), Scalar(right)) => { + BinaryTypeCoercer::new(&left, &arithmetic_operator(operator), &right) + .get_result_type() + .map(Scalar) + .unwrap_or(Unknown) + } + _ => Unknown, + }, + ValueExpression::ScalarFunction { name, args } + | ValueExpression::AggregateFunction { name, args, .. } => { + let first = || { + args.first() + .map(|value| self.expression_type(value)) + .unwrap_or(Unknown) + }; + match name.to_lowercase().as_str() { + "count" | "size" | "length" | "tointeger" | "to_epoch_ms" => { + Scalar(DataType::Int64) + } + "tofloat" => Scalar(DataType::Float64), + "tolower" | "lower" | "toupper" | "upper" | "tostring" => { + Scalar(DataType::Utf8) + } + "date" => Scalar(DataType::Date32), + "datetime" => Scalar(DataType::Timestamp( + TimeUnit::Millisecond, + Some("UTC".into()), + )), + "collect" => List(Box::new(first())), + "head" | "__index" => first().element(), + "range" => List(Box::new(Scalar(DataType::Int64))), + "nodes" => List(Box::new(Unknown)), + "__list" => List(Box::new(common_type( + args.iter() + .map(|value| self.expression_type(value)) + .collect(), + ))), + "coalesce" => common_type( + args.iter() + .map(|value| self.expression_type(value)) + .collect(), + ), + // SUM/AVG output widths depend on DataFusion's accumulator type. + "min" | "max" => first(), + _ => Unknown, + } + } + ValueExpression::Case { whens, otherwise } => common_type( + whens + .iter() + .map(|(_, value)| self.expression_type(value)) + .chain(otherwise.iter().map(|value| self.expression_type(value))) + .collect(), + ), + ValueExpression::Boolean(_) => Scalar(DataType::Boolean), + ValueExpression::Map(fields) => Map(fields + .iter() + .map(|(name, value)| (name.clone(), self.expression_type(value))) + .collect()), + ValueExpression::Field { base, field } => { + self.property_expression_type(self.expression_type(base), field) + } + ValueExpression::VectorLiteral(_) => List(Box::new(Scalar(DataType::Float32))), + ValueExpression::VectorDistance { .. } | ValueExpression::VectorSimilarity { .. } => { + Scalar(DataType::Float64) + } + // Their local scopes have ended; do not infer from similarly named outer variables. + ValueExpression::ListComprehension { .. } + | ValueExpression::PatternComprehension { .. } + | ValueExpression::Parameter(_) => Unknown, + } + } + + pub(super) fn validate_binary( + &self, + left: &ValueExpression, + operator: Operator, + right: &ValueExpression, + ) -> Result<()> { + use ExpressionType::{Entity, Scalar}; + let left_type = self.expression_type(left); + let right_type = self.expression_type(right); + if !left_type.known() || !right_type.known() { + return Ok(()); + } + let comparison = matches!( + operator, + Operator::Eq + | Operator::NotEq + | Operator::Lt + | Operator::LtEq + | Operator::Gt + | Operator::GtEq + ); + let compatible = match (&left_type, &right_type) { + (Scalar(left), Scalar(right)) => { + let string_numeric = (left.is_numeric() + && matches!( + right, + DataType::Utf8 | DataType::LargeUtf8 | DataType::Utf8View + )) + || (right.is_numeric() + && matches!( + left, + DataType::Utf8 | DataType::LargeUtf8 | DataType::Utf8View + )); + !string_numeric + && BinaryTypeCoercer::new(left, &operator, right) + .get_result_type() + .is_ok() + } + (Entity(left, _), Entity(right, _)) => comparison && left == right, + // Collection comparison rules remain with the planner; arithmetic on entities does not. + (Entity(..), _) | (_, Entity(..)) => false, + _ => comparison, + }; + if compatible { + Ok(()) + } else { + Err(GraphError::PlanError { + message: format!( + "Type mismatch in '{} {} {}': {} and {}", + display_value(left), + operator, + display_value(right), + display_type(&left_type), + display_type(&right_type) + ), + location: snafu::Location::new(file!(), line!(), column!()), + }) + } + } +} + +fn common_type(types: Vec) -> ExpressionType { + let mut known = types + .into_iter() + .filter(|value| !matches!(value, ExpressionType::Scalar(DataType::Null))); + let Some(first) = known.next() else { + return ExpressionType::Unknown; + }; + if known.all(|value| value == first) { + first + } else { + ExpressionType::Unknown + } +} + +fn display_type(value: &ExpressionType) -> String { + match value { + ExpressionType::Scalar(data_type) => data_type.to_string(), + ExpressionType::Entity(kind, _) => format!("{kind:?}"), + _ => format!("{value:?}"), + } +} + +fn display_value(value: &ValueExpression) -> String { + match value { + ValueExpression::Property(property) => { + format!("{}.{}", property.variable, property.property) + } + ValueExpression::Variable(name) => name.clone(), + ValueExpression::Literal(PropertyValue::Integer(value)) => value.to_string(), + ValueExpression::Literal(PropertyValue::Float(value)) => value.to_string(), + ValueExpression::Literal(PropertyValue::String(value)) => format!("{value:?}"), + ValueExpression::Arithmetic { + left, + operator, + right, + } => format!( + "({} {} {})", + display_value(left), + arithmetic_operator(operator), + display_value(right) + ), + _ => format!("{value:?}"), + } +} + +pub(super) fn arithmetic_operator(operator: &ArithmeticOperator) -> Operator { + match operator { + ArithmeticOperator::Add => Operator::Plus, + ArithmeticOperator::Subtract => Operator::Minus, + ArithmeticOperator::Multiply => Operator::Multiply, + ArithmeticOperator::Divide => Operator::Divide, + ArithmeticOperator::Modulo => Operator::Modulo, + } +} + +pub(super) fn comparison_operator(operator: &ComparisonOperator) -> Operator { + match operator { + ComparisonOperator::Equal => Operator::Eq, + ComparisonOperator::NotEqual => Operator::NotEq, + ComparisonOperator::LessThan => Operator::Lt, + ComparisonOperator::LessThanOrEqual => Operator::LtEq, + ComparisonOperator::GreaterThan => Operator::Gt, + ComparisonOperator::GreaterThanOrEqual => Operator::GtEq, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use arrow_schema::{Field, Schema}; + use lance_graph_catalog::{InMemoryCatalog, SimpleTableSource}; + + fn analyze(query: &str) -> SemanticResult { + let schema = Arc::new(Schema::new(vec![ + Field::new("id", DataType::Int64, false), + Field::new("name", DataType::Utf8, true), + Field::new("age", DataType::Int32, true), + Field::new( + "created", + DataType::Timestamp(TimeUnit::Microsecond, Some("UTC".into())), + true, + ), + ])); + let catalog = InMemoryCatalog::new() + .with_node_source("Person", Arc::new(SimpleTableSource::new(schema))); + let config = GraphConfig::builder() + .node_table("Person", "id") + .build() + .unwrap(); + SemanticAnalyzer::new(config) + .with_catalog(Arc::new(catalog)) + .analyze( + &crate::parser::parse_cypher_query(query).unwrap(), + &HashMap::new(), + ) + .unwrap() + } + + #[test] + fn rejects_known_operand_types_in_their_scope() { + for (query, expression, types) in [ + ("MATCH (p:Person) WHERE p.name = 1 RETURN p.id", "p.name = 1", ["Utf8", "Int64"]), + ("MATCH (p:Person) RETURN p + 1", "p + 1", ["Node", "Int64"]), + ("MATCH (p:Person) WITH p.name AS value RETURN value * 2", "value * 2", ["Utf8", "Int64"]), + ("MATCH (p:Person) WITH collect(p.name) AS names UNWIND names AS name RETURN name / 2", "name / 2", ["Utf8", "Int64"]), + ("MATCH (p:Person) RETURN [p IN [p.name] | p + 1]", "p + 1", ["Utf8", "Int64"]), + ] { + let errors = analyze(query).errors.join("\n"); + assert!(errors.contains("Type mismatch") && errors.contains(expression) + && types.iter().all(|name| errors.contains(name)), "{query}: {errors}"); + } + } + + #[test] + fn preserves_coercions_nulls_and_unknown_types() { + for query in [ + "MATCH (p:Person) RETURN p.age + 0.5", + "MATCH (p:Person) WHERE p.created > '2020-01-01T00:00:00Z' RETURN p.id", + "MATCH (p:Person) RETURN p.age + null", + "MATCH (p:Person) RETURN p.unknown + 1", + "MATCH (p:Person) WITH p AS q RETURN q.age + 1", + "MATCH (p:Person) RETURN [p IN [p.age] | p + 1], p.name", + "MATCH (p:Person) RETURN [p IN [p.name, p.age] | p + 1]", + "MATCH (p:Person) WITH collect(p) AS people UNWIND people AS person RETURN person.age + 1", + ] { + let errors = analyze(query).errors; + assert!(errors.is_empty(), "{query}: {errors:?}"); + } + } +} diff --git a/docs/api-reference.md b/docs/api-reference.md index 81ffd03..eefe80f 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -32,7 +32,7 @@ Keys match labels case-insensitively. | Exception | When | | --- | --- | -| `ValueError` | Invalid query, unknown label, missing dataset or column, unsupported parameter type, empty `datasets` | +| `ValueError` | Invalid query, incompatible known operand types (with expression and type details), unknown label, missing dataset or column, unsupported parameter type, empty `datasets` | | `NotImplementedError` | A Cypher feature that parses but isn't supported | | `RuntimeError` | Execution or I/O failures | | `TypeError` | A dataset value of an unsupported type, such as a LanceDB `Table` | diff --git a/docs/cypher.md b/docs/cypher.md index b7e4674..dca8916 100644 --- a/docs/cypher.md +++ b/docs/cypher.md @@ -283,6 +283,11 @@ case-insensitive. `match (P:person) return p.NAME` works, and the output column | Exception | Raised when | | --- | --- | -| `ValueError` | The query doesn't parse, refers to an unknown label, or a dataset is missing a configured column. | +| `ValueError` | The query doesn't parse, refers to an unknown label, a dataset is missing a configured column, or a comparison/arithmetic expression has incompatible known types. Type errors name the expression and its operand types. | | `NotImplementedError` | The query parses but uses an unsupported feature (for example a function that isn't implemented, or more than 20 hops). | | `RuntimeError` | Execution fails, for example when a Lance dataset can't be read. | + +Known comparison and arithmetic types are checked against the current dataset schemas before planning. +For example, comparing a string property with a number reports the expression and both types in a +`ValueError`. Mixed numeric types and supported date/timestamp coercions remain valid. Types that cannot +be determined in the current scope are left to the planner, so these checks are not a complete type system. diff --git a/python/python/tests/test_engine_types.py b/python/python/tests/test_engine_types.py new file mode 100644 index 0000000..a629b00 --- /dev/null +++ b/python/python/tests/test_engine_types.py @@ -0,0 +1,97 @@ +# SPDX-License-Identifier: Apache-2.0 +# SPDX-FileCopyrightText: Copyright The lance-csr Authors + +"""Schema-aware errors retain the expression, operand types and lexical scope.""" + +from datetime import datetime, timezone + +import pyarrow as pa +import pytest +from lance_csr import CypherEngine, GraphConfig + + +@pytest.fixture(params=[False, True]) +def engine(request): + config = ( + GraphConfig.builder() + .node_table("Person", "id") + .rel_table("KNOWS", "Person", "Person", "src", "dst") + .build() + ) + return CypherEngine( + config, + { + "Person": pa.table( + { + "id": [1, 2], + "name": ["Ada", "Bob"], + "age": pa.array([30, 40], pa.int32()), + "created": [datetime(2020, 1, 1, tzinfo=timezone.utc)] * 2, + } + ), + "KNOWS": pa.table({"src": [1], "dst": [2], "label": ["friend"]}), + }, + use_csr=request.param, + ) + + +@pytest.mark.parametrize( + ("query", "expression", "types"), + [ + ( + "MATCH (p:Person) WHERE p.name = 1 RETURN p.id", + "p.name = 1", + ("Utf8", "Int64"), + ), + ("MATCH (p:Person) RETURN p + 1", "p + 1", ("Node", "Int64")), + ( + "MATCH (p:Person) WITH p.name AS value RETURN value * 2", + "value * 2", + ("Utf8", "Int64"), + ), + ( + "MATCH (p:Person) WITH collect(p.name) AS names UNWIND names AS name " + "RETURN name / 2", + "name / 2", + ("Utf8", "Int64"), + ), + ( + "MATCH (p:Person) RETURN [p IN [p.name] | p + 1]", + "p + 1", + ("Utf8", "Int64"), + ), + ( + "MATCH (p:Person)-[r:KNOWS]->(q:Person) WHERE r.label = 1 RETURN q.id", + "r.label = 1", + ("Utf8", "Int64"), + ), + ], +) +def test_type_errors_name_expression_and_operands(engine, query, expression, types): + with pytest.raises(ValueError, match="Type mismatch") as error: + engine.execute(query) + assert expression in str(error.value) + assert all(name in str(error.value) for name in types) + + +def test_numeric_temporal_and_null_coercions(engine): + result = engine.execute( + "MATCH (p:Person) WHERE p.created >= $since " + "RETURN p.age + 0.5 AS adjusted, p.age + null AS missing ORDER BY adjusted", + {"since": datetime(2020, 1, 1, tzinfo=timezone.utc)}, + ) + assert result.to_pylist() == [ + {"adjusted": 30.5, "missing": None}, + {"adjusted": 40.5, "missing": None}, + ] + + +def test_comprehension_shadow_does_not_change_outer_type(engine): + result = engine.execute( + "MATCH (p:Person) RETURN [p IN [p.age] | p + 1] AS ages, p.name AS name " + "ORDER BY name" + ) + assert result.to_pylist() == [ + {"ages": [31], "name": "Ada"}, + {"ages": [41], "name": "Bob"}, + ]