Add AtTimeZoneExpression and some relevant translations

[roji]

• Adds AtTimeZoneExpression support to relational, along with support in QuerySqlGenerator, SqlNullabilityProcessor.
• Adds SQL Server-specific EF.Functions.AtTimeZone for datetimeoffset -> datetimeoffset and for datetime/datetime2 -> datetimeoffset

Closes #26199
Closes #26971

[smitpatel]

Build failing.

TimeZoneInfo.ConvertTimeBySystemTimeZoneId

Does string timezone identifier C# always matches the Server side strings? If not, we should add translation for this. EF.Functions one is fine since it corresponds to server always.

[roji]

Does string timezone identifier C# always matches the Server side strings? If not, we should add translation for this. EF.Functions one is fine since it corresponds to server always.

Here's what the docs say:

This method retrieves information on the time zone whose identifier is specified by the destinationTimeZoneId parameter from the registry on Windows systems and from the ICU Library on Linux and macOS.

So the interpretation of that string on .NET is platform-dependent, based on whether on Windows or Linux - so even in pure .NET this parameter isn't cross-platform (i.e. no fixed meaning). I think the method corresponds well for this translation (I've also done this for PG), but I can change to EF.Functions if you think that's better.

[bricelam]

🤔 I wonder if we should translate when the string is localtime or utc on SQLite. Probably a bit of a stretch...

.NET	SQL
EF.Functions.AtTimeZone(date, "localtime")	datetime($date, 'localtime')
EF.Functions.AtTimeZone(date, "utc")	datetime($date, 'utc')

[roji]

Don't forget there's DateTime.ToUniversalTime and ToLocalTime, should we just translate those? That seems like a pretty good fit if SQLite does the right thing...

Otherwise if we have to do it on EF.Functions, I'd go with two different functions rather than a string parameter (e.g. AtUtc, AtLocalTime)

[roji]

@smitpatel updated PR ready for review

[stevendarby]

Does this function work when the time zone is a column, and if so is it worth adding a test for that? I read a good blog on storing time zones in the database... 😃

[sommmen]

Any reason this is still not merged in or?

[smitpatel]

Should we assign typemapping here or should it be inferred from outside (if this is top level we assign default anyway).
Hypothetical scenario
a + (b + c) where c is our above translation and a is column with type mapping. If we assign type mapping here then b will take default from here rather than b/c taking type mapping from a which could be more appropriate.

[roji]

I remember thinking about this... The thing is, the AT TIME ZONE construct is documented very clearly to return a SQL Server datetimeoffset, so I'm leaning towards encoding that in the translation rather than allowing inference.

Here's an opposite scenario: .Where(b => EF.Functions.AtTimeZone(b.ColumnWithTypeMapping, "UTC") == someParameter. In this case someParameter has no type mapping, but I think we'd want it to be inferred to datetimeoffset based on the result of AtTimeZone on the other side...

But am fine either way if you want to leave unassigned and allow inference - let me know. I guess it's a general question when translating to functions which have a single, documented return type...

[smitpatel]

The scenario you mentioned is comparison so if neither side has type mapping then it will get default one for the type. expression.Type is DateTimeOffset and default of that we need to assign then inference would work fine for that.
Isn't datetimeoffset time on SqlServer has different precision or is it datetime thing only?

[roji]

Good questions.

Yeah, datetimeoffset does support precision and scale (docs); and it bubbles up those facets from the operand (for both datetime2 and datetimeoffset operands).

So if I'm getting things right, that means that if the operand has a type mapping (column), we should bubble it up (and for datetime/datetime2 mapping, bubble up the facets and return the appropriate datetimeoffset mapping. But if the operand has no mapping, we shouldn't set a result mapping to allow it to be inferred from the outside.

Does that sound good? If so I'll implement that.

[roji]

For reference, here's the T-SQL with the investigation:

-- Default precision/scale for datetimeoffset is 34, 7
SELECT SQL_VARIANT_PROPERTY(CAST('2020-01-01T12:00:00+02:00' AS datetimeoffset), 'BaseType'); -- datetimeoffset
SELECT SQL_VARIANT_PROPERTY(CAST('2020-01-01T12:00:00+02:00' AS datetimeoffset), 'Precision'); -- 34
SELECT SQL_VARIANT_PROPERTY(CAST('2020-01-01T12:00:00+02:00' AS datetimeoffset), 'Scale'); -- 7

-- datetimeoffset with non-default precision/scale
SELECT SQL_VARIANT_PROPERTY(CAST('2020-01-01T12:00:00+02:00' AS datetimeoffset(2)), 'BaseType'); -- datetimeoffset
SELECT SQL_VARIANT_PROPERTY(CAST('2020-01-01T12:00:00+02:00' AS datetimeoffset(2)), 'Precision'); -- 29
SELECT SQL_VARIANT_PROPERTY(CAST('2020-01-01T12:00:00+02:00' AS datetimeoffset(2)), 'Scale'); -- 2

-- AT TIME ZONE with datetimeoffset operand - bubbles up precision/scale
SELECT SQL_VARIANT_PROPERTY(CAST('2020-01-01T12:00:00+02:00' AS datetimeoffset(2)) AT TIME ZONE 'UTC', 'Precision'); -- 29
SELECT SQL_VARIANT_PROPERTY(CAST('2020-01-01T12:00:00+02:00' AS datetimeoffset(2)) AT TIME ZONE 'UTC', 'Scale'); -- 2

-- AT TIME ZONE with datetimeoffset operand - bubbles up precision/scale
SELECT SQL_VARIANT_PROPERTY(CAST('2020-01-01T12:00:00' AS datetime2(2)) AT TIME ZONE 'UTC', 'Precision'); -- 29
SELECT SQL_VARIANT_PROPERTY(CAST('2020-01-01T12:00:00' AS datetime2(2)) AT TIME ZONE 'UTC', 'Scale'); -- 2

[smitpatel]

Bubbling up as-is type mapping and inferring from outside sounds good. Bubbling precision/scale up is a good thing though we don't do it anywhere else at present so I will let you decide if you want to do it or not. If it is too complex, we can choose to not do it. datetimeoffset(2) will fit in just fine with datetimeoffset(7) type so there is no data loss or truncation.

[roji]

OK, I went ahead and implemented bubbling of precision - in both directions. This means that for x.SomeColumn == EF.Functions.AtTimeZone(someParam, "UTC"), someParam will get SomeColumn's precision, just as if it's compared directly without AtTimeZone. This stuff may be important - I'm not sure how SQL Server behaves when comparing datetimeoffsets with different precisions...

Note that there's now a SqlServerSqlExpressionFactory to have the specific bubbling logic. This is a bit more complicated than I originally thought, let me know what you think and if we should simplify.

[smitpatel]

Minor comments

[ghost]

Hello @roji!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

[smitpatel]

Did you remove SqlServerSqlExpressionFactory?

[roji]

Nope, just botched the squashing...