Fix to #5179 - Query: Select Where Navigation returns wrong results o…

…n relational providers

Problem was that when creating groups in GroupJoin we would only look at the inner key selector to differentiate between groups. However in some cases (specifically 1-Many navigations coming from the many side - e.g. order.Customer) outer element constantly changes, while inner element stays the same. If later in the pipeline outer elements are being used, e.g. in projection, we get incorrect results.

Fix is to also look at changes between outer elements and create a new group every time either outer element or a inner key changes.

Also as part of this change we improve result verification for the navigation tests (previously we would only verify that the result count is as expected).

src/Microsoft.EntityFrameworkCore.Relational/Query/ExpressionVisitors/Internal/BufferedEntityShaper`.cs

@@ -25,6 +25,15 @@ public BufferedEntityShaper(
 
         public override Type Type => typeof(TEntity);
 
+        public virtual object GetKey(QueryContext queryContext, ValueBuffer valueBuffer)
+        {
+            return queryContext.QueryBuffer.GetEntityKey(
+                Key,
+                new EntityLoadInfo(valueBuffer, Materializer),
+                queryStateManager: IsTrackingQuery,
+                throwOnNullKey: !AllowNullResult);
+        }
+
         public virtual TEntity Shape(QueryContext queryContext, ValueBuffer valueBuffer)
         {
             Debug.Assert(queryContext != null);

src/Microsoft.EntityFrameworkCore.Relational/Query/ExpressionVisitors/Internal/BufferedOffsetEntityShaper.cs

@@ -22,6 +22,9 @@ public BufferedOffsetEntityShaper(
         {
         }
 
+        public override object GetKey(QueryContext queryContext, ValueBuffer valueBuffer)
+            => base.GetKey(queryContext, valueBuffer.WithOffset(ValueBufferOffset));
+
         public override TEntity Shape(QueryContext queryContext, ValueBuffer valueBuffer)
             => base.Shape(queryContext, valueBuffer.WithOffset(ValueBufferOffset));
 

src/Microsoft.EntityFrameworkCore.Relational/Query/ExpressionVisitors/Internal/IShaper.cs

@@ -10,6 +10,7 @@ namespace Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal
 {
     public interface IShaper<out T>
     {
+        object GetKey([NotNull] QueryContext queryContext, ValueBuffer valueBuffer);
         T Shape([NotNull] QueryContext queryContext, ValueBuffer valueBuffer);
         bool IsShaperForQuerySource([NotNull] IQuerySource querySource);
         void SaveAccessorExpression([NotNull] QuerySourceMapping querySourceMapping);

src/Microsoft.EntityFrameworkCore.Relational/Query/ExpressionVisitors/Internal/QueryFlattener.cs

@@ -170,6 +170,13 @@ public CompositeShaper(
                 _materializer = materializer;
             }
 
+            public object GetKey(QueryContext queryContext, ValueBuffer valueBuffer)
+            {
+                return new CompositeKey(
+                    _outerShaper.GetKey(queryContext, valueBuffer),
+                    _innerShaper.GetKey(queryContext, valueBuffer));
+            }
+
             public TResult Shape(QueryContext queryContext, ValueBuffer valueBuffer)
                 => _materializer(
                     _outerShaper.Shape(queryContext, valueBuffer),
@@ -192,5 +199,50 @@ public override Expression GetAccessorExpression(IQuerySource querySource)
                 => _outerShaper.GetAccessorExpression(querySource)
                    ?? _innerShaper.GetAccessorExpression(querySource);
         }
+
+        private class CompositeKey
+        {
+            private readonly object _outerKey;
+            private readonly object _innerKey;
+
+            public CompositeKey(object outerKey, object innerKey)
+            {
+                _outerKey = outerKey;
+                _innerKey = innerKey;
+            }
+
+            public override bool Equals(object obj)
+            {
+                var other = obj as CompositeKey;
+                if (other == null)
+                {
+                    return false;
+                }
+
+                return _outerKey != null
+                    ? _outerKey.Equals(other._outerKey) : other._outerKey == null
+                    && _innerKey != null
+                    ? _innerKey.Equals(other._innerKey) : other._innerKey == null;
+            }
+
+            public override int GetHashCode()
+            {
+                unchecked
+                {
+                    var hashCode = 0;
+                    if (_outerKey != null)
+                    {
+                        hashCode = _outerKey.GetHashCode();
+                    }
+
+                    if (_innerKey != null)
+                    {
+                        hashCode += (hashCode * 397) ^ _innerKey.GetHashCode();
+                    }
+
+                    return hashCode;
+                }
+            }
+        }
     }
 }

src/Microsoft.EntityFrameworkCore.Relational/Query/ExpressionVisitors/Internal/UnbufferedEntityShaper.cs

@@ -24,6 +24,9 @@ public UnbufferedEntityShaper(
 
         public override Type Type => typeof(TEntity);
 
+        public virtual object GetKey(QueryContext queryContext, ValueBuffer valueBuffer) =>
+            queryContext.StateManager.TryGetEntryKey(Key, valueBuffer, !AllowNullResult);
+
         public virtual TEntity Shape(QueryContext queryContext, ValueBuffer valueBuffer)
         {
             if (IsTrackingQuery)

src/Microsoft.EntityFrameworkCore.Relational/Query/ExpressionVisitors/Internal/ValueBufferShaper.cs

@@ -17,6 +17,9 @@ public ValueBufferShaper([NotNull] IQuerySource querySource)
 
         public override Type Type => typeof(ValueBuffer);
 
+        public virtual object GetKey(QueryContext queryContext, ValueBuffer valueBuffer)
+            => null;
+
         public virtual ValueBuffer Shape(QueryContext queryContext, ValueBuffer valueBuffer)
             => valueBuffer;
     }

src/Microsoft.EntityFrameworkCore.Relational/Query/QueryMethodProvider.cs

@@ -240,6 +240,7 @@ private static IEnumerable<TResult> _GroupJoin<TOuter, TInner, TKey, TResult>(
                         }
                         else
                         {
+                            var currentOuterEntityKey = outerShaper.GetKey(queryContext, sourceEnumerator.Current);
                             var currentGroupKey = innerKeySelector(inner);
 
                             innerGroupJoinInclude?.Include(inner);
@@ -255,6 +256,16 @@ private static IEnumerable<TResult> _GroupJoin<TOuter, TInner, TKey, TResult>(
                                     break;
                                 }
 
+                                if (currentOuterEntityKey != null)
+                                {
+                                    var nextOuterEntityKey = outerShaper.GetKey(queryContext, sourceEnumerator.Current);
+
+                                    if (!currentOuterEntityKey.Equals(nextOuterEntityKey))
+                                    {
+                                        break;
+                                    }
+                                }
+
                                 inner = innerShaper.Shape(queryContext, sourceEnumerator.Current);
 
                                 if (inner == null)