MDEV-39226: Push whole multi-table update/delete down into engines - #5529
Open
bsrikanth-mariadb wants to merge 1 commit into
Open
MDEV-39226: Push whole multi-table update/delete down into engines#5529bsrikanth-mariadb wants to merge 1 commit into
bsrikanth-mariadb wants to merge 1 commit into
Conversation
bsrikanth-mariadb
marked this pull request as draft
August 11, 2026 10:54
bsrikanth-mariadb
force-pushed
the
13.2-MDEV-39226-direct-multi_table-update-delete
branch
5 times, most recently
from
August 14, 2026 08:16
f2ce815 to
44765e4
Compare
spetrunia
reviewed
Aug 14, 2026
| const char *pushed_select_text= "PUSHED SELECT"; | ||
| const char *pushed_update_text= "PUSHED DOWN UPDATE"; | ||
| const char *pushed_delete_text= "PUSHED DOWN DELETE"; | ||
|
|
Member
There was a problem hiding this comment.
Please follow the pattern: if we use PUSHED SELECT, let's add PUSHED UPDATE , not PUSHED DOWN UPDATE.
Contributor
Author
There was a problem hiding this comment.
Yeah, true. But, felt "PUSHED DOWN XXXX" was sounding better.
Anyways, will keep them consistent.
bsrikanth-mariadb
force-pushed
the
13.2-MDEV-39226-direct-multi_table-update-delete
branch
3 times, most recently
from
August 18, 2026 07:04
b931524 to
eb052c3
Compare
bsrikanth-mariadb
changed the title
bsrikanth-mariadb
force-pushed
the
13.2-MDEV-39226-direct-multi_table-update-delete
branch
4 times, most recently
from
August 19, 2026 02:19
7f1147b to
168a3b1
Compare
Give storage engines a way to take over an entire multi-table UPDATE/DELETE, the way they can already take over a SELECT. Without it the join, the row matching and every modification run in the SQL layer even when an engine could do the whole statement itself in one step; a single-table UPDATE/DELETE already avoids this via direct_update_rows()/direct_delete_rows(), but a multi-table statement has no primary handler object to drive that path. This adds a generic, engine-agnostic pushdown interface: the SQL layer offers the statement to the engine, and if the engine accepts it, it performs the whole thing and reports only the row counts. - Split select_handler into a pushdown_handler base with select_handler (result set) and a new multi_upddel_handler (runs a whole UPDATE/DELETE, reports row counts, reported as PUSHED UPDATE/PUSHED DELETE); add handlerton::create_multi_upddel, looked up in Sql_cmd_dml::execute_inner(). - multi_update/multi_delete gain direct_update_delete_done(), which records the engine's counts so send_eof() binlogs and replies without the SQL-layer loop; it forces statement-format binlogging so the change still replicates under binlog_format=ROW, and errors out instead of silently dropping counts for an unsupported result object. - FederatedX implements the interface as the reference engine used to test correctness: it prints the statement back and runs it remotely, passes the engine's error code/SQLSTATE through, reads the matched count from the remote info string, executes IGNORE locally, and only pushes down when all tables share one remote server (same as SELECT/derived/unit pushdown). Test: federated.federatedx_pushdown_upd_del.
bsrikanth-mariadb
force-pushed
the
13.2-MDEV-39226-direct-multi_table-update-delete
branch
from
August 20, 2026 05:22
168a3b1 to
f59b0bd
Compare
bsrikanth-mariadb
marked this pull request as ready for review
August 20, 2026 05:41
spetrunia
reviewed
Aug 20, 2026
| static multi_upddel_handler * | ||
| create_federatedx_multi_upddel_handler(THD *thd, SELECT_LEX *sel_lex) | ||
| { | ||
| if (!use_pushdown || !is_supported_update_delete(thd->lex->sql_command)) |
Member
There was a problem hiding this comment.
What is this for? Do we get into this function for non-UPDATE/DELETE ?
I have added
DBUG_ASSERT(is_supported_update_delete(thd->lex->sql_command));
and it survived the tests.
Member
|
Please apply this patch: cleanups.patch |
spetrunia
reviewed
Aug 20, 2026
| str_eq(a->hostname, b->hostname) && | ||
| str_eq(a->socket, b->socket) && | ||
| str_eq(a->username, b->username) && | ||
| str_eq(a->password, b->password); |
Member
There was a problem hiding this comment.
Even password?
I'm not sure if it's possible to see different data depending on the user... Is password necessary?
spetrunia
reviewed
Aug 20, 2026
|
|
||
| ha_federatedx_multi_upddel_handler::~ha_federatedx_multi_upddel_handler() | ||
| = default; | ||
|
|
spetrunia
reviewed
Aug 20, 2026
| The range of the error codes of the client library, CR_MIN_ERROR and | ||
| CR_MAX_ERROR of errmsg.h. That header cannot be included here because it | ||
| defines ER, which the server defines differently. | ||
| */ |
Member
There was a problem hiding this comment.
If ER is the only problem, can one use
#undef ER
#include ...
?
spetrunia
requested changes
Aug 20, 2026
spetrunia
left a comment
spetrunia
left a comment
Member
There was a problem hiding this comment.
See above input.
Also, should create_multi_upddel accept SELECT_LEX argument, or LEX would be more meaningful?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Give storage engines a way to take over an entire multi-table
UPDATE/DELETE, the way they can already take over a SELECT. Without it the
join, the row matching and every modification run in the SQL layer even
when an engine could do the whole statement itself in one step; a
single-table UPDATE/DELETE already avoids this via
direct_update_rows()/direct_delete_rows(), but a multi-table statement has
no primary handler object to drive that path.
This adds a generic, engine-agnostic pushdown interface: the SQL layer
offers the statement to the engine, and if the engine accepts it, it
performs the whole thing and reports only the row counts.
(result set) and a new multi_upddel_handler (runs a whole UPDATE/DELETE,
reports row counts, reported as PUSHED UPDATE/PUSHED DELETE); add
handlerton::create_multi_upddel, looked up in Sql_cmd_dml::execute_inner().
the engine's counts so send_eof() binlogs and replies without the
SQL-layer loop; it forces statement-format binlogging so the change still
replicates under binlog_format=ROW, and errors out instead of silently
dropping counts for an unsupported result object.
correctness: it prints the statement back and runs it remotely, passes
the engine's error code/SQLSTATE through, reads the matched count from the
remote info string, executes IGNORE locally, and only pushes down when all
tables share one remote server (same as SELECT/derived/unit pushdown).
Test: federated.federatedx_pushdown_upd_del.