From bebaa9e45110b44e541e037edd1009fb3e2382ac Mon Sep 17 00:00:00 2001 From: Jean Paul Duchens Pacheco Date: Thu, 25 Jun 2026 15:32:19 -0400 Subject: [PATCH 1/4] fix(typing): type ORDER BY aggregate keys by result, reject non-orderable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ORDER BY referencing an aggregate column was typed as `Star` ("Aggregate output typing is a separate gap; pass-through"). That laundered a non-orderable aggregate result (e.g. `COLLECT_LIST` -> List) past the §22.14 comparability check: it passed typecheck, then at runtime the comparator returned "incomparable" -> treated as Equal -> the ORDER BY was silently ignored (a type-safety hole, since `Star` is reserved for base types in the gradual design). Type the aggregate by its result via `check_aggregator` (consistent with how `Expr::Agg` is already typed: COUNT->Int, SUM->numeric, MIN/MAX-> element type, COLLECT_LIST->List). A List/Record-valued aggregate is now rejected statically; scalar aggregates keep working. No runtime checks. Scope: closes the aggregate path only. Other Star-laundering sites (record constants in `simple_type_of_value`, repetition `Group`, unknown list/record property values) remain and are a separate decision. Test: typecheck_rejects_list_aggregate_in_sort_key. Co-Authored-By: Claude Opus 4.8 --- src/typing/checker.rs | 8 +++++--- tests/typecheck_gaps_order_by_test.rs | 17 +++++++++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/typing/checker.rs b/src/typing/checker.rs index 22ff420..24ab610 100644 --- a/src/typing/checker.rs +++ b/src/typing/checker.rs @@ -147,8 +147,10 @@ impl Typechecker { SortKey::Expr(e) => self.check_expr(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, + // 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 \ @@ -173,7 +175,7 @@ impl Typechecker { // 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::Aggregate { agg, .. }) => self.check_aggregator(agg, env), None => { self.errors.push(format!( "ORDER BY column reference #{col} is out of bounds for the \ diff --git a/tests/typecheck_gaps_order_by_test.rs b/tests/typecheck_gaps_order_by_test.rs index 9a4113b..f831429 100644 --- a/tests/typecheck_gaps_order_by_test.rs +++ b/tests/typecheck_gaps_order_by_test.rs @@ -290,3 +290,20 @@ 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}" + ); +} From 10dea40d0f16bb403d1185c905743e0b47938ebf Mon Sep 17 00:00:00 2001 From: Jean Paul Duchens Pacheco Date: Tue, 30 Jun 2026 01:23:46 -0400 Subject: [PATCH 2/4] fix(typing): reject constant records as ORDER BY sort keys (local hardening) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A constant record (`RECORD { name: 'Alice' }`, all-literal fields) folds to `Value::Record` and is typed by `simple_type_of_value` as `Star` (the phase-1 punt). That laundered a non-orderable record past the §22.14 comparability check: it passed typecheck, then at runtime the comparator returned "incomparable" -> treated as Equal -> ORDER BY silently ignored (a type-safety hole; `Star` is reserved for base types). Fix is LOCAL to ORDER BY, mirroring the aggregate fix on this branch: - new `order_key_type` types a sort-key `Expr::Const` via `const_order_type`, which (unlike `simple_type_of_value`) types records — including nested records — as `Record`, so `is_orderable_per_iso_22_14` rejects them. - applied at the three sort-key typing points (SortKey::Expr, SortKey::Column, SortKey::ColumnField). The ColumnField path now extracts real field types, so `ORDER BY r.scalarField` keeps working while `ORDER BY r.recordField` is rejected. - the global constant typing (`Expr::Const` in `check_expr`) is unchanged, so RETURN/WHERE/COALESCE still see a constant record as `Star`. Blast radius stays inside ORDER BY. No runtime checks. Tests: reject const record / nested record field as sort key; accept RETURN of a const record; accept ORDER BY over a scalar record field. Scope: closes the record-constant path. The genuinely-unknown property `Star` case (whether `Star` on a property stays orderable, assuming base-typed properties) is left as an open design question for review. Co-Authored-By: Claude Opus 4.8 --- src/typing/checker.rs | 37 ++++++++++++++++++++-- tests/typecheck_gaps_order_by_test.rs | 44 +++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 3 deletions(-) diff --git a/src/typing/checker.rs b/src/typing/checker.rs index 24ab610..29a84ab 100644 --- a/src/typing/checker.rs +++ b/src/typing/checker.rs @@ -144,9 +144,9 @@ 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), + 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). @@ -174,7 +174,7 @@ 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::Expr { expr, .. }) => self.order_key_type(expr, env), Some(ReturnItem::Aggregate { agg, .. }) => self.check_aggregator(agg, env), None => { self.errors.push(format!( @@ -214,6 +214,21 @@ impl Typechecker { } } + /// Type of an ORDER BY sort-key expression, hardened against the + /// `simple_type_of_value` punt that launders a constant record to + /// `Star` (which is reserved for base types in the gradual design). A + /// record literal is not a base type and is not orderable without + /// Feature GA04, so type it as `Record` here — locally to ORDER BY — + /// so the §22.14 check below rejects it. The global constant typing + /// (`Expr::Const`) is left unchanged, keeping the blast radius to sort + /// keys (RETURN/WHERE/COALESCE still see a constant record as `Star`). + fn order_key_type(&mut self, e: &Expr, env: &TypeEnvironment) -> SimpleType { + if let Expr::Const(v) = e { + return const_order_type(v); + } + 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 { @@ -1134,6 +1149,22 @@ fn create_context(desc: &Option, t: VariableType) -> TypeEnvironment /// element/field types would require recursive typing of values, which /// fppc doesn't do (it has no list literals). Documented in /// `docs/internals/typechecker_migration.md` as a phase-1 punt. +/// Like `simple_type_of_value`, but does not launder a record to `Star`: +/// a constant record (possibly nested) types as `Record` so the ORDER BY +/// comparability check (§22.14) rejects it. Used only for sort keys, via +/// `order_key_type`; the global constant typing keeps the `Star` punt. +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), + } +} + fn simple_type_of_value(v: &Value) -> SimpleType { match v { // Null literal is the SQL untyped null: it inhabits every type for diff --git a/tests/typecheck_gaps_order_by_test.rs b/tests/typecheck_gaps_order_by_test.rs index f831429..0194455 100644 --- a/tests/typecheck_gaps_order_by_test.rs +++ b/tests/typecheck_gaps_order_by_test.rs @@ -307,3 +307,47 @@ fn typecheck_rejects_list_aggregate_in_sort_key() { "got: {err}" ); } + +// ISO §22.14: a record is not orderable (no Feature GA04). A *constant* +// record folds to `Star` via `simple_type_of_value` (the phase-1 punt), +// which would launder it past the comparability check. `order_key_type` +// types it as `Record` locally to ORDER BY so it is rejected, without +// touching the global constant typing. +#[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()); +} + +// 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}" + ); +} From 9e1bd16381ad2c3e866d54b8f5909a399141704b Mon Sep 17 00:00:00 2001 From: Jean Paul Duchens Pacheco Date: Tue, 30 Jun 2026 21:25:23 -0400 Subject: [PATCH 3/4] fix(typing): reject repetition-group variables as ORDER BY sort keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same class of Star-laundering as the record/aggregate cases: `variable_type_to_simple_type` types a `VariableType::Group` (a `{n,m}` repetition binding) as `Star` (the projection punt), so `ORDER BY ` passed the §22.14 comparability check and then silently no-op'd at runtime (the group projects to Null -> all "Equal" -> no reorder). Extend the local `order_key_type` (ORDER BY only) to type a group variable as `Group` so the check rejects it. The global `variable_type_to_simple_type` punt is unchanged, so projection of a group keeps its current behaviour. Consistent with "Star is a base type only": a group is not a base type and must not be laundered to Star at a comparability site. Test: `ORDER BY` over a `{1,2}` repetition group is rejected. Co-Authored-By: Claude Opus 4.8 --- src/typing/checker.rs | 17 ++++++++++++++--- tests/typecheck_gaps_order_by_test.rs | 13 +++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/typing/checker.rs b/src/typing/checker.rs index 29a84ab..4ebc746 100644 --- a/src/typing/checker.rs +++ b/src/typing/checker.rs @@ -223,10 +223,21 @@ impl Typechecker { /// (`Expr::Const`) is left unchanged, keeping the blast radius to sort /// keys (RETURN/WHERE/COALESCE still see a constant record as `Star`). fn order_key_type(&mut self, e: &Expr, env: &TypeEnvironment) -> SimpleType { - if let Expr::Const(v) = e { - return const_order_type(v); + match e { + // A constant record (possibly nested) folds to `Value::Record` + // and is laundered to `Star` by `simple_type_of_value`; type it + // precisely so the comparability check rejects it. + Expr::Const(v) => const_order_type(v), + // A repetition-group variable is not a base type either; + // `variable_type_to_simple_type` launders `Group` to `Star`. + // Type it as `Group` here so the check rejects it (ordering by a + // group of matched edges/nodes is not a comparable value). + 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), } - self.check_expr(e, env) } /// Reject unbounded repetition unless its nearest prefix makes it finite. diff --git a/tests/typecheck_gaps_order_by_test.rs b/tests/typecheck_gaps_order_by_test.rs index 0194455..ed4568a 100644 --- a/tests/typecheck_gaps_order_by_test.rs +++ b/tests/typecheck_gaps_order_by_test.rs @@ -339,6 +339,19 @@ fn typecheck_accepts_record_scalar_field_in_sort_key() { assert!(r.is_ok(), "got: {:?}", r.err()); } +// A repetition-group variable (bound by `{n,m}`) is not a base type and +// is not orderable. `variable_type_to_simple_type` launders `Group` to +// `Star`; `order_key_type` types it as `Group` so ORDER BY rejects it. +#[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() { From bfb35c2c2e73f7fa8eef52e4aba9bae23fe82d2f Mon Sep 17 00:00:00 2001 From: Jean Paul Duchens Pacheco Date: Sat, 4 Jul 2026 21:56:27 -0400 Subject: [PATCH 4/4] docs(typing): tidy ORDER BY hardening comments, restore simple_type_of_value doc Consolidate the record/group laundering rationale into order_key_type's doc comment and drop the per-arm repeats. Re-attach the doc comment that const_order_type's insertion had split off simple_type_of_value, and trim the test comments that re-explained the same mechanism. Co-Authored-By: Claude Fable 5 --- src/typing/checker.rs | 43 +++++++++++---------------- tests/typecheck_gaps_order_by_test.rs | 13 ++++---- 2 files changed, 23 insertions(+), 33 deletions(-) diff --git a/src/typing/checker.rs b/src/typing/checker.rs index 4ebc746..3afb835 100644 --- a/src/typing/checker.rs +++ b/src/typing/checker.rs @@ -214,24 +214,17 @@ impl Typechecker { } } - /// Type of an ORDER BY sort-key expression, hardened against the - /// `simple_type_of_value` punt that launders a constant record to - /// `Star` (which is reserved for base types in the gradual design). A - /// record literal is not a base type and is not orderable without - /// Feature GA04, so type it as `Record` here — locally to ORDER BY — - /// so the §22.14 check below rejects it. The global constant typing - /// (`Expr::Const`) is left unchanged, keeping the blast radius to sort - /// keys (RETURN/WHERE/COALESCE still see a constant record as `Star`). + /// 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 { - // A constant record (possibly nested) folds to `Value::Record` - // and is laundered to `Star` by `simple_type_of_value`; type it - // precisely so the comparability check rejects it. Expr::Const(v) => const_order_type(v), - // A repetition-group variable is not a base type either; - // `variable_type_to_simple_type` launders `Group` to `Star`. - // Type it as `Group` here so the check rejects it (ordering by a - // group of matched edges/nodes is not a comparable value). Expr::Var(name) => match env.get(name) { Some(VariableType::Group(_)) => SimpleType::Group(Box::new(SimpleType::Star)), _ => self.check_expr(e, env), @@ -1154,16 +1147,10 @@ fn create_context(desc: &Option, t: VariableType) -> TypeEnvironment } } -/// Map a literal `Value` to its `SimpleType`. -/// -/// List and Record values get a deliberately loose type — the precise -/// element/field types would require recursive typing of values, which -/// fppc doesn't do (it has no list literals). Documented in -/// `docs/internals/typechecker_migration.md` as a phase-1 punt. -/// Like `simple_type_of_value`, but does not launder a record to `Star`: -/// a constant record (possibly nested) types as `Record` so the ORDER BY -/// comparability check (§22.14) rejects it. Used only for sort keys, via -/// `order_key_type`; the global constant typing keeps the `Star` punt. +/// 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( @@ -1176,6 +1163,12 @@ fn const_order_type(v: &Value) -> SimpleType { } } +/// Map a literal `Value` to its `SimpleType`. +/// +/// List and Record values get a deliberately loose type — the precise +/// element/field types would require recursive typing of values, which +/// fppc doesn't do (it has no list literals). Documented in +/// `docs/internals/typechecker_migration.md` as a phase-1 punt. fn simple_type_of_value(v: &Value) -> SimpleType { match v { // Null literal is the SQL untyped null: it inhabits every type for diff --git a/tests/typecheck_gaps_order_by_test.rs b/tests/typecheck_gaps_order_by_test.rs index ed4568a..01974ac 100644 --- a/tests/typecheck_gaps_order_by_test.rs +++ b/tests/typecheck_gaps_order_by_test.rs @@ -308,11 +308,9 @@ fn typecheck_rejects_list_aggregate_in_sort_key() { ); } -// ISO §22.14: a record is not orderable (no Feature GA04). A *constant* -// record folds to `Star` via `simple_type_of_value` (the phase-1 punt), -// which would launder it past the comparability check. `order_key_type` -// types it as `Record` locally to ORDER BY so it is rejected, without -// touching the global constant typing. +// 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"); @@ -339,9 +337,8 @@ fn typecheck_accepts_record_scalar_field_in_sort_key() { assert!(r.is_ok(), "got: {:?}", r.err()); } -// A repetition-group variable (bound by `{n,m}`) is not a base type and -// is not orderable. `variable_type_to_simple_type` launders `Group` to -// `Star`; `order_key_type` types it as `Group` so ORDER BY rejects it. +// 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");