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
26 changes: 26 additions & 0 deletions docs/testing/semantic-coverage-matrix.md
Original file line number Diff line number Diff line change
Expand Up @@ -656,3 +656,29 @@ zero regressions; unit 944/944; functional clean):
evaluates), though the Quantifier1-4 [8]/[9] scenarios additionally need varlen
`relationships(p)`/`nodes(p)` through an aggregating WITH + GROUP-BY-on-list
(deeper, deferred).

## Coverage update (2026-06-02) — ORDER BY in WITH flows to downstream aggregation; overflow-safe temporal ordering (+10)

`transform_with.c`, `runtime/udf_helpers.c`. Verified via the TCK harness (rigorous
full pass-set diff: **zero regressions, +10**, 3776→3786; unit 944/944; functional
clean). Closes the entire `WithOrderBy1` [45] cluster ("Sort order should be
consistent with comparisons where comparisons are defined", all 10 type examples).

- **`WITH … ORDER BY <input-var> WITH collect(x)` now sees rows in sorted order.**
An ORDER BY on a non-aggregating WITH was only pushed into the WITH's CTE body
when it referenced a *dropped* input variable (WithOrderBy2 [21]-[24]); for an
ORDER BY over a *kept* input variable it stayed on the outer SELECT, which a
subsequent aggregating WITH (`collect`) never observed — so `collect` aggregated
in UNWIND order. Now, when every ORDER BY identifier is a live input variable and
the WITH has no LIMIT/SKIP, the ORDER BY is *also* emitted inside the CTE body
(`order_expr_all_live_input` gate) so the downstream aggregate's input is ordered
(SQLite preserves an ordered subquery's row order through `json_group_array`).
The outer ORDER BY is still emitted for final-output ordering.
- **Temporal `<`/`>` ordering no longer overflows int64 for far-future years.**
`gql_order_cmp_func`'s temporal path compared `parse_temporal_ns()` values, where
`epoch_seconds * 1e9` overflows int64 around year ~2262 and wraps negative — so
`localdatetime/datetime({year: 9999})` compared as the *smallest* value, making
`<` disagree with the ORDER BY key (which uses the zero-padded ISO string). New
`cmp_temporal_strings()` parses each operand to `(epoch_seconds, sub_second_ns)`
and compares componentwise (no overflow across the full Cypher year range),
falling back to lexical compare on parse failure.
77 changes: 74 additions & 3 deletions src/backend/runtime/udf_helpers.c
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,11 @@ static int gql_cmp_json_vals(sqlite3 *db,
* Called by transform_binary_operation's LT/GT/LTE/GTE path —
* each operand is transformed exactly once. */
static int64_t parse_temporal_ns(const char *s); /* defined below */
/* Overflow-safe temporal ordering: compares two ISO temporal strings by their
* UTC instant using (epoch_seconds, sub_second_ns) components, so far-future
* years whose epoch-nanoseconds exceed int64 don't wrap. Returns -1/0/1, or
* -2 if either operand fails to parse. (WithOrderBy1 [45].) */
static int cmp_temporal_strings(const char *a, const char *b); /* defined below */

/* Heuristic: does the text look like a Cypher temporal value (time 'HH:MM…' or
* date/datetime 'YYYY-MM-DD…')? Used so ordered comparison of two temporals
Expand Down Expand Up @@ -475,9 +480,11 @@ void gql_order_cmp_func(
if (!a || !b) { sqlite3_result_null(context); return; }
if (looks_temporal(a) && looks_temporal(b)) {
/* Compare two temporal values by UTC instant, not lexically, so a
* tz offset is honored (Temporal7 [3]: 10:00+01:00 < 09:35+00:00). */
int64_t na = parse_temporal_ns(a), nb = parse_temporal_ns(b);
cmp = (na < nb) ? -1 : (na > nb) ? 1 : 0;
* tz offset is honored (Temporal7 [3]: 10:00+01:00 < 09:35+00:00).
* Use the overflow-safe (seconds, ns) comparison so far-future
* years (9999) don't wrap int64 epoch-ns (WithOrderBy1 [45]). */
cmp = cmp_temporal_strings(a, b);
if (cmp == -2) cmp = strcmp(a, b); /* unparseable → lexical */
} else {
cmp = strcmp(a, b);
}
Expand Down Expand Up @@ -1554,6 +1561,70 @@ static int64_t parse_temporal_ns(const char *s) {
return (int64_t)epoch * 1000000000LL + ns;
}

/* Parse an ISO temporal string into UTC epoch SECONDS plus sub-second nanos,
* without the int64 overflow that (epoch * 1e9) suffers for far-future years.
* Mirrors parse_temporal_ns's field extraction. Returns false on parse error. */
static bool parse_temporal_secs_ns(const char *s, int64_t *out_secs, int64_t *out_ns) {
if (!s) return false;
int y = 1970, mo = 1, d = 1, h = 0, mi = 0, sec = 0;
int64_t ns = 0;
int tz_offset_min = 0;
const char *time_start = NULL;
if (strlen(s) >= 10 && s[4] == '-') {
if (sscanf(s, "%d-%d-%d", &y, &mo, &d) < 3) return false;
if (s[10] == 'T' || s[10] == ' ') time_start = s + 11;
} else if (strlen(s) >= 5 && s[2] == ':') {
time_start = s;
} else {
return false;
}
if (time_start) {
sscanf(time_start, "%d:%d:%d", &h, &mi, &sec);
const char *dot = strchr(time_start, '.');
if (dot) {
char buf[10] = { '0','0','0','0','0','0','0','0','0', 0 };
int i;
for (i = 0; i < 9 && dot[i + 1] >= '0' && dot[i + 1] <= '9'; i++)
buf[i] = dot[i + 1];
ns = atoll(buf);
}
const char *tz_from = dot ? dot + 1 : time_start;
const char *tz = NULL;
for (const char *q = tz_from; *q; q++) {
if (*q == 'Z' || *q == '+' || (*q == '-' && q > time_start + 2)) { tz = q; break; }
}
if (tz) {
if (*tz == 'Z') tz_offset_min = 0;
else {
int sign = (*tz == '+') ? 1 : -1;
int oh = 0, om = 0;
if (sscanf(tz + 1, "%d:%d", &oh, &om) >= 1 ||
sscanf(tz + 1, "%2d%2d", &oh, &om) >= 1)
tz_offset_min = sign * (oh * 60 + om);
}
}
}
struct tm t;
memset(&t, 0, sizeof(t));
t.tm_year = y - 1900; t.tm_mon = mo - 1; t.tm_mday = d;
t.tm_hour = h; t.tm_min = mi; t.tm_sec = sec;
time_t epoch = timegm(&t);
epoch -= tz_offset_min * 60;
*out_secs = (int64_t)epoch;
*out_ns = ns;
return true;
}

static int cmp_temporal_strings(const char *a, const char *b) {
int64_t sa, na, sb, nb;
if (!parse_temporal_secs_ns(a, &sa, &na) ||
!parse_temporal_secs_ns(b, &sb, &nb))
return -2;
if (sa != sb) return (sa < sb) ? -1 : 1;
if (na != nb) return (na < nb) ? -1 : 1;
return 0;
}

/* Build openCypher Duration JSON object from a signed total-nanoseconds value.
* Returns: {"_iso8601": "...", "months": 0, "days": D, "seconds": S, "nanosecondsOfSecond": N}
*
Expand Down
76 changes: 73 additions & 3 deletions src/backend/transform/transform_with.c
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,60 @@ static bool expr_refs_dropped_input_var(cypher_transform_context *ctx, ast_node
}
}

/* True if every identifier referenced by `expr` is live in the CURRENT
* (input) scope — i.e. the expression can be evaluated inside the WITH's CTE
* body (where the input tables are in FROM). Used to decide whether a
* non-aggregating WITH's ORDER BY can be pushed into the CTE body so a
* DOWNSTREAM aggregating WITH (e.g. `... ORDER BY value WITH collect(x)`)
* sees rows in sorted order. Mirrors validate_identifiers_in_scope_ex's walk;
* projected-only aliases (not yet input vars) make this return false, so they
* stay on the outer path. (WithOrderBy1 [45].) */
static bool order_expr_all_live_input(cypher_transform_context *ctx, ast_node *expr)
{
if (!expr) return true;
switch (expr->type) {
case AST_NODE_IDENTIFIER: {
cypher_identifier *id = (cypher_identifier*)expr;
if (!id->name) return true;
return transform_var_lookup(ctx->var_ctx, id->name) != NULL;
}
case AST_NODE_PROPERTY:
return order_expr_all_live_input(ctx, ((cypher_property*)expr)->expr);
case AST_NODE_BINARY_OP: {
cypher_binary_op *b = (cypher_binary_op*)expr;
return order_expr_all_live_input(ctx, b->left) &&
order_expr_all_live_input(ctx, b->right);
}
case AST_NODE_NOT_EXPR:
return order_expr_all_live_input(ctx, ((cypher_not_expr*)expr)->expr);
case AST_NODE_NULL_CHECK:
return order_expr_all_live_input(ctx, ((cypher_null_check*)expr)->expr);
case AST_NODE_FUNCTION_CALL: {
cypher_function_call *fc = (cypher_function_call*)expr;
if (fc->args)
for (int i = 0; i < fc->args->count; i++)
if (!order_expr_all_live_input(ctx, fc->args->items[i])) return false;
return true;
}
case AST_NODE_LIST: {
cypher_list *l = (cypher_list*)expr;
if (l->items)
for (int i = 0; i < l->items->count; i++)
if (!order_expr_all_live_input(ctx, l->items->items[i])) return false;
return true;
}
case AST_NODE_SUBSCRIPT: {
cypher_subscript *s = (cypher_subscript*)expr;
return order_expr_all_live_input(ctx, s->expr) &&
order_expr_all_live_input(ctx, s->index) &&
order_expr_all_live_input(ctx, s->slice_start) &&
order_expr_all_live_input(ctx, s->slice_end);
}
default:
return true; /* literals, parameters, etc. */
}
}

/*
* Transform an expression to a dynamically allocated string.
* Uses a temporary buffer to capture output, then returns the result.
Expand Down Expand Up @@ -670,16 +724,30 @@ int transform_with_clause(cypher_transform_context *ctx, cypher_with *with)
nm = ((cypher_property*)it->expr)->property_name;
if (nm) proj_names[proj_n++] = (char*)nm;
}
bool needs = false, has_agg_order = false;
bool needs = false, has_agg_order = false, all_live_input = true;
for (int i = 0; i < with->order_by->count; i++) {
cypher_order_by_item *oi = (cypher_order_by_item*)with->order_by->items[i];
if (find_aggregating_call(oi->expr)) has_agg_order = true;
if (expr_refs_dropped_input_var(ctx, oi->expr, proj_names, proj_n)) needs = true;
if (!order_expr_all_live_input(ctx, oi->expr)) all_live_input = false;
}
/* Aggregating ORDER BY exprs (e.g. ORDER BY count(x) + 1) interact with
* the WITH's GROUP BY and must stay on the outer path. (WithOrderBy4 [16].) */
if (has_agg_order) needs = false;
if (needs) {

/* Push the ORDER BY into the CTE body when either:
* (a) `needs` — it references an input variable the WITH drops, which
* the outer SELECT over the CTE cannot resolve (WithOrderBy2
* [21]-[24]); this REPLACES the outer ORDER BY; or
* (b) every ORDER BY identifier is a live input variable and the WITH
* has no LIMIT/SKIP — ordering the CTE rows so a DOWNSTREAM
* aggregating WITH (`... ORDER BY value WITH collect(x)`) sees
* sorted input. SQLite preserves an ordered subquery's row order
* through json_group_array. The outer ORDER BY is still emitted
* for final-output ordering. (WithOrderBy1 [45].) */
bool no_limit = !with->limit && !with->skip;
bool dup_for_downstream = !needs && !has_agg_order && all_live_input && no_limit;
if (needs || dup_for_downstream) {
dbuf_append(&cte_body, " ORDER BY ");
for (int i = 0; i < with->order_by->count; i++) {
cypher_order_by_item *oi = (cypher_order_by_item*)with->order_by->items[i];
Expand All @@ -693,7 +761,9 @@ int transform_with_clause(cypher_transform_context *ctx, cypher_with *with)
oexpr, odir, oexpr, odir);
free(oe);
}
order_pushed_to_cte = true;
/* Only the dropped-var case suppresses the outer ORDER BY; the
* downstream-ordering duplicate keeps it for final output. */
if (needs) order_pushed_to_cte = true;
}
}

Expand Down
Loading