Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Query: TimeSpan with DateTime arithmetic operations are not supported #6025

Closed
AsValeO opened this issue Jul 8, 2016 · 15 comments
Closed

Query: TimeSpan with DateTime arithmetic operations are not supported #6025

AsValeO opened this issue Jul 8, 2016 · 15 comments
Labels
closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed. type-enhancement

Comments

@AsValeO
Copy link

AsValeO commented Jul 8, 2016

Edit by @divega:

Fixing this issue is not necessarily about fixing the overflow or type inference problems we hit immediately with the different expressions in this bug but about finding translations of those expressions that actually work. Likely when we find expressions like this we could transform at least some of them into a combination of DateTime.Add and DateTime.Substract, client evaluation, etc., that don't even require creating parameters of type TimeSpan.

Original customer report

The issue

After updating from RC1 this issue with TimeSpan and SqlDbType.Time appeared.
Building.AddDate type is datetime in Azure SQL DB, DateTime in generated entity class.

using (var db = new MyContext())
            {
                var bs = await db.Building.AsNoTracking()
                    .Where(
                        x =>
                            DateTime.Now - x.AddDate > TimeSpan.FromDays(30)).ToListAsync();
            }

Exception message: SqlDbType.Time overflow. Value '30.00:00:00' is out of range. Must be between 00:00:00.0000000 and 23:59:59.9999999.

Further technical details

DB: Azure SQL DB
EF Core version: 1.0.0
Operating system: Windows Server 2012
Visual Studio version: 2015U3

@ajcvickers
Copy link
Member

Note for triage: It seems this was also a problem with the old stack, and from a little searching it doesn't seem like there is an obviously better type mapping.

@ajcvickers
Copy link
Member

Just found this on the backlog: #770

@rowanmiller
Copy link
Contributor

@divega we want to discuss this with you when you are back

@rowanmiller rowanmiller added this to the 1.1.0 milestone Jul 8, 2016
@rowanmiller rowanmiller modified the milestones: 1.2.0, 1.1.0-preview1 Oct 6, 2016
@ajcvickers ajcvickers modified the milestones: 2.0.0-preview1, 2.0.0 Apr 19, 2017
@AsValeO
Copy link
Author

AsValeO commented Jun 17, 2017

EFC 2.0 RC

q = q.Where(x => x.Added + TimeSpan.FromDays(3) >= DateTime.Now);

Failed to convert parameter value from a TimeSpan to a DateTime.
Object must implement IConvertible.
Type: InvalidCastException

Well, year has passed, still can't work with TimeSpan. It worked in EFC 1.0 RC1.
I think this is nessesary feature for EFC 2.0.
Or is there at least any workaround here?

@divega divega removed this from the 2.0.0 milestone Jun 18, 2017
@divega
Copy link
Contributor

divega commented Jun 18, 2017

Clearing up milestone do that we can discuss in triage.

@ajcvickers
Copy link
Member

Notes for triage: Looks like a couple of different things are happening here. In original bug, TimeSpan was being converted to Time, but the value overflowed. This is somewhat expected since the mapping for TimeSpan is to Time, but Time can easily overflow. This is discussed in #242 for mapped properties.

The new exception is different--it indicates that query is attempting to create a DateTime parameter but using a TimeSpan object. SqlClient cannot handle this and so throws. I suspect this is happening due to type inference in the query pipeline, but I haven't verified this.

So, fundamentally, this issue is not about the general mapping of TimeSpan, which is covered by #242, but instead is about what query should do when translating this either in terms of creating parameters of the "correct" type (which can overflow) or doing some more complex translation which avoids this.

@AsValeO As a workaround, you can force client evaluation of the query:

q = q.ToList().Where(x => x.Added + TimeSpan.FromDays(3) >= DateTime.Now);

Before RC1 client evaluation was likely happening all the time, which is why it appeared to be "working".

@AndriySvyryd
Copy link
Member

Confirming @ajcvickers findings. The second exception is from a query bug, but even if fixed it would result in the first exception anyway.

@AndriySvyryd AndriySvyryd removed their assignment Jun 21, 2017
@AndriySvyryd AndriySvyryd removed this from the 2.0.0 milestone Jun 21, 2017
@AndriySvyryd
Copy link
Member

AndriySvyryd commented Jun 21, 2017

Another workaround is to calculate the deadline on the client:

var deadline = DateTime.Now + TimeSpan.FromDays(3);
q = q.Where(x => x.Added >= deadline);

But both of these workarounds suffer from DateTime.Now being evaluated on the client instead of the server, which might lead to different results.

@ajcvickers ajcvickers added this to the 2.0.0 milestone Jun 23, 2017
@smitpatel smitpatel removed this from the 2.0.0 milestone Jun 26, 2017
@ajcvickers ajcvickers added this to the 2.0.0 milestone Jun 26, 2017
@smitpatel
Copy link
Member

smitpatel commented Jun 26, 2017

Since + is overloaded in DateTime which uses internal code to computation the new DateTime & there is no direct translation available in SqlServer. (+ is not defined for datetime & time value).

The best work around to achieve this in SqlServer would be to use DateTime.Add* functions instead of TimeSpan. The query in first post can be re-written as follows to give same result.

var bs = await db.Building.AsNoTracking()
    .Where(
        x => DateTime.Now > x.AddDate.AddDays(30)).ToListAsync();

If you are using different TimeSpan.From* function then find appropriate match in DateTime.Add* for that. EF Core translates DateTime.Add* functions to server.

It will also avoid issue of overflow.

@smitpatel
Copy link
Member

There are multiple issues around this area.

  • The second exception comes from query because we generate local parameter of TimeSpan but when creating DbParameter we use inferred typemapping from the other column which is DateTime. And parameter of DateTime cannot be created with value of TimeSpan. Generally this error is not encountered because the types of both nodes of BinaryExpression are same. In this rare case + is overloaded with 2 different types of args. Hence the error.
  • If we make query pipeline to handle above issue then we hit the overflow issue as in very first post. So Adding TimeSpan may not always work.
  • Even if the value does not cause the overflow, the + operator on SqlServer does not work on data types datetime & time. Hence there is no direct SqlServer translation available for this method. In future we can transform when this method is encountered to something translatable but it would be complex & prone to loss of data.

Closing the issue as the work-around translate to server & also avoid overflow issue. We can address client eval or future translation if there is enough customer feedback.

@smitpatel
Copy link
Member

@divega - Can you put appropriate labels for this issue? I am not sure what is correct one.

@divega divega changed the title TimeSpan to SqlDbType.Time exception Query: TimeSpan arirthmetic with DateTime is not supported Jun 26, 2017
@divega divega changed the title Query: TimeSpan arirthmetic with DateTime is not supported Query: TimeSpan with DateTime arithmetic operations are not supported Jun 26, 2017
@divega divega modified the milestones: Backlog, 2.0.0 Jun 26, 2017
@divega
Copy link
Contributor

divega commented Jun 26, 2017

@smitpatel I found myself copying the majority of the contents of the issue. I ended up repurposing it instead. Feel free to improve the title or the first comment.

@Ciantic
Copy link

Ciantic commented Jul 15, 2017

I throw my obvious suggestion here too:

Map it like this: TimeSpan.FromSeconds(VALUE FROM DATABASE) and when storing throw this in the DB TimeSpan.TotalSeconds. it should cover years.

It would not cover microseconds but I think most often TimeSpans need values greater than 24 hours, than precision of microseconds.

And when TimeSpan is just a integer of seconds, the queries are easier to compose I think.

@ajcvickers
Copy link
Member

@smitpatel to comment.

@ajcvickers ajcvickers removed this from the Backlog milestone Nov 15, 2019
@smitpatel
Copy link
Member

Filed #18939 for subset of these we can actually translate. Closing this issue as won't fix.

@smitpatel smitpatel added closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed. and removed area-query propose-close labels Nov 16, 2019
@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed. type-enhancement
Projects
None yet
Development

No branches or pull requests

7 participants