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

Allow creation of an update/delete DbCommand from an EF IQueryable #33167

Closed
clement911 opened this issue Feb 27, 2024 · 6 comments
Closed

Allow creation of an update/delete DbCommand from an EF IQueryable #33167

clement911 opened this issue Feb 27, 2024 · 6 comments

Comments

@clement911
Copy link
Contributor

clement911 commented Feb 27, 2024

I would like to propose new APIs to build update or delete commands from an EF relational IQueryable.

This is in contrast to the existing ExecuteDetele and ExecuteUpdate methods, which execute the commands immediately and internally.

I'm thinking that it should look similar to #19358

My eventual hope is that these new APIs can be used to implement batching of multiple updates/deletes. See #33143 .
I think it makes sense to start with these smaller APIs before tackling batching.

The RelationalQueryableExtensions.cs file already contains the extensions to execute deletes and updates. I am thinking of adding 2 new extension methods to this same file.

Existing extensions:

public static int ExecuteDelete<TSource>(this IQueryable<TSource> source)

public static int ExecuteUpdate<TSource>(
this IQueryable<TSource> source,
Expression<Func<SetPropertyCalls<TSource>, SetPropertyCalls<TSource>>> setPropertyCalls)

New extensions (to be implemented):

public static DbCommand CreateDeleteDbCommand<TSource>(this IQueryable<TSource> source);
public static DbCommand CreateUpdateDbCommand<TSource>(this IQueryable<TSource> source, Expression<Func<SetPropertyCalls<TSource>, SetPropertyCalls<TSource>>> setPropertyCalls);

(The existing extensions have async versions, but this is not relevant here since no DB call is issued to generated the commands.)

Example:

   var query = context.Widgets.Where(i => /*some predicate*/);
   var command = query.CreateDeleteDbCommand();

I would like to have a try at implementing these. However, I would love to get your thoughts on this high level design of the public surface API first.

@roji
Copy link
Member

roji commented Feb 27, 2024

See previous discussion in #33143.

First, I'm not sure I see why a new CreateDeleteDbcommand() is needed - why doesn't the existing CreateDbCommand() cover it?

For updates, CreateDbCommand() indeed doesn't work since the setters need to be specified; in other words, CreateDbCommand() requires an IQueryable to work, but ExecuteUpdate combines both the specification of the setters and the actual execution/evaluation of the IQueryable, so there's no place to specify CreateDbCommand().

However, that problem is in no way specific to ExecuteUpdate - there are other terminating operators which accept arguments, just like ExecuteUpdate (Max, Average...), and these are similarly incompatible with CreateDbCommand(). Introducing a CreateMaxDbCommand(), CreateAverageDbCommand() doesn't scale and is a bad user experience, so we need a better API here.

As I wrote in #33143 (comment):

Having a ToQueryString variant which accepts the query as an argument (i.e. context.Blogs.ToQueryString(b => b....ExecuteUpdate()) could be a way around this, if it's considered important enough.

This would be a single API, covering all query/update APIs (ExecuteUpdate, Max, Average...). We can open an issue to track that, but implementing this is going to be far from trivial - I wouldn't recommend it without a strong background in query processing and how EF works internally.

Does that make sense @clement911?

@clement911
Copy link
Contributor Author

Thanks for the quick answer @roji

First, I'm not sure I see why a new CreateDeleteDbcommand() is needed - why doesn't the existing CreateDbCommand() cover it?

I'm not sure what you mean.
CreateDbCommand() creates a SELECT command (ExecuteReader) whereas CreateDeleteDbcommand() would create a DELETE command (ExecuteNonQuery).

However, that problem is in no way specific to ExecuteUpdate - there are other terminating operators which accept arguments, just like ExecuteUpdate (Max, Average...), and these are similarly incompatible with CreateDbCommand(). Introducing a CreateMaxDbCommand(), CreateAverageDbCommand() doesn't scale and is a bad user experience, so we need a better API here.

I see your point here. It would be best to have a single API that handles all terminating operators (both ExecuteNonQuery and ExecuteScalar expressions).

context.Blogs.ToQueryString(b => b....ExecuteUpdate()) is an idea.

However if you intend for ToQueryString to be an extension method of DbSet, then it would not be possible to call it if the user only had an IQueryable (but no DbContext or DbSet). By contrast, the existing CreateDbCommand works on IQueryable. I also find CreateDbCommand is more discoverable and intuitive.

Maybe a way to get the best of both world would be to add a new overload for CreateDbCommand, which takes an additional parameter to capture the terminating operator.

public static DbCommand CreateDbCommand<TSource, TResult>(this IQueryable<TSource> source, 
                                            Expression<Func<IQueryable<TSource>, TResult>> terminatingOperator);

Example:

   var query = context.Widgets.Where(i => /*some predicate*/);
   var deleteCmd = query.CreateDbCommand(q => q.ExecuteDelete());
   var countCmd = query.CreateDbCommand(q => q.Count());
   var maxCmd = query.CreateDbCommand(q => q.Max());
   var updateCmd = query.CreateDbCommand(q => q.ExecuteUpdate(i => i.SetProperty(i => i.LastUpdatedAt, DateTime.Now));

p.s: I just watched https://www.youtube.com/watch?v=1Ld3dtnTrMw . It was excellent and I would definitely love it if you could make another video to go deeper in the internals of the visitors and related patterns 👍

@roji
Copy link
Member

roji commented Feb 27, 2024

Maybe a way to get the best of both world would be to add a new overload for CreateDbCommand, which takes an additional parameter to capture the terminating operator.

Tha's probably possible, but breaking up the query in such a way seems pretty extreme and quite unclear. In any case, since ToQueryString and CreateDbCommand don't actually need the DbSet type in their signature, the proposed methods can just hang off of DbContext itself:

context.ToQueryString(c => c.Blogs.Max(...));

We have similar (theoretical) ideas for a Query() method on DbContext to address some issues in LINQ, such as the inability to distinguish between parameter and extent in non-lambda top-level operators (e.g. context.Blogs.Skip(1) vs. context.Blogs.Skip(i)).

p.s: I just watched https://www.youtube.com/watch?v=1Ld3dtnTrMw . It was excellent and I would definitely love it if you could make another video to go deeper in the internals of the visitors and related patterns 👍

Thanks, I'm happy you liked it! Yeah, this is a topic I could talk about for hours and hours :) We'll plan for an additional query internals standup at some point...

@clement911
Copy link
Contributor Author

I think you're right that it would be a much better API if the user doesn't need to break up their query.
Thanks. I'll keep an eye on this issue.

@roji
Copy link
Member

roji commented Feb 27, 2024

@clement911 I've opened #33181 with a summarizing write-up, and will close this in favor that.

@roji
Copy link
Member

roji commented Feb 27, 2024

Duplicate of #33181

@roji roji marked this as a duplicate of #33181 Feb 27, 2024
@roji roji closed this as not planned Won't fix, can't repro, duplicate, stale Feb 27, 2024
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

2 participants