Add function mapping for Ticks to sqlserver#35465
Add function mapping for Ticks to sqlserver#35465z505985755 wants to merge 3 commits intodotnet:mainfrom
Conversation
@dotnet-policy-service agree |
|
@z505985755 Are you going to fix the tests? |
| sqlExpressionFactory.Divide( | ||
| sqlExpressionFactory.Function( | ||
| "DATEPART", | ||
| arguments: new[] { sqlExpressionFactory.Fragment("nanosecond"), instance! }, |
There was a problem hiding this comment.
Am noting that this duplicate instance twice in the translation; if that's a complicated expression (e.g. with a subquery), that's bad for performance - for those cases, we tend to do the translation only if we know that's it's a simple column/parameter, rather than a complex expression. I also saw a brief discussion over multiple translation possibilities (possibly some not supported in older SQL Server), I'd lean towards one that doesn't duplicate expressions - even if it's not supported across all versions/variants - rather than one that's supported everything but duplicates.
(regardless, this is missing test coverage)
There was a problem hiding this comment.
If use datediff_big to calculate the microsecond difference and then multiply it by 10, may lose one digit of precision because the datetime2 data type supports up to 7 decimal places. If use datediff_big to calculate the nanosecond difference and then divide it by 100, it will cause an overflow.
So, I can't think of a solution without using complex expressions.What are you suggesting?
yes,I ran some tests |
The CI failures seem related to the changes. Those need to fixed. |
I noticed this error when executing the test script. It seems that the assertion sql is inconsistent with the generated sql due to the original lack of support for datetime_Ticks. This is also consistent with a bug in CI. How should this be fixed? |
|
You need to adjust baselines. It's fine that the SQL produced is now different. |
|
Set |
|
@z505985755 You need to address the comment from @roji above. Basically translate only if the instance expression is column or parameter. If it's something more complex, bail out. Also tests need to be added to properly cover this - especially around different datatypes and edge values. |
|
Closing for inactivity. |
5 participants
##35436