MDEV-16417 MDEV-20865 Prerequisites and cleanups - #5586
Conversation
|
|
There was a problem hiding this comment.
Pull request overview
Refactors FRM metadata and prelocking infrastructure in preparation for foreign-key metadata storage.
Changes:
- Centralizes and refactors extra2 FRM serialization.
- Updates prelocking state handling and FK-dependent tests/scripts.
- Limits compatibility warnings and improves parallel gcov reliability.
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
sql/unireg.h |
Removes relocated FRM definitions. |
sql/unireg.cc |
Cleans up FRM construction and extra2 writing. |
sql/table.h |
Removes relocated visibility constant. |
sql/table.cc |
Uses Extra2_info for FRM parsing. |
sql/sql_update.cc |
Chains multi-update prelocking reset. |
sql/sql_delete.cc |
Chains multi-delete prelocking reset. |
sql/sql_base.h |
Moves prelocking state into the base strategy. |
sql/sql_base.cc |
Refactors prelocking extension calls. |
sql/mysqld.cc |
Restricts removed-option warnings. |
sql/datadict.h |
Defines extra2 structures and helpers. |
sql/datadict.cc |
Implements Extra2_info reading and writing. |
scripts/mariadb_system_tables.sql |
Reorders help-table creation. |
scripts/mariadb_system_tables_fix.sql |
Reorders help-table alteration. |
mysys/my_init.c |
Restricts deprecated-name warnings. |
mysql-test/suite/innodb/t/instant_alter_index_rename.test |
Adds a valid referenced table. |
mysql-test/suite/innodb/r/instant_alter_index_rename.result |
Updates expected output. |
mysql-test/main/union.test |
Creates and drops FK references correctly. |
mysql-test/main/union.result |
Updates expected output. |
mysql-test/main/system_mysql_db_fix50568.test |
Reorders FK-related help tables. |
mysql-test/main/system_mysql_db_fix50568.result |
Updates expected output. |
mysql-test/main/system_mysql_db_fix50117.test |
Reorders FK-related help tables. |
mysql-test/main/system_mysql_db_fix50117.result |
Updates expected output. |
mysql-test/main/system_mysql_db_fix50030.test |
Reorders FK-related help tables. |
mysql-test/main/system_mysql_db_fix50030.result |
Updates expected output. |
mysql-test/main/system_mysql_db_fix40123.test |
Reorders FK-related help tables. |
mysql-test/main/system_mysql_db_fix40123.result |
Updates expected output. |
mysql-test/main/opt_context_store_ddls.test |
Corrects FK drop order. |
mysql-test/main/opt_context_store_ddls.result |
Includes FK metadata and new order. |
mysql-test/main/insert_notembedded.test |
Corrects FK target and lifecycle order. |
mysql-test/main/insert_notembedded.result |
Updates expected output. |
mysql-test/main/fetch_first.test |
Adds referenced index and fixes drop order. |
mysql-test/main/fetch_first.result |
Updates expected output. |
CMakeLists.txt |
Enables atomic gcov profile updates. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
afa1d1a to
7f90efe
Compare
7f90efe to
48292c9
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 33 changed files in this pull request and generated 2 comments.
Suppressed comments (1)
sql/datadict.cc:298
- Remove the extra “in” so this TODO reads grammatically.
TODO: now this is used in by MDEV-20865 but this can be also used by
5fdfb2a to
b58d55b
Compare
Two compatibility warnings are meant for end users but only add noise to debug/test builds: mysqld is commonly started through a legacy "mysqld" symlink, and test .cnf files still reference long-removed options. Emit them in release builds only. - mysys/my_init.c: the deprecated-invocation-name warning (EE_NAME_DEPRECATED), shown when the program is run via a non-"mariadb" symlink to a "mariadb*" binary -- now compiled under #ifdef NDEBUG. - sql/mysqld.cc: the "'<opt>' was removed ... exists only for compatibility" warning for OPT_REMOVED_OPTION -- now under #ifdef DBUG_OFF.
Fix coverage data corruption by enabling atomic profile updates When compiling with coverage instrumentation, concurrent or forked test executions can cause corruption of coverage data files (.gcda), resulting in negative hit counts and invalid coverage reports. Adding the compiler flag `-fprofile-update=atomic` ensures that updates to coverage counters are performed atomically, preventing race conditions during profile data writes. This change eliminates warnings about unexpected negative hit counts and improves the accuracy and reliability of coverage measurements in parallel or multithreaded test environments.
- Moved has_prelocking_list into Prelocking_strategy (removed it from Multiupdate_prelocking_strategy). It is now initialized in Prelocking_strategy::reset(), so reset() overrides (Multiupdate_prelocking_strategy, Multidelete_prelocking_strategy) chain to the base implementation to set it. - Moved extend_table_list() into Prelocking_strategy; - Made maybe_need_prelocking() virtual method. Now maybe_need_prelocking() can be different per prelocking strategy.
datadict.h is appropriate place for such types. These data types are used by Extra2_info, which is added to datadict.h later. unireg.cc is for older FRM routines. datadict.cc accepts new and refactored routines.
42e3a6d to
e796b86
Compare
Cleanups: - build_frm_image(): clearer names and named constants - engine_table_options_frm_image(): mark in/out argument directions Improvements: - build_frm_image(): guard against overflow of the 2-byte key-info offset - extra2_read_len(): validate the length against the remaining bytes build_frm_image(): the key-information offset is stored in a 2-byte field, yet only each individual extra2 value was length-checked. A definition with enough extra2 content could push frm.length past 0xFFFF, silently truncating the offset in int2store() and yielding an unreadable frm. Reject such a definition before the store. extra2_read_len(): the short-form (1-byte) length was returned without verifying that its payload fits before the segment end, unlike the long-form encoding. A truncated or corrupt frm could produce an out-of-range span that callers consume before any later boundary check. Both encodings are now checked against the remaining bytes.
Current implementation of extra2_write_len() does not guarantee of writing correct 2-byte values as it skips writing zero at the beginning. Refined DBUG_ASSERT() to more truthful limit.
read_extra2() is now Extra2_info::read() Additional assertions for checking size consistency. Extra2_info::write() is used by further MDEV-20865 development.
Storing foreign key metadata in TABLE_SHARE makes opening a foreign table
preopen its referenced tables, so a referenced table must be created
before the foreign one and dropped after it. Rearrange the order of
CREATE/DROP commands in tests and system scripts accordingly.
Affected: the help tables in mariadb_system_tables{,_fix}.sql (with
system_mysql_db_fix* results), fetch_first, union, insert_notembedded,
instant_alter_index_rename and opt_context_store_ddls; some also index a
referenced column or add a missing referenced table, and
opt_context_store_ddls re-records the now-shown MyISAM foreign key.
dcb6474 to
6c125ea
Compare
Preparatory refactorings and cleanups that precede the main "Store foreign
key metadata in TABLE_SHARE" work (MDEV-16417 / MDEV-20865). On their own they
introduce no user-visible feature change — they reshape the FRM/data-dictionary
code, prelocking, tests and build tooling so the FK-in-TABLE_SHARE commits apply
cleanly.
FRM / data dictionary
next to the new/refactored datadict routines (
unireg.ccstays for the legacyFRM code); they feed the upcoming
Extra2_info.skipped a leading zero); tighten the related
DBUG_ASSERT.read_extra2()becomesExtra2_info::read()with size-consistency assertions;Extra2_info::write()is used by later MDEV-20865 work.
Prelocking
has_prelocking_listandextend_table_list()into the basePrelocking_strategy, initializehas_prelocking_listinreset()(subclasses chain to base), and makemaybe_need_prelocking()virtual so it can vary per strategy.Tests & system scripts
metadata in TABLE_SHARE makes opening a foreign table preopen its referenced
tables, referenced tables must be created before (and dropped after) the foreign
ones. Reorder CREATE/DROP in the help-table scripts
(
mariadb_system_tables{,_fix}.sql) and several tests; some also index areferenced column or add a missing referenced table.
Build / tooling
mysqld-symlink and removed-optioncompatibility warnings in release builds only, so they don't pollute debug/test
output.
-fprofile-update=atomicso parallel/forked test runs don'tcorrupt
.gcdacoverage data.