MDEV-41340: Push a merged view down like the table it stands for - #5788
Open
bsrikanth-mariadb wants to merge 1 commit into
Conversation
A multi-table UPDATE/DELETE through a mergeable view wasn't pushed down
into FederatedX at all, even when the view was a transparent stand-in for
one of the engine's own tables. The eligibility check (find_multi_upddel_
handler -> get_fed_table_for_pushdown) ran before JOIN::optimize()'s
DT_MERGE step, so it only ever saw the view's own not-yet-merged
TABLE_LIST placeholder, whose table (if any) isn't FederatedX-backed, and
rejected the whole statement.
Naively fixing that by treating an unmerged view as equivalent to a plain
derived table (defer to its inner SELECT, as already done for anonymous
FROM-list subqueries) is unsafe for a genuinely materialized view: the
statement is reproduced as text via TABLE_LIST::print(), which always
prints a named view by its name, merged or not; a view that needs
materialization (e.g. because of LIMIT) has no such name on the remote
server, so the pushed-down statement referenced a table that only exists
locally, and failed with ER_NO_SUCH_TABLE.
A second, related problem: a merged view's underlying table keeps the
table name/alias it has inside the view's own definition, not the alias
the statement used for the view. If that collides with another reference
to the same table elsewhere in the statement (a self-join through the
view), the printed statement can't tell the two occurrences apart, and
either the remote server rejects it as an ambiguous "not unique
table/alias" statement, or worse, a column silently binds to the wrong
occurrence.
- sql_select.cc: Sql_cmd_dml::execute_inner() now runs the DT_MERGE step
before asking engines whether they can take over the statement, so a
merged view's real underlying table(s) are visible to the check instead
of just the view's own placeholder. Idempotent, so it does not repeat
the same step inside optimize_inner() further down.
- federatedx_pushdown.cc: get_fed_table_for_pushdown()'s per-table check is
now the recursive check_fed_table_for_pushdown(), which:
- skips a merged view's own inert placeholder but recurses into what it
actually stands for, however DT_MERGE represented it (a spliced
sibling TABLE_LIST, or a NESTED_JOIN wrapping the placeholder when the
view is itself one of the statement's targets);
- rejects a named view that still needs materialization instead of
deferring to its inner SELECT, closing the ER_NO_SUCH_TABLE gap above;
- rejects pushdown outright when the same remote table would be printed
under colliding names, closing the self-join gap above.
Test: federated.federatedx_pushdown_upd_del.
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
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.
A multi-table UPDATE/DELETE through a mergeable view wasn't pushed down into FederatedX at all, even when the view was a transparent stand-in for one of the engine's own tables. The eligibility check (find_multi_upddel_ handler -> get_fed_table_for_pushdown) ran before JOIN::optimize()'s DT_MERGE step, so it only ever saw the view's own not-yet-merged TABLE_LIST placeholder, whose table (if any) isn't FederatedX-backed, and rejected the whole statement.
Naively fixing that by treating an unmerged view as equivalent to a plain derived table (defer to its inner SELECT, as already done for anonymous FROM-list subqueries) is unsafe for a genuinely materialized view: the statement is reproduced as text via TABLE_LIST::print(), which always prints a named view by its name, merged or not; a view that needs materialization (e.g. because of LIMIT) has no such name on the remote server, so the pushed-down statement referenced a table that only exists locally, and failed with ER_NO_SUCH_TABLE.
A second, related problem: a merged view's underlying table keeps the table name/alias it has inside the view's own definition, not the alias the statement used for the view. If that collides with another reference to the same table elsewhere in the statement (a self-join through the view), the printed statement can't tell the two occurrences apart, and either the remote server rejects it as an ambiguous "not unique table/alias" statement, or worse, a column silently binds to the wrong occurrence.
Test: federated.federatedx_pushdown_upd_del.