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?

[rclabo]

I appreciate your work on this. I want the new (ie correct) behavior but am not currently experiencing it. I'm running Microsoft.Data.SqlClient 4.1 which must be before the fix. Which version contains the fix? Is there a NuGet package for it that I can use?

[Wraith2]

It should be in all current versions, this is quite an old issue.

Can you make your test sql into a small standalone c# test program and open a new issue with it please? that way it will get tracked correctly and be easier to reproduce and possibly fix.

[dbrownems]

I just tested with the repro at the top of this issue. Using Microsoft.Data.SqlClient 4.1 the behavior is correct, the ROWVERSION is returned as Null. Using System.Data.SqlClient 4.8.3, the old behavior is the same, the ROWVERSION is returned as an empty byte[].

[rclabo]

I'd probably need some help writing this up sufficiently and perhaps I'm barking up the wrong tree here but the issue I'm encountering smells very similar to the issue reported here. More specifically, instead of getting back a null as I expect for the joined rowVersion I'm getting back an empty byte array instead. This issue appears to run as deep as SqlServer itself, but it's reader.GetSqlBinary(fieldIndex) that blows up because apparently it's not expecting to get a byte array with nothing in it.

create database rowVersionTest;
use rowVersionTest;

CREATE TABLE [dbo].[tableA](
	[id] [int] IDENTITY(1000,1) NOT NULL,
	[name] [varchar](50) COLLATE SQL_Latin1_General_CP1_CI_AS NULL,
	[rowVersion] [timestamp] NOT NULL
)
GO

CREATE TABLE [dbo].[tableB](
	[id] [int] IDENTITY(1000,1) NOT NULL,
	[tableAId] [int] NULL,
	[name] [varchar](50) COLLATE SQL_Latin1_General_CP1_CI_AS NULL,
	[rowVersion] [timestamp] NOT NULL
)
GO

insert into tableA (name) values ('Ron');
insert into tableA (name) values ('James');
insert into tableA (name) values ('Jonathan');

insert into tableB (name, tableAId ) values ('Toyota', 1000);
insert into tableB (name, tableAId) values ('Toyota', 1002);

select a.id, a.name, a.rowVersion, b.id as b_Id, b.name as b_name, b.rowVersion as b_rowVersion
from tableA a
left join tableB b on a.id = b.tableAId;

Running the above code produces these results:

Notice in the results above that row 2 has a b_rowVersion value of 0x rather then null . I realize that this isn't likely to be fixed at the SqlServer level so that null could be returned but if I use SqlDataReader to read in that value via the GetSqlBinary method it sure shouldn't throw an Exception, but in Microsoft.Data.SqlClient 4.1 it does in fact throw and Exception.

[Wraith2]

That is a slightly weird behaviour. That value is null as far as sql is concerned, if you try to datalength or null check it you'll get consistent behaviour for a null value. You should not get an exception when asking for the value through GetSqlBinary

Sql Management Studio uses this library so a change here would also affect it. It may be that management studio isn't prepared for the SqlBinary.Null value and doesn't check for IsNull instead just asks for the length and formats it. That would explain the display weirdness there. @DavoudEshtehari is there anyone on the management studio team we could ping on that?

[dbrownems]

BTW select cast(null as rowversion) rv will repro this.

[ErikEJ]

SSMS uses System.Data.SqlClient.

(Version 19 in preview uses M.D.S.)

[Wraith2]

BTW select cast(null as rowversion) rv will repro this.

the SqlBinary crash or the 0x in management studio?

[rclabo]

That is a slightly weird behavior.

Exactly. And I think you are being slightly kind :-)

It may be that management studio isn't prepared for the SqlBinary.Null value and doesn't check for IsNull instead just asks for the length and formats it.

Totally agree.

Here another interesting thing about this case. Even though GetSqlBinary erroneously throws when reading b_rowVersion, using GetSqlValue to read it DOES return a null object. It's this later approach that I resorted to using as a work around. But clearly GetSqlBinary should return SqlBinary.Null.

[rclabo]

select cast(null as rowversion) rv

Wow that's impressive that you were able to boil it down to that. Very cool. Now just try to read that in with GetSqlBinary. It does in fact throw I just tried it.

[Wraith2]

Opened #1684 to track it.