[Issue #75] fix: normalise whole-number JSON floats to integers on round-trip - #76
Closed
McNultyyy wants to merge 17 commits into
Closed
[Issue #75] fix: normalise whole-number JSON floats to integers on round-trip#76McNultyyy wants to merge 17 commits into
McNultyyy wants to merge 17 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>
The EmulatorFlaky trait was blanket-excluding 12 tests from all emulator runs, creating a regression gap. The root cause was a single test (ConcurrentReadsOfNonExistent) overwhelming the emulator with 50 parallel requests. Instead of excluding the entire class, make the concurrency adaptive: - 50 concurrent reads for in-memory (fast, no resource constraints) - 10 concurrent reads for emulator targets (same assertion, lower volume) Changes: - Remove [Trait(TestTraits.Target, TestTraits.EmulatorFlaky)] from Issue18EdgeCaseIntegrationTests - Remove EmulatorFlaky constant from TestTraits.cs - Remove Target!=EmulatorFlaky filter from scripts/run-tests.ps1 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Windows emulator can return 404 with substatus 0 during an intermediate startup state (between 403/1008 and fully ready). This was causing the EmulatorWarmup tool and EmulatorRetry to treat it as fatal, failing the parity workflow before tests even ran. Add 404/0 to the transient error set in both: - tests/EmulatorWarmup/Program.cs (warmup tool) - tests/.../EmulatorSession.cs (shared retry helper) 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>
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>
…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>
…it' into mcnultyyy/merge-prs-65-66-68
…ases-linux' into mcnultyyy/merge-prs-65-66-68
) In real Cosmos DB, FilterPredicate on patch operations treats missing properties as null, so 'FROM c WHERE c.prop = null' matches documents where the property is absent (e.g. due to NullValueHandling.Ignore). Add treatUndefinedAsNull parameter to the WHERE evaluation chain, passed as true only from the two FilterPredicate call sites. This preserves existing query semantics (undefined != null) while fixing the FilterPredicate behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Queries without ORDER BY now return documents in insertion order, matching real Cosmos DB behavior. Previously documents were returned in hash-map order due to ConcurrentDictionary enumeration. Added insertion-order tracking list to InMemoryContainer that maintains document position across create, replace, upsert, and delete operations. GetAllItemsForPartition() now iterates this ordered list instead of the dictionary directly. Fixes #72 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comprehensive edge case tests covering: - Large batch (55 docs) insertion order - Multi-partition interleaved insertion order - Repeated replaces preserving position - Mixed operations sequence (create/delete/replace/upsert) - SELECT projections maintaining order - WHERE filter preserving relative order - TOP and OFFSET/LIMIT pagination order - Empty container and single document edge cases - Delete all + recreate ordering - Upsert-only creates ordering - DISTINCT VALUE preserving first occurrence order - COUNT and SUM aggregates not crashing - Cross-partition vs single-partition order - Rapid sequential creates order - ORDER BY overriding insertion order - Range filter preserving relative order - Multiple non-consecutive deletes preserving remaining order Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…round-trip Adds Issue75DecimalScaleTests covering three bugs: 1. Whole-number decimals (1500m) stored as float 1500.0 instead of integer 1500 2. Fractional decimals with trailing zeros (100.50m) losing trailing zero on round-trip 3. SUM aggregate returning 750.0 on Linux vs 750 on Windows (platform inconsistency) All round-trip tests currently fail. SUM tests pass on Windows but would fail on Linux. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…sue #75) Real Cosmos DB uses a JavaScript engine (IEEE 754 doubles) which normalises whole-number JSON floats to integers on round-trip: - '1500.0' → 1500 (not 1500.0) - '0.0' → 0 (not 0.0) - '100.50' → 100.5 (double strips trailing zero naturally) - SUM(250, 500) → 750 (not platform-dependent '750.0') Changes: - JsonParseHelpers.cs: add NormalizeNumbers() that converts JTokenType.Float values where d == Math.Truncate(d) to JValue(long) after parsing - InMemoryContainer.cs: apply long→double coercion for UDF args (JavaScript has no int/float distinction); fix pre-existing format issues in test files - src/Directory.Build.props: bump version to 4.0.21 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
McNultyyy
marked this pull request as ready for review
May 26, 2026 17:18
McNultyyy
changed the title
McNultyyy
marked this pull request as draft
May 27, 2026 06:44
Update QueryPlanTests and QueryPlanDeepDiveTests to reflect that single-aggregate projection queries (e.g. SELECT SUM(...) AS alias FROM c) now have their aggregates cleared via isSingleAggregateProjectionBypass. This prevents the SDK's AggregateQueryPipelineStage from crashing on Linux with a missing 'payload' field. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
Problem
Real Cosmos DB uses a JavaScript engine (IEEE 754 doubles). This normalises numbers on round-trip in ways our emulator wasn't replicating:
1500m(whole-number decimal)15001500.0❌0m(zero decimal)00.0❌SUM(250, 500)750750(Windows) /750.0(Linux) ❌100.50m100.5100.5✅ (already correct)Root Cause
Newtonsoft.Json's
EnsureDecimalPlace()serialises whole-numberdecimalvalues with a.0suffix (e.g.1500m→"1500.0"). The emulator stored this literal string and returned it, whereas real Cosmos DB normalises all numbers through its JavaScript engine which converts whole-number floats to integers.Fix
JsonParseHelpers.cs— addedNormalizeNumbers(JToken)called after everyJObject.Load/JToken.Load. It traverses the token tree and converts anyJTokenType.Floatvalue whered == Math.Truncate(d)to aJValue(long)(integer token).doublenaturally strips trailing zeros (e.g.100.50 → 100.5), so fractional values need no special handling.InMemoryContainer.cs— at the UDF C# handler call site,longargs are coerced todoublebefore being passed to the user's handler. JavaScript has no int/float distinction (all numbers are IEEE 754 doubles), so UDF handlers may reasonably cast args asdouble. TheNormalizeNumberschange converts stored whole-number floats tolong; this coercion restores the expecteddoubletype at the UDF boundary.The existing
NormalizeNumericResulthelper (applied to SUM/AVG aggregates) already covered Bug 3 and remains in place.Tests
8 integration tests in
Issue75DecimalScaleTests.cscovering:1500m→"1500")0m→"0")100.50m→"100.5")All 8 tests pass (net8 + net10). Integration test suite: 345/345 pass on both targets. No new unit test failures introduced.
Version
Bumped to
4.0.21.