A DELETE containing a single table, an index hint, an impossible WHERE
condition, and a window function will take the multi-delete code path
but never initialize tables for deletion, leading to a crash. Such
a statement would never delete rows from the target table. Record in the
multi_delete whether it was ever initialized for execution, and don't
attempt to delete anything if it wasn't initialized.
The index hint forces the single table DELETE to take the multi-table
codepath. Since this case has an impossible WHERE condition, we set
subq_exit_fl which later causes JOIN::optimize_stage2 to skip the
multi-delete table initialization. It's not safe to attempt
initialization when trying to find a "tableless" subquery plan, so
defend against this case with the new multi-delete flag added by
this commit.
DELETE skips the row without reporting anything when error is returned by handler::ha_rnd_pos()
Report every error except the two that genuinely mean "the row is not there
any more".
Lex::save_list contents remain from a previous query that has invalid
syntax and isn't reset when processing a new query.
We initialize this structure along with it's peer in LEX::start.
MDEV-38295 Recursive CTE usage leaks memory when not used in a top level select
During st_select_lex::cleanup() we assume that leaf tables and any
associated recursive table references can only be present when a join
structure is present. After MDEV-37220 this is no longer true.
We shift our cleanup routine out of the test for a join.
We add Multidelete_prelocking_strategy::handle_end() in the same manner
as Multiupdate_prelocking_strategy::handle_end() (but with minimal
processing as yet) to populate these leaf_tables in the outermost
select when it is a delete statement.
We add in a call to free_underlaid_joins() to the error processing section
of Sql_cmd_dml::execute to free up temporary tables that were allocated
in prepare, but not freed up when execute is bypassed because an error
occured during prepare.
Sergei Petrunia: Better comments for MDEV-38272, MDEV-38272
Approved by: Sergei Petrunia (sergey@mariadb.com)
ft_handler isn't getting initialized for subqueries inside explain
delete/update queries. However, ft_handler is accessed inside ha_ft_read(),
and is the reason for NULL pointer exception.
This is not the case with non-explain delete/update queries, as
well as explain/non-explain select queries.
Follow the approach the SELECT statements are using in
JOIN::optimize_constant_subqueries(): remove SELECT_DESCRIBE
flag when invoking optimization of constant subqueries.
Single-table UPDATE/DELETEs have SELECT_LEX but don't have JOIN.
So, we make optimize_constant_subqueries() not to be a member
of JOIN class, and instead move it to SELECT_LEX, and then
invoke it from single-table UPDATE/DELETE as well as for SELECT queries.
When deleting from multiple tables, the DELETE operator uses a temporary table
to collect the row IDs of rows that will be deleted. If this table grows too
large, then we convert it from a heap temporary table to a persistent (on-disk)
temporary table to avoid unbounded memory usage. There are several error codes
that gate creating this persistent temporary table but themselves don't
indicate a problem with the DELETE operation. However, regardless of this error
code value, we need to clear it after deciding whether or not to create the
persistent temporary table. If we don't clear this value, then the error
code's stale state prevents binlog replication when its value is muxed with
local_error during multi_delete::send_eof.
Problem:
After the re-design of `UPDATE` and `DELETE` in MDEV-28883, the call
to find select handler is missing. This prevents the server from
handring over multi-update/multi-delete queries to storage engines
capable of executng such queries on their own (e.g., ColumnStore).
MDEV-32382 introduced a check in `find_select_handler_inner` function,
that blocked pushdown of queries involving CTEs, without checking if
the storage engine is capable of handling such queries.
Fix:
Add a call to find the select handler for the engine involved in the
multi-update/multi-delete query, allowing the storage engines to
execute such queries.
Fix the `find_select_handler_inner` function by allowing the storage
engine's create_select functions to decide the pushdown of queries
involving CTEs.
a fix for MDEV-15990 has unintentionally added
HA_EXTRA_REMEMBER_POS/HA_EXTRA_RESTORE_POS on the code path for normal,
not versioned, deletes.
Let's restore the old logic.
followup for 8001679af6
Timestamp-versioned row deletion was exposed to a collisional problem: if
current timestamp wasn't changed, then a sequence of row delete+insert could
get a duplication error. A row delete would find another conflicting history row
and return an error.
This is true both for REPLACE and DELETE statements, however in REPLACE, the
"optimized" path is usually taken, especially in the tests. There, delete+insert
is substituted for a single versioned row update. In the end, both paths end up
as ha_update_row + ha_write_row.
The solution is to handle a history collision somehow.
From the design perspective, the user shouldn't experience history rows loss,
unless there's a technical limitation.
To the contrary, trxid-based changes should never generate history for the same
transaction, see MDEV-15427.
If two operations on the same row happened too quickly, so that they happen at
the same timestamp, the history row shouldn't be lost. We can still write a
history row, though it'll have row_start == row_end.
We cannot store more than one such historical row, as this will violate the
unique constraint on row_end. So we will have to phisically delete the row if
the history row is already available.
In this commit:
1. Improve TABLE::delete_row to handle the history collision: if an update
results with a duplicate error, delete a row for real.
2. use TABLE::delete_row in a non-optimistic path of REPLACE, where the
system-versioned case now belongs entirely.
We had a protection against it, by allowing versioned delete if:
trx->id != table->vers_start_id()
For replace this check fails: replace calls ha_delete_row(record[2]), but
table->vers_start_id() returns the value from record[0], which is irrelevant.
The same problem hits Field::is_max, which may have checked the wrong record.
Fix:
* Refactor Field::is_max to optionally accept a pointer as an argument.
* Refactor vers_start_id and vers_end_id to always accept a pointer to the
record. there is a difference with is_max is that is_max accepts the pointer to
the
field data, rather than to the record.
Method val_int() would be too effortful to refactor to accept the argument, so
instead the value in record is fetched directly, like it is done in
Field_longlong.
after 633417308f (MDEV-37312) lookup_handler is locked with F_WRLCK,
because it may be used for deleting rows.
And lookup_handler is locked with F_WRLCK after prune_partitions(),
but the main handler is locked before, and might expects all
partitions to be in the read least, non-pruned.
Let's prepare the lookup handler before prune_partitions().
- Removed duplicate words, like "the the" and "to to"
- Removed duplicate lines (one double sort line found in mysql.cc)
- Fixed some typos found while searching for duplicate words.
Command used to find duplicate words:
egrep -rI "\s([a-zA-Z]+)\s+\1\s" | grep -v param
Thanks to Artjoms Rimdjonoks for the command and pointing out the
spelling errors.
Timestamp-versioned row deletion was exposed to a collisional problem: if
current timestamp wasn't changed, then a sequence of row delete+insert could
get a duplication error. A row delete would find another conflicting history row
and return an error.
This is true both for REPLACE and DELETE statements, however in REPLACE, the
"optimized" path is usually taken, especially in the tests. There, delete+insert
is substituted for a single versioned row update. In the end, both paths end up
as ha_update_row + ha_write_row.
The solution is to handle a history collision somehow.
From the design perspective, the user shouldn't experience history rows loss,
unless there's a technical limitation.
To the contrary, trxid-based changes should never generate history for the same
transaction, see MDEV-15427.
If two operations on the same row happened too quickly, so that they happen at
the same timestamp, the history row shouldn't be lost. We can still write a
history row, though it'll have row_start == row_end.
We cannot store more than one such historical row, as this will violate the
unique constraint on row_end. So we will have to phisically delete the row if
the history row is already available.
In this commit:
1. Improve TABLE::delete_row to handle the history collision: if an update
results with a duplicate error, delete a row for real.
2. use TABLE::delete_row in a non-optimistic path of REPLACE, where the
system-versioned case now belongs entirely.
We had a protection against it, by allowing versioned delete if:
trx->id != table->vers_start_id()
For replace this check fails: replace calls ha_delete_row(record[2]), but
table->vers_start_id() returns the value from record[0], which is irrelevant.
The same problem hits Field::is_max, which may have checked the wrong record.
Fix:
* Refactor Field::is_max to optionally accept a pointer as an argument.
* Refactor vers_start_id and vers_end_id to always accept a pointer to the
record. there is a difference with is_max is that is_max accepts the pointer to
the
field data, rather than to the record.
Method val_int() would be too effortful to refactor to accept the argument, so
instead the value in record is fetched directly, like it is done in
Field_longlong.
It was observed that, when doing multiple DELETE on tables:
-> if there was duplicate data to delete, then no ddls were dumped to the trace.
-> However, when tested with no data being deleted from the tables,
then ddls of the tables were getting dumped to the trace.
The problem is that store_tables_context_in_trace() is not getting
invoked, from mysql_execute_command() in all the situations.
The reason is that multi-table DELETE returned error when it had deleted duplicate rows.
Multi-table DELETE actually finds record combinations to delete,
and when it has found let's say {t1.rowX, t2.rowY, t3.rowZ}, it will attempt to
save t1.rowX in the temptable for t1, t2.rowY in the temptable for t2 and so forth.
When saving the row to be deleted, in the temp table, it can encounter error
121 (HA_ERR_FOUND_DUPP_KEY), and it is propagated to the caller.
As a fix, I have marked that there is no error when HA_ERR_FOUND_DUPP_KEY error_code is
noticed in the multi_delete::send_data()
Also expand vcol field index coverings to include indexes covering all
the fields in the expression. The reasoning goes as follows: let f(c1,
c2, ..., cn) be a function on applied to columns c1, c2, ..., cn, if
f(...) is covered by an index, so should vc whose expression is
f(...).
For example, if t.vf = t.c1 + t.c2, and t has three indexes (vf), (c1,
c2), (c1).
Before this change, vf's index covering is a singleton {(vf)}. Let's call
that the "conventional" index covering.
After this change vf's index covering is now {(vf), (c1, c2)}, since
(c1, c2) covers both c1 and c2. Let's call (c1, c2) in this case the
"extra" covering.
With the coverings updated, when an index in the "extra" covering is
chosen for keyread, the vcol also needs to be calculated. In this case
we mark vcol in the table read_set, and ensure it is computed.
With these changes, we see various improvements, including from using
full table scan + filesort to full index scan + filesort when ORDER BY
an indexed vcol (here vc = c + 1 is a vcol and both c and vc are
indexes):
explain select c + 1 from t order by vc;
id select_type table type possible_keys key key_len ref rows Extra
-1 SIMPLE t ALL NULL NULL NULL NULL 10000 Using filesort
+1 SIMPLE t index NULL c 5 NULL 10000 Using index; Using filesort
The substitutions are followed updates to all_fields which include a
copy of the ORDER BY/GROUP BY item pointers, as well as corresponding
updates to ref_pointer_array so that the all_fields and
ref_pointer_array remain in sync.
Another, related change is the recomputation of table index covering
on substitutions. It not only reflects the correct table index
covering after the substitutions, but also improve executions where
the vcol index can be chosen, such as this example (here vc = c + 1
and vc is the only index in the table), from full table scan +
filesort to full index scan:
select vc from t order by c + 1;
We do it in SELECT as well as in single table DELETE/UPDATE.
Multi-delete code invokes ha_rnd_end after applying deferred deletes, following
the pattern as in multi-update.
The CSV engine batches deletes made via calls to delete_row, then applies them
all at once during ha_rnd_end. For each row batched, the CSV engine decrements
an internal counter of the total rows in the table. The multi-delete code was
not calling ha_rnd_end, so this internal counter was not consistent with the
total number of rows in the table, triggering the assertion.
In the CSV engine, explicitly delete the destination file before renaming the
source file over it. This avoids a file rename problem on Windows. This
change would have been necessary before the latest multi-delete changes, had
this test case existed at that point in time, because the end_read_record call
in do_table_deletes swallowed the rename failure on Windows.
replication problems
DELETE HISTORY did not process parameterized PS properly as the
history expression was checked on prepare stage when the parameters
was not yet substituted. In that case check_units() succeeded as there
is no invalid type: Item_param has type_handler_null which is
inherited from string type and this is valid type for history
expression. The warning was thrown when the expression was evaluated
for comparison on delete execution (when the parameter was already
substituted).
The fix postpones check_units() until the first PS execution. We have
to postpone where conditions processing until the first execution and
update select_lex.where on every execution as it is reset to the state
after prepare.
When processing queries like
INSERT INTO t1 (..) SELECT .. FROM t1, t2 ...,
there is a single query block (i.e., a single SELECT_LEX) for both INSERT and
SELECT parts. During hints resolution, when hints are attached to particular
TABLE_LIST's, the search is performed by table name across the whole
query block.
So, if a table mentioned in an optimizer hint is present in the INSERT part,
the hint is attached to the that table. This is obviously wrong as
optimizer hints are supposed to only affect the SELECT part of
an INSERT..SELECT clause.
This commit disables possible attaching hints to tables in the INSERT part
and fixes some other bugs related to INSERT..SELECT statements processing
It places a limit N (a timeout value in milliseconds) on how long
a statement is permitted to execute before the server terminates it.
Syntax:
SELECT /*+ MAX_EXECUTION_TIME(milliseconds) */ ...
Only top-level SELECT statements support the hint.
MDEV-36563 Assertion `!mysql_bin_log.is_open()' failed in
THD::mark_tmp_table_as_free_for_reuse
The purpose of this commit is to ensure that creation and changes of
temporary tables are properly and predicable logged to the binary
log. It also fixes some bugs where ROW logging was used in MIXED mode,
when STATEMENT would be a better (and expected) choice.
In this comment STATEMENT stands for logging to binary log in
STATEMENT format, MIXED stands for MIXED binlog format and ROW for ROW
binlog format.
New rules for logging of temporary tables
- CREATE of temporary tables are now by default binlogged only if
STATEMENT binlog format is used. If it is binlogged, 1 is stored in
TABLE_SHARE->table_creation_was_logged. The user can change this
behavior by setting create_temporary_table_binlog_formats to
MIXED,STATEMENT in which case the create is logged in statement
format also in MIXED mode (as before).
- Changes to temporary tables are only binlogged if and only if
the CREATE was logged. The logging happens under STATEMENT or MIXED.
If binlog_format=ROW, temporary table changes are not binlogged. A
temporary table that are changed under ROW are marked as 'not up to
date in binlog' and no future row changes are logged. Any usage of
this temporary table will force row logging of other tables in any
future statements using the temporary table to be row logged.
- DROP TEMPORARY is binlogged only of the CREATE was binlogged.
Changes done:
- Row logging is forced for any statement using temporary tables that
are not up to date in the binary log.
(Before the row logging was forced if the user has a temporary table)
- If there is any changes to the temporary table that is not binlogged,
the table is marked as not up to date.
- TABLE_SHARE->table_creation_was_logged has a new definition for
temporary tables:
0 Table creating was not logged to binary log
1 Table creating was logged to binary log and table is up to date.
2 Table creating was logged to binary log but some changes where
not logged to binary log.
Table is not up to date in binary log is defined as value 0 or 2.
- If a multi-table-update or multi-table-delete fails then
all updated temporary tables are marked as not up to date.
- Enforce row logging if the query is using temporary tables
that are not up to date.
Before row logging was enforced if the user had any
temporary tables.
- When dropping temporary tables use IF EXISTS. This ensures
that slave will not stop if it had crashed and lost the
temporary tables.
- Remove comment and version from DROP /*!4000 TEMPORARY.. generated when
a connection closes that has open temporary tables. Added 'generated by
server' at the end of the DROP.
Bugs fixed:
- When using temporary tables with commands that forced row based,
like INSERT INTO temporary_table VALUES (UUID()), this was never
logged which causes the temporary table to be inconsistent on
master and slave.
- Used binlog format is now clearly defined. It is now only depending
on the current binlog_format and the tables used.
Before it was depending on the user had ANY temporary tables and
the state of 'current_stmt_binlog_format' set by previous queries.
This also caused temporary tables to be logged to binary log in
some cases.
- CREATE TABLE t1 LIKE not_logged_temporary_table caused replication
to stop.
- Rename of not binlogged temporary tables where binlogged to binary log
which caused replication to stop.
Changes in behavior:
- By default create_temporary_table_binlog_formats=STATEMENT, which
means that CREATE TEMPORARY is not logged to binary log under MIXED
binary logging. This can be changed by setting
create_temporary_table_binlog_formats to MIXED,STATEMENT.
- Using temporary tables that was not logged to the binary log will
cause any query using them for updating other tables to be logged in
ROW format. Before all queries was logged in ROW format if the user had
any temporary tables, even if they were not used by the query.
- Generated DROP TEMPORARY TABLE is now always using IF EXISTS and
has a "generated by server" comment in the binary log.
The consequences of the above is that manipulations of a lot of rows
through temporary tables will by default be be slower in mixed mode.
For example:
BEGIN;
CREATE TEMPORARY TABLE tmp AS SELECT a, b, c FROM
large_table1 JOIN large_table2 ON ...;
INSERT INTO other_table SELECT b, c FROM tmp WHERE a <100;
DROP TEMPORARY TABLE tmp;
COMMIT;
By default this will create a huge entry in the binary log, compared
to just a few hundred bytes in statement mode. However the change in
this commit will make usage of temporary tables more reliable and
predicable and is thus worth it. Using statement mode or
create_temporary_table_binlog_formats can be used to avoid this issue.
Added capability to create a trigger associated with several trigger
events. For this goal, the syntax of the CREATE TRIGGER statement
was extended to support the syntax structure { event [ OR ... ] }
for the `trigger_event` clause. Since one trigger will be able to
handle several events it should be provided a way to determine what
kind of event is handled on execution of a trigger. For this goal
support of the clauses INSERTING, UPDATING , DELETING was added by
this patch. These clauses can be used inside a trigger body to detect
what kind of trigger action is currently processed using the following
boilerplate:
IF INSERTING THEN ...
ELSIF UPDATING THEN ...
ELSIF DELETING THEN ...
In case one of the clauses INSERTING, UPDATING, DELETING specified in
a trigger's body not matched with a trigger event type, the error
ER_INCOMPATIBLE_EVENT_FLAG is emitted.
After this patch be pushed, one Trigger object will be associated with
several trigger events. It means that the array
Table_triggers_list::triggers
can contain several pointers to the same Trigger object in array members
corresponding to different events. Moreover, support of several trigger
events for the same trigger requires that the data members `next` and
`action_order` of the Trigger class be converted to arrays to store
relating information per trigger event base.
Ability to specify the same trigger for different event types results in
necessity to handle invalid cases on execution of the multi-event
trigger, when the OLD or NEW qualifiers doesn't match a current event
type against that the trigger is run. The clause OLD should produces
the NULL value for INSERT event, whereas the clause NEW should produce
the NULL value for DELETE event.
Avoid ASAN failure by collecting statistics from Result objects
before cleaning them up. In related single-table cases, statistics
are maintained directly by the single-table update and delete
functions.
* Let GCC `-Wformat` check formats sent to these `my_vsnprintf_ex` users
* Migrate them from the old extension specifiers
to the new `-Wformat`-compatible suffixes
* rpl.rpl_system_versioning_partitions updated for MDEV-32188
* innodb.row_size_error_log_warnings_3 changed error for MDEV-33658
(checks are done in a different order)
Reintroduces delete_while_scanning optimization for multi_delete.
Reverse some test changes from the initial feature devlopment now
that we delete-on-the-fly once again.
