[DO NOT MERGE] Fix COUNT(expr) with nested ternary expressions miscounting documents (#64)#65
Draft
McNultyyy wants to merge 2 commits into
Draft
[DO NOT MERGE] Fix COUNT(expr) with nested ternary expressions miscounting documents (#64)#65McNultyyy wants to merge 2 commits into
McNultyyy wants to merge 2 commits into
Conversation
… 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>
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>
b96b92d to
2ea4370
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #64 —
COUNT(expr > 0 ? 1 : undefined)incorrectly counts documents where the expression evaluates toundefinedwhen nested ternary expressions are involved.Root Cause
ExprToStringinCosmosSqlParserdid not parenthesiseTernaryExpressionorCoalesceExpressionnodes when they appeared as operands of higher-precedence operators (binary, unary, BETWEEN, IN, LIKE).When the SDK transforms a query like:
The handler round-trips it through
SimplifySdkQuery→ExprToString→ re-parse. Without parentheses, the re-parsed AST has completely different semantics:IS_DEFINED(c.creditValue) ? c.creditValue.amount : (c.grossValue.amount > 0 ? 1 : undefined)(IS_DEFINED(c.creditValue) ? c.creditValue.amount : c.grossValue.amount) > 0 ? 1 : undefinedFix
Added
WrapIfLowPrecedencehelper 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:
Version
Bumped to 4.0.18.