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

End-user way to add custom sql expression #26522

Closed
Clemkd opened this issue Nov 3, 2021 · 5 comments
Closed

End-user way to add custom sql expression #26522

Clemkd opened this issue Nov 3, 2021 · 5 comments

Comments

@Clemkd
Copy link

Clemkd commented Nov 3, 2021

Hi!

I have a few projects that are currently using EF Core 5. Each time I need a new expression to take advantage of managed code instead of strings for raw sql. In some cases I am able to use the proposed expression classes, but in other cases I can't and it is a pain to look for a workaround.

For example, I want a "max/min over partition by" expression (SQL Server, Postgres, MySQL, ...), so I create it by instantiating the RowNumberExpression that is closest to what I want, then replace the prints to meet my need. BUT the SqlNullabilityExpression throws an exception as you might expect.

I see I'm not the only one who wants to add custom expressions without waiting for providers to accept and implement these expressions. I think it's too bad to restrict the end user even though I understand that these custom expressions can be a source of errors or performance penalties, but it's understandable that this is the responsibility of the developers who wrote them.

So I'm asking if you can come up with a way to add new custom expressions for end users, who are aware of the associated risks, but without doing more tricky things to do what they're looking for.
This would bring a lot of flexibility to the library, which I think is sorely lacking when queries get slightly more complex.

#26147
#26199

@roji
Copy link
Member

roji commented Nov 3, 2021

@Clemkd we had some discussion around this in #26199.

To summarize, adding a new expression is an advanced scenario which requires changes in multiple services in the query pipeline (nullability processing, inference in the SQL expression factory, SQL generation). This is a rare scenario, and we really don't recommend users do this - in many cases raw SQL can be used successfully as a much simpler alternative.

However, there's nothing stopping users from doing this if they so desire - we do not restrict this in any way... It's simply up to users to replace the provider's services with versions that support the new expression. Of the top of my head, I don't think there's anything we could do in EF Core to make this better/easier, aside maybe from documenting the process and the affected (but this is such a rare scenario that this doc would likely be very low priority).

Do you have anything particular in mind you're expecting us to do here?

@Clemkd
Copy link
Author

Clemkd commented Nov 3, 2021

in many cases raw SQL can be used successfully as a much simpler alternative

Yes, but with the loss of code management, while it is useful for example if you refactor a property of an entity.

I don't have anything specific in mind, naively I was wondering if the query pipeline would be able to handle in a later version to inject custom expressions, or anything like this (for advanced purposes).

Or in another way, add expressions that could allow more complex queries. For example, an expression which can be translated to an array of expressions whitout delimiter, or a binary expression without operand (both could be used into the HasDbTranslation mehod).
Example of AtTimeZone which can be created by an end user using this naive idea, as long as it is not yet implemented by a provider :

modelBuilder
    .HasDbFunction(typeof(Extensions).GetMethod(nameof(Extensions.ToTimeZone)))
    .HasTranslation(x =>
    {
        var columnExpression = x.First() as ColumnExpression;
        var clauseExpression = new SqlFragmentExpression(" AT TIME ZONE ");
        var timeZoneExpression = x.Skip(1).First() as SqlConstantExpression;

        var expression = new SqlExpressions(columnExpression, clauseExpression, timeZoneExpression);

        return expression;
    }

@roji
Copy link
Member

roji commented Nov 3, 2021

@Clemkd the point is that the above HasTranslation code is insufficient, and that various services need to be made aware of the expression type as well, as noted above (nullability processor, sql generator, possibly some others). So while adding a new custom expression is possible, it cannot be as simple as the above.

@smitpatel
Copy link
Contributor

I think the points from earlier discussion remains still valid.

  • A supported SQL is characteristic of a provider. Hence any augmentation of SQL tree is a task of the provider code (and not the user). If user wants to extend the tree which provider is not willing to extend (or do it till provider adds support), user can override multiple services to create own forked provider.
  • Query pipeline allows augmentation from users in translation through 2 mechanisms
    • UDFs - Since they are clearly defined by users, providers cannot translate them by default. We made this fast path by allowing user to just register and we would integrate it with translation.
    • Custom translation through IMethodCallTranslator (via plugin) or HasTranslation. This allows user to translate something to provider supported SQL tree. Again this could be integrate something user wishes to do particularly different in realm of supported SQL tree.

Regarding end-user way of adding custom SQL expression,

  • We need to use services so providers can override behavior as needed. That means end-user also need to replace services. This can get into rabbit hole of either wrong services being used or every service has to do look up (plug-in like) imposing perf penalty.
  • If we encapsulate the logic inside the expression itself then adding a custom expression becomes very easy but then built-in expression behavior change becomes complicated for providers to implement.

I believe, the way current design is useful, augmentable at the expense of slight delay from providers.

@ajcvickers
Copy link
Member

Note from triage: closing as by-design per discussion above and discussions on linked issues.

@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
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

4 participants