Fix to #18211 - Throw better exception for Last without order by

dotnet · Feb 6, 2020 · 9e716d6 · 9e716d6
1 parent dd843a2
commit 9e716d6
Show file tree

Hide file tree

Showing 7 changed files with 54 additions and 9 deletions.
diff --git a/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs
@@ -552,7 +552,9 @@ protected override ShapedQueryExpression TranslateLastOrDefault(
             var selectExpression = (SelectExpression)source.QueryExpression;
             if (selectExpression.Orderings.Count == 0)
             {
-                return null;
+                throw new InvalidOperationException(
+                    CoreStrings.LastUsedWithoutOrderBy(returnDefault ?
+                        nameof(Queryable.LastOrDefault) : nameof(Queryable.Last)));
             }
 
             if (predicate != null)

diff --git a/src/EFCore/Properties/CoreStrings.Designer.cs b/src/EFCore/Properties/CoreStrings.Designer.cs
diff --git a/src/EFCore/Properties/CoreStrings.resx b/src/EFCore/Properties/CoreStrings.resx
@@ -1266,4 +1266,7 @@
   <data name="SkipNavigationInUseBySkipNavigation" xml:space="preserve">
     <value>The skip navigation '{skipNavigation}' cannot be removed because it is set as the inverse of the skip navigation '{inverseSkipNavigation}' on '{referencingEntityType}'. All referencing skip navigations must be removed before this skip navigation can be removed.</value>
   </data>
+  <data name="LastUsedWithoutOrderBy" xml:space="preserve">
+    <value>Queries performing '{method}' operation must have a deterministic sort order. Rewrite the query to apply an OrderBy clause on the sequence before calling '{method}'.</value>
+  </data>
 </root>
diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindAggregateOperatorsQueryCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindAggregateOperatorsQueryCosmosTest.cs
@@ -896,6 +896,17 @@ FROM root c
 WHERE ((c[""Discriminator""] = ""Customer"") AND (c[""CustomerID""] = ""ALFKI""))");
         }
 
+        [ConditionalTheory(Skip = "Issue #17246")]
+        public override async Task LastOrDefault_when_no_order_by(bool async)
+        {
+            await base.LastOrDefault_when_no_order_by(async);
+
+            AssertSql(
+                @"SELECT c
+FROM root c
+WHERE ((c[""Discriminator""] = ""Customer"") AND (c[""CustomerID""] = ""ALFKI""))");
+        }
+
         public override async Task Last_Predicate(bool async)
         {
             await base.Last_Predicate(async);

diff --git a/test/EFCore.InMemory.FunctionalTests/Query/NorthwindAggregateOperatorsQueryInMemoryTest.cs b/test/EFCore.InMemory.FunctionalTests/Query/NorthwindAggregateOperatorsQueryInMemoryTest.cs
@@ -54,5 +54,11 @@ public override Task Last_when_no_order_by(bool async)
         {
             return base.Last_when_no_order_by(async);
         }
+
+        [ConditionalTheory(Skip = "Issue#17386")]
+        public override Task LastOrDefault_when_no_order_by(bool async)
+        {
+            return base.LastOrDefault_when_no_order_by(async);
+        }
     }
 }
diff --git a/test/EFCore.Specification.Tests/Query/NorthwindAggregateOperatorsQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/NorthwindAggregateOperatorsQueryTestBase.cs
@@ -976,13 +976,28 @@ public virtual Task Last(bool async)
 
         [ConditionalTheory]
         [MemberData(nameof(IsAsyncData))]
-        public virtual Task Last_when_no_order_by(bool async)
+        public virtual async Task Last_when_no_order_by(bool async)
         {
-            return AssertTranslationFailed(
-                () => AssertLast(
-                    async,
-                    ss => ss.Set<Customer>().Where(c => c.CustomerID == "ALFKI"),
-                    entryCount: 1));
+            Assert.Equal(Diagnostics.CoreStrings.LastUsedWithoutOrderBy(nameof(Enumerable.Last)),
+                (await Assert.ThrowsAsync<InvalidOperationException>(
+                    () => AssertLast(
+                        async,
+                        ss => ss.Set<Customer>().Where(c => c.CustomerID == "ALFKI"),
+                        entryCount: 1)
+                    )).Message);
+        }
+
+        [ConditionalTheory]
+        [MemberData(nameof(IsAsyncData))]
+        public virtual async Task LastOrDefault_when_no_order_by(bool async)
+        {
+            Assert.Equal(Diagnostics.CoreStrings.LastUsedWithoutOrderBy(nameof(Enumerable.LastOrDefault)),
+                (await Assert.ThrowsAsync<InvalidOperationException>(
+                    () => AssertLastOrDefault(
+                        async,
+                        ss => ss.Set<Customer>().Where(c => c.CustomerID == "ALFKI"),
+                        entryCount: 1)
+                    )).Message);
         }
 
         [ConditionalTheory]

diff --git a/test/EFCore.Specification.Tests/Query/NorthwindIncludeQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/NorthwindIncludeQueryTestBase.cs
@@ -334,8 +334,8 @@ var customer
         public virtual void Include_collection_with_last_no_orderby(bool useString)
         {
             using var context = CreateContext();
-            Assert.Contains(
-                CoreStrings.TranslationFailed("").Substring(21),
+            Assert.Equal(
+                CoreStrings.LastUsedWithoutOrderBy(nameof(Enumerable.Last)),
                 Assert.Throws<InvalidOperationException>(
                     () => useString
                         ? context.Set<Customer>().Include("Orders").Last()