[DO NOT MERGE] Fix string literal aliases in aggregate queries returning null on Linux + add cross-platform CI#68
Draft
McNultyyy wants to merge 5 commits into
Draft
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>
McNultyyy
force-pushed
the
mcnultyyy/fix-string-literal-aliases-linux
branch
from
8fb346e to
2313249
Compare
McNultyyy
changed the title
McNultyyy
changed the base branch from
mcnultyyy/fix-count-ternary-parenthesization
to
main
String literal aliases (e.g. SELECT 'Settlement' AS Label, COUNT(1) ...) returned null on Linux because ProjectAggregateFields treated them as property paths via SelectToken. On Windows the bug was masked by ServiceInterop's native aggregate pipeline. The fix checks for non-identifier SqlExpr nodes in the else branch and evaluates them via EvaluateSqlExpression, matching the existing pattern used in the GROUP BY aggregate path. Covers string, numeric, boolean, and null literal aliases with integration tests. Bumps version to 4.0.19. Closes #67 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
McNultyyy
force-pushed
the
mcnultyyy/fix-string-literal-aliases-linux
branch
from
2313249 to
520abfb
Compare
…gences ServiceInterop.dll is only available on Windows, causing fundamentally different code paths to execute. Adding a windows-latest integration test job ensures both paths are tested on every PR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
McNultyyy
changed the title
McNultyyy
changed the title
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
1 participant
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 #67 — String literal aliases in aggregate queries (e.g.
SELECT 'Settlement' AS Label, COUNT(1) AS ItemCount FROM c) returnednullon Linux where ServiceInterop.dll is unavailable.Root Cause
Two-part issue:
ProjectAggregateFields(InMemoryContainer.cs): Non-aggregate SELECT fields were unconditionally treated as property paths. For'Settlement' AS Label, this calledjObj.SelectToken("'Settlement'")which returns null. Fixed by evaluating non-identifierSqlExprnodes (literals, function calls) viaEvaluateSqlExpression.DefaultQueryPlanStrategy: The multi-aggregate bypass only triggered when there were 2+ aggregates. ForSELECT 42 AS X, COUNT(1) AS N(1 aggregate + 1 literal), the SDK's aggregate pipeline wasn't bypassed and crashed with"Underlying object does not have an 'payload' field". Fixed by adding aisLiteralWithAggregateBypasscondition.Why Windows vs Linux
DefaultQueryPlanStrategy→ both issues surfaceChanges
src/CosmosDB.InMemoryEmulator/InMemoryContainer.cs— Evaluate literal/expression fields inProjectAggregateFieldssrc/CosmosDB.InMemoryEmulator/DefaultQueryPlanStrategy.cs— Bypass SDK aggregate pipeline when literals appear alongside aggregatestests/.../Issue67StringLiteralAliasTests.cs— 5 integration tests (string, numeric, boolean, null literals + multi-query).github/workflows/_build-and-test.yml— Addintegration-windowsjob + rename existing job tointegration-linuxsrc/Directory.Build.props— Version bump to 4.0.19CI Improvement
This bug was platform-specific and only caught when the release workflow ran on Linux. To prevent this class of bug in future, integration tests now run on both
ubuntu-latestandwindows-latestin CI, ensuring both ServiceInterop and non-ServiceInterop code paths are exercised on every PR.Testing
v4.0.19-beta.1published and verified