SqlServer: Generate False predicate when skip/take both are 0

src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs

@@ -3105,27 +3105,29 @@ private bool Equals(SelectExpression selectExpression)
         // This does not take internal states since when using this method SelectExpression should be finalized
         [Obsolete("Use the overload which does not require distinct & alias parameter.")]
         public SelectExpression Update(
-            [NotNull] List<ProjectionExpression> projections,
-            [NotNull] List<TableExpressionBase> tables,
+            [NotNull] IReadOnlyList<ProjectionExpression> projections,
+            [NotNull] IReadOnlyList<TableExpressionBase> tables,
             [CanBeNull] SqlExpression predicate,
-            [CanBeNull] List<SqlExpression> groupBy,
+            [NotNull] IReadOnlyList<SqlExpression> groupBy,
             [CanBeNull] SqlExpression having,
-            [CanBeNull] List<OrderingExpression> orderings,
+            [NotNull] IReadOnlyList<OrderingExpression> orderings,
             [CanBeNull] SqlExpression limit,
             [CanBeNull] SqlExpression offset,
             bool distinct,
             [CanBeNull] string alias)
         {
             Check.NotNull(projections, nameof(projections));
             Check.NotNull(tables, nameof(tables));
+            Check.NotNull(groupBy, nameof(groupBy));
+            Check.NotNull(orderings, nameof(orderings));
 
             var projectionMapping = new Dictionary<ProjectionMember, Expression>();
             foreach (var kvp in _projectionMapping)
             {
                 projectionMapping[kvp.Key] = kvp.Value;
             }
 
-            return new SelectExpression(alias, projections, tables, groupBy, orderings)
+            return new SelectExpression(alias, projections.ToList(), tables.ToList(), groupBy.ToList(), orderings.ToList())
             {
                 _projectionMapping = projectionMapping,
                 Predicate = predicate,
@@ -3152,12 +3154,12 @@ public SelectExpression Update(
         /// <returns> This expression if no children changed, or an expression with the updated children. </returns>
         // This does not take internal states since when using this method SelectExpression should be finalized
         public SelectExpression Update(
-            [NotNull] List<ProjectionExpression> projections,
-            [NotNull] List<TableExpressionBase> tables,
+            [NotNull] IReadOnlyList<ProjectionExpression> projections,
+            [NotNull] IReadOnlyList<TableExpressionBase> tables,
             [CanBeNull] SqlExpression? predicate,
-            [NotNull] List<SqlExpression> groupBy,
+            [NotNull] IReadOnlyList<SqlExpression> groupBy,
             [CanBeNull] SqlExpression? having,
-            [NotNull] List<OrderingExpression> orderings,
+            [NotNull] IReadOnlyList<OrderingExpression> orderings,
             [CanBeNull] SqlExpression? limit,
             [CanBeNull] SqlExpression? offset)
         {
@@ -3172,7 +3174,7 @@ public SelectExpression Update(
                 projectionMapping[kvp.Key] = kvp.Value;
             }
 
-            return new SelectExpression(Alias, projections, tables, groupBy, orderings)
+            return new SelectExpression(Alias, projections.ToList(), tables.ToList(), groupBy.ToList(), orderings.ToList())
             {
                 _projectionMapping = projectionMapping,
                 Predicate = predicate,

src/EFCore.SqlServer/Query/Internal/SkipTakeCollapsingExpressionVisitor.cs

@@ -0,0 +1,109 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System.Collections.Generic;
+using System.Linq.Expressions;
+using JetBrains.Annotations;
+using Microsoft.EntityFrameworkCore.Query;
+using Microsoft.EntityFrameworkCore.Query.SqlExpressions;
+using Microsoft.EntityFrameworkCore.Utilities;
+
+#nullable enable
+
+namespace Microsoft.EntityFrameworkCore.SqlServer.Query.Internal
+{
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    public class SkipTakeCollapsingExpressionVisitor : ExpressionVisitor
+    {
+        private readonly ISqlExpressionFactory _sqlExpressionFactory;
+
+        private IReadOnlyDictionary<string, object> _parameterValues;
+        private bool _canCache;
+
+        /// <summary>
+        ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+        ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+        ///     any release. You should only use it directly in your code with extreme caution and knowing that
+        ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+        /// </summary>
+        public SkipTakeCollapsingExpressionVisitor([NotNull] ISqlExpressionFactory sqlExpressionFactory)
+        {
+            Check.NotNull(sqlExpressionFactory, nameof(sqlExpressionFactory));
+
+            _sqlExpressionFactory = sqlExpressionFactory;
+            _parameterValues = null!;
+        }
+
+        /// <summary>
+        ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+        ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+        ///     any release. You should only use it directly in your code with extreme caution and knowing that
+        ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+        /// </summary>
+        public virtual SelectExpression Process(
+            [NotNull] SelectExpression selectExpression,
+            [NotNull] IReadOnlyDictionary<string, object> parametersValues,
+            out bool canCache)
+        {
+            Check.NotNull(selectExpression, nameof(selectExpression));
+            Check.NotNull(parametersValues, nameof(parametersValues));
+
+            _parameterValues = parametersValues;
+            _canCache = true;
+
+            var result = (SelectExpression)Visit(selectExpression);
+
+            canCache = _canCache;
+
+            return result;
+        }
+
+        /// <summary>
+        ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+        ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+        ///     any release. You should only use it directly in your code with extreme caution and knowing that
+        ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+        /// </summary>
+        protected override Expression VisitExtension(Expression extensionExpression)
+        {
+            if (extensionExpression is SelectExpression selectExpression)
+            {
+                if (IsZero(selectExpression.Limit)
+                    && IsZero(selectExpression.Offset))
+                {
+                    return selectExpression.Update(
+                        selectExpression.Projection,
+                        selectExpression.Tables,
+                        selectExpression.GroupBy.Count > 0 ? selectExpression.Predicate : _sqlExpressionFactory.Constant(false),
+                        selectExpression.GroupBy,
+                        selectExpression.GroupBy.Count > 0 ? _sqlExpressionFactory.Constant(false) : null,
+                        new List<OrderingExpression>(0),
+                        limit: null,
+                        offset: null);
+                }
+
+                bool IsZero(SqlExpression? sqlExpression)
+                {
+                    switch (sqlExpression)
+                    {
+                        case SqlConstantExpression constant:
+                            return ((int)constant.Value) == 0;
+                        case SqlParameterExpression parameter:
+                            _canCache = false;
+                            return ((int)_parameterValues[parameter.Name]) == 0;
+
+                        default:
+                            return false;
+                    }
+                }
+            }
+
+            return base.VisitExtension(extensionExpression);
+        }
+    }
+}

src/EFCore.SqlServer/Query/Internal/SqlServerParameterBasedSqlProcessor.cs

@@ -48,6 +48,11 @@ public override SelectExpression Optimize(
 
             var optimizedSelectExpression = base.Optimize(selectExpression, parametersValues, out canCache);
 
+            optimizedSelectExpression = new SkipTakeCollapsingExpressionVisitor(Dependencies.SqlExpressionFactory)
+                .Process(optimizedSelectExpression, parametersValues, out var canCache2);
+
+            canCache &= canCache2;
+
             return (SelectExpression)new SearchConditionConvertingExpressionVisitor(Dependencies.SqlExpressionFactory)
                 .Visit(optimizedSelectExpression);
         }

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

@@ -4202,6 +4202,12 @@ public override Task First_on_collection_in_projection(bool async)
             return base.First_on_collection_in_projection(async);
         }
 
+        [ConditionalTheory(Skip = "Non embedded collection subquery Issue#17246")]
+        public override Task Skip_0_Take_0_works_when_constant(bool async)
+        {
+            return base.Skip_0_Take_0_works_when_constant(async);
+        }
+
         private void AssertSql(params string[] expected)
             => Fixture.TestSqlLoggerFactory.AssertBaseline(expected);
 

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

@@ -1288,9 +1288,10 @@ public virtual Task Element_selector_with_case_block_repeated_inside_another_cas
                 async,
                 ss => from order in ss.Set<Order>()
                       group new
-                          {
-                              IsAlfki = order.CustomerID == "ALFKI", OrderId = order.OrderID > 1000 ? order.OrderID : -order.OrderID
-                          } by
+                      {
+                          IsAlfki = order.CustomerID == "ALFKI",
+                          OrderId = order.OrderID > 1000 ? order.OrderID : -order.OrderID
+                      } by
                           new { order.OrderID }
                       into g
                       select new { g.Key.OrderID, Aggregate = g.Sum(s => s.IsAlfki ? s.OrderId : -s.OrderId) });
@@ -2326,6 +2327,35 @@ public virtual Task GroupBy_aggregate_join_another_GroupBy_aggregate(bool async)
                 elementSorter: o => o.Key);
         }
 
+        [ConditionalTheory]
+        [MemberData(nameof(IsAsyncData))]
+        public virtual Task GroupBy_aggregate_after_skip_0_take_0(bool async)
+        {
+            return AssertQuery(
+                async,
+                ss => ss.Set<Order>()
+                    .Skip(0)
+                    .Take(0)
+                    .GroupBy(o => o.CustomerID)
+                    .Select(g => new { g.Key, Total = g.Count() }),
+                elementSorter: o => o.Key);
+        }
+
+        [ConditionalTheory]
+        [MemberData(nameof(IsAsyncData))]
+        public virtual Task GroupBy_skip_0_take_0_aggregate(bool async)
+        {
+            return AssertQuery(
+                async,
+                ss => ss.Set<Order>()
+                    .Where(e => e.OrderID > 10500)
+                    .GroupBy(o => o.CustomerID)
+                    .Skip(0)
+                    .Take(0)
+                    .Select(g => new { g.Key, Total = g.Count() }),
+                elementSorter: o => o.Key);
+        }
+
         #endregion
 
         #region GroupByAggregateChainComposition
@@ -2523,7 +2553,8 @@ public virtual Task GroupBy_group_Distinct_Select_Distinct_aggregate(bool async)
                         g =>
                             new
                             {
-                                g.Key, Max = g.Distinct().Select(e => e.OrderDate).Distinct().Max(),
+                                g.Key,
+                                Max = g.Distinct().Select(e => e.OrderDate).Distinct().Max(),
                             }),
                 elementSorter: e => e.Key);
         }
@@ -2540,7 +2571,8 @@ public virtual Task GroupBy_group_Where_Select_Distinct_aggregate(bool async)
                         g =>
                             new
                             {
-                                g.Key, Max = g.Where(e => e.OrderDate.HasValue).Select(e => e.OrderDate).Distinct().Max(),
+                                g.Key,
+                                Max = g.Where(e => e.OrderDate.HasValue).Select(e => e.OrderDate).Distinct().Max(),
                             }),
                 elementSorter: e => e.Key);
         }

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

@@ -6336,5 +6336,31 @@ public virtual Task SkipWhile_throws_meaningful_exception(bool async)
                     async,
                     ss => ss.Set<Customer>().OrderBy(c => c.CustomerID).SkipWhile(c => c.CustomerID != "Foo").Skip(1)));
         }
+
+        [ConditionalTheory]
+        [MemberData(nameof(IsAsyncData))]
+        public virtual async Task Skip_0_Take_0_works_when_parameter(bool async)
+        {
+            await AssertQuery(
+                async,
+                ss => ss.Set<Customer>().OrderBy(c => c.CustomerID).Skip(0).Take(0));
+
+            await AssertQuery(
+                async,
+                ss => ss.Set<Customer>().OrderBy(c => c.CustomerID).Skip(1).Take(1),
+                entryCount: 1);
+        }
+
+        [ConditionalTheory]
+        [MemberData(nameof(IsAsyncData))]
+        public virtual Task Skip_0_Take_0_works_when_constant(bool async)
+        {
+            return AssertQueryScalar(
+                async,
+                ss => ss.Set<Customer>().Where(c => c.CustomerID.StartsWith("F"))
+                        .OrderBy(c => c.CustomerID)
+                        .Select(e => e.Orders.OrderBy(o => o.OrderID).Skip(0).Take(0).Any()),
+                assertOrder: true);
+        }
     }
 }

test/EFCore.SqlServer.FunctionalTests/Query/NorthwindGroupByQuerySqlServerTest.cs

@@ -2327,6 +2327,32 @@ GROUP BY [o0].[CustomerID]
 ) AS [t0] ON [t].[Key] = [t0].[Key]");
         }
 
+        public override async Task GroupBy_aggregate_after_skip_0_take_0(bool async)
+        {
+            await base.GroupBy_aggregate_after_skip_0_take_0(async);
+
+            AssertSql(
+                @"SELECT [t].[CustomerID] AS [Key], COUNT(*) AS [Total]
+FROM (
+    SELECT [o].[CustomerID]
+    FROM [Orders] AS [o]
+    WHERE 0 = 1
+) AS [t]
+GROUP BY [t].[CustomerID]");
+        }
+
+        public override async Task GroupBy_skip_0_take_0_aggregate(bool async)
+        {
+            await base.GroupBy_skip_0_take_0_aggregate(async);
+
+            AssertSql(
+                @"SELECT [o].[CustomerID] AS [Key], COUNT(*) AS [Total]
+FROM [Orders] AS [o]
+WHERE [o].[OrderID] > 10500
+GROUP BY [o].[CustomerID]
+HAVING 0 = 1");
+        }
+
         public override async Task GroupBy_with_grouping_key_using_Like(bool async)
         {
             await base.GroupBy_with_grouping_key_using_Like(async);

test/EFCore.SqlServer.FunctionalTests/Query/NorthwindMiscellaneousQuerySqlServerTest.cs

@@ -5300,6 +5300,40 @@ FROM [Employees] AS [e0]
 ) AS [t2]");
         }
 
+        public override async Task Skip_0_Take_0_works_when_parameter(bool async)
+        {
+            await base.Skip_0_Take_0_works_when_parameter(async);
+
+            AssertSql(
+                @"SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region]
+FROM [Customers] AS [c]
+WHERE 0 = 1",
+                //
+                @"@__p_0='1'
+
+SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region]
+FROM [Customers] AS [c]
+ORDER BY [c].[CustomerID]
+OFFSET @__p_0 ROWS FETCH NEXT @__p_0 ROWS ONLY");
+        }
+
+        public override async Task Skip_0_Take_0_works_when_constant(bool async)
+        {
+            await base.Skip_0_Take_0_works_when_constant(async);
+
+            AssertSql(
+                @"SELECT CASE
+    WHEN EXISTS (
+        SELECT 1
+        FROM [Orders] AS [o]
+        WHERE 0 = 1) THEN CAST(1 AS bit)
+    ELSE CAST(0 AS bit)
+END
+FROM [Customers] AS [c]
+WHERE [c].[CustomerID] LIKE N'F%'
+ORDER BY [c].[CustomerID]");
+        }
+
         private void AssertSql(params string[] expected)
             => Fixture.TestSqlLoggerFactory.AssertBaseline(expected);