feat: supported multiple rows update for sharding key update

[yogesh1801]

No description provided.

[levkk]

Apparently it was easy. Could you include some test coverage?

[yogesh1801]

@levkk 2 problems what i face is

1. perf regression on insert rows, earlier if original shard == final_shard then directly run update on the shard, but now since we expect multiple rows this is not possible as UPDATE uses complete WHERE clause from the origin query so now even if same shard both INSERT and DELETE Runs, so there is a tradeoff

2. For tests i was thinking to run something like UPDATE SET id = id + 10 WHERE id IN ( 1, 2 ) but looks like id + 10 type expressions are not supported, if i do something like id = 11 it gives primary key violation so for tests should i remove PK constraint from the table for just this test.

[levkk]

Regression on insert rows, earlier if original shard == final_shard then directly run update on the shard, but now since we expect multiple rows this is not possible as UPDATE uses complete WHERE clause from the origin query so now even if same shard both INSERT and DELETE Runs, so there is a tradeoff

You should still be able to validate that each row matches the original shard - just loop over the results and perform the same check we do for one row. The only difference is all checks have to pass, or we fallback to the insert/delete workflow.

For tests i was thinking to run something like UPDATE SET id = id + 10 WHERE id IN ( 1, 2 ) but looks like id + 10 type expressions are not supported, if i do something like id = 11 it gives primary key violation so for tests should i remove PK constraint from the table for just this test.

You can do this instead:

UPDATE ... SET id = 5 WHERE true;

(it was 10 before, or an ID that matches a different shard actually).

[yogesh1801]

You should still be able to validate that each row matches the original shard - just loop over the results and perform the same check we do for one row. The only difference is all checks have to pass, or we fallback to the insert/delete workflow.

Thought of this but how likely is this case that all of them belong to same bag?, though there is no harm in checking all rows and see if they all match the same shard since it will be a quick check.

[levkk]

Thought of this but how likely is this case that all of them belong to same bag?, though there is no harm in checking all rows and see if they all match the same shard since it will be a quick check.

Could be high, e.g.:

UPDATE users SET tenant_id = 7 WHERE user_id IN ($1);

That's going to return a lot of rows but if they belong to the same tenant in the first place, and the new tenant_id is on the same shard, we could save ourselves a lot of work.

[yogesh1801]

@levkk have added the above suggestion could you review

[yogesh1801]

not sure, why tests failing

Ok they passed :)

[codecov]

Codecov Report

❌ Patch coverage is 87.50000% with 7 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
.../frontend/client/query_engine/multi_step/update.rs	87.50%	7 Missing ⚠️

📢 Thoughts on this report? Let us know!