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

Type converters, comparers etc. cannot be supported if they contain expressions incompatible with expresion tree #32717

Open
roji opened this issue Jan 4, 2024 · 1 comment

Comments

@roji
Copy link
Member

roji commented Jan 4, 2024

Following fixes in #32659, Npgsql's NpgsqlArrayConverter can now be successfully translated from LINQ to C#. However, that still doesn't work with the compiled model, since the value converter accepts an Expression<Func<...>> parameter, but the LINQ tree contains statements and loops, which are invalid there. This means that converters/comparers can currently only be used if they're restricted to the set of nodes that are valid inside C# expression trees.

A possible way around this is to allow value converters to be constructed without an expression tree; for example, implementations would simply extend ValueConverter and override regular methods as usual. Then, the materializer would then check whether an expression is tree is available, and if not, it would fall back to calling the delegate from the tree rather than integrating an expression tree inline.

Another alternative is to specifically detect this case in LinqToCSharpSyntaxTranslator, and to emit C# code creating the LINQ expression nodes, rather than the C# code directly ("quoting" them). So instead of emitting 1 == 2, we'd emit Expression.Binary(Expression.Constant(1), Expression.Constant(2)). We don't currently have a need for quoting elsewhere, and this is quite a heavy approach.

@roji roji changed the title Type converters, comparers etc. cannot be supported if they contain expression incompatible with expresion tree Type converters, comparers etc. cannot be supported if they contain expressions incompatible with expresion tree Jan 4, 2024
@ajcvickers ajcvickers added this to the Backlog milestone Jan 18, 2024
@AndriySvyryd
Copy link
Member

We could also compile the materializer itself, see #32923

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants