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

Feature request: ExecuteAssert #33329

Closed
alrz opened this issue Mar 15, 2024 · 12 comments
Closed

Feature request: ExecuteAssert #33329

alrz opened this issue Mar 15, 2024 · 12 comments

Comments

@alrz
Copy link
Member

alrz commented Mar 15, 2024

Doing a basic "query, validate, update" would need two or more transactions/roundtrips to the database.

var user = context.Users.Where(..);
if (!user.IsVerified) throw new("user not verified");
/*update user*/
context.SaveChanges();

Using an API like ExecuteAssert could fail the transaction without fetching the data first and sending the update back.

using var transaction = context.Database.BeginTransaction(); // don't need this if you only need the assertion on one entity
context.Users.Where(..)
   .ExecuteAssert(user => u.IsVerified, "user not verified")
   .ExecuteUpdate(user => /*update user*/);
context.SaveChanges();
transaction.Commit();

It would be possible to add the assertion to where clause, but then it's not clear which assertion failed to report it. So an alternative API could look like this:

context.Users.Where(..)
   .Where(user => u.IsVerified).Assert("user not verified")
   .ExecuteUpdate(user => /*update user*/);
@alrz alrz changed the title Feature request: ExecuteAssertAsync Feature request: ExecuteAssert Mar 15, 2024
@roji
Copy link
Member

roji commented Mar 15, 2024

I'm not quite sure what ExecuteAssert would actually do - can you provide more details on exactly which SQL you're expecting all the above to produce?

@alrz
Copy link
Member Author

alrz commented Mar 15, 2024

It's basically an early return for IF NOT EXIST ( query ). If true, then it would either return the assertion message or raiseerror based on impl strategy. The client would get a concrete exception with user message to be reported (same logic in the original example in OP). Note the message string doesn't need to be present in the query, I think a mapping on the client would suffice.

@roji
Copy link
Member

roji commented Mar 15, 2024

What's stopping you from just implementing that yourself today with LINQ? You could run the query once with Any() composed on top and check the results (the "assertion"), and if it's true you can go ahead and do ExecuteUpdate. This should be easily wrappable in an extension method if you really want to.

@alrz
Copy link
Member Author

alrz commented Mar 15, 2024

@roji

What's stopping you from just implementing that yourself today with LINQ? You could run the query once with Any()

I think the whole point of using ExecuteUpdate is to avoid sending out a SELECT - saving a database roundtrip.

int n = context.Users.Where(u => u.Id == id && 
                                 u.IsEmailVerified && 
                                 u.IsPhoneVerified)
    .ExecuteUpdate(setters => setters.SetProperty(b => b.SomeProp, true));
if (n == 0) // not clear if email is not verified or phone; assuming user exists

However with that, you'd lose the ability to see which precondition has failed, so you'd need to run a query first.

var user = context.Users.Where(u => u.Id == id).First();
if (!user.IsEmailVerified ) return "email not verified";
if (!user.IsPhoneVerified) return "phone not verified";
user.SomeProp = true;
context.SaveChanges();

Ideally you would send the precondition itself in the same query and get the result back

try {
  context.Users.Where(u => u.Id == id)
    .Assert(u => u.IsEmailVerified, "email not verified")
    .Assert(u => u.IsPhoneVerified, "phone not verified")
    .ExecuteUpdate(...);
} catch (AssertionFailedException e) {
   return e.UserMessage; // the intended message to return
}

(I factored out "user not found" here but I think that could be done using Assert(u => true, "user not found"))

@roji
Copy link
Member

roji commented Mar 15, 2024

I'm really not understanding - how would your "ExecuteAssert" prevent an additional query to do the assert? We can't do magic with the database - if you want a query to be sent before the update in order to pre-verify something, then that query has to be sent.

To get your point across, please include the full SQL commands that your proposed LINQ above would generate.

@alrz
Copy link
Member Author

alrz commented Mar 15, 2024

Sure, here's a simple sample,

IF NOT EXISTS (SELECT 1 FROM Users WHERE UserId = @UserId AND IsVerified = 1)
BEGIN
    SELECT 'User is not verified.' AS ErrorMessage;
    RETURN;
END;

UPDATE Users
SET SomeProperty = 1
WHERE UserId = @UserId;

@roji
Copy link
Member

roji commented Mar 15, 2024

That looks like it would return "User is not verified" for both the case where the user is unverified and for the case where no such user exists. If that's your goal, you can simply do a single ExecuteUpdate for the following:

UPDATE Users
SET SomeProperty = 1
WHERE UserId = @UserId AND IsVerified = 1;

You can then check how many rows were actually updated (ExecuteUpdate returns that).

@alrz
Copy link
Member Author

alrz commented Mar 15, 2024

and for the case where no such user exists

Yeah I noticed that, it should probably use something to effect of

IF 1 != ALL (SELECT IsVerified FROM Users WHERE UserId = @UserId)

So that it would pass where no such user exists.

If that's your goal, you can simply do a single ExecuteUpdate for the following:

UPDATE Users
SET SomeProperty = 1
WHERE UserId = @UserId AND IsVerified = 1;

As explained in #33329 (comment) the goal is to know which part of the query has failed so it can be reported in the app.

@roji
Copy link
Member

roji commented Mar 15, 2024

OK.

Simply executing the "assertion" query is already possible with regular link - simply suffix the query with the LINQ Any() operator to check if any users match whatever it is you want to test; there's no need for a new "assertion" concept or operator for that specific purpose.

So I'm assuming what you're asking here is to be able to execute both the query and the update in the same roundtrip, rather than in 2; if so, that's covered in a general case by #10879. In other words, I don't think it makes sense to introduce some special "assertion" API specifically for this case, but rather a general batching mechanism, which allows mixing in arbitrary queries/updates.

Does that make sense?

@alrz
Copy link
Member Author

alrz commented Mar 15, 2024

that's covered in a general case by #10879

Yes, if it were to allow an update as part of the batch that would perfectly cover this case - I were just confused by the title "batching read".

@roji
Copy link
Member

roji commented Mar 15, 2024

Great - updates are definitely covered by that issue as well (I think there are even some discussions on what exactly the API would look like around those).

@roji
Copy link
Member

roji commented Mar 15, 2024

Duplicate of #10879

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