Adding the SqlJson type #2734
Adding the SqlJson type #2734
Conversation
Remove unused Adding all the json tests Revert "Remove unused" This reverts commit c448b6e. Remove unused
|
This PR cannot be merged, since the tests which work with package reference would fail to compile. |
| { | ||
| get | ||
| { | ||
| if (IsNull) | ||
| { | ||
| throw new SqlNullValueException(); | ||
| } | ||
| else | ||
| { | ||
| return _jsonString!; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
nit: if you want to start using more modern C# code, this can be a simple expression-bodied method with a conditional operator:
| { | |
| get | |
| { | |
| if (IsNull) | |
| { | |
| throw new SqlNullValueException(); | |
| } | |
| else | |
| { | |
| return _jsonString!; | |
| } | |
| } | |
| } | |
| => IsNull ? throw new SqlNullValueException() : _jsonString!; |
| _isNull = true; | ||
| } | ||
|
|
||
| private static void ValidateJson(string jsonString) |
There was a problem hiding this comment.
Is this additional parsing of the entire document necessary (with the associated perf overhead)? I'm assuming that SQL Server would throw anyway on any bad input etc...
|
Do we still need this PR? |
We have already merged the SqlJson class in a separate PR. I think this one can be dropped |
|
[like] Apoorv Deshmukh reacted to your message:
…________________________________
From: Deepak Saini ***@***.***>
Sent: Sunday, October 6, 2024 7:22:39 PM
To: dotnet/SqlClient ***@***.***>
Cc: Apoorv Deshmukh ***@***.***>; Mention ***@***.***>
Subject: Re: [dotnet/SqlClient] Adding the SqlJson type (PR #2734)
Do we still need this PR? @saurabh500<https://github.com/saurabh500> @apoorvdeshmukh<https://github.com/apoorvdeshmukh> @deepaksa1<https://github.com/deepaksa1>
We have already merged the SqlJson class in a separate PR. I think this one can be dropped
—
Reply to this email directly, view it on GitHub<#2734 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABMWK35HRWASWTELA3J6Z3LZ2GEX7AVCNFSM6AAAAABLXBY5TKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOJVGU2TENRQGE>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Fixes #2665
Brings in the SqlJson implementation along with the tests.
The ref will be updated #2670 closer to the release.