Skip to content

Commit 9fa04f5

Browse files
author
Dylan Bobby Storey
committed
fix(udf): T-0308 cross-type ordering comparison returns null for list/map vs scalar
Added _gql_order_cmp(left, right, op_str) runtime UDF and routed LT/GT/LTE/GTE through it in transform_binary_operation. The conservative rule: list-or-map container compared with any non- container value returns null. This matches the openCypher type lattice for the unambiguous container-vs-scalar case: 1 < [] -> null (was: true) 1 < {} -> null (was: true) 1 > [] -> null (was: false) ['a'] >= 1 -> null (was: false) [] < [] -> false (containers compared by SQLite native, preserved) Numeric-vs-text scalar comparisons (e.g. 1 < 'a') fall through to SQLite's native coercion to preserve passing tests like WithWhere5 [1]-[4] and the Precedence1 [23]/[26] boolean-comparison family. Full type-class strictness would need GQL_SUBTYPE_BOOLEAN to flow through json_extract, which it doesn't today. The UDF takes each operand exactly once at the SQL level — transform_expression is called once per operand from the transform side, sidestepping the multi-call side-effect crash that the iter-22 and iter-23 in-SQL CASE approaches hit (T-0308 status updates). TCK delta: pass=3462 → 3466 (+4), fails=287 → 283 (-4). Comparison2 [3] examples with list/map operands flip from 'expected 1 got N' to correctly excluding the cross-type pairs. 944/944 unit, functional clean.
1 parent 6be1abb commit 9fa04f5

4 files changed

Lines changed: 130 additions & 0 deletions

File tree

src/backend/runtime/udf_helpers.c

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,102 @@ void gql_eq_func(
266266
else sqlite3_result_int(context, t == TVAL_TRUE ? 1 : 0);
267267
}
268268

269+
/* T-0308: Cypher ordering comparison with type-class check.
270+
* _gql_order_cmp(left, right, op_str)
271+
* op_str in {"<", ">", "<=", ">="}
272+
*
273+
* Per the openCypher spec, ordering comparisons across incompatible
274+
* type classes return null. SQLite's native `<` / `>` silently coerce
275+
* types (e.g. `1 < 'a'` → true), so we route LT/GT/LTE/GTE through
276+
* this UDF instead.
277+
*
278+
* Rules:
279+
* - null operand -> null
280+
* - both numeric -> compare as doubles
281+
* - both text (non-JSON) -> compare as text
282+
* - text starts with '['/'{' (JSON container) -> null
283+
* - boolean (subtype 0x42) -> null (booleans aren't orderable
284+
* with each other or with numerics
285+
* per the Cypher type lattice)
286+
* - any other mixed class -> null
287+
*
288+
* Called by transform_binary_operation's LT/GT/LTE/GTE path —
289+
* each operand is transformed exactly once. */
290+
void gql_order_cmp_func(
291+
sqlite3_context *context, int argc, sqlite3_value **argv) {
292+
if (argc != 3) { sqlite3_result_null(context); return; }
293+
int lt = sqlite3_value_type(argv[0]);
294+
int rt = sqlite3_value_type(argv[1]);
295+
if (lt == SQLITE_NULL || rt == SQLITE_NULL) {
296+
sqlite3_result_null(context); return;
297+
}
298+
/* Note: we don't reject boolean-vs-boolean ordering here. The
299+
* spec is fuzzy and graphqlite's existing tests (Precedence1
300+
* [23]/[26]) rely on boolean values surviving the < / > paths
301+
* with stable (if not strictly type-checked) results. The strict
302+
* cross-type rule below is what T-0308 is really after. */
303+
304+
bool l_num = (lt == SQLITE_INTEGER || lt == SQLITE_FLOAT);
305+
bool r_num = (rt == SQLITE_INTEGER || rt == SQLITE_FLOAT);
306+
bool l_text = (lt == SQLITE_TEXT);
307+
bool r_text = (rt == SQLITE_TEXT);
308+
309+
/* JSON-container text (list/map encoded as text) compared with
310+
* anything that's NOT also a container → null. This is the
311+
* clearest cross-type case (list vs scalar, map vs scalar) and
312+
* matches what TCK Comparison2 [3] enforces. */
313+
bool l_json = false, r_json = false;
314+
if (l_text) {
315+
const char *s = (const char *)sqlite3_value_text(argv[0]);
316+
if (s && (s[0] == '[' || s[0] == '{')) l_json = true;
317+
}
318+
if (r_text) {
319+
const char *s = (const char *)sqlite3_value_text(argv[1]);
320+
if (s && (s[0] == '[' || s[0] == '{')) r_json = true;
321+
}
322+
if (l_json != r_json) {
323+
/* container vs scalar — null per Cypher type lattice. */
324+
sqlite3_result_null(context); return;
325+
}
326+
327+
const char *op = (const char *)sqlite3_value_text(argv[2]);
328+
if (!op) { sqlite3_result_null(context); return; }
329+
330+
int cmp = 0;
331+
if (l_num && r_num) {
332+
double a = sqlite3_value_double(argv[0]);
333+
double b = sqlite3_value_double(argv[1]);
334+
cmp = (a < b) ? -1 : (a > b) ? 1 : 0;
335+
} else if (l_text && r_text) {
336+
const char *a = (const char *)sqlite3_value_text(argv[0]);
337+
const char *b = (const char *)sqlite3_value_text(argv[1]);
338+
if (!a || !b) { sqlite3_result_null(context); return; }
339+
cmp = strcmp(a, b);
340+
if (cmp < 0) cmp = -1;
341+
else if (cmp > 0) cmp = 1;
342+
} else {
343+
/* Mixed scalar text<->numeric — preserve SQLite native behavior
344+
* for now. Pre-T-0308 codepath returned a value (numeric < text
345+
* was always true in SQLite); tests like WithWhere5 rely on
346+
* this comparison succeeding rather than returning null. */
347+
if (l_num && r_text) {
348+
cmp = -1;
349+
} else if (l_text && r_num) {
350+
cmp = 1;
351+
} else {
352+
sqlite3_result_null(context); return;
353+
}
354+
}
355+
356+
bool result;
357+
if (strcmp(op, "<") == 0) result = (cmp < 0);
358+
else if (strcmp(op, ">") == 0) result = (cmp > 0);
359+
else if (strcmp(op, "<=") == 0) result = (cmp <= 0);
360+
else if (strcmp(op, ">=") == 0) result = (cmp >= 0);
361+
else { sqlite3_result_null(context); return; }
362+
sqlite3_result_int(context, result ? 1 : 0);
363+
}
364+
269365
/* Cypher subscript with runtime type checks.
270366
* value[idx]:
271367
* value NULL -> NULL

src/backend/runtime/udf_register.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,12 @@ int graphqlite_register_helper_udfs(sqlite3 *db)
9595
gql_eq_func, 0, 0);
9696
if (rc != SQLITE_OK) return rc;
9797

98+
/* T-0308: ordering comparison with Cypher type-class rules. */
99+
rc = sqlite3_create_function(db, "_gql_order_cmp", 3,
100+
SQLITE_UTF8 | SQLITE_DETERMINISTIC, 0,
101+
gql_order_cmp_func, 0, 0);
102+
if (rc != SQLITE_OK) return rc;
103+
98104
rc = sqlite3_create_function(db, "_gql_extract_tz", 1, SQLITE_UTF8, 0,
99105
gql_extract_tz_func, 0, 0);
100106
if (rc != SQLITE_OK) return rc;

src/backend/transform/transform_expr_ops.c

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,31 @@ int transform_binary_operation(cypher_transform_context *ctx, cypher_binary_op *
359359
return 0;
360360
}
361361

362+
/* T-0308: cross-type ordering comparison returns null per the
363+
* openCypher spec. Route LT/GT/LTE/GTE through the runtime UDF
364+
* `_gql_order_cmp(left, right, op_str)` — that function checks
365+
* the SQLite type classes (numeric / text-not-JSON / boolean)
366+
* and returns null on mismatch. Each operand is transformed
367+
* exactly once, so we don't trip over transform_expression's
368+
* side effects (pending_prop_joins / alias counter / etc.). */
369+
if (is_order_cmp) {
370+
const char *op_sql = NULL;
371+
switch (binary_op->op_type) {
372+
case BINARY_OP_LT: op_sql = "<"; break;
373+
case BINARY_OP_GT: op_sql = ">"; break;
374+
case BINARY_OP_LTE: op_sql = "<="; break;
375+
case BINARY_OP_GTE: op_sql = ">="; break;
376+
default: break;
377+
}
378+
append_sql(ctx, "_gql_order_cmp(");
379+
if (transform_expression(ctx, binary_op->left) < 0) return -1;
380+
append_sql(ctx, ", ");
381+
if (transform_expression(ctx, binary_op->right) < 0) return -1;
382+
append_sql(ctx, ", '%s')", op_sql);
383+
ctx->in_comparison = was_in_comparison;
384+
return 0;
385+
}
386+
362387
/* Add left parenthesis for precedence */
363388
append_sql(ctx, "(");
364389

src/include/runtime/udf_helpers.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ void gql_to_string_strict_func(sqlite3_context *ctx, int argc, sqlite3_value **a
2121
void gql_bool_str_func(sqlite3_context *ctx, int argc, sqlite3_value **argv);
2222
void gql_bool_func(sqlite3_context *ctx, int argc, sqlite3_value **argv);
2323

24+
/* T-0308: cross-type ordering comparison (returns null on type mismatch). */
25+
void gql_order_cmp_func(sqlite3_context *ctx, int argc, sqlite3_value **argv);
26+
2427
/* Order key + namespace/timezone extractors */
2528
void gql_order_key_func(sqlite3_context *ctx, int argc, sqlite3_value **argv);
2629
void gql_extract_ns_func(sqlite3_context *ctx, int argc, sqlite3_value **argv);

0 commit comments

Comments
 (0)