[Phase 3] MSSQL JSON - test fixture + REST CRUD tests#3720
Open
souvikghosh04 wants to merge 7 commits into
Open
[Phase 3] MSSQL JSON - test fixture + REST CRUD tests#3720souvikghosh04 wants to merge 7 commits into
souvikghosh04 wants to merge 7 commits into
Conversation
Mirrors the merged vector-type feature (#3677). CI now runs SQL Server 2025 (#3697), so the native json column is created unconditionally - no server-version gating needed. - profiles table (id, metadata json) + 5 seed rows (simple/array/nested/unicode/null) (T002) - Profile entity in dab-config.MsSql.json + config-generator command, REST + GraphQL enabled, anon/authenticated CRUD (T003) - MsSqlRestJsonTypesTests: GET list/by-pk/null/array/nested/unicode, POST, PUT, PATCH, PATCH-to-null, DELETE (T011,T013,T015,T017,T019)
souvikghosh04
marked this pull request as ready for review
July 14, 2026 13:17
souvikghosh04
requested review from
Alekhya-Polavarapu,
Aniruddh25,
JerryNixon,
RubenCerna2079,
aaronburtle,
anushakolan,
rusamant,
sourabh1007,
stuartpa and
vadeveka
as code owners
July 14, 2026 13:17
Contributor
There was a problem hiding this comment.
Pull request overview
Adds SQL Server 2025 native JSON end-to-end coverage to the MSSQL integration test suite by extending the MsSql schema fixture, test config, and adding REST CRUD tests for the new profiles.metadata (json) column. This fits into the existing pattern used for recently-added SQL Server 2025-only datatypes (e.g., vector) to validate behavior through the REST API.
Changes:
- Add
profilestable with ajsoncolumn and seeded rows to the MSSQL test schema. - Add
Profileentity to the MSSQL test config and config-generator command list. - Add REST integration tests covering read + CRUD mutations for JSON values (semantic comparison via JSON parsing).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Service.Tests/SqlTests/RestApiTests/MsSqlRestJsonTypesTests.cs | New REST integration test suite validating JSON column round-tripping (read + CRUD). |
| src/Service.Tests/DatabaseSchema-MsSql.sql | Adds profiles table and seed data for JSON scenarios. |
| src/Service.Tests/dab-config.MsSql.json | Adds Profile entity mapping for REST/GraphQL. |
| config-generators/mssql-commands.txt | Adds generator commands to reproduce the Profile entity config. |
…les seed The vector_type_table block left IDENTITY_INSERT ON (it was previously the last such block). Inserting the profiles seed after it triggered 'IDENTITY_INSERT is already ON for table vector_type_table', aborting schema init for the whole MsSql suite. Turn vector OFF before the profiles ON/OFF block.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
- Align profiles identity seed to IDENTITY(5001, 1) per the schema convention (line 74) - Tighten ParseMetadata to assert metadata is a JSON string (proves the Phase 2 string-contract), no raw-object fallback - GetJsonTypeList now asserts each row metadata is a JSON string or null - Fix PatchJsonType_ToNull doc comment to reference id/2
…snapshot CI on SQL Server 2025 shows a native json column is inlined by DAB's FOR JSON PATH projection as a nested JSON object (like vector columns become native arrays), not an escaped string. DAB applies no special handling - SQL Server inlines it. Correct the read-side assertions accordingly: - ParseMetadata now asserts metadata.ValueKind == Object and returns it directly (no string parse). - GetJsonTypeList asserts each row metadata is a JSON object or null. - Update doc comments to describe the FOR JSON PATH inlining behavior. Also refresh ConfigurationTests.TestReadingRuntimeConfigForMsSql.verified.txt to include the new Profile entity (only delta).
…op key-fields) CI regenerates dab-config.MsSql.json from config-generators/mssql-commands.txt before running tests. The 'add Profile' command specifies no --source.key-fields (profiles is a table, so the PK is inferred from the DB schema, consistent with VectorType and every other table entity). The hand-added key-fields made the committed config - and the verified snapshot - diverge from the generated config, so TestReadingRuntimeConfigForMsSql failed on CI (Source.KeyFields present in verified, absent in the generated config it deserializes). Remove key-fields from the Profile source and refresh the snapshot to match.
aaronburtle
reviewed
Jul 22, 2026
| /// PUT /api/Profile/id/1 - Verify a full update replaces the metadata payload, then restore it. | ||
| /// </summary> | ||
| [TestMethod] | ||
| public async Task PutJsonType_Update() |
Contributor
There was a problem hiding this comment.
PutJsonType_Update, PatchJsonType_Update, and PatchJsonType_ToNull all mutate shared rows, and then restore with a follow-up call. If any assertions fail before the restore, then the rows are corrupted for any tests that run afterwards. Consider wrapping the mutate/assert/restore in a try-finally for safety.
ie:
try
{
// mutate + assert
}
finally
{
// restore original value
}
Contributor
There was a problem hiding this comment.
We have a similar smell in our current vector tests that we should probably fix too, but not in this PR.
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.
What this PR does
Adds the database test fixture and REST integration tests for SQL Server 2025
JSONcolumns. It builds on Phase 2 (which taught the engine to treatJSONas a normalstring) and proves the behavior end-to-end through the REST API.This follows the exact pattern of the recently merged vector data type feature (#3677), and is unblocked by CI now running SQL Server 2025 (#3697).
What's included
profilestable with anidand ametadata jsoncolumn, plus 5 seed rows covering a simple object, an array, a deeply nested object, unicode + emoji, andNULL.Profileentity (REST + GraphQL enabled) in the MsSql test config and config generator.MsSqlRestJsonTypesTests: read (list, by id, null, array, nested, unicode), insert, update (PUT/PATCH), clear-to-null, and delete.Because DAB treats
JSONas a string, the tests parsemetadataand compare it semantically, so they're robust to any whitespace / key-order normalization the engine applies.No gating needed
Unlike the earlier plan, the table is created unconditionally (no server-version guard) — CI runs SQL Server 2025 and the schema already uses the 2025-only
vectortype the same way.Delivery plan
Notes
jsontype).JSONdata type for MSSQL #2768.