Query: Implement First/Single/Last OrDefault throwing behavior

Subquery produces top(1) for Single/SingleOrDefault
Single/First/SingleOrDefault in subquery is treated as if FirstOrDefault
Last/LastOrDefault throws when there is no ordering on SelectExpression to reverse
Resolves #15559

.gitignore

@@ -1,4 +1,4 @@
-QueryBaseline.cs
+QueryBaseline.txt
 
 ## Ignore Visual Studio temporary files, build results, and
 ## files generated by popular Visual Studio add-ons.

src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs

@@ -25,6 +25,7 @@ public class RelationalQueryableMethodTranslatingExpressionVisitor : QueryableMe
         private readonly RelationalProjectionBindingExpressionVisitor _projectionBindingExpressionVisitor;
         private readonly IModel _model;
         private readonly ISqlExpressionFactory _sqlExpressionFactory;
+        private readonly bool _subquery;
 
         public RelationalQueryableMethodTranslatingExpressionVisitor(
             QueryableMethodTranslatingExpressionVisitorDependencies dependencies,
@@ -40,6 +41,7 @@ public RelationalQueryableMethodTranslatingExpressionVisitor(
             _projectionBindingExpressionVisitor = new RelationalProjectionBindingExpressionVisitor(this, _sqlTranslator);
             _model = model;
             _sqlExpressionFactory = sqlExpressionFactory;
+            _subquery = false;
         }
 
         protected virtual RelationalQueryableMethodTranslatingExpressionVisitorDependencies RelationalDependencies { get; }
@@ -54,6 +56,7 @@ protected RelationalQueryableMethodTranslatingExpressionVisitor(
             _weakEntityExpandingExpressionVisitor = parentVisitor._weakEntityExpandingExpressionVisitor;
             _projectionBindingExpressionVisitor = new RelationalProjectionBindingExpressionVisitor(this, _sqlTranslator);
             _sqlExpressionFactory = parentVisitor._sqlExpressionFactory;
+            _subquery = true;
         }
 
         protected override Expression VisitMethodCall(MethodCallExpression methodCallExpression)
@@ -555,12 +558,17 @@ private SqlBinaryExpression CreateJoinPredicate(
         protected override ShapedQueryExpression TranslateLastOrDefault(
             ShapedQueryExpression source, LambdaExpression predicate, Type returnType, bool returnDefault)
         {
+            var selectExpression = (SelectExpression)source.QueryExpression;
+            if (selectExpression.Orderings.Count == 0)
+            {
+                return null;
+            }
+
             if (predicate != null)
             {
                 source = TranslateWhere(source, predicate);
             }
 
-            var selectExpression = (SelectExpression)source.QueryExpression;
             selectExpression.ReverseOrderings();
             selectExpression.ApplyLimit(TranslateExpression(Expression.Constant(1)));
 
@@ -849,7 +857,7 @@ protected override ShapedQueryExpression TranslateSingleOrDefault(ShapedQueryExp
             }
 
             var selectExpression = (SelectExpression)source.QueryExpression;
-            selectExpression.ApplyLimit(TranslateExpression(Expression.Constant(2)));
+            selectExpression.ApplyLimit(TranslateExpression(Expression.Constant(_subquery ? 1 : 2)));
 
             if (source.ShaperExpression.Type != returnType)
             {

test/EFCore.Cosmos.FunctionalTests/Query/SimpleQueryCosmosTest.cs

@@ -347,6 +347,12 @@ FROM root c
 WHERE (c[""Discriminator""] = ""Employee"")");
         }
 
+        [ConditionalTheory(Skip = "Issue #17246")]
+        public override Task Where_query_composition_entity_equality_one_element_Single(bool isAsync)
+        {
+            return base.Where_query_composition_entity_equality_one_element_Single(isAsync);
+        }
+
         [ConditionalTheory(Skip = "Issue#17246")]
         public override async Task Where_query_composition_entity_equality_one_element_FirstOrDefault(bool isAsync)
         {
@@ -358,6 +364,12 @@ FROM root c
 WHERE (c[""Discriminator""] = ""Employee"")");
         }
 
+        [ConditionalTheory(Skip = "Issue #17246")]
+        public override Task Where_query_composition_entity_equality_one_element_First(bool isAsync)
+        {
+            return base.Where_query_composition_entity_equality_one_element_First(isAsync);
+        }
+
         [ConditionalTheory(Skip = "Issue #17246")]
         public override async Task Where_query_composition_entity_equality_no_elements_SingleOrDefault(bool isAsync)
         {
@@ -369,6 +381,12 @@ FROM root c
 WHERE (c[""Discriminator""] = ""Employee"")");
         }
 
+        [ConditionalTheory(Skip = "Issue #17246")]
+        public override Task Where_query_composition_entity_equality_no_elements_Single(bool isAsync)
+        {
+            return base.Where_query_composition_entity_equality_no_elements_Single(isAsync);
+        }
+
         [ConditionalTheory(Skip = "Issue#17246")]
         public override async Task Where_query_composition_entity_equality_no_elements_FirstOrDefault(bool isAsync)
         {
@@ -380,6 +398,24 @@ FROM root c
 WHERE (c[""Discriminator""] = ""Employee"")");
         }
 
+        [ConditionalTheory(Skip = "Issue #17246")]
+        public override Task Where_query_composition_entity_equality_no_elements_First(bool isAsync)
+        {
+            return base.Where_query_composition_entity_equality_no_elements_First(isAsync);
+        }
+
+        [ConditionalTheory(Skip = "Issue #17246")]
+        public override Task Where_query_composition_entity_equality_multiple_elements_SingleOrDefault(bool isAsync)
+        {
+            return base.Where_query_composition_entity_equality_multiple_elements_SingleOrDefault(isAsync);
+        }
+
+        [ConditionalTheory(Skip = "Issue #17246")]
+        public override Task Where_query_composition_entity_equality_multiple_elements_Single(bool isAsync)
+        {
+            return base.Where_query_composition_entity_equality_multiple_elements_Single(isAsync);
+        }
+
         [ConditionalTheory(Skip = "Issue#17246")]
         public override async Task Where_query_composition_entity_equality_multiple_elements_FirstOrDefault(bool isAsync)
         {
@@ -391,6 +427,12 @@ FROM root c
 WHERE (c[""Discriminator""] = ""Employee"")");
         }
 
+        [ConditionalTheory(Skip = "Issue #17246")]
+        public override Task Where_query_composition_entity_equality_multiple_elements_First(bool isAsync)
+        {
+            return base.Where_query_composition_entity_equality_multiple_elements_First(isAsync);
+        }
+
         [ConditionalTheory(Skip = "Issue #17246")]
         public override async Task Where_query_composition2(bool isAsync)
         {

test/EFCore.InMemory.FunctionalTests/Query/IncludeInMemoryTest.cs

@@ -18,5 +18,11 @@ public IncludeInMemoryTest(IncludeInMemoryFixture fixture, ITestOutputHelper tes
         public override void Include_collection_with_client_filter(bool useString)
         {
         }
+
+        [ConditionalTheory(Skip = "Issue #16963")]
+        public override void Include_collection_with_last_no_orderby(bool useString)
+        {
+            base.Include_collection_with_last_no_orderby(useString);
+        }
     }
 }

test/EFCore.InMemory.FunctionalTests/Query/SimpleQueryInMemoryTest.cs

@@ -381,5 +381,47 @@ public override Task Projection_when_client_evald_subquery(bool isAsync)
 
         [ConditionalTheory(Skip = "Issue #16963")]
         public override Task Select_bool_closure_with_order_by_property_with_cast_to_nullable(bool isAsync) => null;
+
+        [ConditionalTheory(Skip = "Issue #16963")]
+        public override Task Where_query_composition_entity_equality_one_element_Single(bool isAsync)
+        {
+            return base.Where_query_composition_entity_equality_one_element_Single(isAsync);
+        }
+
+        [ConditionalTheory(Skip = "Issue #16963")]
+        public override Task Where_query_composition_entity_equality_one_element_First(bool isAsync)
+        {
+            return base.Where_query_composition_entity_equality_one_element_First(isAsync);
+        }
+
+        [ConditionalTheory(Skip = "Issue #16963")]
+        public override Task Where_query_composition_entity_equality_no_elements_Single(bool isAsync)
+        {
+            return base.Where_query_composition_entity_equality_no_elements_Single(isAsync);
+        }
+
+        [ConditionalTheory(Skip = "Issue #16963")]
+        public override Task Where_query_composition_entity_equality_no_elements_First(bool isAsync)
+        {
+            return base.Where_query_composition_entity_equality_no_elements_First(isAsync);
+        }
+
+        [ConditionalTheory(Skip = "Issue #16963")]
+        public override Task Where_query_composition_entity_equality_multiple_elements_SingleOrDefault(bool isAsync)
+        {
+            return base.Where_query_composition_entity_equality_multiple_elements_SingleOrDefault(isAsync);
+        }
+
+        [ConditionalTheory(Skip = "Issue #16963")]
+        public override Task Where_query_composition_entity_equality_multiple_elements_Single(bool isAsync)
+        {
+            return base.Where_query_composition_entity_equality_multiple_elements_Single(isAsync);
+        }
+
+        [ConditionalTheory(Skip = "Issue #16963")]
+        public override Task Last_when_no_order_by(bool isAsync)
+        {
+            return base.Last_when_no_order_by(isAsync);
+        }
     }
 }

test/EFCore.Relational.Specification.Tests/TransactionTestBase.cs

@@ -51,7 +51,7 @@ public virtual void SaveChanges_can_be_used_with_no_transaction()
                     });
 
 
-                context.Entry(context.Set<TransactionCustomer>().Last()).State = EntityState.Added;
+                context.Entry(context.Set<TransactionCustomer>().OrderBy(c => c.Id).Last()).State = EntityState.Added;
 
                 Assert.Throws<DbUpdateException>(() => context.SaveChanges());
 
@@ -176,7 +176,7 @@ public virtual async Task SaveChanges_uses_enlisted_transaction(bool async, bool
                             Name = "Bobble"
                         });
 
-                    context.Entry(context.Set<TransactionCustomer>().Last()).State = EntityState.Added;
+                    context.Entry(context.Set<TransactionCustomer>().OrderBy(c => c.Id).Last()).State = EntityState.Added;
 
                     if (async)
                     {
@@ -250,7 +250,7 @@ public virtual async Task SaveChanges_uses_enlisted_transaction_after_connection
                             Name = "Bobble"
                         });
 
-                    context.Entry(context.Set<TransactionCustomer>().Last()).State = EntityState.Added;
+                    context.Entry(context.Set<TransactionCustomer>().OrderBy(c => c.Id).Last()).State = EntityState.Added;
 
                     context.Database.AutoTransactionsEnabled = true;
                 }
@@ -302,7 +302,7 @@ public virtual async Task SaveChanges_uses_enlisted_transaction_connectionString
                             Name = "Bobble"
                         });
 
-                    context.Entry(context.Set<TransactionCustomer>().Last()).State = EntityState.Added;
+                    context.Entry(context.Set<TransactionCustomer>().OrderBy(c => c.Id).Last()).State = EntityState.Added;
 
                     if (async)
                     {
@@ -347,7 +347,7 @@ public virtual async Task SaveChanges_uses_ambient_transaction(bool async, bool
                             Name = "Bobble"
                         });
 
-                    context.Entry(context.Set<TransactionCustomer>().Last()).State = EntityState.Added;
+                    context.Entry(context.Set<TransactionCustomer>().OrderBy(c => c.Id).Last()).State = EntityState.Added;
 
                     if (async)
                     {
@@ -421,7 +421,7 @@ public virtual async Task SaveChanges_uses_ambient_transaction_with_connectionSt
                             Name = "Bobble"
                         });
 
-                    context.Entry(context.Set<TransactionCustomer>().Last()).State = EntityState.Added;
+                    context.Entry(context.Set<TransactionCustomer>().OrderBy(c => c.Id).Last()).State = EntityState.Added;
 
                     if (async)
                     {
@@ -464,7 +464,7 @@ public virtual void SaveChanges_throws_for_suppressed_ambient_transactions(bool
                             Name = "Bobble"
                         });
 
-                    context.Entry(context.Set<TransactionCustomer>().Last()).State = EntityState.Added;
+                    context.Entry(context.Set<TransactionCustomer>().OrderBy(c => c.Id).Last()).State = EntityState.Added;
 
                     using (new TransactionScope(TransactionScopeOption.Suppress))
                     {
@@ -501,7 +501,7 @@ public virtual void SaveChanges_uses_enlisted_transaction_after_ambient_transact
                             Name = "Bobble"
                         });
 
-                    context.Entry(context.Set<TransactionCustomer>().Last()).State = EntityState.Added;
+                    context.Entry(context.Set<TransactionCustomer>().OrderBy(c => c.Id).Last()).State = EntityState.Added;
                 }
 
                 using (var transaction = new CommittableTransaction(TimeSpan.FromMinutes(10)))

test/EFCore.Specification.Tests/ConcurrencyDetectorTestBase.cs

@@ -98,15 +98,15 @@ public virtual Task Last_logs_concurrent_access_nonasync()
             return ConcurrencyDetectorTest(
                 c =>
                 {
-                    var result = c.Products.Last();
+                    var result = c.Products.OrderBy(p => p.ProductID).Last();
                     return Task.FromResult(false);
                 });
         }
 
         [ConditionalFact]
         public virtual Task Last_logs_concurrent_access_async()
         {
-            return ConcurrencyDetectorTest(c => c.Products.LastAsync());
+            return ConcurrencyDetectorTest(c => c.Products.OrderBy(p => p.ProductID).LastAsync());
         }
 
         [ConditionalFact]

test/EFCore.Specification.Tests/Query/IncludeTestBase.cs

@@ -371,25 +371,19 @@ var customer
             }
         }
 
-        [ConditionalTheory(Skip = "Issue#15559")]
+        [ConditionalTheory]
         [InlineData(false)]
         [InlineData(true)]
         public virtual void Include_collection_with_last_no_orderby(bool useString)
         {
             using (var context = CreateContext())
             {
-                var customer
-                    = useString
-                        ? context.Set<Customer>()
-                            .Include("Orders")
-                            .Last()
-                        : context.Set<Customer>()
-                            .Include(c => c.Orders)
-                            .Last();
-
-                Assert.NotNull(customer);
-                Assert.Equal(7, customer.Orders.Count);
-                Assert.Equal(8, context.ChangeTracker.Entries().Count());
+                Assert.Equal(
+                    CoreStrings.TranslationFailed(@"Last<Customer>(Select<Customer, Customer>(    source: DbSet<Customer>,     selector: (c) => IncludeExpression(        c,         MaterializeCollectionNavigation(Navigation: Customer.Orders (<Orders>k__BackingField, List<Order>) Collection ToDependent Order Inverse: Customer PropertyAccessMode.Field, Where<Order>(            source: DbSet<Order>,             predicate: (o) => Property<string>(c, ""CustomerID"") == Property<string>(o, ""CustomerID""))), Orders)))"),
+                    RemoveNewLines(Assert.Throws<InvalidOperationException>(
+                        () => useString
+                            ? context.Set<Customer>().Include("Orders").Last()
+                            : context.Set<Customer>().Include(c => c.Orders).Last()).Message));
             }
         }
 
@@ -4327,6 +4321,8 @@ private static void CheckIsLoaded(
             }
         }
 
+        private string RemoveNewLines(string message) => message.Replace("\n", "").Replace("\r", "");
+
         protected virtual void ClearLog()
         {
         }

test/EFCore.Specification.Tests/Query/SimpleQueryTestBase.ResultOperators.cs

@@ -1084,12 +1084,16 @@ public virtual Task Last(bool isAsync)
 
         [ConditionalTheory]
         [MemberData(nameof(IsAsyncData))]
-        public virtual Task Last_when_no_order_by(bool isAsync)
+        public virtual async Task Last_when_no_order_by(bool isAsync)
         {
-            return AssertLast<Customer>(
-                isAsync,
-                cs => cs.Where(c => c.CustomerID == "ALFKI"),
-                entryCount: 1);
+            Assert.Equal(
+                CoreStrings.TranslationFailed(@"Last<object>(Select<Customer, object>(    source: Where<Customer>(        source: DbSet<Customer>,         predicate: (c) => c.CustomerID == ""ALFKI""),     selector: (c) => (object)c))"),
+                RemoveNewLines(
+                    (await Assert.ThrowsAsync<InvalidOperationException>(
+                        () => AssertLast<Customer>(
+                                        isAsync,
+                                        cs => cs.Where(c => c.CustomerID == "ALFKI"),
+                                        entryCount: 1))).Message));
         }
 
         [ConditionalTheory]
@@ -1492,7 +1496,8 @@ public virtual async Task Contains_with_local_anonymous_type_array_closure(bool
                                 o => ids.Contains(
                                     new
                                     {
-                                        Id1 = o.OrderID, Id2 = o.ProductID
+                                        Id1 = o.OrderID,
+                                        Id2 = o.ProductID
                                     })), entryCount: 1))).Message));
         }