Handle binary keys of different lengths in update pipeline

Fixes #16888 Note that `StructuralEqualityComparer.Equals` does not suffer from this problem.
dotnet · Aug 6, 2019 · 4561939 · 4561939
1 parent a1cbf34
commit 4561939
Show file tree

Hide file tree

Showing 7 changed files with 91 additions and 10 deletions.
diff --git a/src/EFCore.Relational/Update/Internal/ModificationCommandComparer.cs b/src/EFCore.Relational/Update/Internal/ModificationCommandComparer.cs
@@ -151,6 +151,16 @@ private static readonly MethodInfo _compareMethod
         private static readonly MethodInfo _structuralCompareMethod
             = typeof(ModificationCommandComparer).GetTypeInfo().GetDeclaredMethod(nameof(CompareStructureValue));
 
-        private static int CompareStructureValue<T>(T x, T y) => StructuralComparisons.StructuralComparer.Compare(x, y);
+        private static int CompareStructureValue<T>(T x, T y)
+        {
+            if (x is Array array1
+                && y is Array array2
+                && array1.Length != array2.Length)
+            {
+                return array1.Length - array2.Length;
+            }
+
+            return StructuralComparisons.StructuralComparer.Compare(x, y);
+        }
     }
 }
diff --git a/test/EFCore.Specification.Tests/BuiltInDataTypesTestBase.cs b/test/EFCore.Specification.Tests/BuiltInDataTypesTestBase.cs
@@ -1442,32 +1442,96 @@ public virtual void Can_insert_and_read_back_with_binary_key()
 
             using (var context = CreateContext())
             {
-                context.Set<BinaryKeyDataType>().Add(
+                context.Set<BinaryKeyDataType>().AddRange(
                     new BinaryKeyDataType
                     {
-                        Id = new byte[] { 1, 2, 3 }
+                        Id = new byte[] { 1, 2, 3 },
+                        Ex = "X1"
+                    },
+                    new BinaryKeyDataType
+                    {
+                        Id = new byte[] { 1, 2, 3, 4 },
+                        Ex = "X3"
+                    },
+                    new BinaryKeyDataType
+                    {
+                        Id = new byte[] { 1, 2, 3, 4, 5 },
+                        Ex = "X2"
                     });
 
-                context.Set<BinaryForeignKeyDataType>().Add(
+                context.Set<BinaryForeignKeyDataType>().AddRange(
                     new BinaryForeignKeyDataType
                     {
                         Id = 77,
+                        BinaryKeyDataTypeId = new byte[] { 1, 2, 3, 4 }
+                    },
+                    new BinaryForeignKeyDataType
+                    {
+                        Id = 777,
                         BinaryKeyDataTypeId = new byte[] { 1, 2, 3 }
+                    },
+                    new BinaryForeignKeyDataType
+                    {
+                        Id = 7777,
+                        BinaryKeyDataTypeId = new byte[] { 1, 2, 3, 4, 5 }
                     });
 
-                Assert.Equal(2, context.SaveChanges());
+                Assert.Equal(6, context.SaveChanges());
             }
 
-            using (var context = CreateContext())
+            BinaryKeyDataType QueryByBinaryKey(DbContext context, byte[] bytes)
             {
-                var entity = context
+                return context
                     .Set<BinaryKeyDataType>()
                     .Include(e => e.Dependents)
-                    .Where(e => e.Id == new byte[] { 1, 2, 3 })
+                    .Where(e => e.Id == bytes)
                     .ToList().Single();
+            }
+
+            using (var context = CreateContext())
+            {
+                var entity1 = QueryByBinaryKey(context, new byte[] { 1, 2, 3 });
+                Assert.Equal(new byte[] { 1, 2, 3 }, entity1.Id);
+                Assert.Equal(1, entity1.Dependents.Count);
+
+                var entity2 = QueryByBinaryKey(context, new byte[] { 1, 2, 3, 4 });
+                Assert.Equal(new byte[] { 1, 2, 3, 4 }, entity2.Id);
+                Assert.Equal(1, entity2.Dependents.Count);
+
+                var entity3 = QueryByBinaryKey(context, new byte[] { 1, 2, 3, 4, 5 });
+                Assert.Equal(new byte[] { 1, 2, 3, 4, 5 }, entity3.Id);
+                Assert.Equal(1, entity3.Dependents.Count);
+
+                entity3.Ex = "Xx1";
+                entity2.Ex = "Xx3";
+                entity1.Ex = "Xx7";
+
+                entity1.Dependents.Single().BinaryKeyDataTypeId = new byte[]
+                {
+                    1, 2, 3, 4, 5
+                };
+
+                entity2.Dependents.Single().BinaryKeyDataTypeId = new byte[]
+                {
+                    1, 2, 3, 4, 5
+                };
 
-                Assert.Equal(new byte[] { 1, 2, 3 }, entity.Id);
-                Assert.Equal(new byte[] { 1, 2, 3 }, entity.Dependents.First().BinaryKeyDataTypeId);
+                context.SaveChanges();
+            }
+
+            using (var context = CreateContext())
+            {
+                var entity1 = QueryByBinaryKey(context, new byte[] { 1, 2, 3 });
+                Assert.Equal("Xx7", entity1.Ex);
+                Assert.Equal(0, entity1.Dependents.Count);
+
+                var entity2 = QueryByBinaryKey(context, new byte[] { 1, 2, 3, 4 });
+                Assert.Equal("Xx3", entity2.Ex);
+                Assert.Equal(0, entity2.Dependents.Count);
+
+                var entity3 = QueryByBinaryKey(context, new byte[] { 1, 2, 3, 4, 5 });
+                Assert.Equal("Xx1", entity3.Ex);
+                Assert.Equal(3, entity3.Dependents.Count);
             }
         }
 
@@ -2334,6 +2398,8 @@ protected class BinaryKeyDataType
         {
             public byte[] Id { get; set; }
 
+            public string Ex { get; set; }
+
             public ICollection<BinaryForeignKeyDataType> Dependents { get; set; }
         }
 

diff --git a/test/EFCore.SqlServer.FunctionalTests/BuiltInDataTypesSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/BuiltInDataTypesSqlServerTest.cs
@@ -2498,6 +2498,7 @@ public virtual void Columns_have_expected_data_types()
 
             const string expected = @"BinaryForeignKeyDataType.BinaryKeyDataTypeId ---> [nullable varbinary] [MaxLength = 900]
 BinaryForeignKeyDataType.Id ---> [int] [Precision = 10 Scale = 0]
+BinaryKeyDataType.Ex ---> [nullable nvarchar] [MaxLength = -1]
 BinaryKeyDataType.Id ---> [varbinary] [MaxLength = 900]
 BuiltInDataTypes.Enum16 ---> [smallint] [Precision = 5 Scale = 0]
 BuiltInDataTypes.Enum32 ---> [int] [Precision = 10 Scale = 0]

diff --git a/test/EFCore.SqlServer.FunctionalTests/ConvertToProviderTypesSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/ConvertToProviderTypesSqlServerTest.cs
@@ -32,6 +32,7 @@ public virtual void Columns_have_expected_data_types()
 
             const string expected = @"BinaryForeignKeyDataType.BinaryKeyDataTypeId ---> [nullable nvarchar] [MaxLength = 450]
 BinaryForeignKeyDataType.Id ---> [int] [Precision = 10 Scale = 0]
+BinaryKeyDataType.Ex ---> [nullable nvarchar] [MaxLength = -1]
 BinaryKeyDataType.Id ---> [nvarchar] [MaxLength = 450]
 BuiltInDataTypes.Enum16 ---> [bigint] [Precision = 19 Scale = 0]
 BuiltInDataTypes.Enum32 ---> [bigint] [Precision = 19 Scale = 0]

diff --git a/test/EFCore.SqlServer.FunctionalTests/CustomConvertersSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/CustomConvertersSqlServerTest.cs
@@ -27,6 +27,7 @@ public virtual void Columns_have_expected_data_types()
 
             const string expected = @"BinaryForeignKeyDataType.BinaryKeyDataTypeId ---> [nullable varbinary] [MaxLength = 900]
 BinaryForeignKeyDataType.Id ---> [int] [Precision = 10 Scale = 0]
+BinaryKeyDataType.Ex ---> [nullable nvarchar] [MaxLength = -1]
 BinaryKeyDataType.Id ---> [varbinary] [MaxLength = 900]
 BuiltInDataTypes.Enum16 ---> [bigint] [Precision = 19 Scale = 0]
 BuiltInDataTypes.Enum32 ---> [bigint] [Precision = 19 Scale = 0]

diff --git a/test/EFCore.SqlServer.FunctionalTests/EverythingIsBytesSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/EverythingIsBytesSqlServerTest.cs
@@ -31,6 +31,7 @@ public virtual void Columns_have_expected_data_types()
 
             const string expected = @"BinaryForeignKeyDataType.BinaryKeyDataTypeId ---> [nullable varbinary] [MaxLength = 900]
 BinaryForeignKeyDataType.Id ---> [varbinary] [MaxLength = 4]
+BinaryKeyDataType.Ex ---> [nullable varbinary] [MaxLength = -1]
 BinaryKeyDataType.Id ---> [varbinary] [MaxLength = 900]
 BuiltInDataTypes.Enum16 ---> [varbinary] [MaxLength = 2]
 BuiltInDataTypes.Enum32 ---> [varbinary] [MaxLength = 4]

diff --git a/test/EFCore.SqlServer.FunctionalTests/EverythingIsStringsSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/EverythingIsStringsSqlServerTest.cs
@@ -32,6 +32,7 @@ public virtual void Columns_have_expected_data_types()
 
             const string expected = @"BinaryForeignKeyDataType.BinaryKeyDataTypeId ---> [nullable nvarchar] [MaxLength = 450]
 BinaryForeignKeyDataType.Id ---> [nvarchar] [MaxLength = 64]
+BinaryKeyDataType.Ex ---> [nullable nvarchar] [MaxLength = -1]
 BinaryKeyDataType.Id ---> [nvarchar] [MaxLength = 450]
 BuiltInDataTypes.Enum16 ---> [nvarchar] [MaxLength = -1]
 BuiltInDataTypes.Enum32 ---> [nvarchar] [MaxLength = -1]