SqlServer does not read DbNull for null value in rowversion column

[smitpatel]

As @ErikEJ found #21269 (comment)

OK, rowversion is never null, and always gets a value assigned by the database engine: A nonnullable rowversion column is semantically equivalent to a binary(8) column. A nullable rowversion column is semantically equivalent to a varbinary(8) column.

In our shaper when reading value of the column solely,
we generate checks but all checks are bypassed and we throw IndexOutOfRangeException.

dataReader.IsDBNull(0) ? default(Nullable<ulong>) : (Nullable<ulong>)dataReader.GetFieldValue<byte[]>(0) == null ? 0 : BitConverter.ToUInt64(
        value: NumberToBytesConverter<ulong>.ReverseLong(dataReader.GetFieldValue<byte[]>(0)), 
        startIndex: 0);

Method body of ReverseLong which generates the error

        private static byte[] ReverseLong(byte[] bytes)
            => new[] { bytes[7], bytes[6], bytes[5], bytes[4], bytes[3], bytes[2], bytes[1], bytes[0] };

May be we need to add customized converter for SqlServer in this case.

[smitpatel]

Added a disabled regression test in codebase

[bdebaere]

As I discovered by me in my original issue, rowversion cannot be NULL in Sql Server, NULL is represented by 0x. This means that the IsDBNull check which is used by EntityFrameworkCore is not enough for rowversion columns.

The bug is the following. In the BufferedDataReader.BufferedDataRecord.ReadRow method the following check is performed if (!(_tempNulls[_currentRowNumber * _nullCount + nullIndex] = _underlyingReader.IsDBNull(i))). For a reason unknown to me IsDBNull always returns false when the data type is timestamp (rowversion), see my StackOverflow post about it at https://stackoverflow.com/questions/62410830/timestamp-isdbnull.
What this means is that, in the case of timestamp its value will always be Array.Empty, see screenshot below.

Unless there's something obvious I'm missing here which would render this all wrong, in which case I apologize in advance, these are the paths I currently see:

1. Extend the IsDBNull check with a type check on rowversion, could be based on EntityFrameworkCore metadata or DbDataReader.GetDataTypeName (the last one seems rather dirty);
2. Add a value converter specifically for rowversion which can deal with empty arrays instead of the default NumberToBytesConverter;
3. Adjust the NumberToBytesConverter to be able to handle empty arrays, as is done currently.

Do let me know which one of the above paths you wish to take, or if you wish to take a different path, so that I can adjust my pull request.

I'm glad you are finally looking into the direction of a customized converter for Sql Server, as I suggested. Unless you can contact the Sql Server team about this, but due to backwards compatibility they might be afraid to correct it.

[smitpatel]

@bdebaere - Neither of the aforementioned options are right solution.

[bdebaere]

@smitpatel

May be we need to add customized converter for SqlServer in this case.

is equal to

Add a value converter specifically for rowversion which can deal with empty arrays instead of the default NumberToBytesConverter;