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

Refactor BytesToDateTimeConverter, so it works out-of-the-box with compiled models. #1847

Merged
merged 1 commit into from
Feb 27, 2024

Conversation

lauxjpn
Copy link
Collaborator

@lauxjpn lauxjpn commented Feb 27, 2024

Our previous BytesToDateTimeConverter implementation used methods calls in its convert to/from expressions that used a variable that was supplied to the convert expression. That is not supported by EF Core's LinqToCSharpSyntaxTranslator.VisitInvocation() implementation (would be trivial to implement though), which assumes that an InvocationExpression always contains a lambda - which is not the case.

We also accessed a private static field, which cannot be referenced from outside the class. Since the compiled model code generator does not create an instance of the value converter in question by default, but instead creates a general value converter that just uses the same expressions that we define in ours, no private fields could be accessed.

The docs state:

Conversions are defined using two Func expression trees: one from ModelClrType to ProviderClrType and the other from ProviderClrType to ModelClrType. Expression trees are used so that they can be compiled into the database access delegate for efficient conversions. The expression tree may contain a simple call to a conversion method for complex conversions.

While this seems to be just a suggestion, it seems to be a requirement for value converters in compiled models.

Fixes #1846

@lauxjpn lauxjpn added this to the 9.0.0 milestone Feb 27, 2024
@lauxjpn lauxjpn self-assigned this Feb 27, 2024
@lauxjpn lauxjpn merged commit 284e9a5 into PomeloFoundation:master Feb 27, 2024
21 checks passed
@lauxjpn lauxjpn deleted the fix/issue1846 branch February 27, 2024 18:53
@lauxjpn lauxjpn modified the milestones: 9.0.0, 8.0.1 Mar 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BytesToDateTimeConverter creation is incompatible with compiled model code generation
1 participant