Skip to content

Commit 20ab387

Browse files
author
Dylan Bobby Storey
committed
validate: UndefinedVariable on new aggregate in ORDER BY (restricted mode)
WithOrderBy4 [13] / [14] expect SyntaxError when ORDER BY introduces a NEW aggregate function call that isn't part of the projection: WITH a.num2 % 3 AS mod, min(sum) AS min ORDER BY sum(sum) --------^^^^^^^^^ -- sum() is a fresh aggregate not in projection -> error The existing order_by_walk_check already skipped INSIDE aggregate calls (correct — grouping keys aren't free vars there), but didn't check whether the aggregate itself was valid to use. Extended the walker to record top-level aggregate function names from the projection and reject ORDER BY aggregates whose function name isn't among them. Repeating count(*) when count(*) is projected still works (function name matches). TCK delta: pass=3448 -> 3450 (+2), fails -2. 944/944 unit, functional clean.
1 parent 2768a78 commit 20ab387

1 file changed

Lines changed: 41 additions & 10 deletions

File tree

src/backend/transform/transform_validate.c

Lines changed: 41 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -950,14 +950,32 @@ static int check_ambiguous_aggregation(const ast_node *expr,
950950

951951
/* Recursive walker for ORDER BY: accept if every non-aggregate free
952952
* reference is in `available` or, in property-path form,
953-
* `available_props`. Skips inside aggregate calls. */
953+
* `available_props`. Skips inside aggregate calls — but the aggregate
954+
* itself must be one whose function name appears in the projection
955+
* (passed as `projected_agg_names`); otherwise it's a "new" aggregate
956+
* which is invalid in restricted mode. */
954957
static int order_by_walk_check(const ast_node *expr,
955958
const name_set *available,
956959
const name_set *available_props,
960+
const name_set *projected_agg_names,
957961
char **error_message)
958962
{
959963
if (!expr) return 0;
960-
if (ast_is_aggregate_func(expr)) return 0; /* inside-agg is fine */
964+
if (ast_is_aggregate_func(expr)) {
965+
/* WithOrderBy4 [13]/[14]: ORDER BY introduces a NEW aggregate
966+
* not present in the projection. Per spec, ORDER BY in restricted
967+
* mode can only re-use projection-aggregates. */
968+
const cypher_function_call *fc = (const cypher_function_call *)expr;
969+
if (fc->function_name && projected_agg_names &&
970+
!nset_contains(projected_agg_names, fc->function_name)) {
971+
set_error(error_message,
972+
"SyntaxError: UndefinedVariable: ORDER BY introduces aggregate `%s` "
973+
"that is not in the projection",
974+
fc->function_name);
975+
return -1;
976+
}
977+
return 0;
978+
}
961979
if (expr->type == AST_NODE_IDENTIFIER) {
962980
const char *n = ((const cypher_identifier *)expr)->name;
963981
if (!nset_contains(available, n)) {
@@ -979,29 +997,29 @@ static int order_by_walk_check(const ast_node *expr,
979997
if (nset_contains(available_props, path)) return 0;
980998
}
981999
/* Otherwise recurse into the base. */
982-
return order_by_walk_check(p->expr, available, available_props, error_message);
1000+
return order_by_walk_check(p->expr, available, available_props, projected_agg_names, error_message);
9831001
}
9841002
switch (expr->type) {
9851003
case AST_NODE_BINARY_OP: {
9861004
const cypher_binary_op *b = (const cypher_binary_op *)expr;
987-
if (order_by_walk_check(b->left, available, available_props, error_message) < 0) return -1;
988-
return order_by_walk_check(b->right, available, available_props, error_message);
1005+
if (order_by_walk_check(b->left, available, available_props, projected_agg_names, error_message) < 0) return -1;
1006+
return order_by_walk_check(b->right, available, available_props, projected_agg_names, error_message);
9891007
}
9901008
case AST_NODE_NOT_EXPR:
991-
return order_by_walk_check(((const cypher_not_expr *)expr)->expr, available, available_props, error_message);
1009+
return order_by_walk_check(((const cypher_not_expr *)expr)->expr, available, available_props, projected_agg_names, error_message);
9921010
case AST_NODE_FUNCTION_CALL: {
9931011
const cypher_function_call *f = (const cypher_function_call *)expr;
9941012
if (f->args) {
9951013
for (int i = 0; i < f->args->count; i++) {
996-
if (order_by_walk_check(f->args->items[i], available, available_props, error_message) < 0) return -1;
1014+
if (order_by_walk_check(f->args->items[i], available, available_props, projected_agg_names, error_message) < 0) return -1;
9971015
}
9981016
}
9991017
return 0;
10001018
}
10011019
case AST_NODE_SUBSCRIPT: {
10021020
const cypher_subscript *s = (const cypher_subscript *)expr;
1003-
if (order_by_walk_check(s->expr, available, available_props, error_message) < 0) return -1;
1004-
return order_by_walk_check(s->index, available, available_props, error_message);
1021+
if (order_by_walk_check(s->expr, available, available_props, projected_agg_names, error_message) < 0) return -1;
1022+
return order_by_walk_check(s->index, available, available_props, projected_agg_names, error_message);
10051023
}
10061024
default:
10071025
return 0;
@@ -1047,6 +1065,11 @@ static int validate_order_by_with_projection(ast_list *order_by,
10471065
* `available_props` as "base.prop" strings. */
10481066
name_set available; nset_init(&available);
10491067
name_set available_props; nset_init(&available_props);
1068+
/* Aggregate function names that appear at top-level in the
1069+
* projection. ORDER BY may only repeat aggregates that match
1070+
* these names; introducing a new aggregate (sum(x) when
1071+
* projection has min(x)) is invalid. */
1072+
name_set projected_agg_names; nset_init(&projected_agg_names);
10501073
/* Storage for synthesized "base.prop" strings — kept alive while
10511074
* we run the order-by walk, then freed below. */
10521075
char **prop_storage = NULL;
@@ -1075,16 +1098,24 @@ static int validate_order_by_with_projection(ast_list *order_by,
10751098
}
10761099
}
10771100
}
1101+
/* Catch top-level aggregate calls in the projection — their
1102+
* function name is what ORDER BY is allowed to reuse. */
1103+
if (ast_is_aggregate_func(it->expr)) {
1104+
const cypher_function_call *fc = (const cypher_function_call *)it->expr;
1105+
if (fc->function_name) nset_add(&projected_agg_names, fc->function_name);
1106+
}
10781107
}
10791108

10801109
int rc = 0;
10811110
for (int i = 0; i < order_by->count && rc == 0; i++) {
10821111
cypher_order_by_item *obi = (cypher_order_by_item *)order_by->items[i];
10831112
if (!obi || !obi->expr) continue;
1084-
rc = order_by_walk_check(obi->expr, &available, &available_props, error_message);
1113+
rc = order_by_walk_check(obi->expr, &available, &available_props,
1114+
&projected_agg_names, error_message);
10851115
}
10861116
nset_free(&available);
10871117
nset_free(&available_props);
1118+
nset_free(&projected_agg_names);
10881119
for (int i = 0; i < prop_storage_count; i++) free(prop_storage[i]);
10891120
free(prop_storage);
10901121
return rc;

0 commit comments

Comments
 (0)