When a select containing 32 ranges is made on a table containing a
compound key with 32 parts, the range optimizer can run off the end
of a stack variable, invalidly overwriting subsequent stack variables.
In the struct st_sel_arg_range_seq, we have an array
RANGE_SEQ_ENTRY stack[MAX_REF_PARTS];
MAX_REF_PARTS is 32.
check_quick_select
/ sel_arg_range_seq_init
initialises stack[0] as NOT a key part
/ sel_arg_range_seq_next
iterates through the key parts, adding key part n to stack[n+1]
key part #32 gets referenced by step_down_to(), setting seq->i off the
end of the array.
Fix: RANGE_SEQ_ENTRY stack[MAX_REF_PARTS+1];
The above change exposed an issue with key length calculation on
MS Windows. Calling make_prev_keypart_map(32) caused the resultant
bitmap to be calculated as (1UL << 32) - 1. Using the MSVC compiler
this resulted in an empty key length calculation during
handler::index_read_map, causing an assertion in ha_innobase::index_read().
As we only need 32 bits to represent our key map, we change the type thus
-typedef ulong key_part_map;
+typedef uint32 key_part_map;
We correct make_keypart_map() and make_prev_keypart_map() to call our
overflow safe my_set_bits(). We also correct bka_range_seq_next()
and bkah_range_seq_next() to use make_prev_keypart_map().
We also add some DBUG_ASSERTS in key_part_map processing elsewhere,
exposing some issues in our BNLH implementation. We cap the number of
keyuse parts here, altering the explain output of 2 of our tests.
Numerous places needed bit shift operations altered to use
make*keymap_part and various format strings needed to be corrected.
Problem:
Executing a range select using an index that includes virtual/generated columns
(e.g. SELECT ... WHERE c BETWEEN ... ORDER BY d) could trigger:
Assertion 'marked_for_read()' failed in Field access during execution.
Cause:
The keyread initialization path was populating table->read_set using
mark_index_columns(), which follows the non-recursive index marking path.
This only marks direct index fields and does not resolve dependencies of
virtual/generated columns.
As a result, when an index contained virtual columns, their dependent base
columns were not included in read_set, leading to invalid access during
later evaluation of virtual columns.
Fix:
Replace mark_index_columns() with mark_index_columns_for_read() in the
keyread initialization path. This ensures the recursive read-marking logic is
used, so virtual/generated column dependencies are correctly propagated into
read_set before execution.
Problem: Executing queries that require virtual/generated column
evaluation during filesort trigger debug assertions due to missing
columns in read_set. Release builds return wrong results: rows that do
not match the WHERE, or missing rows that do.
Cause: find_all_keys() temporarily assigns TABLE::tmp_set as both
read_set and write_set (keyread does the same). InnoDB calls
TABLE::update_virtual_field() in the middle of the
scan (row_sel_sec_rec_is_for_clust_rec() ->
innobase_get_computed_value()) when it checks a secondary index record
against an older row version, a delete-marked record, or at READ
UNCOMMITTED. TABLE::update_virtual_field() calls
bitmap_clear_all(&tmp_set) before evaluating virtual column
dependencies. Since all three pointers share the same underlying
bitmap buffer, clearing tmp_set also clears the active
read_set/write_set, causing required columns to appear missing during
execution and triggering the assertion.
Fix: The fix uses a local bitmap instead of tmp_set for the virtual
column dependency walk in TABLE::update_virtual_field(), so the active
read_set/write_set is not touched.
Test: vcol.vcol_keys_innodb covers READ UNCOMMITTED (MDEV-36896) and a
concurrent uncommitted update under REPEATABLE READ (MDEV-41365).
Duplicated by MDEV-41365.
The local variable key_part_end that was introduced in
commit 3b1c2e58ab (MDEV-40571)
would trigger a GCC warning. The warning is bogus; in fact,
before the variable will be read, it will have been initialised
when either keys==0 or i==0, which covers each code path
that does not lead to a return.
Applying HEX encoding write writting an ENUM/SET TYPELIB to FRM
if the TYPELIB has 0x00 bytes in the value.
This HEX encoding was earlier used only to write UCS2/UTF16/UTF32 TYPELIBs.
A new flag FIELDFLAG_FRM_HEX_ENCODED_TYPELIB was added to indicate
that the TYPELIB is hex encoded. It's used only inside FRM.
Note, it's mangled with FIELDFLAG_TREAT_BIT_AS_CHAR.
This should not be harmful:
- BIT and ENUM/SET columns are handled by two separate code branches
when opening an FRM
- The flag is unset immediately after decoding TYPELIB, so the rest
of the code does not se an unexpected flag combination.
numerous checks that the frm is valid, no OOB reads,
values make sense (number of keyparts not less than number of keys,
no keys means no keyparts, number of long unique fields is not larger than
number of fields, fields values in the record don't overlap and don't
go over record ends, and so on). most asserts were changed to if()'s
Introduce CAN_BE_NULL annotation into Table_check_intact checks.
By default columns are considered NOT NULL, nullable
columns must be explicitly marked with CAN_BE_NULL.
All TABLE_FIELD_TYPE arrays are fixed as above.
Assisted-By: Claude:claude-4.6-sonnet
1.
Only allow #mysql50# prefix on table names that
can not be decoded from a file name to a table name.
It is an error to put this prefix on a name that can be accessed
as a valid table name without a prefix. Use `t1` not `#mysql50#t1`,
use `a-b` not `#mysql50#a@002db`.
2.
Don't allow creation of new tables with the #mysql50# prefix.
Except when in SQLCOM_ALTER_DB_UPGRADE - main.upgrade test runs
ALTER DATABASE `#mysql50#mysqltest-1` UPGRADE DATA DIRECTORY NAME;
This command moves tables from the old db name to the new db name,
and some of these tables also have old #mysql50# names.
3.
Also don't allow to create such databases, views, partitions,
subpartitions, sequences, triggers.
Three problems:
1. Option value specified as `foo bar` was not quoted,
printed as foo bar, breaking CREATE TABLE.
2. Option value that included */ was printed as is, in unknown
options when the whole option was commented out, this broke the
comment: /* opt='val*/ue' */
3. Option names can include */ too, `opt*/ion`=1
Fix: simplify value printing, ignore the original quoting, always
print the value quoted as a string. Break comment markers with an
escape (as *\/) if inside a comment. As \-escapes don't work inside,
backticks, we cannot use `-quoting for option values at all.
For the same reason we cannot allow */ inside option names, thus
let's only allow keywords for option names and reject fancy names
even if IGNORE_BAD_TABLE_OPTIONS is in force.
A couple of things interact in this scenario.
1. A virtual column whose value depends on session state, such as a
VCOL calling DATE_FORMAT, has fix_fields (via fix_expr) run on it
again before each INSERT. For a table reused from the cache this runs in
Vcol_expr_context before setup_tables sets the table map. The test
first opens t1 with a SELECT that fails before setup_tables, leaving
it cached with map zero.
2. When the connection character set needs conversion, CONCAT (more
generally, any charset aggregation) wraps that column in a charset
converter. The converter evaluates any argument reported as
constant, so it calls val_str() on the column as part of
fix_fields, before a row exists and without the column in the read
set. On DEBUG builds this fails the marked_for_read assertion;
otherwise it reads an unread column.
Consequently:
When a table instance is opened, fix_fields runs with table->map
already nonzero, but the fix_fields (via fix_expr) call from vcol_fix_expr
did not have it nonzero; so set table->map=1 during Vcol_expr_context::init.
The original Vcol_expr_context::init() (MDEV-24176) called
init_lex_with_single_table, which set table->map= 1 as a side effect.
A later merge of 10.2 into 10.3 (6f6c74b0d1) replaced that call with
a backup of table->expr_arena, dropping the side effect.
The destructor of the Vcol_expr_context will reset the map back to its
original value.
Remove the unscoped #pragma GCC diagnostic ignored
"-Wdeprecated-declarations" from include/violite.h and replace all
sprintf/vsprintf calls with snprintf/vsnprintf across the codebase.
Where possible, pass actual buffer sizes as function parameters
instead of using hardcoded constants or recomputing strlen().
Notable API changes:
- get_date(): added to_len parameter for buffer size
- nice_time(), end_timer() in client/mysql.cc: added buff_size
- generate_new_name(): added name_size parameter
- calc_md5(): added buffer_size parameter (fixes off-by-one)
- MD5_HASH_TO_STRING macro: added _size parameter
- Added MYSQL_UDF_MAX_RESULT_LENGTH constant in mysql_com.h
Also fixed a pre-existing bug in sql/rpl_mi.cc where the ellipsis
"..." was written to the wrong buffer (dbuff instead of buff).
Vendored code is excluded: extra/readline, extra/wolfssl,
wsrep-lib, libmariadb, storage/mroonga/vendor, zlib.
Rename cloning methods of class Item and its descendants
in the following way:
(from) (to)
do_build_clone -> deep_copy
build_clone -> deep_copy_with_checks
do_get_copy -> shallow_copy
get_copy -> shallow_copy_with_checks
to better reflect their functionality.
Also make Item::deep_copy() and shallow_copy() protected.
Outside users should call deep_copy_with_checks()
and shallow_copy_with_checks().
The issue was that unpack_vcol_info_from_frm() wrongly linked the used
sequence tables into tables->internal_tables when more than one sequence
table was used.
Other things:
- Fixed internal_table_exists() to take db into account.
(This is making the code easier to read. As we where comparing
pointers the old code also worked).
- 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.
Issue:
When applying 'SELECT NEXT VALUE..' on applier node with
binlog_row_image=MINIMAL and log-binlog enabled, applier
node fails with below error:
Slave SQL: Could not execute Write_rows_v1 event on table monitor.seq_moni_num; Unknown error, Error_code: 1105; handler error No Error!; the event's master log FIRST, end_log_pos 0, Internal MariaDB error code: 1105
To reproduce run below command on the first/active node:
> CREATE SEQUENCE `seq_test` start with 1 minvalue 1 maxvalue 9223372036854775806 increment by 0 cache 1000 nocycle ENGINE=InnoDB;
> SELECT NEXT VALUE FOR seq_test;
> SELECT NEXT VALUE FOR seq_test;
The applier node will leave the cluster after executing
the 'SELECT NEXT VALUE' with below error:
ERROR] Slave SQL: Could not execute Write_rows_v1 event on table test.seq_test; Unknown error, Error_code: 1105; handler error No Error!; the event's master log FIRST, end_log_pos 0, Internal MariaDB error code: 1105
[Warning] WSREP: Event 3 Write_rows_v1 apply failed: 195, seqno 14511334511
Solution:
When binary loggging is enabled and binlog_row_image is set to
'MINIMAL', then 'SELECT NEXT VALUE' fails to apply on applier node.
It fails with error HA_ERR_SEQUENCE_INVALID_DATA 195 in
sequence_definition::check_and_adjust() because sequence variables
like min_value, max_value, start are 0.
The marking of all columns in 'TABLE::mark_columns_per_binlog_row_image()'
will prevent update/set column values for the sequence table.
For the sequence table column bitmap sent from master is only used.
Signed-off-by: Julius Goryavsky <julius.goryavsky@mariadb.com>
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.
Lots of different cases, SELECT, SELECT DEFAULT(),
UPDATE t SET x=DEFAULT, prepares statements,
opening of a table for the I_S, prelocking (so TL_WRITE),
insert with subquery (so SQLCOM_SELECT), etc.
Don't check NEXTVAL privileges in fix_fields() anymore, it cannot
possibly handle all the cases correctly. Make a special method
Item_func_nextval::check_access() for that and invoke it from
* fix_fields on explicit SELECT NEXTVAL()
(but not if NEXTVAL() is used in a DEFAULT clause)
* when DEFAULT bareword in used in, say, UPDATE t SET x=DEFAULT
(but not if DEFAULT() itself is used in a DEFAULT clause)
* in CREATE TABLE
* in ALTER TABLE ALGORITHM=INPLACE (that doesn't go CREATE TABLE path)
* on INSERT
helpers
* Virtual_column_info::check_access() to walk the item tree and invoke
Item::check_access()
* TABLE::check_sequence_privileges() to iterate default expressions
and invoke Virtual_column_info::check_access()
also, single-table UPDATE in prepared statements now associates
value items with fields just as multi-update already did, fixes the
case of PREPARE s "UPDATE t SET x=?"; EXECUTE s USING DEFAULT.
The sequence that causes the issue:
1. file->row_logging is false because slave-bin was not open;
2. TABLE::mark_columns_per_binlog_row_image() didn't mark column for
read because file->row_logginbg is false. This was implemented in
e53ad95b73 (MDEV-6877);
3. TABLE::update_virtual_fields() didn't update virtual field value
because column is not marked for read;
4. calc_row_difference() sees o_len as UNIV_SQL_NULL, but new row
value is "1". The virtual column is added to update vector;
5. row_upd() tries to update secondary index, but row_upd_sec_step()
doesn't see old value in the index.
The patch does mark_virtual_column_with_deps() via
column_bitmaps_signal() in case of rgi_slave in
mark_columns_per_binlog_row_image() so that non-stored virtual columns
are marked for update in slave thread.
Also fixed column_bitmaps_signal() for partitioning where the signal
did not reach the partition storage engine.
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.
MariaDB server crashes when a query includes a derived table
containing unnamed column (eg: `SELECT '' from t`). When `Item`
object representing such unnamed column was checked for valid,
non-empty name in `TABLE_LIST::create_field_translation`, the
server crahsed(assertion `item->name.str && item->name.str[0]`
failed).
This fix removes the redundant assertion. The assert was a strict
debug guard that's no longer needed because the code safely handles
empty strings without it.
Selecting `''` from a derived table caused `item->name.str`
to be an empty string. While the pointer itself wasn't `NULL`
(`item->name.str` is `true`), its first character (`item->name.str[0]`)
was null terminator, which evaluates to `false` and eventually made
the assert fail. The code immediately after the assert can safely
handle empty strings and the assert was guarding against something
which the code can already handle.
Includes `mysql-test/main/derived.test` to verify the fix.
Calling a stored routine that executes a join on three or more tables
and referencing not-existent column name in the USING clause resulted in
a crash on its second invocation.
Server crash taken place by the reason of dereferencing null pointer
in condition of DBUG_ASSERT inside the method
Field_iterator_natural_join::next()
There the data member
cur_column_ref->table_field->field
has the nullptr value that was reset at the end of first
execution of a stored routine when the standalone procedure
cleanup_items() called by the method sp_head::execute.
Later this data member is not re-initialized and never referenced
in any place except the DBUG_ASSERT on second and later invocations
of the stored routine.
To fix the issue, the assert's condition should be augmented by
a condition '|| !cur_column_ref->table_field' before dereferencing
cur_column_ref->table_field. Such extra checking is aligned with
conditions used by DBUG_ASSERT macros used by implementation of
the class Field_iterator_table_ref that aggregated the class
Field_iterator_natural_join.
it's incorrect to zero out table->triggers->extra_null_bitmap
before a statement, because if insert uses an explicit field list
and omits a field that has no default value, the field should
get NULL implicitly. So extra_null_bitmap should have 1s for all
fields that have no defaults
* create extra_null_bitmap_init and initialize it as above
* copy extra_null_bitmap_init to extra_null_bitmap for inserts
* still zero out extra_null_bitmap for updates/deletes where
all fields definitely have a value
* make not_null_fields_have_null_values() to send
ER_NO_DEFAULT_FOR_FIELD for fields with no default and no value,
otherwise creation of a trigger with an empty body would change the
error message
MDEV-28127 did is_equal() which compared vcol expressions
literally. But another table vcol expression is not equal because of
different table name.
We implement another comparison method is_identical() which respects
different table name in vcol comparison. If any field item points to
table_A and compared field item points to table_B, such items are
treated as equal in (table_A, table_B) comparison. This is done by
cloning table_B expression and renaming any table_B entries to table_A
in it.
DELAYED with virtual columns
Segfault was cause by two different copies of same Field instance in
prepared delayed insert. One was made by
Delayed_insert::get_local_table() (see make_new_field()). That copy
went through parse_vcol_defs() and received new vcol_info->expr.
Another one was made by copy_keys_from_share() by this code:
/*
We are using only a prefix of the column as a key:
Create a new field for the key part that matches the index
*/
field= key_part->field=field->make_new_field(root, outparam, 0);
field->field_length= key_part->length;
So, key_part and table got different objects of same field and the
crash was because key_part->field->vcol_info->expr is NULL.
The fix does update_keypart_vcol_info() to update vcol_info->expr in
key_part->field.
Cleanup: memdup_vcol() is static inline instead of macro + check OOM.
This commit updates default memory allocations size used with MEM_ROOT
objects to minimize the number of calls to malloc().
Changes:
- Updated MEM_ROOT block sizes in sql_const.h
- Updated MALLOC_OVERHEAD to also take into account the extra memory
allocated by my_malloc()
- Updated init_alloc_root() to only take MALLOC_OVERHEAD into account as
buffer size, not MALLOC_OVERHEAD + sizeof(USED_MEM).
- Reset mem_root->first_block_usage if and only if first block was used.
- Increase MEM_ROOT buffers sized used by my_load_defaults, plugin_init,
Create_tmp_table, allocate_table_share, TABLE and TABLE_SHARE.
This decreases number of malloc calls during queries.
- Use a small buffer for THD->main_mem_root in THD::THD. This avoids
multiple malloc() call for new connections.
I tried the above changes on a complex select query with 12 tables.
The following shows the number of extra allocations that where used
to increase the size of the MEM_ROOT buffers.
Original code:
- Connection to MariaDB: 9 allocations
- First query run: 146 allocations
- Second query run: 24 allocations
Max memory allocated for thd when using with heap table: 61,262,408
Max memory allocated for thd when using Aria tmp table: 419,464
After changes:
Connection to MariaDB: 0 allocations
- First run: 25 allocations
- Second run: 7 allocations
Max memory allocated for thd when using with heap table: 61,347,424
Max memory allocated for thd when using Aria table: 529,168
The new code uses slightly more memory, but avoids memory fragmentation
and is slightly faster thanks to much fewer calls to malloc().
Reviewed-by: Sergei Golubchik <serg@mariadb.org>
A prepared SELECT statement because of CF_REEXECUTION_FRAGILE needs to
check the table is the same definition as previously otherwise a
re-prepare of the statement can occur.
When running many 'SELECT DEFAULT(name) FROM table1_containing_sequence'
in parallel the TABLE_LIST::is_the_same_definition may be called when
m_table_ref_type is TABLE_REF_NULL because it hasn't been checked yet.
In this case populate the TABLE_LIST with the values determined by the
TABLE_SHARE and allow the execution to continue.
As a result of this, the main.ps_ddl test doesn't need to reprepare
as the defination hasn't changed. This is another case where
TABLE_LIST::is_the_same_definition is called when m_table_ref_type is
TABLE_REF_NULL, but that doesn't mean that the defination is different.
