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.
Reason:
=======
Following the changes in MDEV-38734, the server no longer marks
all indexed virtual columns during an UPDATE operation.
Consequently, ha_innobase::update() populates the upd_t vector
with old_vrow but omits the actual data for these virtual columns.
Despite this omission, trx_undo_page_report_modify() continues to
write metadata for indexed virtual columns into the undo log. Because
the actual values are missing from the update vector, the undo log
entry is recorded without the historical data for these columns.
When the purge thread processes the undo log to reconstruct a
previous record state for MVCC, it identifies an indexed virtual
column but finds no associated data.
The purge thread incorrectly interprets this missing data as a NULL
value, rather than a "missing/unrecorded" value. The historical
record is reconstructed with an incorrect NULL for the virtual column.
This causes the purge thread to incorrectly identify and purge
records that are not actually delete-marked, leading to abort
of server.
Solution:
=========
ha_innobase::column_bitmaps_signal(): Revert the column-marking
logic to the state prior to commit a4e4a56720, ensuring all
indexed virtual columns are unconditionally marked during an UPDATE.
The previous "optimization" attempted to manually detect indexed
column changes before marking virtual columns. The manual check
for indexed column modifications is redundant. InnoDB already
provides the UPD_NODE_NO_ORD_CHANGE flag within row_upd_step().
This flag is being used in trx_undo_page_report_modify() and
trx_undo_read_undo_rec() should log or read virtual column values.
Refactored column_bitmaps_signal() to accept a mark_for_update
parameter that controls when indexed virtual columns are marked.
TABLE::mark_columns_needed_for_update() is the only place that needs
mark_for_update=true because only UPDATE operations need to mark
indexed virtual columns for InnoDB's undo logging mechanism.
INSERT operation is already handled by
TABLE::mark_virtual_columns_for_write(insert_fl=true).
Even the commit a4e4a56720 changes are
going to affect TABLE::mark_virtual_column_for_write(false) and
It is called during UPDATE operation, and that's when
column_bitmaps_signal() needs to mark indexed virtual columns.
Online DDL has separate code path which is handled by
row_log_mark_virtual_cols() for all DML operations
SEQUENCE is a pseudo-engine intended to produce sequential numbers.
This engine has HTON_HIDDEN flag, and optimizer costs for such engines
are zeroed and marked with MEM_UNDEFINED() macro.
However, some code paths still examine optimizer costs during
execution of queries involving SEQUENCE tables. For example,
deletion from a sequence table, despite being invalid, still evaluates
costs of access to the table before producing an error. This makes
MSAN trigger on the error "use-of-uninitialized-value".
Not to overcomplicate the logic for handling such scenarios,
this patch simply removes MEM_UNDEFINED() macro for engines with
HTON_HIDDEN flag.
The original symptom of this was an assertion 'marked_for_read()' failing in
RBR with unique blobs and binlog_row_image=MINIMAL. The problem was that the
hidden DB_ROW_HASH_1 virtual column was included in the after-image of the
update, but the underlying blob column was not being updated, so it was not
in the read_set/write_set.
It seems clearly wrong to include the DB_ROW_HASH_1 in the after-image when
the underlying blob isn't even being updated. The cause of this is the
following commit:
Author: Monty <monty@mariadb.org>
Date: Wed May 23 22:42:29 2018 +0300
MDEV-15243 Crash with virtual fields and row based binary logging
That patch removed a check for if the underlying fields of a virtual column
were being updated, and just added them unconditionally. This seems wrong.
So revert that part of the commit, restoring the logic to only add a virtual
column if any underlying field is actually in the write_set.
Also fix a typo in that commit where a code reformat accidentally reversed a
condition.
Also fix an assertion when InnoDB goes to update secondary indexes: If any
part of the primary key is being updated, then add all virtual columns that
are part of secondary indexes to the read_set.
Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
When evaluating a CHECK constraint during INSERT/UPDATE IGNORE, an evaluation error (like ER_DATA_OUT_OF_RANGE) can be set in the Diagnostics Area.
If the failure is ignored (downgraded to a warning), verify_constraints() was not clearing the error state from the DA before returning.
This led to a crash in my_ok() because it asserts that no error is set in the DA.
Fixed by calling clear_error() inside verify_constraints() when ignore_failure is true, ensuring the DA is ready for the subsequent OK packet.
Outdated table_list->derived_result after first trigger run. It was
allocated on statment mem_root and freed when trigger procedure
finishes.
Next INSERT trigger on new view in mysql_derived_prepare() jumps to
exit on error before it had a chance to allocate new derived_result.
It jumps here:
797 if ((res= sl->handle_derived(lex, DT_PREPARE)))
798 goto exit;
and the allocation is later:
837 if ((!derived->is_with_table_recursive_reference() ||
838 !derived->derived_result) &&
839 !(derived->derived_result= new (thd->mem_root) select_unit(thd)))
840 DBUG_RETURN(TRUE); // out of memory
on exit it does destruction of already freed object from previous statement:
901 exit:
...
921 if (res)
922 {
923 if (!derived->is_with_table_recursive_reference())
924 {
...
927 delete derived->derived_result;
The most obvious fix is to cleanup derived_result when the TABLE_LIST
is reinited for new use. Also as a precaution the patch does the
cleanup after delete operator.
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().
They were based on the maximum possible key tuple length, which can be
much larger than the real data size.
The return value is used by handler::keyread_time(), which is used
to estimate the cost of range access.
This could cause range access not to be picked, even if it uses
the clustered PK and reads about 8% of the table.
The fix is to add KEY::stat_storage_length (next to KEY::rec_per_key) and
have the storage engine fill it in handler::info(HA_STATUS_CONST).
Currently, only InnoDB fills this based on its internal statistics:
index->stat_index_size and ib_table->stat_n_rows.
Also changed:
- In handler::calculate_costs(), use ha_keyread_clustered_time() when
computing clustered PK read cost, not ha_keyread_time().
The fix is OFF by default and enabled by setting FIX_INDEX_LOOKUP_COST flag
in @@new_mode.
The NULL-aware index statistics fix is now controlled by the
FIX_INDEX_STATS_FOR_ALL_NULLS flag and disabled by default
for preserving execution plan stability in stable versions.
To enable:
SET @@new_mode = 'FIX_INDEX_STATS_FOR_ALL_NULLS';
Or via command line:
--new-mode=FIX_INDEX_STATS_FOR_ALL_NULLS
Or in configuration file:
[mysqld]
new_mode=FIX_INDEX_STATS_FOR_ALL_NULLS
`all_nulls_key_parts` bitmap is now calculated at set_statistics_for_table()
When all values in an indexed column are NULL, EITS statistics show
avg_frequency == 0. This commit adds logic to distinguish between
"no statistics available" and "all values are NULL" scenarios.
For NULL-rejecting conditions (e.g., t1.col = t2.col), when statistics
confirm all indexed values are NULL, the optimizer can now return a
very low cardinality estimate (1.0) instead of unknown (0.0), since
NULL = NULL never matches.
For non-NULL-rejecting conditions (e.g., t1.col <=> t2.col),
normal cardinality estimation continues to apply since matches are possible.
Changes:
- Added KEY::rec_per_key_null_aware() to check nulls_ratio from column
statistics when avg_frequency is 0
- Modified best_access_path() in sql_select.cc to use the new
rec_per_key_null_aware() method for ref access cost estimation
- The optimization works with single-column and composite indexes,
checking each key part's NULL-rejecting status via notnull_part bitmap
(Preparation for the main patch)
set_statistics_for_table() incorrectly treated indexes with all NULL
values the same as indexes with no statistics, because avg_frequency
is 0 in both cases. This caused the optimizer to ignore valid EITS
data and fall back to engine statistics.
Additionally, KEY::actual_rec_per_key() would fall back to engine
statistics even when EITS was available, and used incorrect pointer
comparison (rec_per_key == 0 instead of nullptr).
Fix by adding Index_statistics::stats_were_read flag to track per-index
whether statistics were actually read from persistent tables, and
restructuring actual_rec_per_key() to prioritize EITS when available.
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.
