dotnet / SqlClient

Microsoft.Data.SqlClient provides database connectivity to SQL Server for .NET applications.
MIT License
836 stars 278 forks source link

API Proposal: SqlJson class design to support JSON datatype #2665

Open apoorvdeshmukh opened 1 month ago

apoorvdeshmukh commented 1 month ago

Background: As part of enhancing support in SqlClient for JSON datatype for Sql Server as mentioned in #2622, SqlDbType needed to be enhanced with enum called Json and introduction of a SqlType class for JSON type was needed. An enum called Json with value 35 in SqlDbType was recently added in the runtime repo through issue #103925 The next follow up item is to provide a SqlType class to work with the JSON data. This proposal talks about the design of SqlJson class which aims to represent the JSON data stored in or retrieved from a server.

API Proposal

using System;
using System.Data.SqlTypes;
using System.Text.Json;

namespace Microsoft.Data.SqlTypes
{
    /// <summary>
    /// Represents the Json Data type in SQL Server.
    /// </summary>
    public class SqlJson : INullable
    {
        /// <summary>
        /// Parameterless constructor. Initializes a new instance of the SqlJson class which 
        /// represents a null JSON value.
        /// </summary>
        public SqlJson() { }

        /// <summary>
        /// Takes a <see cref="string"/> as input and initializes a new instance of the SqlJson class.
        /// </summary>
        /// <param name="jsonString"></param>
        public SqlJson(string jsonString) { }

        /// <summary>
        /// Takes a <see cref="JsonDocument"/> as input and initializes a new instance of the SqlJson class.
        /// </summary>
        /// <param name="jsonDoc"></param>
        public SqlJson(JsonDocument jsonDoc) { }

        /// <inheritdoc/>
        public bool IsNull => throw null;

        /// <summary>
        /// Represents a null instance of the <see cref="SqlJson"/> type.
        /// </summary>
        public static SqlJson Null { get { throw null; } }

        /// <summary>
        /// Gets the string representation of the Json content of this <see cref="SqlJson" /> instance.
        /// </summary>
        public string Value { get ; }
    }
}

Additionally, SqlDataReader will be enhanced with addition of API for returning SqlJson type

        /// <summary>
        /// Gets the value of the specified column as JSON value.
        /// </summary>
        virtual public SqlJson GetSqlJson(int i)
apoorvdeshmukh commented 1 month ago

CC: @saurabh500 @David-Engel @roji @uc-msft @deepaksa1

ErikEJ commented 1 month ago

Assume this will be .NET 8+ only, or??

apoorvdeshmukh commented 1 month ago

Assume this will be .NET 8+ only, or??

We will support netfx and all the runtime versions.

roji commented 1 month ago

.NET 9 and above

Just pointing out that SqlDbType.Json isn't necessary for making read work, and and should also not be necessary for making write work with JsonDocument. Even for writing strings as JSON, there's the possibility of the user hardcodIng the numeric enum value - this is ugly and hacky, but it would allow people to use JSON without forcing them to upgrade to .net 9. So I'm just wondering if the plan is to only enable JSON support for 9.

ErikEJ commented 1 month ago

@roji Reply was edited 🥳

saurabh500 commented 1 month ago

The namespace should be Microsoft.Data.SqlTypes

saurabh500 commented 1 month ago

@David-Engel what is the guidance about the docs for public APIs? Do we create an XML file like it exists for other public APIs today? Also, I see that the docs when I hover over in VS, is broken. The related issue is here. https://github.com/dotnet/SqlClient/issues/2013 So for the docs of this new type what should be done ? Do you create the docs for this in a more compliant way (not sure what the technical term is)

JRahnama commented 1 month ago

@saurabh500 I have been working on converting the docs to a more modern style to address issue #2013. However, this is a big job, and it will take some more time for me to complete as I am doing it alongside my scheduled tasks. When it is done, I need to request a sample published test from the documentation team to ensure everything works as expected, so that will take some time.

So far, I have noticed a few issues with the documentation contents inside the CDATA block, which have not been marked by the compiler as it reads them as a text block.

So, I think for now we need to continue the old style as the job needs to be done in a separate PR, but I will wait for @David-Engel 's comment on this.

saurabh500 commented 1 month ago

Thanks @JRahnama for the inputs.

JRahnama commented 1 month ago

So for the docs of this new type what should be done ? Do you create the docs for this in a more compliant way (not sure what the technical term is)

To add more on this, I have found out below:

  1. all tables and lists needs to be converted to (this is in progress)
  2. All CDATA need to be converted to sections (this part is done in my work).
  3. all xrefs needs to be converted to , this will need more consideration as it does not accept %2A at the end of the link and each overload needs to be specified. For example <see cref="T:Microsoft.Data.SqlClient.SqlCommand()" /> will link to default constructor and <see cref=" Microsoft.Data.SqlClient.SqlCommand(string, SqlConnection)" /> will link to other constructor.
  4. For Note and Caution sections I need to test to see if we can use SandCastle xml

this is how it will look after changes are done: image

image

Wraith2 commented 1 month ago

We should have a stream constructor so that people don't have to go through string to load data.

saurabh500 commented 1 month ago

@Wraith2 For the SqlTypes i.e. SqlJson we wont be adding streaming support.

This is because these types need to validate the datatype before sending it out to the server. For JSON, the stream will need to be parsed till the end to validate if the JSON is valid, and will diminish the value of stream on SqlJson

However streaming is needed with SqlParameter.Value when SqlDbType=Json in accordance with this article https://learn.microsoft.com/en-us/dotnet/framework/data/adonet/sqlclient-streaming-support

Also this will be interesting when we use SqlBulkCopy to stream Json data from one Sql server to another.