Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 43 additions & 6 deletions src/typing/checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand All @@ -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 \
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -1126,6 +1147,22 @@ fn create_context(desc: &Option<Descriptor>, 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
Expand Down
71 changes: 71 additions & 0 deletions tests/typecheck_gaps_order_by_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
);
}
Loading