Skip to content

Commit a302058

Browse files
author
Dylan Bobby Storey
committed
validate: T-0234 — LIMIT/SKIP allow constant expressions (no var refs)
Previous check rejected any non-literal/non-parameter expression in LIMIT or SKIP, including constant computations like: WITH n LIMIT toInteger(ceil(1.7)) RETURN count(*) -- ReturnSkipLimit2 [6] Per the openCypher spec, LIMIT/SKIP accept any expression that evaluates to a non-negative integer at compile time. Only expressions that reference variables from the surrounding scope (`LIMIT n.count`) are invalid. Added expr_has_var_reference() — walks the AST looking for IDENTIFIER / PROPERTY / SUBSCRIPT bases. The SKIP/LIMIT validator now allows any expression with no var refs (the runtime handles the actual value check) and rejects expressions with var refs as NonConstantExpression. TCK delta: pass=3461 → 3462 (+1), errors=79 → 77 (-2). The other error scenario flipped to a result fail (executor LIMIT-through- WITH still has its own bug; that's separate from validation). 944/944 unit, functional clean.
1 parent c1ac2f3 commit a302058

1 file changed

Lines changed: 64 additions & 4 deletions

File tree

src/backend/transform/transform_validate.c

Lines changed: 64 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -713,9 +713,67 @@ static int validate_expr_typed(ast_node *expr, const var_type_ctx *vctx, char **
713713
}
714714
}
715715

716-
/* SKIP / LIMIT must be a non-negative integer constant or a parameter.
717-
* Expressions referencing variables (e.g. `LIMIT n.count`) and negative
718-
* literals are rejected at compile time per the openCypher spec. */
716+
/* True if `expr` contains any free variable reference (identifier or
717+
* property access whose base is an identifier). Walks function-call
718+
* args, binary ops, list/map literals, etc. Used by validate_skip_limit
719+
* to allow constant-valued expressions like `toInteger(ceil(1.7))`. */
720+
static bool expr_has_var_reference(const ast_node *expr)
721+
{
722+
if (!expr) return false;
723+
switch (expr->type) {
724+
case AST_NODE_IDENTIFIER:
725+
return true;
726+
case AST_NODE_PROPERTY:
727+
return true; /* base is implicitly a var */
728+
case AST_NODE_SUBSCRIPT: {
729+
const cypher_subscript *s = (const cypher_subscript *)expr;
730+
return expr_has_var_reference(s->expr) ||
731+
expr_has_var_reference(s->index);
732+
}
733+
case AST_NODE_BINARY_OP: {
734+
const cypher_binary_op *b = (const cypher_binary_op *)expr;
735+
return expr_has_var_reference(b->left) ||
736+
expr_has_var_reference(b->right);
737+
}
738+
case AST_NODE_NOT_EXPR:
739+
return expr_has_var_reference(((const cypher_not_expr *)expr)->expr);
740+
case AST_NODE_NULL_CHECK:
741+
return expr_has_var_reference(((const cypher_null_check *)expr)->expr);
742+
case AST_NODE_FUNCTION_CALL: {
743+
const cypher_function_call *f = (const cypher_function_call *)expr;
744+
if (!f->args) return false;
745+
for (int i = 0; i < f->args->count; i++) {
746+
if (expr_has_var_reference(f->args->items[i])) return true;
747+
}
748+
return false;
749+
}
750+
case AST_NODE_LIST: {
751+
const cypher_list *l = (const cypher_list *)expr;
752+
if (!l->items) return false;
753+
for (int i = 0; i < l->items->count; i++) {
754+
if (expr_has_var_reference(l->items->items[i])) return true;
755+
}
756+
return false;
757+
}
758+
case AST_NODE_MAP: {
759+
const cypher_map *m = (const cypher_map *)expr;
760+
if (!m->pairs) return false;
761+
for (int i = 0; i < m->pairs->count; i++) {
762+
const cypher_map_pair *p = (const cypher_map_pair *)m->pairs->items[i];
763+
if (p && expr_has_var_reference(p->value)) return true;
764+
}
765+
return false;
766+
}
767+
default:
768+
return false;
769+
}
770+
}
771+
772+
/* SKIP / LIMIT must be a non-negative integer constant, a parameter, or
773+
* a constant expression that doesn't reference variables. Expressions
774+
* referencing variables (e.g. `LIMIT n.count`) and negative literals
775+
* are rejected at compile time per the openCypher spec.
776+
* `LIMIT toInteger(ceil(1.7))` is valid (ReturnSkipLimit2 [6]). */
719777
static int validate_skip_limit(ast_node *expr, const char *kw, char **error_message)
720778
{
721779
if (!expr) return 0;
@@ -740,7 +798,9 @@ static int validate_skip_limit(ast_node *expr, const char *kw, char **error_mess
740798
return -1;
741799
}
742800
if (expr->type == AST_NODE_PARAMETER) return 0;
743-
/* Identifiers, property access, function calls, etc. — non-constant. */
801+
/* Constant-valued expression (no var reference) is OK; runtime checks
802+
* value. References to variables are rejected at compile time. */
803+
if (!expr_has_var_reference(expr)) return 0;
744804
char buf[160];
745805
snprintf(buf, sizeof(buf),
746806
"SyntaxError: NonConstantExpression: %s must be a constant integer or parameter",

0 commit comments

Comments
 (0)