-
Notifications
You must be signed in to change notification settings - Fork 380
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
Change return type of all EF.Functions.DateDiff
extension methods related to units of SECOND
or smaller from int
to long
#1875
Comments
Probably an oversight. @roji? |
As far as I can tell, the SQL Server EF.Functions.DateDiff* methods translate to DATEDIFF, not DATEDIFF_BIG (code); we seem to translate to DATEDIFF_BIG only when the user specifies DateTimeOffset.ToUnixTimeSeconds/ToUnixTimeMilliseconds. We could introduce EF.Functions.DateDiffBig* overloads - see dotnet/efcore#32278 which tracks this. All this seems to be in line with the general policy that stuff in EF.Functions corresponds directly to SQL functions (as much as possible)... FWIW the Pomelo provider deviates from this in having EF.Functions.DateDiff rather than EF.Functions.TimestampDiff. In any case, SQL Server and MySQL seem to be different on this, with SQL Server having two functions (DATEDIFF returning int and DATEDIFF_BIG returning bigint), and MySQL having just one TIMESTAMPDIFF returning a bigint. |
I vote for new |
@roji You're right, I did not look close enough. As for Pomelo, unless we want to duplicate the extension methods and then mark all the old ones as obsolete (which we could arguably do), it would be a bit late in the game for us to rename them now to mimic the original MySQL function names. We will probably leave it as is for now (its probably close enough), unless the community thinks a name change is worth it.
@moander If this comment is directed at the Pomelo implementations: We will change the return type of the methods to (Shouldn't be an issue, because the compiler should complain, if the types in existing code don't match anymore.) |
The value returned for
TIMESTAMPDIFF
function calls of valid dates can exceedInt32.MaxValue
, but currently, all ourEF.Functions.DateDiff
extension methods useint
as the return type.We should change this behavior to return
long
instead ofint
. At least for extension methods related to units ofSECOND
or smaller.@ajcvickers The SQL Server provider currently uses
int
as the return type for everything as well, even though it translates those calls toDATEDIFF_BIG
. An oversight or a conscious choice?The text was updated successfully, but these errors were encountered: