Problem:
=========
During commit, server calls prepare_commit_versioned to
determine the transaction modified system-versioned data.
Due to binlog_do_db option, we disable the binlog for the
statement. But prepare_commit_versioned() is being
called only when binlog is enabled for the statement.
Fix:
===
prepare_commit_versioned() should happen irrespective
of binlog state. So if the server has any read-write operation
then we should call prepare_commit_versioned().
prep_alter_part_table upon re-partitioning by system time
memcmp() tries to compare beyond the last member of interval because
sizeof(Vers_part_info::interval) is 80. It is sizeof of variable,
sizeof of type is 76.
Now we compare interval_t struct C++ way.
The assertion was to make sure we don't do vers_set_hist_part() for
SELECT (or any non-DML). But actually we must do it if SELECT calls
some function that does DML. Patch moves the assertion to non-routines
only.
differently react to SQL_MODE => unusable SHOW CREATE
Use abort_on_warning dependent on strict mode over create new table
like it is done for copy data and inplace alter.
This patch adds for "--ps-protocol" second execution
of queries "SELECT".
Also in this patch it is added ability to disable/enable
(--disable_ps2_protocol/--enable_ps2_protocol) second
execution for "--ps-prototocol" in testcases.
First UPDATE under START TRANSACTION does nothing (nstate= nstate),
but anyway generates history. Since update vector is empty we get into
(!uvect->n_fields) branch which only adds history row, but does not do
update. After that we get current row with wrong (old) row_start value
and because of that second UPDATE tries to insert history row again
because it sees trx->id != row_start which is the guard to avoid
inserting multiple trx_id-based history rows under same transaction
(because we have same trx_id and we get duplicate error and this bug
demostrates that). But this try anyway fails because PK is based on
row_end which is constant under same transaction, so PK didn't change.
The fix moves vers_make_update() to an earlier stage of
calc_row_difference(). Therefore it prepares update vector before
(!uvect->n_fields) check and never gets into that branch, hence no
need to handle versioning inside that condition anymore.
Now trx->id and row_start are equal after first UPDATE and we don't
try to insert second history row.
== Cleanups and improvements ==
ha_innobase::update_row():
vers_set_fields and vers_ins_row are cleaned up into direct condition
check. SQLCOM_ALTER_TABLE check now is not used as this is dead code,
assertion is done instead.
upd_node->is_delete is set in calc_row_difference() just to keep
versioning code as much in one place as possible. vers_make_delete()
is still located in row_update_for_mysql() as this is required for
ha_innodbase::delete_row() as well.
row_ins_duplicate_error_in_clust():
Restrict DB_FOREIGN_DUPLICATE_KEY to the better conditions.
VERSIONED_DELETE is used specifically to help lower stack to
understand what caused current insert. Related to MDEV-29813.
Constraints processing row_ins_check_foreign_constraint() was not
called because row_upd_check_references_constraints() didn't see
update as delete: node->is_delete was false.
Since MDEV-30378 we check for TRG_EVENT_DELETE to detect versioned
delete in ha_innobase::update_row().
Now we can use TRG_EVENT_DELETE to set upd_node->is_delete, so
constraints processing is triggered correctly.
When replicating MDL events for a table that uses system versioning
without primary keys, ensure that for data sets with duplicate
records, the updates to these records with duplicates are enacted on
the correct row. That is, there was a bug (reported in MDEV-30430)
such that the function to find the row to update would stop after
finding the first matching record. However, in the absence of
primary keys, the version of the record is needed to compare the row
to ensure we are updating the correct one.
The fix, therefore, updates the record comparison functionality to
use system version columns when there are no primary keys on the
table.
Reviewed By:
============
Andrei Elkin <andrei.elkin@mariadb.com>
EXPLAIN EXTENDED for an UPDATE/DELETE/INSERT/REPLACE statement did not
produce the warning containing the text representation of the query
obtained after the optimization phase. Such warning was produced for
SELECT statements, but not for DML statements.
The patch fixes this defect of EXPLAIN EXTENDED for DML statements.
The main difference in code path between EQ_REF and REF is that for
REF we have to do an extra read_next on the index to check that there
is no more matching rows.
Before this patch we added a preference of EQ_REF by ensuring that REF
would always estimate to find at least 2 rows.
This patch adds the cost of the extra key read_next to REF access and
removes the code that limited REF to at least 2 rows. For some queries
this can have a big effect as the total estimated rows will be halved
for each REF table with 1 rows.
multi_range cost calculations are also changed to take into account
the difference between EQ_REF and REF.
The effect of the patch to the test suite:
- About 80 test case changed
- Almost all changes where for EXPLAIN where estimated rows for REF
where changed from 2 to 1.
- A few test cases using explain extended had a change of 'filtered'.
This is because of the estimated rows are now closer to the
calculated selectivity.
- A very few test had a change of table order.
This is because the change of estimated rows from 2 to 1 or the small
cost change for REF
(main.subselect_sj_jcl6, main.group_by, main.dervied_cond_pushdown,
main.distinct, main.join_nested, main.order_by, main.join_cache)
- No key statistics and the estimated rows are now smaller which cased
estimated filtering to be lower.
(main.subselect_sj_mat)
- The number of total rows are halved.
(main.derived_cond_pushdown)
- Plans with 1 row changed to use RANGE instead of REF.
(main.group_min_max)
- ALL changed to REF
(main.key_diff)
- Key changed from ref + index_only to PRIMARY key for InnoDB, as
OPTIMIZER_ROW_LOOKUP_COST + OPTIMIZER_ROW_NEXT_FIND_COST is smaller than
OPTIMIZER_KEY_LOOKUP_COST + OPTIMIZER_KEY_NEXT_FIND_COST.
(main.join_outer_innodb)
- Cost changes printouts
(main.opt_trace*)
- Result order change
(innodb_gis.rtree)
This was done after discussions with Igor, Sanja and Bar.
The main reason for removing the deprication was to ensure that MariaDB
is always backward compatible whenever possible.
Other things:
- Added statistics counters, mainly for the feedback plugin.
- INTO OUTFILE
- INTO variable
- If INTO is using the old syntax (end of query)
This includes all test changes from
"Changing all cost calculation to be given in milliseconds"
and forwards.
Some of the things that caused changes in the result files:
- As part of fixing tests, I added 'echo' to some comments to be able to
easier find out where things where wrong.
- MATERIALIZED has now a higher cost compared to X than before. Because
of this some MATERIALIZED types have changed to DEPENDEND SUBQUERY.
- Some test cases that required MATERIALIZED to repeat a bug was
changed by adding more rows to force MATERIALIZED to happen.
- 'Filtered' in SHOW EXPLAIN has in many case changed from 100.00 to
something smaller. This is because now filtered also takes into
account the smallest possible ref access and filters, even if they
where not used. Another reason for 'Filtered' being smaller is that
we now also take into account implicit filtering done for subqueries
using FIRSTMATCH.
(main.subselect_no_exists_to_in)
This is caluculated in best_access_path() and stored in records_out.
- Table orders has changed because more accurate costs.
- 'index' and 'ALL' for small tables has changed to use 'range' or
'ref' because of optimizer_scan_setup_cost.
- index can be changed to 'range' as 'range' optimizer assumes we don't
have to read the blocks from disk that range optimizer has already read.
This can be confusing in the case where there is no obvious where clause
but instead there is a hidden 'key_column > NULL' added by the optimizer.
(main.subselect_no_exists_to_in)
- Scan on primary clustered key does not report 'Using Index' anymore
(It's a table scan, not an index scan).
- For derived tables, the number of rows is now 100 instead of 2,
which can be seen in EXPLAIN.
- More tests have "Using index for group by" as the cost of this
optimization is now more correct (lower).
- A primary key could be preferred for a normal key, even if it would
access more rows, as it's faster to do 1 lokoup and 3 'index_next' on a
clustered primary key than one lookup trough a secondary.
(main.stat_tables_innodb)
Notes:
- There was a 4.7% more calls to best_extension_by_limited_search() in
the main.greedy_optimizer test. However examining the test results
it looked that the plans where slightly better (eq_ref where more
chained together) so I assume this is ok.
- I have verified a few test cases where there was notable/unexpected
changes in the plan and in all cases the new optimizer plans where
faster. (main.greedy_optimizer and some others)
node->is_delete was incorrectly set to NO_DELETE for a set of operations.
In general we shouldn't rely on sql_command and look for more abstract ways
to control the behavior.
trg_event_map seems to be a suitable way. To mind replica nodes, it is ORed
with slave_fk_event_map, which stores trg_event_map when replica has
triggers disabled.
1. In case of system-versioned table add row_end into FTS_DOC_ID index
in fts_create_common_tables() and innobase_create_key_defs().
fts_n_uniq() returns 1 or 2 depending on whether the table is
system-versioned.
After this patch recreate of FTS_DOC_ID index is required for
existing system-versioned tables. If you see this message in error
log or server warnings: "InnoDB: Table db/t1 contains 2 indexes
inside InnoDB, which is different from the number of indexes 1
defined in the MariaDB" use this command to fix the table:
ALTER TABLE db.t1 FORCE;
2. Fix duplicate history for secondary unique index like it was done
in MDEV-23644 for clustered index (932ec586aa). In case of
existing history row which conflicts with currently inseted row we
check in row_ins_scan_sec_index_for_duplicate() whether that row
was inserted as part of current transaction. In that case we
indicate with DB_FOREIGN_DUPLICATE_KEY that new history row is not
needed and should be silently skipped.
3. Some parts of MDEV-21138 (7410ff436e) reverted. Skipping of
FTS_DOC_ID index for history rows made problems with purge
system. Now this is fixed differently by p.2.
4. wait_all_purged.inc checks that we didn't affect non-history rows
so they are deleted and purged correctly.
Additional FTS fixes
fts_init_get_doc_id(): exclude history rows from max_doc_id
calculation. fts_init_get_doc_id() callback is used only for crash
recovery.
fts_add_doc_by_id(): set max value for row_end field.
fts_read_stopword(): stopwords table can be system-versioned too. We
now read stopwords only for current data.
row_insert_for_mysql(): exclude history rows from doc_id validation.
row_merge_read_clustered_index(): exclude history_rows from doc_id
processing.
fts_load_user_stopword(): for versioned table retrieve row_end field
and skip history rows. For non-versioned table we retrieve 'value'
field twice (just for uniformity).
FTS tests for System Versioning now include maybe_versioning.inc which
adds 3 combinations:
'vers' for debug build sets sysvers_force and
sysvers_hide. sysvers_force makes every created table
system-versioned, sysvers_hide hides WITH SYSTEM VERSIONING
for SHOW CREATE.
Note: basic.test, stopword.test and versioning.test do not
require debug for 'vers' combination. This is controlled by
$modify_create_table in maybe_versioning.inc and these
tests run WITH SYSTEM VERSIONING explicitly which allows to
test 'vers' combination on non-debug builds.
'vers_trx' like 'vers' sets sysvers_force_trx and sysvers_hide. That
tests FTS with trx_id-based System Versioning.
'orig' works like before: no System Versioning is added, no debug is
required.
Upgrade/downgrade test for System Versioning is done by
innodb_fts.versioning. It has 2 combinations:
'prepare' makes binaries in std_data (requires old server and OLD_BINDIR).
It tests upgrade/downgrade against old server as well.
'upgrade' tests upgrade against binaries in std_data.
Cleanups:
Removed innodb-fts-stopword.test as it duplicates stopword.test
For commands
(1) alter table t1 add partition (partition p2);
(2) alter table t1 add partition (partition px history);
It printed the same error message:
Wrong partitioning type, expected type: `SYSTEM_TIME`
For (1) it is not clear from the syntax that we are trying to add
HASH partition. For (2) it is not clear that the table partitioning is
different than SYSTEM_TIME. Now it prints what type we are trying to
add to what type of partitioning.
Fixed warning unused function rename_field_in_list() for compilation
without partitioning.
For commands
(1) alter table t1 add partition (partition p2);
(2) alter table t1 add partition (partition px history);
It printed the same error message:
Wrong partitioning type, expected type: `SYSTEM_TIME`
For (1) it is not clear from the syntax that we are trying to add
HASH partition. For (2) it is not clear that the table partitioning is
different than SYSTEM_TIME. Now it prints what type we are trying to
add to what type of partitioning.
Specifically:
Revert "MDEV-29664 Assertion `!n_mysql_tables_in_use' failed in innobase_close_connection"
This reverts commit ba875e9396.
Revert "MDEV-29620 Assertion `next_insert_id == 0' failed in handler::ha_external_lock"
This reverts commit aa08a7442a.
Revert "MDEV-29628 Memory leak after CREATE OR REPLACE with foreign key"
This reverts commit c579d66ba6.
Revert "MDEV-29609 create_not_windows test fails with different result"
This reverts commit cb583b2f1b.
Revert "MDEV-29544 SIGSEGV in HA_CREATE_INFO::finalize_locked_tables"
This reverts commit dcd66c3814.
Revert "MDEV-28933 CREATE OR REPLACE fails to recreate same constraint name"
This reverts commit cf6c517632.
Revert "MDEV-28933 Moved RENAME_CONSTRAINT_IDS to include/sql_funcs.h"
This reverts commit f1e1c1335b.
Revert "MDEV-28956 Locking is broken if CREATE OR REPLACE fails under LOCK TABLES"
This reverts commit a228ec80e3.
Revert "MDEV-25292 gcol.gcol_bugfixes --ps fix"
This reverts commit 24fff8267d.
Revert "MDEV-25292 Disable atomic replace for slave-generated or-replace"
This reverts commit 2af15914cb.
Revert "MDEV-25292 backup_log improved"
This reverts commit 34398a20b5.
Revert "MDEV-25292 Atomic CREATE OR REPLACE TABLE"
This reverts commit 93c8252f02.
Revert "MDEV-25292 Table_name class for (db, table_name, alias)"
This reverts commit d145dda9c7.
Revert "MDEV-25292 ha_table_exists() cleanup and improvement"
This reverts commit 409b8a86de.
Revert "MDEV-25292 Cleanups"
This reverts commit 595dad83ad.
Revert "MDEV-25292 Refactoring: moved select_field_count into Alter_info."
This reverts commit f02af1d229.
don't set vers_write=false if one vers column was used explicitly,
instead do vers_update_fields() for columns that do not have explicit
value. So, if row_start has the value and row_end not, row_end will
get max by default.
* clarify the help text for --system-versioning-insert-history
* move the vers_write=false check from Item_field::fix_fields()
next to other vers field checks in find_field_in_table()
* move row_start validation from handler::write_row() next to
vers_update_fields()
* make secure_timestamp check to happen in one place only,
extract it into a function is_set_timestamp_vorbidden().
* overwriting vers fields is an error, just like setting @@timestamp
* don't run vers_insert_history() for every row
1. system_versioning_insert_history session variable allows
pseudocolumns ROW_START and ROW_END be specified in INSERT,
INSERT..SELECT and LOAD DATA.
2. Cleaned up select_insert::send_data() from setting vers_write as
this parameter is now set on TABLE initialization.
4. Replication of system_versioning_insert_history via option_bits in
OPTIONS_WRITTEN_TO_BIN_LOG.
To prevent ASAN heap-use-after-poison in the MDEV-16549 part of
./mtr --repeat=6 main.derived
the initialization of Name_resolution_context was cleaned up.
