Skip to content

[DO NOT MERGE] Fix COUNT(expr) with nested ternary expressions miscounting documents (#64)#65

Draft
McNultyyy wants to merge 2 commits into
mainfrom
mcnultyyy/fix-count-ternary-parenthesization
Draft

[DO NOT MERGE] Fix COUNT(expr) with nested ternary expressions miscounting documents (#64)#65
McNultyyy wants to merge 2 commits into
mainfrom
mcnultyyy/fix-count-ternary-parenthesization

Conversation

@McNultyyy
Copy link
Copy Markdown
Collaborator

Summary

Fixes #64COUNT(expr > 0 ? 1 : undefined) incorrectly counts documents where the expression evaluates to undefined when nested ternary expressions are involved.

Root Cause

ExprToString in CosmosSqlParser did not parenthesise TernaryExpression or CoalesceExpression nodes when they appeared as operands of higher-precedence operators (binary, unary, BETWEEN, IN, LIKE).

When the SDK transforms a query like:

SELECT VALUE COUNT((IS_DEFINED(c.creditValue) ? c.creditValue.amount : c.grossValue.amount) > 0 ? 1 : undefined) FROM c

The handler round-trips it through SimplifySdkQueryExprToString → re-parse. Without parentheses, the re-parsed AST has completely different semantics:

  • Before fix: IS_DEFINED(c.creditValue) ? c.creditValue.amount : (c.grossValue.amount > 0 ? 1 : undefined)
  • After fix: (IS_DEFINED(c.creditValue) ? c.creditValue.amount : c.grossValue.amount) > 0 ? 1 : undefined

Fix

Added WrapIfLowPrecedence helper method that wraps ternary and coalesce sub-expressions in parentheses when they appear inside operators with higher precedence, ensuring the serialized SQL re-parses to the same AST.

Test Coverage

Added 10 integration tests covering:

  • Simple ternary in SELECT VALUE (condition true/false)
  • Nested ternary in SELECT VALUE (inner resolves, comparison succeeds/fails)
  • Simple ternary in COUNT (condition false → not counted)
  • Mixed documents with COUNT (only matching docs counted)
  • Nested ternary in COUNT — the exact bug scenario (inner resolves to 0, comparison fails → not counted)
  • Nested ternary in COUNT (inner resolves to positive → counted)
  • Fallback path when IS_DEFINED is false
  • Multi-aggregate query mirroring the original issue's pattern

Version

Bumped to 4.0.18.

… round-trip (#64)

ExprToString did not wrap TernaryExpression or CoalesceExpression in
parentheses when they appeared as operands of higher-precedence binary,
unary, BETWEEN, IN, or LIKE operators. When the SDK's transformed query
was round-tripped through SimplifySdkQuery → re-parse, the missing
parentheses produced a different AST — causing COUNT(expr) to evaluate
the wrong condition and miscount documents.

Added WrapIfLowPrecedence helper that wraps ternary/coalesce nodes in
parens when they appear in operator positions that would otherwise
re-parse with different associativity.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@McNultyyy McNultyyy requested a review from lemonlion as a code owner May 18, 2026 22:04
@McNultyyy McNultyyy marked this pull request as draft May 18, 2026 22:07
Documents how to publish beta/prerelease and stable packages via the
existing release.yml workflow (tag push convention) for AI/LLM agent
discoverability.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@McNultyyy McNultyyy force-pushed the mcnultyyy/fix-count-ternary-parenthesization branch from b96b92d to 2ea4370 Compare May 19, 2026 07:40
@McNultyyy McNultyyy changed the title Fix COUNT(expr) with nested ternary expressions miscounting documents (#64) [DO NOT MERGE] Fix COUNT(expr) with nested ternary expressions miscounting documents (#64) May 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: COUNT(expr > 0 ? 1 : undefined) incorrectly counts undefined values

1 participant