Query: Generate parameter for member accesses on enclosing class

[azabluda]

The first method doesn't differ much from the second one, yet it passes whilst the other one fails. The error message shows no connection to the new stricter LINQ translation approach of 3.0, which is probably not the case here anyway.

[TestClass]
public class UnitTest1
{
    public DateTime CurrentDate => DateTime.Now;

    [TestMethod]
    public void TestMethod_PASS()
    {
        using (var context = new NorthwindContext())
        {
            context.Orders.Where(o => o.OrderDate < DateTime.Now).ToList();
        }
    }

    [TestMethod]
    public void TestMethod_FAIL()
    {
        using (var context = new NorthwindContext())
        {
            context.Orders.Where(o => o.OrderDate < CurrentDate).ToList();
        }
    }

    public class Order
    {
        public int OrderID { get; set; }

        public DateTime OrderDate { get; set; }
    }

    public class NorthwindContext : DbContext
    {
        public DbSet<Order> Orders { get; set; }

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
            =>  optionsBuilder.UseSqlServer("Data Source=(localdb)\\MSSQLLocalDB; Database=Northwind; Integrated Security=True");
    }
}

Test Name:	TestMethod_FAIL
Test Outcome:	Failed
Result StackTrace:	
at Microsoft.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at Microsoft.Data.SqlClient.SqlInternalConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at Microsoft.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, Boolean callerHasConnectionLock, Boolean asyncClose)
   at Microsoft.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady)
   at Microsoft.Data.SqlClient.SqlDataReader.TryHasMoreRows(Boolean& moreRows)
   at Microsoft.Data.SqlClient.SqlDataReader.TryReadInternal(Boolean setTimeout, Boolean& more)
   at Microsoft.Data.SqlClient.SqlDataReader.Read()
   at Microsoft.EntityFrameworkCore.Storage.RelationalDataReader.Read()
   at Microsoft.EntityFrameworkCore.Query.RelationalShapedQueryCompilingExpressionVisitor.QueryingEnumerable`1.Enumerator.MoveNext()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at UnitTestProject1.UnitTest1.TestMethod_FAIL()
Result Message:	
Test method UnitTestProject1.UnitTest1.TestMethod_FAIL threw exception: 
Microsoft.Data.SqlClient.SqlException: Conversion failed when converting date and/or time from character string.

Further technical details

EF Core version: 3.0.0-preview8.19405.11
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Windows 10
IDE: Visual Studio Professional 2019 16.2.2

[azabluda]

Two remarks:

1. TestMethod_FAIL() translates to SQL without parameter binding, which seems to be a regression from 2.x.

2. The default store type mapping for DateTime is apparently datetime2 (with the longer literal '2019-01-01T00:00:00.0000000' vs '2019-01-01T00:00:00.000'). This caused the a.m. SqlException in my legacy database. Where can I read more about the default type mapping conventions for SqlServer provider?

[smitpatel]

https://docs.microsoft.com/en-us/ef/core/modeling/relational/data-types

If you have a data type in database which is different from default then you have to manually configure it to tell EF which data type to use. Once you specify correct one, query would work automatically.

[azabluda]

Thanks for the link. This is where I actually learned about the default "DateTime => datetime2" mapping, yet I was hoping for a more comprehensive mapping table listing all data types.

Agree, with the manual mapping it works, but the generated non-parameterized query is still sub-optimal (or at least unreasonably different from 2.x).

[smitpatel]

the generated non-parameterized query is still sub-optimal (or at least unreasonably different from 2.x).

Can you also share generated queries and explain why it is sub-optimal?

[azabluda]

With manual mapping of OrderDate to 'datetime' my TestMethod_FAIL no longer fails, but the generated queries are different depending on the EF version

-- 3.0.0-preview8.19405.11
SELECT [o].[OrderID], [o].[OrderDate]
FROM [Orders] AS [o]
WHERE [o].[OrderDate] < '2019-08-15T18:26:13.227'

-- 2.2.6
SELECT [o].[OrderID], [o].[OrderDate]
FROM [Orders] AS [o]
WHERE [o].[OrderDate] < @__CurrentDate_0

I'd say I'd rather prefer the old variant according to https://www.red-gate.com/simple-talk/sql/t-sql-programming/performance-implications-of-parameterized-queries/

When a parameterized query is used, SQL Server can maintain just one execution plan in its plan cache and use it over and over again for different values supplied to this statement, just like a stored procedure. From a performance and resource utilization perspective, this approach is much more economical.

[smitpatel]

If you use local variable from closure then it would generate parameter since you can change the value of variable in local closure. When constants appear in expression (other than constant of closure), then there is no way to change value in expression tree hence we inline them.

[azabluda]

Yes, I also noticed that with local variables it works as before (sounds like a workaround yet quite painful considering the amount of queries to revisit).

Just out of curiosity why had it been decided to translate a property expression to @__CurrentDate_0 in 2.x? Is the same reasoning not applicable for 3.x?

[smitpatel]

In 2.x, we used Remotion.Linq library which did some processing around it so we never get to see actual expression tree in that way. As it stands in 3.0, there is no way to identify such difference inside expression tree to generate parameter. The only thing is possible is to generate parameter for every constant we use but that may not be really useful.

[azabluda]

I see. In my eyes though this sudden change in behavior may have implications serious enough for being at least worth mentioning on the "breaking changes" list.

Of course the impact depends on the actual code, but who knows how many programs out there are using properties in LINQ queries today? Database servers may suffer greatly if they suddenly get bombarded with zillions of SQL queries which only differ in some excessively inlined values.

I'd rather say everything that isn't a hard-coded expression like Where(o => o.OrderDate < new DateTime(2019, 1, 1)) should generate a parameter. Not sure if reverting the default behavior back closer to 2.x is possible as the netcore3 release is approaching soon, yet I kindly ask you to raise this topic on a team meeting when you have time. Thanks!