fix(cypher): ignore unbound OPTIONAL MATCH values in COUNT - #2241

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

• It will not be closed for inactivity. No stale bot touches pull requests here.
• It may still sit a while before a human reads it. That is on us, not on you.
• Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

• Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
• Get CI green, or say which failures you believe are pre-existing.
• Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
• Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

Merged — thank you. This is the correct semantics, and the test is the kind we can rely on.

COUNT(t) over an OPTIONAL MATCH that found nothing must be 0, not 1: the row exists so the outer MATCH can still report it, but there is no t to count. Skipping the increment when the binding has no value is exactly right, and putting the guard next to the aggregate accumulation keeps it where the next reader will find it.

I checked that your test can fail rather than taking the green suite at face value — with src/cypher/cypher.c reverted and your test left in place:

tests/test_cypher.c:1762: "1" != "0"

That matters here: this repository has shipped tests that passed with the fix removed, so a passing suite is not by itself evidence. Yours earns it.

One thing you may want to look at next, entirely optional. The guard is scoped to COUNT, but the same unbound value reaches the other aggregates through the shared path just below it. For SUM the damage is invisible — strtod on an absent value yields 0.0 and adding zero is harmless. For AVG it is not: entry->counts[ci] is still incremented for the unbound row, so the denominator grows while the numerator does not, and the average comes back too low. MIN/MAX will likewise fold a spurious 0.0 into the comparison.

If you would like to extend this to the other aggregates, I would happily review it — and if you would rather stop here, that is a complete fix for the bug you set out to fix, and I will open a follow-up myself with credit to you for finding the shape of it.