Copilot commented on code in PR #2406:
URL: https://github.com/apache/age/pull/2406#discussion_r3149456634
##########
src/backend/parser/cypher_clause.c:
##########
@@ -1769,6 +1815,62 @@ static Node *make_predicate_case_expr(ParseState
*pstate, Node *pred,
cexpr->defresult = (Expr *) true_const;
cexpr->location = -1;
}
+ else /* CPFK_SINGLE */
+ {
+ /*
+ * Three WHEN arms, in order:
+ * count(*) FILTER (pred IS TRUE) >= 2 -> false (definitely >1)
+ * bool_or(pred IS NULL) -> NULL (unknown arm)
+ * count(*) FILTER (pred IS TRUE) = 1 -> true (exactly one)
+ * else -> false (zero true)
+ *
+ * The >=2 arm is tested first so that 2-or-more definite trues
+ * win over any null predicates, matching Neo4j semantics.
+ */
+ CaseWhen *when0, *when3;
+ Node *count_ge_2, *count_eq_1;
+ Node *count_true_a, *count_true_b;
+ Node *int_2 = (Node *) makeConst(INT8OID, -1, InvalidOid,
sizeof(int64),
+ Int64GetDatum(2), false,
FLOAT8PASSBYVAL);
+ Node *int_1 = (Node *) makeConst(INT8OID, -1, InvalidOid,
sizeof(int64),
+ Int64GetDatum(1), false,
FLOAT8PASSBYVAL);
Review Comment:
`makeConst()` is creating `INT8OID` constants but uses `FLOAT8PASSBYVAL` for
the `constbyval` flag. This should use the byval macro appropriate for int8
(e.g., `INT8PASSBYVAL`, or derive it via type metadata) to avoid incorrect
Datum handling on platforms where pass-by-value differs.
```suggestion
Int64GetDatum(2), false,
INT8PASSBYVAL);
Node *int_1 = (Node *) makeConst(INT8OID, -1, InvalidOid,
sizeof(int64),
Int64GetDatum(1), false,
INT8PASSBYVAL);
```
##########
src/backend/parser/cypher_clause.c:
##########
@@ -1676,25 +1676,71 @@ static Node *make_bool_or_agg(ParseState *pstate, Node
*arg)
return (Node *) agg;
}
+/*
+ * Helper: build a fully-transformed `count(*) FILTER (WHERE filter)` Aggref.
+ *
+ * The filter must already be a transformed boolean expression.
+ */
+static Node *make_count_star_filter_agg(ParseState *pstate, Node *filter)
+{
+ Aggref *agg;
+ Oid count_oid;
+
+ /* count() -- the zero-argument count-star form */
+ count_oid = LookupFuncName(list_make1(makeString("count")),
+ 0, NULL, false);
+
+ agg = makeNode(Aggref);
+ agg->aggfnoid = count_oid;
+ agg->aggtype = INT8OID;
Review Comment:
`make_count_star_filter_agg` hard-codes the aggregate return type as
`INT8OID`. While `count(*)` returns bigint in standard PostgreSQL, it’s more
robust to derive the return type from `count_oid` (as the parser/transform
layer typically does) to avoid subtle issues if function resolution changes or
if this helper is reused for other aggregates in the future.
##########
src/backend/parser/cypher_clause.c:
##########
@@ -1676,25 +1676,71 @@ static Node *make_bool_or_agg(ParseState *pstate, Node
*arg)
return (Node *) agg;
}
+/*
+ * Helper: build a fully-transformed `count(*) FILTER (WHERE filter)` Aggref.
+ *
+ * The filter must already be a transformed boolean expression.
+ */
+static Node *make_count_star_filter_agg(ParseState *pstate, Node *filter)
+{
+ Aggref *agg;
+ Oid count_oid;
+
+ /* count() -- the zero-argument count-star form */
+ count_oid = LookupFuncName(list_make1(makeString("count")),
+ 0, NULL, false);
+
+ agg = makeNode(Aggref);
+ agg->aggfnoid = count_oid;
+ agg->aggtype = INT8OID;
+ agg->aggcollid = InvalidOid;
+ agg->inputcollid = InvalidOid;
+ agg->aggtranstype = InvalidOid; /* filled by planner */
+ agg->aggargtypes = NIL;
+ agg->aggdirectargs = NIL;
+ agg->args = NIL;
+ agg->aggorder = NIL;
+ agg->aggdistinct = NIL;
+ agg->aggfilter = (Expr *) filter;
+ agg->aggstar = true;
+ agg->aggvariadic = false;
+ agg->aggkind = AGGKIND_NORMAL;
+ agg->aggpresorted = false;
+ agg->agglevelsup = 0;
+ agg->aggsplit = AGGSPLIT_SIMPLE;
+ agg->aggno = -1;
+ agg->aggtransno = -1;
+ agg->location = -1;
+
+ pstate->p_hasAggs = true;
+
+ return (Node *) agg;
+}
+
/*
* Helper: build a transformed CASE expression implementing three-valued
- * predicate logic for all(), any(), and none().
+ * predicate logic for all(), any(), none(), and single().
*
- * any(): CASE WHEN bool_or(pred IS TRUE) THEN true
- * WHEN bool_or(pred IS NULL) THEN NULL
- * ELSE false END
+ * any(): CASE WHEN bool_or(pred IS TRUE) THEN true
+ * WHEN bool_or(pred IS NULL) THEN NULL
+ * ELSE false END
*
- * all(): CASE WHEN bool_or(pred IS FALSE) THEN false
- * WHEN bool_or(pred IS NULL) THEN NULL
- * ELSE true END
+ * all(): CASE WHEN bool_or(pred IS FALSE) THEN false
+ * WHEN bool_or(pred IS NULL) THEN NULL
+ * ELSE true END
*
- * none(): CASE WHEN bool_or(pred IS TRUE) THEN false
- * WHEN bool_or(pred IS NULL) THEN NULL
- * ELSE true END
+ * none(): CASE WHEN bool_or(pred IS TRUE) THEN false
+ * WHEN bool_or(pred IS NULL) THEN NULL
+ * ELSE true END
*
- * Empty list: both bool_or calls return NULL (no rows), so the CASE
- * falls through to the default: false for any(), true for all()/none().
- * This matches Cypher's vacuous truth semantics.
+ * single(): CASE WHEN count(*) FILTER (WHERE pred IS TRUE) >= 2 THEN false
+ * WHEN bool_or(pred IS NULL) THEN NULL
+ * WHEN count(*) FILTER (WHERE pred IS TRUE) = 1 THEN true
+ * ELSE false END
+ *
+ * Empty list: all aggregates return NULL on zero rows, so the CASE
+ * falls through to the default: false for any()/single(), true for
+ * all()/none(). This matches Cypher's vacuous truth semantics.
Review Comment:
This comment is inaccurate for `single()`: `count(*)` (including `count(*)
FILTER (...)`) returns `0` on zero rows, not `NULL`. The logic still appears to
produce the intended default, but the comment should be corrected (e.g., note
that `bool_or(...)` yields `NULL` on zero rows while `count(*)` yields `0`).
```suggestion
* Empty list: bool_or(...) returns NULL on zero rows, while count(*)
* (including count(*) FILTER (...)) returns 0, so the CASE still falls
* through to the default: false for any()/single(), true for all()/none().
* This matches Cypher's vacuous truth semantics.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
