NumberToBytesConverter on ulong RowVersion property fails with .Include query

[SimonCropp]

Sorry about the detail. but this has a few moving pieces.

(TLDR run this test in the repro)

Scenario

• using a Timestamp/row version
• rowversion is mapped to a ulong property with a NumberToBytesConverter .HasConversion(new NumberToBytesConverter<ulong>()).
• Models

public class Parent
{
    public Guid Id { get; set; } = Guid.NewGuid();
    public Guid? ChildId { get; set; }
    public Child Child { get; set; }
}
public class Child
{
    public Guid Id { get; set; } = Guid.NewGuid();
    public ulong ULongRowVersion { get; set; }
    public Guid ParentId { get; set; }
    public Parent Parent { get; set; }
}

• a single One To One Relationship. and map ULongRowVersion to a rowversion

var child = modelBuilder.Entity<Child>();
child.HasOne(_ => _.Parent)
   .WithOne(_ => _.Child)
   .HasForeignKey<Parent>(_ => _.ChildId);
child.Property(x => x.ULongRowVersion)
   .HasConversion(new NumberToBytesConverter<ulong>())
   .IsRowVersion()
   .IsRequired()
   .HasColumnType("RowVersion");

modelBuilder.Entity<Parent>();

• Data: single parent, no child

var parent = new Parent();
context.Add(parent);
context.SaveChanges();

• execute a query FirstOrDefault query on Parent and .Include the Child property

var firstOrDefault = context.Parents
       .Include(x => x.Child)
       .FirstOrDefault();

Outcome

InvalidOperationException in NumberToBytesConverter.ReverseLong.

System.InvalidOperationException : An exception occurred while reading a database value for property 'Child.ULongRowVersion'. See the inner exception for more information.
---- System.IndexOutOfRangeException : Index was outside the bounds of the array.
   at Microsoft.EntityFrameworkCore.Metadata.Internal.EntityMaterializerSource.ThrowReadValueException[TValue](Exception exception, Object value, IPropertyBase property)
   at lambda_method(Closure , DbDataReader )
   at Microsoft.EntityFrameworkCore.Storage.Internal.TypedRelationalValueBufferFactory.Create(DbDataReader dataReader)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryingEnumerable`1.Enumerator.BufferlessMoveNext(DbContext _, Boolean buffer)
   at Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal.SqlServerExecutionStrategy.Execute[TState,TResult](TState state, Func`3 operation, Func`3 verifySucceeded)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryingEnumerable`1.Enumerator.MoveNext()
   at System.Linq.Enumerable.SelectEnumerableIterator`2.MoveNext()
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqOperatorProvider.<_TrackEntities>d__17`2.MoveNext()
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqOperatorProvider.ExceptionInterceptor`1.EnumeratorExceptionInterceptor.MoveNext()
   at System.Collections.Generic.List`1.AddEnumerable(IEnumerable`1 enumerable)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at Tests.ULongRowVersion() in C:\Code\EfRowVersionRepro\DataModel\Tests.cs:line 46
----- Inner Stack Trace -----
   at Microsoft.EntityFrameworkCore.Storage.ValueConversion.NumberToBytesConverter`1.ReverseLong(Byte[] bytes)
   at lambda_method(Closure , DbDataReader )

The strange thing is, that with my understanding of the above scenario, NumberToBytesConverter should never be called since there are no rows in that table.

Repro

https://github.com/SimonCropp/EfRowVersionRepro

The above failing scenario is in the ULongRowVersionContext and can be executed in the Tests.ULongRowVersion() method.

There are also two other working scenarios.

Using BytesRowVersion

This scenario still uses a ulong for the row version, but with a custom. ULongRowVersionWithWorkAroundContext and can be executed in the Tests.ULongRowVersionWithWorkAround() method.

ULongToBytesConverter is a clone of NumberToBytesConverter. When debugging the exception was occurring in the code generated by the custom expressions, so it made it very difficult to understand what the problem was. When i initially created ULongToBytesConverter i got the same exception as with NumberToBytesConverter. But since i was able to debug i was able to see where the error was occurring and add a workaround.

static ulong ToNumber(byte[] bytes)
{
    if (bytes == null)
    {
        return 0;
    }
    #region check added to avoid exception
    if (bytes.Length == 0)
    {
        return 0;
    }
    #endregion

    if (BitConverter.IsLittleEndian)
    {
        bytes = ReverseLong(bytes);
    }

    return BitConverter.ToUInt64(bytes, 0);
}

Note that i have no idea why this works ;)

Further technical details

• Microsoft.EntityFrameworkCore.SqlServer 2.1.1
• Microsoft.EntityFrameworkCore 2.1.1
• Microsoft SQL Server Express (64-bit) 14.0.1000.169
• Windows Server 2012 R2 Datacenter (6.3)
• TargetFramework: netcoreapp2.0
• Visual Studio 2017 15.7.4)

[ajcvickers]

Notes for triage: NumberToBytesConverter is being called from query with a zero-length byte array, which should never happen even if there are rows in the database. Queries "look" the same in both cases:
Working query log (property is byte[]):

dbug: Microsoft.EntityFrameworkCore.Query[10107]
      => Microsoft.EntityFrameworkCore.Query.RelationalQueryModelVisitor
      (QueryContext queryContext) => IEnumerable<Parent> _InterceptExceptions(
          source: IEnumerable<Parent> _TrackEntities(
              results: IEnumerable<Parent> _ToSequence(() => Parent FirstOrDefault(IEnumerable<Parent> _Select(
                          source: IEnumerable<TransparentIdentifier<Parent, Child>> _ShapedQuery(
                              queryContext: queryContext,
                              shaperCommandContext: SelectExpression:
                                  SELECT TOP(1) [x].[Id], [x].[ChildId], [x.Child].[Id], [x.Child].[ULongRowVersion]
                                  FROM [Parent] AS [x]
                                  LEFT JOIN [Child] AS [x.Child] ON [x].[ChildId] = [x.Child].[Id],
                              shaper: TypedCompositeShaper<BufferedOffsetEntityShaper<Parent>, Parent, BufferedOffsetEntityShaper<Child>, Child, TransparentIdentifier<Parent, Child>>),
                          selector: (TransparentIdentifier<Parent, Child> t1) => Parent _Include(
                              queryContext: queryContext,
                              entity: t1.Outer,
                              included: new object[]{ t1.Inner },
                              fixup: (QueryContext queryContext | Parent entity | object[] included) =>
                              {
                                  void queryContext.QueryBuffer.StartTracking(
                                      entity: entity,
                                      entityType: EntityType: Parent)
                                  return !(bool ReferenceEquals(included[0], null)) ?
                                  {
                                      void queryContext.QueryBuffer.StartTracking(
                                          entity: included[0],
                                          entityType: EntityType: Child)
                                      void SetRelationshipSnapshotValue(
                                          stateManager: queryContext.StateManager,
                                          navigation: Parent.Child,
                                          entity: entity,
                                          value: included[0])
                                      return void SetRelationshipSnapshotValue(
                                          stateManager: queryContext.StateManager,
                                          navigation: Child.Parent,
                                          entity: included[0],
                                          value: entity)
                                  }
                                   :
                                  {
                                      void SetRelationshipIsLoaded(
                                          stateManager: queryContext.StateManager,
                                          navigation: Parent.Child,
                                          entity: entity)
                                      return default(void)
                                  }
                              }
                          )))),
              queryContext: Unhandled parameter: queryContext,
              entityTrackingInfos: { itemType: Parent },
              entityAccessors: List<Func<Parent, object>>
              {
                  Func<Parent, Parent>,
              }
          ),
          contextType: BloggingContext,
          logger: DiagnosticsLogger<Query>,
          queryContext: Unhandled parameter: queryContext)

Broken query log:

dbug: Microsoft.EntityFrameworkCore.Query[10107]
      => Microsoft.EntityFrameworkCore.Query.RelationalQueryModelVisitor
      (QueryContext queryContext) => IEnumerable<Parent> _InterceptExceptions(
          source: IEnumerable<Parent> _TrackEntities(
              results: IEnumerable<Parent> _ToSequence(() => Parent FirstOrDefault(IEnumerable<Parent> _Select(
                          source: IEnumerable<TransparentIdentifier<Parent, Child>> _ShapedQuery(
                              queryContext: queryContext,
                              shaperCommandContext: SelectExpression:
                                  SELECT TOP(1) [x].[Id], [x].[ChildId], [x.Child].[Id], [x.Child].[ULongRowVersion]
                                  FROM [Parent] AS [x]
                                  LEFT JOIN [Child] AS [x.Child] ON [x].[ChildId] = [x.Child].[Id],
                              shaper: TypedCompositeShaper<BufferedOffsetEntityShaper<Parent>, Parent, BufferedOffsetEntityShaper<Child>, Child, TransparentIdentifier<Parent, Child>>),
                          selector: (TransparentIdentifier<Parent, Child> t1) => Parent _Include(
                              queryContext: queryContext,
                              entity: t1.Outer,
                              included: new object[]{ t1.Inner },
                              fixup: (QueryContext queryContext | Parent entity | object[] included) =>
                              {
                                  void queryContext.QueryBuffer.StartTracking(
                                      entity: entity,
                                      entityType: EntityType: Parent)
                                  return !(bool ReferenceEquals(included[0], null)) ?
                                  {
                                      void queryContext.QueryBuffer.StartTracking(
                                          entity: included[0],
                                          entityType: EntityType: Child)
                                      void SetRelationshipSnapshotValue(
                                          stateManager: queryContext.StateManager,
                                          navigation: Parent.Child,
                                          entity: entity,
                                          value: included[0])
                                      return void SetRelationshipSnapshotValue(
                                          stateManager: queryContext.StateManager,
                                          navigation: Child.Parent,
                                          entity: included[0],
                                          value: entity)
                                  }
                                   :
                                  {
                                      void SetRelationshipIsLoaded(
                                          stateManager: queryContext.StateManager,
                                          navigation: Parent.Child,
                                          entity: entity)
                                      return default(void)
                                  }
                              }
                          )))),
              queryContext: Unhandled parameter: queryContext,
              entityTrackingInfos: { itemType: Parent },
              entityAccessors: List<Func<Parent, object>>
              {
                  Func<Parent, Parent>,
              }
          ),
          contextType: BloggingContext,
          logger: DiagnosticsLogger<Query>,
          queryContext: Unhandled parameter: queryContext)

[SimonCropp]

doing a cleanup of my repos. can u take a copy of what u need from https://github.com/SimonCropp/EfRowVersionRepro/ so i can delete it

[ajcvickers]

@SimonCropp I think we have what we need.

[SimonCropp]

@ajcvickers cool. thanks for the followup

[replaysMike]

ah man I hit this too, looking forward to a fix

[replaysMike]

Providing a temporary workaround that worked in my case:

on this table:

public class MyTable {
  public ulong RowVersion { get; set; }
}

using this builder config:

builder.Entity<MyTable>()
                .Property(p => p.RowVersion)
                .HasConversion<byte[]>()
                .IsRowVersion();

migrationBuilder.AddColumn<byte[]>(
                name: "RowVersion",
                table: "MyTable",
                nullable: false,
                defaultValue: new byte[] { });

manually changing the default value to a non-zero length array in the migration fixes it:

migrationBuilder.AddColumn<byte[]>(
                name: "RowVersion",
                table: "MyTable",
                nullable: false,
                defaultValue: new byte[] { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 });

[smitpatel]

@ajcvickers - Where is NumberToBytesConverter?

[ajcvickers]

@smitpatel https://github.com/aspnet/EntityFrameworkCore/blob/master/src/EFCore/Storage/ValueConversion/NumberToBytesConverter.cs

[smitpatel]

This is working in 3.1

[bdebaere]

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.

Modifying the QueryBugsTest.Projecting_entity_with_value_converter_and_include_works test from var result = context.Parents.Include(p => p.Child).FirstOrDefault(); to var result = context.Parents.Select(p => (ulong?)p.Child.ULongRowVersion).FirstOrDefault(); demonstrates the problem: an IndexOutOfRangeException is still thrown even though a cast is added to ulong? as suggested in #20633. This is simply because the cast doesn't accomplish anything, as the underlying value is Array.Empty, not null.

[bdebaere]

You have the following options:

1. You ask the Sql Server team to have a rowversion column return null when its value is coming from a LEFT JOIN to a record which does not exist, instead of 0x which is being returned now.
2. You ask the .NET team to have IsDBNull return true when a column is of data type rowversion and the value is 0x when its value is coming from a LEFT JOIN to a record which does not exist, instead of false which is being returned now.
3. You stop reading further column values from a single entity whose primary key is null when a LEFT JOIN is performed to a record which does not exist.
4. You modify the code to deal with a rowversion value of 0x at some point in the query pipeline e.g. equating 0x to null or checking for an array length of zero.

Let me know what you decide.

[ajcvickers]

Note--in the bug filed here there are no child rows in the table, and hence we should never be reading the row with an empty rowversion. This seems like the critical part to figure out before deciding how to fix the issue.

[bdebaere]

@ajcvickers If you are going with option 3, you will have to always include the primary key columns in every outer join query for it to work.
For example: context.Parents.Select(p => (ulong?)p.Child.ULongRowVersion).FirstOrDefault() needs to translate to

SELECT TOP(1) [c].[Id], [c].[ULongRowVersion]
FROM [Parents] AS [p]
LEFT JOIN [Children] AS [c] ON [p].[ChildId] = [c].[Id]

Or alternatively translate to

SELECT TOP(1) CASE WHEN [c].[Id] IS NULL THEN CAST(1 AS BIT) ELSE CAST(0 AS BIT) END AS [IsNull], [c].[ULongRowVersion]
FROM [Parents] AS [p]
LEFT JOIN [Children] AS [c] ON [p].[ChildId] = [c].[Id]

[billybraga]

I am having this issue in 5.0.4. I can't seem to figure out if the bug was fixed (can't find a commit in here that addresses the issue).

[ajcvickers]

@billybraga If this isn't working for you with 5.0.4, then please open a new issue and attach a small, runnable project or post a small, runnable code listing that reproduces what you are seeing so that we can investigate.

[billybraga]

@ajcvickers Done #24446