don't reload stored routines in the middle of the execution
of a routine. we don't want different iterations of a loop
to see diffefent definitions
For this: remember Cversion in THD on the first sp cache lookup,
after that only compare versions with this value not with Cversion.
Relaxed check, only number of columns and the PK.
Enough to avoid crashes, but doesn't break upgrades and migration
from MySQL as in MDEV-37777.
Added checks everywhere. (flush/create/alter/drop server)
Check mysql.plugin table too.
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).
See also MDEV-30046.
Idempotent write_row works same as REPLACE: if there is a duplicating
record in the table, then it will be deleted and re-inserted, with the
same update optimization.
The code in Rows:log_event::write_row was basically copy-pasted from
write_record.
What's done:
REPLACE operation was unified across replication and sql. It is now
representred as a Write_record class, that holds the whole state, and allows
re-using some resources in between the row writes.
Replace, IODKU and single insert implementations are split across different
methods, reluting in a much cleaner code.
The entry point is preserved as a single Write_record::write_record() call.
The implementation to call is chosen on the constructor stage.
This allowed several optimizations to be done:
1. The table key list is not iterated for every row. We find last unique key in
the order of checking once and preserve it across the rows. See last_uniq_key().
2. ib_handler::referenced_by_foreign_key acquires a global lock. This call was
done per row as well. Not all the table config that allows optimized replace is
folded into a single boolean field can_optimize. All the fields to check are
even stored in a single register on a 64-bit platform.
3. DUP_REPLACE and DUP_UPDATE cases now have one less level of indirection
4. modified_non_trans_tables is checked and set only when it's really needed.
5. Obsolete bitmap manipulations are removed.
Also:
* Unify replace initialization step across implementations:
add prepare_for_replace and finalize_replace
* alloca is removed in favor of mem_root allocation. This memory is reused
across the rows.
* An rpl-related callback is added to the replace branch, meaning that an extra
check is made per row replace even for the common case. It can be avoided with
templates if considered a problem.
Issue: When XA transaction is implicitly rolled back, we keep XA state
XA_ACTIVE and set rm_error to ER_LOCK_DEADLOCK. Other than XA command
we don't check for rm_error and DML and query are executed with a new
transaction.
Fix: One way to fix this issue is to set the XA state to XA_ROLLBACK_ONLY
which is checked while opening table open_tables() and ER_XAER_RMFAIL is
returned for any DML or Query.
Problem:
Empty queries are incremented if no rows are sent to the client in the
EXECUTE phase of select query. With cursor protocol, rows are not sent
during EXECUTE phase; they are sent later in FETCH phase. Hence,
queries executed with cursor protocol are always falsely treated as
empty in EXECUTE phase.
Fix:
For cursor protocol, empty queries are now counted during the FETCH
phase. This ensures counter correctly reflects whether any rows were
actually sent to the client.
Tests included in `mysql-test/main/show.test`.
check sequence privileges in Item_func_nextval::fix_fields(),
just like column privileges are checked in Item_field::fix_fields()
remove sequence specific hacks that kinda made sequence privilege
checks works, but not in all cases. And they were too lax,
didn't requre SELECT privilege for NEXTVAL. Also INSERT privilege looks
wrong here, UPDATE would've been more appropriate, but won't
change that for compatibility reasons.
also fixes
MDEV-36413 User without any privileges to a sequence can read from it and modify it via column default
Identifier names can be empty in the grammar. The check_ident_length
is used from everything from triggers, to partitions, to key names
and UDF names. This change updates 0 length identifiers as valid
without further checking.
Primary keys are one clear case where a empty name is used and
the name.str is a null pointer.
Checking empty names where the key->name.str is a null pointer
results in a UBSAN error in Well_formed_prefix_status further
down the stack which we can avoid.
DROP TABLE on child and UPDATE of parent table can cause an MDL BF-BF
conflict when applied concurrently.
DROP TABLE takes MDL locks on both child and its parent table, however
it only it did not add certification keys for the parent table.
This patch adds the following:
* Append certification keys corresponding to all parent tables
before DROP TABLE replication.
* Fix wsrep_append_fk_parent_table() so that it works when it is
given a table list containing temporary tables.
* Make sure function wsrep_append_fk_parent_table() is only called
for local transaction. That was not the case for ALTER TABLE.
* Add a test case that verifies that UPDATE parent depends on
preceeding DROP TABLE child.
* Adapt galera_ddl_fk_conflict test to work with DROP TABLE as well.
Signed-off-by: Julius Goryavsky <julius.goryavsky@mariadb.com>
The previous commit for fixing MDEV-35446 disabled setting
Galera errors on COM_STMT_PREPARE commands.
As a side effect, a number of tests were started to fail
due to the client receiving different error codes from the
ones expected in the test dependending on whether --ps-protocol
was used.
Also, in the case of test galera_ftwrl, it was found that
it is expected that during COM_STMT_PREPARE command, we
may perform a sync wait operation, which can fail with
LOCK_WAIT_TIMEOUT error.
The revised fix consists in anticipating the call to
wsrep_after_command_before_result(), so that we check for
BF aborts or errors during statement prepare, before sending
back the statement metadata message to client.
Signed-off-by: Julius Goryavsky <julius.goryavsky@mariadb.com>
Test failed sporadically when --ps-protocol was enabled:
a transaction that was BF aborted on COMMIT would succeed
instead of reporting the expected deadlock error.
The reason for the failure was that, depending on timing,
the transaction was BF aborted while the COMMIT statement
was being prepared through a COM_STMT_PREPARE command.
In the failing cases, the transaction was BF aborted
after COM_STMT_PREPARE had already disabled the diagnostics
area of the client. Attempt to override the deadlock error
towards the end of dispatch_command() would be skipped,
resulting in a successful COMMIT even if the transaction
is aborted.
This bug affected the following MTR tests:
- galera_insert_multi
- galera_nopk_unicode
Signed-off-by: Julius Goryavsky <julius.goryavsky@mariadb.com>
Replication of non-transactional engines is experimental and
uses TOI. This naturally means that if there is open transaction
with transactional engine it's changes will be rolled back.
Fixed by adding error message if non-transactional engine
is part of multi-engine transaction with warning.
Signed-off-by: Julius Goryavsky <julius.goryavsky@mariadb.com>
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>
Search conditions were evaluated using val_int(), which was wrong.
Fixing the code to use val_bool() instead.
Details:
- Adding a new item_base_t::IS_COND flag which marks Items used
as <search condition> in WHERE, HAVING, JOIN ON, CASE WHEN clauses.
The flag is at the parse time.
These expressions must be evaluated using val_bool() rather than val_int().
Note, the optimizer creates more Items which are used as search conditions.
Most of these items are not marked with IS_COND yet. This is OK for now,
but eventually these Items can also be fixed to have the flag.
- Adding a method Item::is_cond() which tests if the Item has the IS_COND flag.
- Implementing Item_cache_bool. It evaluates the cached expression using
val_bool() rather than val_int().
Overriding Type_handler_bool::Item_get_cache() to create Item_cache_bool.
- Implementing Item::save_bool_in_field(). It uses val_bool() rather than
val_int() to evaluate the expression.
- Implementing Type_handler_bool::Item_save_in_field()
using Item::save_bool_in_field().
- Fixing all Item_bool_func descendants to implement a virtual val_bool()
rather than a virtual val_int().
- To find places where val_int() should be fixed to val_bool(), a few
DBUG_ASSERT(!is_cond()) where added into val_int() implementations
of selected (most frequent) classes:
Item_field
Item_str_func
Item_datefunc
Item_timefunc
Item_datetimefunc
Item_cache_bool
Item_bool_func
Item_func_hybrid_field_type
Item_basic_constant descendants
- Fixing all places where DBUG_ASSERT() happened during an "mtr" run
to use val_bool() instead of val_int().
Added new test scenario in galera.galera_bf_kill
test to make the issue surface. The tetst scenario has
a multi statement transaction containing a KILL command.
When the KILL is submitted, another transaction is
replicated, which causes BF abort for the KILL command
processing. Handling BF abort rollback while executing
KILL command causes node hanging, in this scenario.
sql_kill() and sql_kill_user() functions have now fix,
to perform implicit commit before starting the KILL command
execution. BEcause of the implicit commit, the KILL execution
will not happen inside transaction context anymore.
Signed-off-by: Julius Goryavsky <julius.goryavsky@mariadb.com>
Replication of MyISAM and Aria DML is experimental and best
effort only. Earlier change make INSERT SELECT on both
MyISAM and Aria to replicate using TOI and STATEMENT
replication. Replication should happen only if user
has set needed wsrep_mode setting.
Note: This commit contains additional changes compared
to those already made for the 10.5 branch.
+ small refactoring after main fix.
Signed-off-by: Julius Goryavsky <julius.goryavsky@mariadb.com>
