Implementation Translate DateDiff by ralmsdeveloper · Pull Request #10264 · dotnet/efcore

ralmsdeveloper · 2017-11-10T18:11:39Z

EF.Functions.DateDiff

var lstPeople = cxt
    .People.Where(p => EF.Functions.DateDiffDay(p.Birth, DateTime.Now) > 0 )
    .ToList();

var lstPeople = cxt
    .People.Where(p => EF.Functions.DateDiffYear(p.Birth, DateTime.Now) > 0 )
    .ToList();

SQL

SELECT [p].[PeopleId], [p].[Birth], [p].[FirstName]
FROM [People] AS [p]
WHERE DATEDIFF(DAY,[p].[Birth],GETDATE()) > 0

SELECT [p].[PeopleId], [p].[Birth], [p].[FirstName]
FROM [People] AS [p]
WHERE DATEDIFF(YEAR,[p].[Birth],GETDATE()) > 0

patricdeveloper · 2017-11-21T01:52:31Z

I really needed that!

smitpatel · 2017-11-21T02:00:31Z

Ref: #9585
6.ii
I discussed with @anpete about this. We arrived at conclusion that we want to provide method functionality similar to SqlMethods. Methods can be found here http://referencesource.microsoft.com/#System.Data.Linq/SqlClient/SqlMethods.cs,5a1e96cbe66f5bf1
Basically it would be DateDiff* methods so that we can provide an in memory implementation along with SQL translation for SqlServer.

smitpatel · 2017-11-21T02:01:05Z

+        /// <param name="dateStart">The start date</param>
+        /// <param name="dateEnd">The end date</param>
+        /// <param name="datePart">datePart</param>
+        public DateDiffExpression([NotNull] Expression dateStart, [NotNull] Expression dateEnd, [CanBeNull] Expression datePart)


There is no need to create DateDiffExpression.
Utilize existing SqlFunctionExpression

smitpatel

The methods should be similar to SqlMethods from Linq to SQL rather than inventing our own. Also put in memory implementation.

smitpatel · 2017-11-21T02:03:24Z

@patricdeveloper - See discussion at #10241 It specifies how to use DateDiff in current released version.

ralmsdeveloper · 2017-11-21T03:17:15Z

~~@smitpatel my initial idea was to translate something like this:~~

DateTime.Now.Subtract(DateTime.Now).TotalDays > 0

~~Using Subtract, we could implement an extension for year, month and .....~~
~~Why would we have this in memory and SQL.~~
~~What do you think about that too?~~

Forget it!

ralmsdeveloper

Done!

smitpatel · 2017-11-21T23:49:06Z

+        /// <param name="startDate">Starting date for the calculation.</param>
+        /// <param name="endDate">Ending date for the calculation.</param>
+        /// <returns>Number of year boundaries crossed between the dates.</returns>
+        public static int? DateDiffYear(


This should go to Core. like EFFunctions.Like
Also tests would go to specs tests and SqlServer would be only provider translating it.

ralmsdeveloper

@smitpatel Done!
I also changed the title to make it more pleasant. 😏

Updated.

smitpatel · 2017-11-22T03:37:15Z

@anpete - Can you take a look at the function introduced on EF.Functions to see if it is according to what is our plan.

patricdeveloper · 2017-11-22T18:38:44Z

@smitpatel
Thank you very much, it worked for me (#10241), but I do not think everyone wants to do this, especially people who need to use this for calculations.

This being the default in EF.Functions will be much better, especially for people who are now coming to .Net Core

smitpatel · 2017-11-22T18:41:12Z

@patricdeveloper - We understand #10241 is work-around only. Ideally we want to add quite a few useful functions which are used by many customers in EF.Functions for easier access. But till we reach there, UDFs are providing a very good work-around. Its not ideal solution but it basically unblock you.

anpete · 2017-11-27T18:38:58Z

+            DateTime? startDate,
+            DateTime? endDate)
+        {
+            if (startDate.HasValue && endDate.HasValue)


nit: ternary expression

anpete · 2017-11-27T18:39:27Z

+            DateTimeOffset? startDate,
+            DateTimeOffset? endDate)
+        {
+            if (startDate.HasValue && endDate.HasValue)


nit: ternary expression

Same for other methods further down

anpete · 2017-11-27T18:50:53Z

@ralmsdeveloper Looks good! Should we implement support for SQLite? It looks like at least something should be possible based on http://www.sqlite.org/lang_datefunc.html

cc @bricelam

ralmsdeveloper · 2017-11-27T18:58:12Z

Thanks @anpete .
I'm going to make these adjustments to SQL Server.

I took a look at http://www.sqlite.org/lang_datefunc.html, and I think it's possible to do that too!
I will analyze this later with more time.

bricelam · 2017-11-27T20:47:17Z

I wrote a lot of possible date and time translations for SQLite here once...

ralmsdeveloper · 2017-11-27T21:02:56Z

Thanks @bricelam, I looked and liked it, it will be very useful.
Once this PR is merged, I'll be doing these SQLite implementations, to let you all free, into more complex things 😄

ralmsdeveloper · 2017-11-29T22:21:54Z

@anpete Anything else I can do here?

anpete · 2017-11-29T22:28:47Z

I think we would prefer to get SQLite as part of this if possible. 😁

smitpatel · 2017-11-29T22:34:35Z

-        {
-            fixture.TestSqlLoggerFactory.Clear();
-        }
+            => fixture.TestSqlLoggerFactory.Clear();


Ok, @smitpatel, I did this to delete warnings, which appeared after the change in .editorconfig 😁

smitpatel

Follow exact translations from #4108 (comment)
With only additional change of explicit cast if its needed.
No translation related to DateTimeOffset on SQLite.

smitpatel · 2017-12-05T19:08:39Z

+
+                Assert.Equal(0, count);
+            }
+        }


Add test for DateDiffNanosecond

smitpatel · 2017-12-05T19:09:15Z

+            { typeof(DateTime).GetRuntimeMethod(nameof(DateTime.AddHours), new[] { typeof(double) }), "hours" },
+            { typeof(DateTime).GetRuntimeMethod(nameof(DateTime.AddMinutes), new[] { typeof(double) }), "minutes" },
+            { typeof(DateTime).GetRuntimeMethod(nameof(DateTime.AddSeconds), new[] { typeof(double) }), "seconds" }
+            // TODO: Future implementation, how to do the translation de TIME ZONE?


Remove these comments. Till we actually decide to support it, there is not much value putting it here.

smitpatel · 2017-12-05T19:13:59Z

+        private static readonly string _sqliteUtc = "'utc'";
+
+        private static readonly MethodInfo _concatExpression
+            = typeof(string).GetRuntimeMethod(nameof(string.Concat), new[] { typeof(Expression), typeof(string) });


No method of string exists which takes expression & string params. It would match with object one instead.

Please do not look at this yet ...

smitpatel · 2017-12-05T19:15:23Z

+
+                if (firstArgument.NodeType == ExpressionType.Convert)
+                {
+                    firstArgument = new ExplicitCastExpression(firstArgument, typeof(string));


Why do we need explicit cast here? Convert to string should be fine. in SQL it will be string value anyway. And we may need convert to align types for add.

smitpatel · 2017-12-05T19:15:41Z

+                var concatDateAdd =
+                    firstArgument.NodeType == ExpressionType.Extension
+                        ? Expression.Add(firstArgument, new SqlFragmentExpression($"' {datePart}'"), _concatExpression)
+                        : (Expression)new SqlFragmentExpression($"'{firstArgument.ToString()}  {datePart}'");


firstArgument.ToString() is big NO.

smitpatel · 2017-12-05T19:32:04Z

+
+            if (_methodInfoDateDiffMapping.TryGetValue(methodCallExpression.Method, out var datePartCalculate))
+            {
+                return new ExplicitCastExpression(


Would these DateTimeOffset based methods be translated? @anpete @bricelam

smitpatel · 2017-12-05T19:33:04Z

+        public virtual Expression Translate(MemberExpression memberExpression)
+        {
+            if (memberExpression.Expression != null
+                && (memberExpression.Member.Name == nameof(DateTime.Date)


avoid this check. Switch is already doing that.

smitpatel · 2017-12-05T19:33:55Z

+        {
+            switch (memberName)
+            {
+                case nameof(DateTime.Year):


make this a dictionary and use trygetvalue. You can do similar change in sqlserver if you want.

smitpatel · 2017-12-05T19:34:47Z

+    /// </summary>
+    public class SqliteDateTimeNowTranslator : IMemberTranslator
+    {
+        private static string _sqliteFormatDate = "'%Y-%m-%d %H:%M:%S'";


@bricelam - Can you check what should be the full date format since it is different from what we generate in SQL for SQLite.

smitpatel · 2017-12-05T19:42:31Z

+                return new ExplicitCastExpression(
+                    Expression.Divide(
+                        Expression.Subtract(
+                            new SqlFunctionExpression(


Translate them exactly how this sql is formed. #4108 (comment)

@smitpatel, but that's what's being done.
This comment was edited in a few minutes, the CAST I had already implemented.
If you look at: #10264 (comment)

This is already being done. The translation is being done.

The current implementation (at the time of reviewing) for date diff was finding seconds for 2 dates and subtracting them diving by some number to change units.
like (Date1.Seconds - Date2.Seconds)/ (const to convert to year) to find out DateDiffYear. But client side code is Date1.Year - Date2.Year
The translation should be same for server/client both as much as possible.
So translation becomes
Cast(strftime("%Y", date1) as int) - Cast(strftime("%Y", date2) as int)

@smitpatel,
For this case of year and second, do only one subtraction, more for the rest, of course, has to be done calculation!

the output for sql, is good now!

--DateDiffDay SELECT "p"."Id", "p"."Data" FROM "Test" AS "p" WHERE CAST((strftime('%s', "p"."Data") - strftime('%s', strftime('%Y-%m-%d %H:%M:%S', 'now', 'localtime'))) / (60 * 60 * 24) AS INTEGER) > 0 --DateDiffMonth SELECT "p"."Id", "p"."Data" FROM "Test" AS "p" WHERE CAST((strftime('%s', "p"."Data") - strftime('%s', strftime('%Y-%m-%d %H:%M:%S', 'now', 'localtime'))) / (60 * 60 * 24 * 365/12) AS INTEGER) > 0 --DateDiffYear SELECT "p"."Id", "p"."Data" FROM "Test" AS "p" WHERE (CAST(strftime('%Y', "p"."Data") AS INTEGER) - CAST(strftime('%Y', strftime('%Y-%m-%d %H:%M:%S', 'now', 'localtime')) AS INTEGER)) > 0 --DateDiffSecond SELECT "p"."Id", "p"."Data" FROM "Test" AS "p" WHERE (CAST(strftime('%S', "p"."Data") AS INTEGER) - CAST(strftime('%S', strftime('%Y-%m-%d %H:%M:%S', 'now', 'localtime')) AS INTEGER)) > 0

There is still mismatch between client version and server output.
If you like then I can build on the top of your PR to implement SQLite translators.

@smitpatel,
My intention was for everything to be perfect.
I did some basic tests and it works for me, I do not see inconsistency, could you guide me?
And yes, feel free to complement the PR!

For example lets take dates
`> var date1 = new DateTime(2016, 2, 27);

var date2 = new DateTime(2016, 3, 1);`

The DateDiffMonth on client side is 12 * (date1.Year - date2.Year) + date1.Month - date2.Month Which would give us result of -1. There is difference of -1 month as we can see there.

When converting this using %s and dividing by no. of seconds in a month. we generate this SQL

sqlite> select (CAST (strftime("%s", "2016-02-27 00:00:00.0000000") AS REAL) - CAST (strftime("%s", "2016-03-01 00:00:00.0000000") AS REAL))/(60 * 60 * 24 * 365 / 12);

Which gives us answer as -0.0986301369863014

Both answers are far apart because in first case even though the dates are 3 days apart only month changes and our client method just consider months. Nothing else.
So if convert everything to seconds and start using it on server then we would be generate results which would change based on client/server eval. Since translation is something which we are introducing we should align the results to make it deterministic.

I believe the client implementation is taken from SqlMethods class from Linq2Sql so we would preserve it and write our translation accordingly.
So above would generate in sql following

select 12 * (CAST (strftime("%Y", "2016-02-27 00:00:00.0000000") AS INT) - CAST (strftime("%Y", "2016-03-01 00:00:00.0000000") AS INT)) + CAST (strftime("%m", "2016-02-27 00:00:00.0000000") AS INT) - CAST (strftime("%m", "2016-03-01 00:00:00.0000000") AS INT)

It is somewhat ugly but it aligns us with client.

If you look at the translation then it is basically putting pieces together what client would have done otherwise using MemberTranslator.

I wanted to do this, but I thought it was strange.
More really that's the way!
@smitpatel thank you very much, I am adapting ...

ralmsdeveloper · 2017-12-05T20:04:09Z

@smitpatel, thanks for looking.
I'll check

ralmsdeveloper · 2017-12-06T02:00:56Z

ping: @smitpatel "ping"🙈

this will not work, when I pass an expression that needs an integer return on the client side.

new SqlFunctionExpression(
                "strftime",
                typeof(string),
                new Expression[] {
                    new SqlFragmentExpression("%d"),
                    new SqlFunctionExpression(
                        "strftime",
                        typeof(string),
                        new Expression[]{
                            new SqlFragmentExpression("%Y-%m-%d %H:%M:%S"),
                            /*date,*/
                            Expression.Add(
                                new SqlFunctionExpression(
                                    "strftime",
                                    typeof(string),
                                    new []
                                    {
                                        new SqlFragmentExpression("%d")
                                        /*date,*/
                                    }),
                                new SqlFragmentExpression(" days"))
                        })
                });

Example:

public virtual Expression Translate(MethodCallExpression methodCallExpression)
{
    if (_methodInfoDatePartMapping.TryGetValue(methodCallExpression.Method, out var datePart))
    {
        var firstArgument = methodCallExpression.Arguments[0];
        ....
       Expression.Add(
           new SqlFunctionExpression(
            "strftime",
            typeof(string),
            new []
             {
                 new SqlFragmentExpression("%d")
                  firstArgument     <<<<<<--------------
                  }),
            new SqlFragmentExpression(" days"));
       ...
    }
    return null;
}

Why is my variable firstArgument:
CONVERT (CAST (strftime (System.Collections.ObjectModel.ReadOnlyCollection`1 [System.Linq.Expressions.Expression] AS Int32))

That will cause an exception of the type:
System.InvalidOperationException: 'The Binary Add operator is not defined for the' System.String 'and' System.Object 'types.'

That's why I created the ExplicitConcatExpression class, to make CAST suitable for that!

smitpatel · 2017-12-06T02:03:45Z

Can you post what is exactly client side linq query? That will give me idea of what is MethodCallExpression.

ralmsdeveloper · 2017-12-06T02:32:16Z

if I do this:
db.Test.Where (p => p.Data.AddDays (p.Data.Day).Day> 10) .ToList ();

The MethodCallExpression method will automatically contain an expression whose return is integer.

Therefore, it is not possible to concatenate with "days" (methodCallExpression.Arguments [0] || days).

Do you understand what I want to explain?
So I have this in development, why does this work. It's more of a cast, but it works.

public virtual Expression Translate(MethodCallExpression methodCallExpression)
{
    if (_methodInfoDatePartMapping.TryGetValue(methodCallExpression.Method, out var datePart))
    {
        var firstArgument = methodCallExpression.Arguments[0];

        //hack - That's not what we will use !!!!
        if (firstArgument.NodeType == ExpressionType.Convert)
        {
            // This is just to run with Expression.Add, and this works!
            // Until I or you have a better idea.
            firstArgument = new ExplicitCastExpression(firstArgument, typeof(string));
        }

        var expressionAdd = firstArgument.NodeType == ExpressionType.Extension
            ? Expression.Add(firstArgument, new SqlFragmentExpression($"' {datePart}'"), _concat)
            : (Expression)new SqlFragmentExpression($"'{firstArgument} {datePart}'");

        return new SqlFunctionExpression(
            functionName: _sqliteFunctionDateFormat,
            returnType: methodCallExpression.Type,
            arguments: new[]
            {
                 new SqlFragmentExpression(_sqliteFormatDate),
                  methodCallExpression.Object,
                 expressionAdd
            });
    }

    return null;
}

smitpatel · 2017-12-06T03:34:34Z

While p.Data.Day is of type int in CLR world, once you translate it to server it becomes
strftime("%d", "p"."Data") which is of type string (strftime returns string).

so in the essence you would translate p.Data.Day as
ExplicitCast(SqlFunction(strftime, string,...), typeof(int)) generating sql with CAST.
When you arrive in this visitor, since you need string type parameter, you can unwrap the explicit cast expression to get the inner string operation and then you can easily concat it with " days".

Essentially, we would explicit cast only when materializing value because we need to read the value as numeric type instead of string but for all internal operations are in string so we can unwrap explicit cast.

translated sql for above would be
CAST(strftime("%d", strftime("%Y-%m-%d %H:%M:%f", "p"."Data", (strftime("%d", "p"."Data") || " days"))) AS INTEGER) > 10

ralmsdeveloper · 2017-12-06T15:26:08Z

@smitpatel
I think we have something nicer now!

SELECT "p"."Id", "p"."Date" 
FROM "Test" AS "p"
WHERE CAST(strftime('%d', strftime('%Y-%m-%d %H:%M:%S', "p"."Date", strftime('%d', "p"."Date") || ' days')) AS INTEGER) > 1

SELECT "p"."Id", "p"."Date"
FROM "Test" AS "p"
WHERE CAST(strftime('%d', strftime('%Y-%m-%d %H:%M:%S', "p"."Date", '1 days')) AS INTEGER) > 1

And for the month DataDiff!

SELECT COUNT(*)
FROM "Orders" AS "c"
WHERE (12 * ((CAST(strftime('%Y', "c"."OrderDate") AS INTEGER) - CAST(strftime('%Y', strftime('%Y-%m-%d %H:%M:%S', 'now', 'localtime')) AS INTEGER)) + (CAST(strftime('%m', "c"."OrderDate") AS INTEGER) - CAST(strftime('%m', strftime('%Y-%m-%d %H:%M:%S', 'now', 'localtime')) AS INTEGER)))) = 0

smitpatel · 2017-12-06T18:12:31Z

@ralmsdeveloper - Can you rebase on latest dev and squash all the commits into one? I will update the SQLite implementation on top of this PR preserving your contribution.

ralmsdeveloper · 2017-12-06T22:11:03Z

Ping: @smitpatel
Updated, I did a little better in the calculation to avoid unnecessary multiplication!

smitpatel · 2017-12-08T21:37:34Z

@ralmsdeveloper - Can you rebase this on latest dev and squash all commits into one? I will update SQLite implementation.

ralmsdeveloper · 2017-12-08T22:23:01Z

Yes, I'll do it now!

* Suporte DateDiff para SQL Server * Suporte DateDiff para SQLite

* DateDiff Support for SQL Server * DateDiff support for SQLite

smitpatel · 2017-12-09T00:16:32Z

@ralmsdeveloper - Please do no work any longer in this PR. It would cause merge conflicts for me on rebase. I will fix if there are any tests issues. I am working on this right now.

smitpatel · 2017-12-09T02:43:11Z

Closing this in favour of #10528
@ralmsdeveloper - I have preserved your commit in that PR for credits. Thank you for contribution.

dnfclas added the cla-already-signed label Nov 10, 2017

ghost removed the cla-already-signed label Nov 14, 2017

ghost deleted a comment from dnfclas Nov 14, 2017

smitpatel reviewed Nov 21, 2017

View reviewed changes

smitpatel previously requested changes Nov 21, 2017

View reviewed changes

ralmsdeveloper force-pushed the Dev1989 branch from 06c6510 to ac635ca Compare November 21, 2017 21:59

ralmsdeveloper commented Nov 21, 2017

View reviewed changes

smitpatel reviewed Nov 21, 2017

View reviewed changes

ralmsdeveloper force-pushed the Dev1989 branch from d90706e to aebfc93 Compare November 22, 2017 00:47

ralmsdeveloper changed the title ~~Implementation Translate DateDiff for SQL Server~~ Implementation Translate DateDiff Nov 22, 2017

ralmsdeveloper commented Nov 22, 2017

View reviewed changes

ajcvickers assigned anpete Nov 22, 2017

anpete reviewed Nov 27, 2017

View reviewed changes

ralmsdeveloper force-pushed the Dev1989 branch from aebfc93 to 7d37a15 Compare November 27, 2017 19:33

smitpatel reviewed Nov 29, 2017

View reviewed changes

smitpatel suggested changes Dec 5, 2017

View reviewed changes

ralmsdeveloper force-pushed the Dev1989 branch from 03ca5ad to 4238487 Compare December 6, 2017 21:50

smitpatel assigned smitpatel and unassigned anpete Dec 7, 2017

ralmsdeveloper force-pushed the Dev1989 branch from 4238487 to 109ad94 Compare December 8, 2017 22:44

ralmsdeveloper added 9 commits December 8, 2017 21:49

Implementation Translate DateDiff

6726633

* Suporte DateDiff para SQL Server * Suporte DateDiff para SQLite

Fix type localtime SQLite

d5b48f0

Fix and cleaning code

0c5f38d

Remove Skip

10cc965

Adjustments for analysis

61925d5

More adjustments and cleaning!

6f7986e

More test and cleaning!

bd6dd4b

More cleaning and tweaking!

ca37dc0

Implementation Translate DateDiff

f970bb7

* DateDiff Support for SQL Server * DateDiff support for SQLite

ralmsdeveloper force-pushed the Dev1989 branch from 109ad94 to f970bb7 Compare December 8, 2017 23:54

Implementation Translate DateDiff

754ecb4

* DateDiff Support for SQL Server * DateDiff support for SQLite

smitpatel closed this Dec 9, 2017

ralmsdeveloper deleted the Dev1989 branch January 13, 2020 14:00

Conversation

ralmsdeveloper commented Nov 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

patricdeveloper commented Nov 21, 2017

Uh oh!

smitpatel commented Nov 21, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smitpatel left a comment

Choose a reason for hiding this comment

Uh oh!

smitpatel commented Nov 21, 2017

Uh oh!

ralmsdeveloper commented Nov 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ralmsdeveloper left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ralmsdeveloper left a comment

Choose a reason for hiding this comment

Uh oh!

smitpatel commented Nov 22, 2017

Uh oh!

patricdeveloper commented Nov 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

smitpatel commented Nov 22, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anpete commented Nov 27, 2017

Uh oh!

ralmsdeveloper commented Nov 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bricelam commented Nov 27, 2017

Uh oh!

ralmsdeveloper commented Nov 27, 2017

Uh oh!

ralmsdeveloper commented Nov 29, 2017

Uh oh!

anpete commented Nov 29, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smitpatel left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

ralmsdeveloper commented Nov 10, 2017 •

edited

Loading

ralmsdeveloper commented Nov 21, 2017 •

edited

Loading

patricdeveloper commented Nov 22, 2017 •

edited

Loading

ralmsdeveloper commented Nov 27, 2017 •

edited

Loading

ralmsdeveloper commented Dec 6, 2017 •

edited

Loading