SqlDataReader incorrectly returns null ROWVERSION as an empty byte[]

[dbrownems]

When returned in a resultset a null ROWVERSION is returned as an empty byte[], instead of DbNull.Value. This problem does not occur when returning using a parameter value, or using a resultset in ODBC (so not a TDS issue).

Here's e a repro:

using System;
using System.Data.Odbc;
using System.Data.SqlClient;
using System.Data.SqlTypes;
using System.Threading;
using System.Threading.Tasks;
using System.Transactions;

namespace ConsoleApp14
{
    class Program
    {

        static void Main(string[] args)
        {

            using (var con = new SqlConnection("Server=localhost;database=tempdb;Integrated Security=true"))
            {
                con.Open();
                var cmd = con.CreateCommand();
                cmd.CommandText = "select cast(null as rowversion) rv";
                using (var rdr = cmd.ExecuteReader())
                {
                    rdr.Read();
                    var allowDbNull = rdr.GetColumnSchema()[0].AllowDBNull;
                    var isNull = rdr.IsDBNull(0);
                    var val = rdr[0];

                    Console.WriteLine($"SqlClient: AllowDbNull {allowDbNull} IsDbNull: {isNull} {val.GetType().Name} {val}");

                }
            }


            using (var con = new SqlConnection("Server=localhost;database=tempdb;Integrated Security=true"))
            {
                con.Open();
                var cmd = con.CreateCommand();
                cmd.CommandText = "select @val = cast(null as rowversion) ";

                var p = cmd.Parameters.Add(new SqlParameter("@val", System.Data.SqlDbType.Timestamp));
                p.Direction = System.Data.ParameterDirection.Output;

                cmd.ExecuteNonQuery();
                {

                   SqlBinary val = (SqlBinary) p.SqlValue;
                   Console.WriteLine($"SqlClient (parameter): IsDbNull: {val.IsNull} {val.GetType().Name} {val}");

                }
            }

            using (var con = new OdbcConnection("Driver={ODBC Driver 17 for SQL Server};Server=localhost;Trusted_Connection=yes"))
            {
                con.Open();
                var cmd = con.CreateCommand();
                cmd.CommandText = "select cast(null as rowversion) rv";
                using (var rdr = cmd.ExecuteReader())
                {
                    rdr.Read();
                    var allowDbNull = rdr.GetSchemaTable().Rows[0]["AllowDBNull"];
                    var isNull = rdr.IsDBNull(0);
                    var val = rdr[0];

                    Console.WriteLine($"ODBC:      AllowDbNull {allowDbNull} IsDbNull: {isNull} {val.GetType().Name} {val}");

                }

            }


        }
    }
}

Expected behavior

SqlClient: AllowDbNull True IsDbNull: True DBNull
SqlClient (parameter): IsDbNull: True SqlBinary Null
ODBC: AllowDbNull True IsDbNull: True DBNull

Actual Behavior

SqlClient: AllowDbNull True IsDbNull: False Byte[] System.Byte[]
SqlClient (parameter): IsDbNull: True SqlBinary Null
ODBC: AllowDbNull True IsDbNull: True DBNull

[davidbaxterbrowne]

Apparently this issue was raised before in .NET Framework but not fixed because it would be a breaking change. Perhaps it can be fixed here, and left as-is in .NET Framework.

See related https://dba.stackexchange.com/questions/250744/why-selecting-rowversion-using-outer-join-never-returns-null/250800

[cheenamalhotra]

Hi @dbrownems @davidbaxterbrowne

With Microsoft.Data.SqlClient, we have some room for flexibility and we also support both .NET Framework and .NET Core.

We may have room for the breaking fix in future (with a planned major bump with other breaking changes combined), only criteria is what's the impact of the issue and how would it affect existing behaviors or if it will cause any regressions.

We can mark this issue as breaking behavior change that needs deep investigation and analyze it in depth when a major version release is planned. Does that sound fair?

[davidbaxterbrowne]

It does to me. This is a very uncommon scenario, as a ROWVERSION column in a table is never actually NULL. It's automatically populated on INSERT and can't be directly updated. The only scenario where you have a Null ROWVERSION is in a query result like in the attached SO issue.

[saurabh500]

If we end up "fixing" this, then this should be done with an app context switch, so that an application that is migrating to Microsoft.Data.SqlClient on netfx, can switch to the old behavior, in case it is not possible to change the check for the migrating application.

[bdebaere]

Related:
https://stackoverflow.com/questions/62410830/rowversion-isdbnull
dotnet/efcore#12518
dotnet/efcore#24446
dotnet/efcore#22256

[Wraith2]

I was just working on fixing this.

Question, should the AppContext switch default to the new behaviour (to return SqlBinary.Null) or the old behaviour? 3.0.0 will contain other breaking changes so I think it might be good to default to the new behaviour and have the switch return it to the old behaviour. Is that ok?

[dbrownems]

I think this behavior is simply wrong, and should be changed by default.

[Wraith2]

The changes would make it:

SqlClient: AllowDbNull True IsDbNull: True DBNull
SqlClient (parameter): IsDbNull: True SqlBinary Null

Trying to get SqlBinary Null on the reader test would be very difficult because it doesn't work in the same way as the parameter does. Anything which is null through the reader is going to return you a DBNull rather than the specific type.

[mburbea]

I'm not sure I follow this. As if I do the following I get back a SqlBinary with the IsNull property true. (haven't tried your branch but I assume that it behave as if I casted the timestamp to binary(8))

using var cmd = new SqlCommand(@"select convert(binary(8),convert(timestamp, null))", conn);
using var reader = await cmd.ExecuteReaderAsync();
await reader.Read();
var type = reader.GetFieldType(0);
var value = reader.GetSqlValue(0);

type is a byte[], and value is indeed a SqlBinary with the IsNull property set to true.

[Wraith2]

I assume that it behave as if I casted the timestamp to binary(8))

Yes. Except you won't have to know to do that anymore which is the fix.

[rclabo]

I'm unclear what happened to this?
It seems a much bigger issue then the discussion here indicates. For example in .Net Core 6 consider the following:

Select a.fieldOne, a.fieldTwo, b.fieldOne, b.rowVersion from tableA a left join tableB b on a.fieldOne = b.fieldOne where a.fieldOne = 'HelloWorld';

In the above code if tableA.fieldOne contain a value for 'HelloWorld' but tableB.fieldOne does not, then we expect to get back the fieldOne and fieldTwo from tableA and we expect to get back null and null for b.fieldOne and b.rowVersion. But currently b.rowVersion DOES NOT return null it returns 0x. If one tries to use reader.GetSqlBinary(fieldIndex); to read that value for the rowVersion it will throw with a System.NullReferenceException. That's totally not cool.

[Wraith2]

The issue was closed by the changes in #998 and the behaviour changed. If you want the old behaviour you can use the compatibility context switch Switch.Microsoft.Data.SqlClient.LegacyRowVersionNullBehavior to return to the old way.

Are you saying that the change as discussed and approved/merged isn't not correct? or that you don't like the way the change works?