Skip to content

Commit cc19476

Browse files
gregfeliceclaude
andcommitted
Address Copilot round 2: NULL-list guard, single() comment, pg_aggregate.h
1. Add NULL-list guard for all predicate functions (all/any/none/single). Wraps the result with CASE WHEN list IS NULL THEN NULL ELSE <result> END in the grammar layer. This fixes single(x IN null WHERE ...) returning false instead of NULL. The expr pointer is safely shared between the NullTest and the predicate function node because AGE's expression transformer creates new nodes without modifying the parse tree in-place. 2. Fix single() block comment in transform_cypher_predicate_function: described LIMIT 2 optimization but implementation uses plain count(*). Updated comment to match actual implementation. 3. Keep #include "catalog/pg_aggregate.h" -- Copilot suggested removal but AGGKIND_NORMAL macro requires it (build fails without it). Regression test: predicate_functions OK. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
1 parent 77a6aea commit cc19476

File tree

4 files changed

+58
-19
lines changed

4 files changed

+58
-19
lines changed

regress/expected/predicate_functions.out

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -155,9 +155,9 @@ $$) AS (result agtype);
155155
(1 row)
156156

157157
--
158-
-- NULL list input: all/any/none return null, single returns false
159-
-- (unnest of NULL produces zero rows; aggregates return NULL over
160-
-- empty input, but count(*) returns 0)
158+
-- NULL list input: all four return null
159+
-- (NULL-list guard in the grammar produces CASE WHEN expr IS NULL
160+
-- THEN NULL ELSE <subquery> END)
161161
--
162162
SELECT * FROM cypher('predicate_functions', $$
163163
RETURN all(x IN null WHERE x > 0)
@@ -188,7 +188,7 @@ SELECT * FROM cypher('predicate_functions', $$
188188
$$) AS (result agtype);
189189
result
190190
--------
191-
false
191+
192192
(1 row)
193193

194194
--
@@ -271,13 +271,13 @@ $$) AS (result agtype);
271271
false
272272
(1 row)
273273

274-
-- single() with null list: count(*) = 0, 0 = 1 -> false
274+
-- single() with null list: NULL (same as other predicate functions)
275275
SELECT * FROM cypher('predicate_functions', $$
276276
RETURN single(x IN null WHERE x > 0)
277277
$$) AS (result agtype);
278278
result
279279
--------
280-
false
280+
281281
(1 row)
282282

283283
--

regress/sql/predicate_functions.sql

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,9 +100,9 @@ SELECT * FROM cypher('predicate_functions', $$
100100
$$) AS (result agtype);
101101

102102
--
103-
-- NULL list input: all/any/none return null, single returns false
104-
-- (unnest of NULL produces zero rows; aggregates return NULL over
105-
-- empty input, but count(*) returns 0)
103+
-- NULL list input: all four return null
104+
-- (NULL-list guard in the grammar produces CASE WHEN expr IS NULL
105+
-- THEN NULL ELSE <subquery> END)
106106
--
107107
SELECT * FROM cypher('predicate_functions', $$
108108
RETURN all(x IN null WHERE x > 0)
@@ -169,7 +169,7 @@ SELECT * FROM cypher('predicate_functions', $$
169169
RETURN single(x IN [null, 5] WHERE x > 0)
170170
$$) AS (result agtype);
171171

172-
-- single() with null list: count(*) = 0, 0 = 1 -> false
172+
-- single() with null list: NULL (same as other predicate functions)
173173
SELECT * FROM cypher('predicate_functions', $$
174174
RETURN single(x IN null WHERE x > 0)
175175
$$) AS (result agtype);

src/backend/parser/cypher_clause.c

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1776,8 +1776,9 @@ static Node *make_predicate_case_expr(ParseState *pstate, Node *pred,
17761776
* Transform a cypher_predicate_function node into a query tree.
17771777
*
17781778
* Generates aggregate-based queries that preserve Cypher's three-valued
1779-
* NULL semantics. The grammar layer adds a CASE WHEN list IS NULL
1780-
* THEN NULL ELSE (subquery) END guard for the null-list case.
1779+
* NULL semantics. The grammar layer wraps the SubLink with a
1780+
* CASE WHEN list IS NULL THEN NULL ELSE (subquery) END guard so all
1781+
* four functions return NULL when the input list is NULL.
17811782
*
17821783
* For all()/any()/none():
17831784
* SELECT CASE WHEN bool_or(pred IS TRUE/FALSE) THEN ...
@@ -1786,13 +1787,11 @@ static Node *make_predicate_case_expr(ParseState *pstate, Node *pred,
17861787
* FROM unnest(list) AS x
17871788
*
17881789
* For single():
1789-
* SELECT count(*) FROM (
1790-
* SELECT 1 FROM unnest(list) AS x WHERE pred IS TRUE LIMIT 2
1791-
* ) sub
1790+
* SELECT count(*)
1791+
* FROM unnest(list) AS x
1792+
* WHERE pred IS TRUE
17921793
*
17931794
* All four use EXPR_SUBLINK so the subquery returns a scalar value.
1794-
* The LIMIT 2 in single() enables early termination: once two matches
1795-
* are found, evaluation stops and count(*) = 2 != 1.
17961795
*/
17971796
static Query *transform_cypher_predicate_function(cypher_parsestate *cpstate,
17981797
cypher_clause *clause)

src/backend/parser/cypher_gram.y

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3404,6 +3404,7 @@ static Node *build_predicate_function_node(cypher_predicate_function_kind kind,
34043404
{
34053405
SubLink *sub;
34063406
cypher_predicate_function *pred_func = NULL;
3407+
Node *result;
34073408

34083409
/* build the predicate function node */
34093410
pred_func = make_ag_node(cypher_predicate_function);
@@ -3422,6 +3423,10 @@ static Node *build_predicate_function_node(cypher_predicate_function_kind kind,
34223423
* All predicate functions now use EXPR_SUBLINK: the transform layer
34233424
* generates aggregate-based queries that return a scalar boolean
34243425
* (for all/any/none) or integer (for single).
3426+
*
3427+
* The transform layer also wraps the result with a NULL-list guard
3428+
* (CASE WHEN list IS NULL THEN NULL ELSE <result> END) to ensure
3429+
* all four functions return NULL when the input list is NULL.
34253430
*/
34263431
sub = makeNode(SubLink);
34273432
sub->subLinkId = 0;
@@ -3443,15 +3448,50 @@ static Node *build_predicate_function_node(cypher_predicate_function_kind kind,
34433448
(Node *) sub,
34443449
make_int_const(1, location),
34453450
location);
3446-
return (Node *) node_to_agtype(eq_expr, "boolean", location);
3451+
result = (Node *) node_to_agtype(eq_expr, "boolean", location);
34473452
}
34483453
else
34493454
{
34503455
/*
34513456
* all()/any()/none(): the subquery returns a boolean directly
34523457
* from the CASE+bool_or() aggregate expression.
34533458
*/
3454-
return (Node *) node_to_agtype((Node *) sub, "boolean", location);
3459+
result = (Node *) node_to_agtype((Node *) sub, "boolean", location);
3460+
}
3461+
3462+
/*
3463+
* NULL-list guard: CASE WHEN expr IS NULL THEN NULL ELSE result END
3464+
*
3465+
* Without this, unnest(NULL) produces zero rows, causing all/any/none
3466+
* to accidentally return NULL (via bool_or over empty input) and
3467+
* single to return false (count(*) = 0). Cypher semantics require
3468+
* all four to return NULL when the input list is NULL.
3469+
*
3470+
* The expr pointer is shared with pred_func->expr. This is safe
3471+
* because AGE's expression transformer (transform_cypher_expr_recurse)
3472+
* creates new nodes rather than modifying the parse tree in-place,
3473+
* so the two references are transformed independently.
3474+
*/
3475+
{
3476+
NullTest *null_test = makeNode(NullTest);
3477+
CaseWhen *case_when = makeNode(CaseWhen);
3478+
CaseExpr *guard = makeNode(CaseExpr);
3479+
3480+
null_test->arg = (Expr *) expr;
3481+
null_test->nulltesttype = IS_NULL;
3482+
null_test->argisrow = false;
3483+
null_test->location = location;
3484+
3485+
case_when->expr = (Expr *) null_test;
3486+
case_when->result = (Expr *) make_null_const(location);
3487+
case_when->location = location;
3488+
3489+
guard->arg = NULL;
3490+
guard->args = list_make1(case_when);
3491+
guard->defresult = (Expr *) result;
3492+
guard->location = location;
3493+
3494+
return (Node *) guard;
34553495
}
34563496
}
34573497

0 commit comments

Comments
 (0)