diff --git a/src/typing/checker.rs b/src/typing/checker.rs index 22ff420..3afb835 100644 --- a/src/typing/checker.rs +++ b/src/typing/checker.rs @@ -144,11 +144,13 @@ impl Typechecker { for spec in specs { let t = match &spec.key { - SortKey::Expr(e) => self.check_expr(e, env), + SortKey::Expr(e) => self.order_key_type(e, env), SortKey::Column(idx) => match returns.and_then(|rs| rs.get(*idx)) { - Some(ReturnItem::Expr { expr, .. }) => self.check_expr(expr, env), - // Aggregate output typing is a separate gap; pass-through. - Some(ReturnItem::Aggregate { .. }) => SimpleType::Star, + Some(ReturnItem::Expr { expr, .. }) => self.order_key_type(expr, env), + // Type the aggregate by its result type so a non-orderable + // result (e.g. COLLECT_LIST -> List) is rejected below instead + // of being laundered through Star (reserved for base types). + Some(ReturnItem::Aggregate { agg, .. }) => self.check_aggregator(agg, env), None => { self.errors.push(format!( "ORDER BY column reference #{idx} is out of bounds for the \ @@ -172,8 +174,8 @@ impl Typechecker { SortKey::ColumnField { col, path } => { // Type the projected column, then walk the record path. let mut t = match returns.and_then(|rs| rs.get(*col)) { - Some(ReturnItem::Expr { expr, .. }) => self.check_expr(expr, env), - Some(ReturnItem::Aggregate { .. }) => SimpleType::Star, + Some(ReturnItem::Expr { expr, .. }) => self.order_key_type(expr, env), + Some(ReturnItem::Aggregate { agg, .. }) => self.check_aggregator(agg, env), None => { self.errors.push(format!( "ORDER BY column reference #{col} is out of bounds for the \ @@ -212,6 +214,25 @@ impl Typechecker { } } + /// Type of an ORDER BY sort-key expression. `Star` is reserved for + /// base types in the gradual design, but two non-base values reach + /// sort keys laundered to `Star`: a constant record (via the + /// `simple_type_of_value` punt) and a repetition-group variable (via + /// `variable_type_to_simple_type`). Neither is orderable without + /// Feature GA04, so type them precisely here — locally to ORDER BY — + /// and let the §22.14 comparability check reject them. The global + /// typing keeps the punt (RETURN/WHERE still see them as `Star`). + fn order_key_type(&mut self, e: &Expr, env: &TypeEnvironment) -> SimpleType { + match e { + Expr::Const(v) => const_order_type(v), + Expr::Var(name) => match env.get(name) { + Some(VariableType::Group(_)) => SimpleType::Group(Box::new(SimpleType::Star)), + _ => self.check_expr(e, env), + }, + _ => self.check_expr(e, env), + } + } + /// Reject unbounded repetition unless its nearest prefix makes it finite. fn check_unbounded_repetition(&mut self, q: &Query) { for m in &q.matches { @@ -1126,6 +1147,22 @@ fn create_context(desc: &Option, t: VariableType) -> TypeEnvironment } } +/// Like `simple_type_of_value`, but a constant record (possibly nested) +/// types as `Record` instead of the `Star` punt, so the ORDER BY +/// comparability check (§22.14) can reject it. Sort keys only, via +/// `order_key_type`. +fn const_order_type(v: &Value) -> SimpleType { + match v { + Value::Record(fields) => SimpleType::Record( + fields + .iter() + .map(|(k, vv)| (k.clone(), const_order_type(vv))) + .collect(), + ), + other => simple_type_of_value(other), + } +} + /// Map a literal `Value` to its `SimpleType`. /// /// List and Record values get a deliberately loose type — the precise diff --git a/tests/typecheck_gaps_order_by_test.rs b/tests/typecheck_gaps_order_by_test.rs index 9a4113b..01974ac 100644 --- a/tests/typecheck_gaps_order_by_test.rs +++ b/tests/typecheck_gaps_order_by_test.rs @@ -290,3 +290,74 @@ fn runtime_treats_list_sort_key_as_equal_so_input_order_preserved() { _ => panic!("expected projected"), } } + +// ISO §22.14: an aggregate whose result type is non-orderable (a List from +// COLLECT_LIST) must be rejected as a sort key, not laundered through Star and +// silently treated as Equal at runtime. Regression for the ORDER BY-over-Star +// soundness gap (Star is reserved for base types). +#[test] +fn typecheck_rejects_list_aggregate_in_sort_key() { + let r = compile_query( + "MATCH (x: User) RETURN x.city, COLLECT_LIST(x.name) GROUP BY x.city \ + ORDER BY COLLECT_LIST(x.name)", + ); + let err = r.expect_err("ORDER BY over a list-valued aggregate must be rejected"); + assert!( + err.contains("comparable") || err.contains("22.14") || err.contains("GA04"), + "got: {err}" + ); +} + +// ISO §22.14: a record is not orderable (no Feature GA04). A constant +// record used to launder to `Star` and slip past the comparability +// check; regression for the `order_key_type` hardening. +#[test] +fn typecheck_rejects_const_record_in_sort_key() { + let r = compile_query("MATCH (x) RETURN RECORD { name: 'Alice' } AS r ORDER BY r"); + let err = r.expect_err("ORDER BY over a record must be rejected"); + assert!( + err.contains("comparable") || err.contains("22.14") || err.contains("GA04"), + "got: {err}" + ); +} + +// The fix is local to ORDER BY: projecting a constant record in RETURN +// must keep working (it was typed as Star globally and still is). +#[test] +fn typecheck_accepts_const_record_in_return() { + let r = compile_query("MATCH (x) RETURN RECORD { name: 'Alice' } AS r"); + assert!(r.is_ok(), "got: {:?}", r.err()); +} + +// Drilling into a *scalar* field of a record is fine: `r.name` is a +// string, which is orderable. +#[test] +fn typecheck_accepts_record_scalar_field_in_sort_key() { + let r = compile_query("MATCH (x) RETURN RECORD { name: 'Alice' } AS r ORDER BY r.name"); + assert!(r.is_ok(), "got: {:?}", r.err()); +} + +// A repetition-group variable (bound by `{n,m}`) is not a base type +// and is not orderable; regression for the `order_key_type` hardening. +#[test] +fn typecheck_rejects_group_variable_in_sort_key() { + let r = compile_query("MATCH (a)-[e]->{1,2}(b) RETURN a, b ORDER BY e"); + let err = r.expect_err("ORDER BY over a repetition group must be rejected"); + assert!( + err.contains("comparable") || err.contains("22.14") || err.contains("GA04"), + "got: {err}" + ); +} + +// Drilling into a field that is itself a record stays non-orderable. +#[test] +fn typecheck_rejects_record_nested_record_field_in_sort_key() { + let r = compile_query( + "MATCH (x) RETURN RECORD { inner: RECORD { name: 'Alice' } } AS r ORDER BY r.inner", + ); + let err = r.expect_err("ORDER BY over a nested record field must be rejected"); + assert!( + err.contains("comparable") || err.contains("22.14") || err.contains("GA04"), + "got: {err}" + ); +}