Fix Linq .Second bug for PostgreSQL et al. #3528
Open
+436
−97
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hazzik
reviewed
Apr 22, 2024
| @@ -33,8 +34,13 @@ public DateTimePropertiesHqlGenerator() | |||
|
|
|||
| public override HqlTreeNode BuildHql(MemberInfo member, Expression expression, HqlTreeBuilder treeBuilder, IHqlExpressionVisitor visitor) | |||
| { | |||
| return treeBuilder.MethodCall(member.Name.ToLowerInvariant(), | |||
| var functionName = member.Name.ToLowerInvariant(); | |||
| if (functionName == "second" && (visitor.SessionFactory as ISessionFactoryImplementor)?.Dialect.Functions.ContainsKey("secondtruncated") == true) | |||
There was a problem hiding this comment.
Can we instead of checking if the function is supported define secondtruncated in the base dialect?
There was a problem hiding this comment.
I would love to, but the only meaningful way to do that, I think, would be to create a new kind of ISQLFunction, which aliases another function.
RegisterFunction("secondtruncated ", new SQLFunctionAlias(this, "second"));
|
Mysql seems to be failing because it needs to use |
gliljas
force-pushed
the
GH3525
branch
2 times, most recently
from
436ff1d to
0ba96aa
Compare
Refactor tests
|
The problem isn't flooring, but rather the old MySQL version used on the TC build. |
Co-authored-by: Frédéric Delaporte <[email protected]>
Co-authored-by: Frédéric Delaporte <[email protected]>
fredericDelaporte
approved these changes
Jan 6, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix for #3525
The actual fix was adding a new HQL function "secondtruncated", which is used if it is available, otherwise it falls back to the old "second". Another option would be to fix the "second" function in the dialects where it doesn't work, but that could be considered a more breaking change. As mentioned in the issue, Hibernate has defined "second" to actually include fractional seconds, and have adjusted the dialects where the corresponding SQL function truncates.
The most changes were in DateTimeTests.cs. I really wanted to use that test class since it already tests exactly this (but not seconds), but to get fractional seconds into the Northwind dataset was a bit icky, so I opted to create specific test data.
I used the approach of comparing the result from the LINQ query with the result from the same query run in memory. Of course that doesn't work in scenarios with null, case sensitivity etc., but here it works just fine.
The MySQL test fails on Github, for some reason. I can't make it fail locally.