Skip to content

Fix SqlDataReader.IsDBNull() for json#2830

Merged
deepaksa1 merged 3 commits intodotnet:feat-jsonsupportfrom
deepaksa1:dev/jsonNullFix
Sep 4, 2024
Merged

Fix SqlDataReader.IsDBNull() for json#2830
deepaksa1 merged 3 commits intodotnet:feat-jsonsupportfrom
deepaksa1:dev/jsonNullFix

Conversation

@deepaksa1
Copy link
Copy Markdown
Contributor

This PR aims to fix SqlDataReader.IsDBNull() API for json data type.
Currently, it returns false for a null json column. Adding the case for json type returns expected output.

@ErikEJ
Copy link
Copy Markdown
Contributor

ErikEJ commented Sep 2, 2024

Possible to add a test?

Copy link
Copy Markdown
Contributor

@apoorvdeshmukh apoorvdeshmukh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Please add a testcase that fails without your fix.

Comment thread src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/JsonTest/JsonTest.cs Outdated
Copy link
Copy Markdown
Contributor

@apoorvdeshmukh apoorvdeshmukh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested few minor changes.

Comment thread src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/JsonTest/JsonTest.cs Outdated
@deepaksa1 deepaksa1 merged commit f5994c8 into dotnet:feat-jsonsupport Sep 4, 2024
@deepaksa1 deepaksa1 deleted the dev/jsonNullFix branch September 4, 2024 09:46
deepaksa1 added a commit to deepaksa1/SqlClient that referenced this pull request Oct 1, 2024
* fix SqlDataReader.IsDBNull() for json

* add testcase

* resolve PR commits
@mdaigle mdaigle added this to the 6.0-preview2 milestone Oct 24, 2024
@David-Engel David-Engel added the Area\Json Use this for issues that are targeted for the Json feature in the driver. label Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area\Json Use this for issues that are targeted for the Json feature in the driver.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants