Issue:
Mariadb acquires additional MDL locks on UPDATE/INSERT/DELETE statements
on table with foreign keys. For example, table t1 references t2, an
UPDATE to t1 will MDL lock t2 in addition to t1.
A replica may deliver an ALTER t1 and UPDATE t2 concurrently for
applying. Then the UPDATE may acquire MDL lock for t1, followed by a
conflict when the ALTER attempts to MDL lock on t1. Causing a BF-BF
conflict.
Solution:
Additional keys for the referenced/foreign table needs to be added
to avoid potential MDL conflicts with concurrent update and DDLs.
Signed-off-by: Julius Goryavsky <julius.goryavsky@mariadb.com>
Valgrind is single threaded and only changes threads as part of
system calls or waits.
Some busy loops were identified and fixed where the server assumes
that some other thread will change the state, which will not happen
with valgrind.
Based on patch by Monty. Original patch introduced VALGRIND_YIELD,
which emits pthread_yield() only in valgrind builds. However it was
agreed that it is a good idea to emit yield() unconditionally, such
that other affected schedulers (like SCHED_FIFO) benefit from this
change. Also avoid pthread_yield() in favour of standard
std::this_thread::yield().
Get rid of need of matherialization for usual INSERT (cache results in
Item_cache* if needed)
- subqueries in VALUE do not see new records in the table we are
inserting to
- subqueries in RETIRNING prohibited to use the table we are inserting to
"t1 JOIN t2 USING(col1,...)" calls mark_common_columns() to mark the
listed columns as used in both used tables, t1 and t2.
Due to a typo bug, it would mark the wrong column in the second table
(t2): instead of t2.col1 it would mark the last column in t2.
The harmful effects included JOIN_TAB(t2)->covering_keys not being
set correctly. This changed the cost to access the table and then
caused different query plans depending on which table was the second
in the JOIN ... USING syntax.
Newer gcc reports:
error: 'rfield' may be used uninitialized [-Werror=maybe-uninitialized]
9041 | unwind_stored_field_offsets(fields, rfield);
After investigation, it turned to be an impossible case:
1. The only way it could be broken is if
if (!(field= fld->field_for_view_update()))
line case would succeed from the first time.
2. Consequent checks initialize rfield.
fld may return NULL in field_for_view_update() only for views.
3. Before fill_record, UPDATE first calls check_fields, where
field_for_view_update() result is already checked. INSERT calls
check_view_insertability that checks that all view fields are
updateable.
It all means that field_for_view_update() cannot be NULL in fill_record,
so the if can be converted to DBUG_ASSERT.
This essentially shifts the responsibility on preliminary
field_for_view_update() check to the caller.
In this patch:
1. convert field_for_view_update() check to DBUG_ASSERT
2. harden unwind_stored_field_offsets function so that it can be used
even if field_for_view_update() is NULL
3. As a consequence, `field` is passed instead of `rfield` as a
terminator.
4. Initialize `field` to NULL to bypass a false-positive warning!
MDEV-34171 denied removing indirect routines/tables after
recover_from_failed_open() for auto-create partition case. Now we are
going further and keep them for any failed table reopen.
MDEV-34171 did not handle correctly open_and_process_routine() after
that skip of sp_remove_not_own_routines(). Now it is fixed by
sroutine_to_open correct usage.
table->move_fields has some limitations:
1. It cannot be used in cascade
2. It should always have a restoring pair
In this case, an error has occurred before the field ptr was restored, returning
from the function in that state. Even in case of an error, the table can be
reused afterwards and table->field[i]->ptr is not reset in between.
The solution is to restore the field pointers immanently whenever they've been
deviated.
Also add an assertion that ensures that table fields are restored after the use
in close_thread_tables.
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
Note: Changes to the test innodb.stats_persistent
in commit e5c4c0842d (MDEV-35443)
are not merged, because the test scenario is impossible
due to commit e66928ab28 (MDEV-33462).
Fix a regression introduced by commit d98ac851 (MDEV-29935, MDEV-26247) causing
MAX_TABLES overflow in `setup_table_map()`. The check for MAX_TABLES was moved
outside of the loop that increments table numbers, allowing overflows during
loop iterations. Since setup_table_map() operates on a 64-bit bitmap, table
numbers exceeding 64 triggered the UBSAN check.
This commit returns the overflow check within the loop and adds a debug
assertion to `setup_table_map()` to ensure no bitmap overrun occurs.
This problem occured for statements like `INSERT INTO t1 SELECT 1`,
which do not have tables in the SELECT part. In such scenarios
SELECT_LEX::insert_tables was not properly set at `setup_tables()`,
and this led to either incorrect execution or a crash
Reviewer: Oleksandr Byelkin <sanja@mariadb.com>
This bug has the same nature as the issues
MDEV-34718: Trigger doesn't work correctly with bulk update
MDEV-24411: Trigger doesn't work correctly with bulk insert
To fix the issue covering all use cases, resetting the thd->bulk_param
temporary to the value nullptr before invoking triggers and restoring
its original value on finishing execution of a trigger is moved to the method
Table_triggers_list::process_triggers
that be invoked ultimately for any kind of triggers.
The problem was that when using clang + asan, we do not get a correct value
for the thread stack as some local variables are not allocated at the
normal stack.
It looks like that for example clang 18.1.3, when compiling with
-O2 -fsanitize=addressan it puts local variables and things allocated by
alloca() in other areas than on the stack.
The following code shows the issue
Thread 6 "mariadbd" hit Breakpoint 3, do_handle_one_connection
(connect=0x5080000027b8,
put_in_cache=<optimized out>) at sql/sql_connect.cc:1399
THD *thd;
1399 thd->thread_stack= (char*) &thd;
(gdb) p &thd
(THD **) 0x7fffedee7060
(gdb) p $sp
(void *) 0x7fffef4e7bc0
The address of thd is 24M away from the stack pointer
(gdb) info reg
...
rsp 0x7fffef4e7bc0 0x7fffef4e7bc0
...
r13 0x7fffedee7060 140737185214560
r13 is pointing to the address of the thd. Probably some kind of
"local stack" used by the sanitizer
I have verified this with gdb on a recursive call that calls alloca()
in a loop. In this case all objects was stored in a local heap,
not on the stack.
To solve this issue in a portable way, I have added two functions:
my_get_stack_pointer() returns the address of the current stack pointer.
The code is using asm instructions for intel 32/64 bit, powerpc,
arm 32/64 bit and sparc 32/64 bit.
Supported compilers are gcc, clang and MSVC.
For MSVC 64 bit we are using _AddressOfReturnAddress()
As a fallback for other compilers/arch we use the address of a local
variable.
my_get_stack_bounds() that will return the address of the base stack
and stack size using pthread_attr_getstack() or NtCurrentTed() with
fallback to using the address of a local variable and user provided
stack size.
Server changes are:
- Moving setting of thread_stack to THD::store_globals() using
my_get_stack_bounds().
- Removing setting of thd->thread_stack, except in functions that
allocates a lot on the stack before calling store_globals(). When
using estimates for stack start, we reduce stack_size with
MY_STACK_SAFE_MARGIN (8192) to take into account the stack used
before calling store_globals().
I also added a unittest, stack_allocation-t, to verify the new code.
Reviewed-by: Sergei Golubchik <serg@mariadb.org>
Fixed by checking handler_stats if it's active instead of
thd->variables.log_slow_verbosity & LOG_SLOW_VERBOSITY_ENGINE.
Reviewed-by: Sergei Petrunia <sergey@mariadb.com>
