Console View
|
Categories: connectors experimental galera main |
|
| connectors | experimental | galera | main | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
|
|
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Yuchen Pei
ycp@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39525 Add type checks to vcol index substitution in WHERE When doing a vcol index substitution in a WHERE condition, when the vcol field is not a supertype of the vcol expression, data truncation may result in wrong results after substitution. To fix that, we add a supertype check to WHERE substitution, just like we do for GROUP BY/ORDER BY substitution. However, this fix can be too strict in some cases. For example, if the condition is WHERE <vcol_expr> > 21 where <vcol_expr> has type BIGINT and <vcol_field> has type TINYINT, the supertype check would fail, but here, data truncation would not result in wrong result, because the truncation clamps at extreme values which are -128 or 127 for TINYINT. The key is that the constant compared to is strictly inside the domain of vcol_field. To that end, we add a fallback check for WHERE substitution, that compares vcol_field type domain with the constants. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Vladislav Vaintroub
vvaintroub@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
fix Windows build: split mariadb_private into headers-only and full parts Split mariadb_private into a headers-only part, linked to static-only plugins, and one that additionally links the server library (to fix unresolved dependencies) for module plugins. Fixes the cycle reported by CMake. Also, rocksdb_aux_lib needs server headers too. Also fixes INSTALL_RUNTIME_DEPS: it walked only one level of LINK_LIBRARIES to find same-build shared libs to exclude, which worked while plugins linked server/mariadbd directly. Now that they link the mariadb_private INTERFACE library instead, server only shows up via its INTERFACE_LINK_LIBRARIES, so the walk is now transitive. Assisted-by: Claude:claude-5-sonnet |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Aleksey Midenkov
midenok@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41157 CREATE DATABASE COMMENT overflows db.opt comment buffer Bug 1: put_dbopt() used strmov() to copy schema_comment into a fixed DATABASE_COMMENT_MAXLEN+1 buffer. validate_comment_length() only truncates comment->length in non-strict sql_mode, leaving comment->str NUL-terminated at its original (unbounded) length. strmov() copies until the source NUL, ignoring the truncated length, overflowing the destination buffer for long comments. The fix uses strmake() bounded by comment->length instead, matching the LEX_CSTRING contract (length is authoritative, str need not be NUL-terminated at length). Bug 2: write_db_opt() used strxnmov() to copy the full un-truncated comment until it ran out of buffer space mid-string with no trailing newline. Which made load_db_opt() silently discard the whole unterminated "comment=" line on the next restart, losing the comment entirely instead of just truncating it. The fix bounds the comment copy into db.opt by the already-validated comment->length via strmake(), instead of relying on the source string's own NUL terminator, matching the put_dbopt() fix. Bug 3: validate_comment_length() only runs on a COMMENT clause given in the current statement. ALTER DATABASE without one instead pulls the existing comment off disk via load_db_opt(), which never bounded it. That unvalidated length then reached write_db_opt()'s own comment= copy into its stack buffer, so a legacy or hand-edited db.opt with an overlong comment= line overflowed it on ALTER DATABASE. The fix: load_db_opt() now clamps the parsed comment to DATABASE_COMMENT_MAXLEN right when it reads the "comment=" line, so every consumer (put_dbopt(), write_db_opt()'s ALTER path) always sees an already-bounded value. The clamp itself must truncate by bytes, not characters: Well_formed_prefix()'s LEX_CSTRING overload takes a character count, but DATABASE_COMMENT_MAXLEN sizes the buffers in bytes. Bug 4: validate_comment_length() had the same character/byte confusion, on the primary CREATE/ALTER DATABASE COMMENT path: it passed max_len as a character count to Well_formed_prefix(), so a multi-byte comment could be truncated to max_len characters instead of bytes. Fixed like load_db_opt(): clamp to max_len bytes first, then find the well-formed prefix. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Thirunarayanan Balathandayuthapani
thiru@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40319 Instant ALTER TABLE rollback corrupts virtual column Problem: ======= ha_innobase_inplace_ctx::~ha_innobase_inplace_ctx() runs, whenever ctx->instant_table is set and frees the old_v_cols exist also. old_v_cols and old_n_v_cols are captured in the constructor as prebuilt_arg->table->v_cols and n_v_cols, i.e. an alias of the live table's own virtual columns, not a copy. By the time this destructor runs, old_table->v_cols is either still that same array. If the failure happened before ctx->instant_column() ever ran like during prepare_inplace_alter_table_dict() or failure happened during the commit phase innobase_instant_try(). In both cases old_v_cols is the table's current, live v_cols array, so this loop destructs dict_v_col_t objects that are still in use. Solution: ======== ha_innobase_inplace_ctx::~ha_innobase_inplace_ctx(): Destruct instant_table->v_cols[], not old_v_cols[]. instant_table is the independently allocated dict_table_t that prepare_instant() built; It owns its own v_cols array, whose dict_v_col_t::v_indexes must be destructed before dict_mem_table_free() reclaims instant_table's memory. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Sergei Petrunia
sergey@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Add comment about Create_tmp_table::m_group | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Aleksey Midenkov
midenok@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41157 CREATE DATABASE COMMENT overflows db.opt comment buffer Bug #1: put_dbopt() used strmov() to copy schema_comment into a fixed DATABASE_COMMENT_MAXLEN+1 buffer. validate_comment_length() only truncates comment->length in non-strict sql_mode, leaving comment->str NUL-terminated at its original (unbounded) length. strmov() copies until the source NUL, ignoring the truncated length, overflowing the destination buffer for long comments. The fix uses strmake() bounded by comment->length instead, matching the LEX_CSTRING contract (length is authoritative, str need not be NUL-terminated at length). Bug #2: strxnmov() copied the full un-truncated comment until it ran out of buffer space mid-string with no trailing newline -- which made load_db_opt() silently discard the whole unterminated "comment=" line on the next restart, losing the comment entirely instead of just truncating it. The fix bounds the comment copy into db.opt by the already-validated comment->length via strmake(), instead of relying on the source string's own NUL terminator, matching the put_dbopt() fix. Bug #3: validate_comment_length() only runs on a COMMENT clause given in the current statement. ALTER DATABASE without one instead pulls the existing comment off disk via load_db_opt(), which never bounded it. That unvalidated length then reached write_db_opt()'s own comment= copy into its stack buffer, so a legacy or hand-edited db.opt with an overlong comment= line overflowed it on ALTER DATABASE. The fix: load_db_opt() now clamps the parsed comment to DATABASE_COMMENT_MAXLEN right when it reads the "comment=" line, so every consumer (put_dbopt(), write_db_opt()'s ALTER path) always sees an already-bounded value. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Thirunarayanan Balathandayuthapani
thiru@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-19574: innodb_stats_method is not honored when innodb_stats_persistent=ON Problem: ======= When persistent statistics are enabled (innodb_stats_persistent=ON), the innodb_stats_method setting is not properly utilized during statistics calculation. The statistics collection functions always use a hardcoded default behavior for NULL value comparison instead of respecting the configured stats method. This affects the accuracy of n_diff_key_vals (distinct key count), particularly for indexes with nullable columns containing NULL values. Moreover, stat_n_non_null_key_vals[] was never computed for persistent statistics; it stayed at the 0 that dict_stats_empty_index() assigns. With innodb_stats_method=nulls_ignored, innodb_rec_per_key() therefore always found n_diff <= n_null and reported one record per key for every index. This impacts the query optimizer, which makes decisions based on inaccurate cardinality estimates. Solution: ======== Introduced IndexLevelStats to collect statistics at a specific B-tree level during index analysis. Introduced PageStats to collect statistics for leaf page analysis. Refactored the following functions: dict_stats_analyze_index_level() to IndexLevelStats::analyze_level() dict_stats_analyze_index_for_n_prefix() to IndexLevelStats::sample_leaf_pages() dict_stats_analyze_index_below_cur() to PageStats::scan_below() dict_stats_scan_page() to PageStats::scan() The innodb_stats_method value is read once per table in dict_stats_update_persistent() and passed down, so that all indexes of a table are analyzed with the same method. Add the stats method name to stat_description when innodb_stats_method has a non-default value. The suffix is dropped when the description is already full. Added the new stat name n_nonnull_fld01, n_nonnull_fld02, etc. with a stats description, to indicate how many non-null values exist for the nth field of the index. This value is retrieved and stored in the index statistics in dict_stats_fetch_index_stats_step(). The counts are per column, not per n-column prefix. rec_get_n_blob_pages(): Calculate the number of externally stored pages for a record, using ceiling division by the usable BLOB page payload (blob_part_size), which differs between ROW_FORMAT=COMPRESSED (zip_size minus FIL_PAGE_DATA) and the other formats (srv_page_size minus the BLOB header and the page trailer). For ROW_FORMAT=COMPRESSED the length in the field reference is the uncompressed length, so the result is an upper bound. When the leaf level is scanned in full, the number of leaf pages that were scanned is reported as n_leaf_pages for a multi level index. Before, result.n_leaf_pages was overwritten with index->stat_n_leaf_pages, which dict_stats_empty_index() had just set to 1, so every index that took the full scan path reported n_leaf_pages=1. Single page indexes report 1. This changes cardinality estimates and therefore leads to multiple changes in existing test cases. Non-null values are counted only at the leaf level, since only leaf pages hold actual records. A full scan of the leaf level counts them exactly. When the level is sampled, the per column count is derived from the sampled leaves with the same formula as n_diff: n_ordinary_leaf_pages * n_non_null_all_analyzed_pages / n_leaf_pages_to_analyze This is an estimate for NOT NULL columns as well: the sampled leaves may hold fewer or more records than the average, and a dive that stops at a boring page contributes nothing to the sum while still counting in the divisor. innodb_rec_per_key(): stat_n_non_null_key_vals[i] holds the number of records in which the i-th indexed column alone is not NULL, while what has to be excluded here is the number of records whose first i+1 columns are all not NULL, because that is the population which the n-column prefix statistic stat_n_diff_key_vals[i] has to be corrected against when innodb_stats_method=nulls_ignored: with NULLs compared as unequal, every record carrying a NULL anywhere in the prefix adds a distinct value of its own to n_diff. PageStats::scan(): n_non_null is accumulated and assigned only for leaf pages, so that a non-leaf scan cannot leave a node pointer count behind when scan_below() stops at a boring page without reaching a leaf. IndexLevelStats::reset_for_level() also clears n_diff[], and dict_stats_analyze_index() zero initializes the buffer backing it, so that a level scan which finds no records (a failed btr_pcur_open_level(), or a non-leaf page whose first record is not marked as the leftmost one on the level) leaves n_diff[] at 0 instead of stale values. IndexLevelStats::sample_leaf_pages() returns early when the group boundaries for the prefix are empty, which is the same condition. IndexLevelStats::analyze_level(): Instead of copying the last record of the page, retain the latch on the page until the record has been compared with the first record of the next page dict_stats_fetch_index_stats_step() no longer resets stat_n_non_null_key_vals[] while processing an n_diff_pfxNN row: dict_stats_empty_table() has already cleared the array before the fetch, and with n_nonnull_fldNN rows now being read too, that reset would make the result depend on the order in which the rows arrive. dict_stats_save(): now static function in dict0stats.cc that takes the innodb_stats_method value, and is removed from dict0stats.h. dict_stats_update_persistent() saves the statistics itself, so its callers no longer have to. Replaced btr_rec_get_externally_stored_len() with rec_get_n_blob_pages() in dict0stats.cc. btr_rec_get_field_ref_offs() and btr_rec_get_field_ref(), together with the BTR_BLOB_HDR_* macros, were moved from btr0cur.cc to btr0cur.h so that rec_get_n_blob_pages() can reuse them; btr_rec_get_field_ref_offs() is now a noexcept function returning size_t. Changed stat_n_diff_key_vals and stat_n_non_null_key_vals from ib_uint64_t* to uint64_t* len_is_stored(): simplified to a single comparison, which is equivalent for the unsigned lengths that it is used with. Removed the unused UNIV_STATS_DEBUG build macro (univ.i) and turned the DEBUG_PRINTF() helper in dict0stats.cc into an unconditional no-op |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Aleksey Midenkov
midenok@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41157 CREATE DATABASE COMMENT overflows db.opt comment buffer Bug #1: put_dbopt() used strmov() to copy schema_comment into a fixed DATABASE_COMMENT_MAXLEN+1 buffer. validate_comment_length() only truncates comment->length in non-strict sql_mode, leaving comment->str NUL-terminated at its original (unbounded) length. strmov() copies until the source NUL, ignoring the truncated length, overflowing the destination buffer for long comments. The fix uses strmake() bounded by comment->length instead, matching the LEX_CSTRING contract (length is authoritative, str need not be NUL-terminated at length). Bug #2: write_db_opt() used strxnmov() to copy the full un-truncated comment until it ran out of buffer space mid-string with no trailing newline. Which made load_db_opt() silently discard the whole unterminated "comment=" line on the next restart, losing the comment entirely instead of just truncating it. The fix bounds the comment copy into db.opt by the already-validated comment->length via strmake(), instead of relying on the source string's own NUL terminator, matching the put_dbopt() fix. Bug #3: validate_comment_length() only runs on a COMMENT clause given in the current statement. ALTER DATABASE without one instead pulls the existing comment off disk via load_db_opt(), which never bounded it. That unvalidated length then reached write_db_opt()'s own comment= copy into its stack buffer, so a legacy or hand-edited db.opt with an overlong comment= line overflowed it on ALTER DATABASE. The fix: load_db_opt() now clamps the parsed comment to DATABASE_COMMENT_MAXLEN right when it reads the "comment=" line, so every consumer (put_dbopt(), write_db_opt()'s ALTER path) always sees an already-bounded value. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Oleksandr Byelkin
sanja@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41144 Assertion `scale <= precision' failed in decimal_bin_size dynamic_column_decimal_read() derived intg/frac from an unbounded var-uint and only checked scale>precision in int domain, so a crafted value could still overflow decimal_bin_size()'s uint16 parameters, tripping its scale<=precision assert (an out-of-bounds array read in release builds). Bound intg/frac against DECIMAL_MAX_POSSIBLE_PRECISION before use, and reject the var-uint decoder's error signal, as the sibling dynamic_column_string_read() already does. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Vladislav Vaintroub
vvaintroub@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
fix macOS: -undefined dynamic_lookup instead of linking mariadbd mariadbd is not exported, so it can't be a dependency of mariadb_private (previous commit dropped that link entirely). On Apple platforms specifically, -bundle_loader isn't the only option - a MODULE there is a loadable bundle, same class as Python/Perl/Ruby native extensions, and -Wl,-undefined,dynamic_lookup is the standard way those defer symbol resolution to load time, same as ELF already does for free. CMake's own Platform/Darwin.cmake confirms this isn't the default for MODULE targets, so it needs to be added explicitly. Scoped to APPLE specifically, not "not Linux" - FreeBSD/OpenBSD/NetBSD and Solaris/illumos are ELF like Linux and never needed anything here. Assisted-by: Claude:claude-5-sonnet |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Aleksey Midenkov
midenok@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41157 CREATE DATABASE COMMENT overflows db.opt comment buffer Bug #1: put_dbopt() used strmov() to copy schema_comment into a fixed DATABASE_COMMENT_MAXLEN+1 buffer. validate_comment_length() only truncates comment->length in non-strict sql_mode, leaving comment->str NUL-terminated at its original (unbounded) length. strmov() copies until the source NUL, ignoring the truncated length, overflowing the destination buffer for long comments. The fix uses strmake() bounded by comment->length instead, matching the LEX_CSTRING contract (length is authoritative, str need not be NUL-terminated at length). Bug #2: write_db_opt() used strxnmov() to copy the full un-truncated comment until it ran out of buffer space mid-string with no trailing newline. Which made load_db_opt() silently discard the whole unterminated "comment=" line on the next restart, losing the comment entirely instead of just truncating it. The fix bounds the comment copy into db.opt by the already-validated comment->length via strmake(), instead of relying on the source string's own NUL terminator, matching the put_dbopt() fix. Bug #3: validate_comment_length() only runs on a COMMENT clause given in the current statement. ALTER DATABASE without one instead pulls the existing comment off disk via load_db_opt(), which never bounded it. That unvalidated length then reached write_db_opt()'s own comment= copy into its stack buffer, so a legacy or hand-edited db.opt with an overlong comment= line overflowed it on ALTER DATABASE. The fix: load_db_opt() now clamps the parsed comment to DATABASE_COMMENT_MAXLEN right when it reads the "comment=" line, so every consumer (put_dbopt(), write_db_opt()'s ALTER path) always sees an already-bounded value. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
bsrikanth-mariadb
srikanth.bondalapati@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40837 Capture sequences used only in column DEFAULT expressions Problem: A sequence referenced only in a column's DEFAULT expression (e.g. "a INT DEFAULT NEXTVAL(s1)") is opened only when a statement evaluates DEFAULT values (INSERT, LOAD DATA, etc). A plain SELECT never opens it, so the sequence never appears in thd->lex->query_tables, and Optimizer_context_recorder:: dump_sql_script() had no way to see it. The dependent table's definition was then captured without the sequence it depends on, making the captured context unusable on replay. Fix: TABLE::internal_tables already holds the sequence tables that a table's DEFAULT expressions depend on, populated whenever the table is opened regardless of statement type. dump_sql_script() now walks TABLE::internal_tables for each dumped table, opens any sequence not already open via open_and_lock_internal_tables(), and dumps each sequence's CREATE SEQUENCE and current value (via the new dump_sequence_context()/dump_sequence_current_value() helpers, factored out of the existing inline SETVAL logic) before the dependent table's own CREATE TABLE statement, so replay can recreate both in the correct order. Tested with a new MTR test in opt_context_store_ddls.test: create a sequence, create a table with a column defaulting to NEXT_VALUE() on that sequence, insert rows, then run a plain SELECT and confirm the captured optimizer context includes both the sequence's and the table's DDL, with the sequence appearing first. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rucha Deodhar
rucha.deodhar@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41181: ASAN heap-buffer-overflow after SELECT JSON_SCHEMA_VALID Analysis: dynstr_set truncates wide/binary strings (like UCS2) using strlen(). Forcing a_res.length = je->value_len afterward creates a mismatch between the tiny allocated buffer and the large expected length, causing an ASAN memcpy overflow. Fix: Fixed by replacing dynstr_set() with dynstr_realloc() and a raw memcpy() using the explicit byte length (je->value_len). |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Yuchen Pei
ycp@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39525 Add type checks to vcol index substitution in WHERE When doing a vcol index substitution in a WHERE condition, when the vcol field is not a supertype of the vcol expression, data truncation may result in wrong results after substitution. To fix that, we add a supertype check to WHERE substitution, just like we do for GROUP BY/ORDER BY substitution. However, this fix can be too strict in some cases. For example, if the condition is WHERE <vcol_expr> > 21 where <vcol_expr> has type BIGINT and <vcol_field> has type TINYINT, the supertype check would fail, but here, data truncation would not result in wrong result, because the truncation clamps at extreme values which are -128 or 127 for TINYINT. The key is that the constant compared to is strictly inside the domain of vcol_field. To that end, we add a fallback check for WHERE substitution, that compares vcol_field type domain with the constants. A hole remains for strings with PAD SPACE collations: any const string for comparison could be a result of truncation, even if much shorter than the capacity of the vcol field. For example, for a table with (a varchar(20), vc varchar(5) as (concat('x',a)), index(vc)) vc = 'xabc' evals to 1 for a row with a = 'abc z' but concat('x',a) = 'xabc' evals to 0. The right hand side is 'xabc' could match a vc of varchar(N) for any N >= 4 because there could be arbitrary number of spaces before the truncation. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Vladislav Vaintroub
vvaintroub@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40967 PROXY protocol host check sent in clear text mid-SSL handshake Defer the host-privileged/host-blocked check for a PROXY-header-derived address until after the client's SSL handshake completes, instead of sending it immediately in clear text. Co-Authored-By: Claude Sonnet 5 <[email protected]> |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Aleksey Midenkov
midenok@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41157 CREATE DATABASE COMMENT overflows db.opt comment buffer Bug #1: put_dbopt() used strmov() to copy schema_comment into a fixed DATABASE_COMMENT_MAXLEN+1 buffer. validate_comment_length() only truncates comment->length in non-strict sql_mode, leaving comment->str NUL-terminated at its original (unbounded) length. strmov() copies until the source NUL, ignoring the truncated length, overflowing the destination buffer for long comments. The fix uses strmake() bounded by comment->length instead, matching the LEX_CSTRING contract (length is authoritative, str need not be NUL-terminated at length). Bug #2: strxnmov() copied the full un-truncated comment until it ran out of buffer space mid-string with no trailing newline -- which made load_db_opt() silently discard the whole unterminated "comment=" line on the next restart, losing the comment entirely instead of just truncating it. The fix bounds the comment copy into db.opt by the already-validated comment->length via strmake(), instead of relying on the source string's own NUL terminator, matching the put_dbopt() fix. Note: put_dbopt() is also reached from load_db_opt() when parsing a db.opt file directly (e.g. one written before this fix, or edited by hand), where the comment length was never validated. put_dbopt() therefore clamps the length to DATABASE_COMMENT_MAXLEN unconditionally, protecting every caller instead of relying on the caller having validated it. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Aleksey Midenkov
midenok@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41157 CREATE DATABASE COMMENT overflows db.opt comment buffer Bug #1: put_dbopt() used strmov() to copy schema_comment into a fixed DATABASE_COMMENT_MAXLEN+1 buffer. validate_comment_length() only truncates comment->length in non-strict sql_mode, leaving comment->str NUL-terminated at its original (unbounded) length. strmov() copies until the source NUL, ignoring the truncated length, overflowing the destination buffer for long comments. The fix uses strmake() bounded by comment->length instead, matching the LEX_CSTRING contract (length is authoritative, str need not be NUL-terminated at length). Bug #2: write_db_opt() used strxnmov() to copy the full un-truncated comment until it ran out of buffer space mid-string with no trailing newline. Which made load_db_opt() silently discard the whole unterminated "comment=" line on the next restart, losing the comment entirely instead of just truncating it. The fix bounds the comment copy into db.opt by the already-validated comment->length via strmake(), instead of relying on the source string's own NUL terminator, matching the put_dbopt() fix. Bug #3: validate_comment_length() only runs on a COMMENT clause given in the current statement. ALTER DATABASE without one instead pulls the existing comment off disk via load_db_opt(), which never bounded it. That unvalidated length then reached write_db_opt()'s own comment= copy into its stack buffer, so a legacy or hand-edited db.opt with an overlong comment= line overflowed it on ALTER DATABASE. The fix: load_db_opt() now clamps the parsed comment to DATABASE_COMMENT_MAXLEN right when it reads the "comment=" line, so every consumer (put_dbopt(), write_db_opt()'s ALTER path) always sees an already-bounded value. The clamp itself must truncate by bytes, not characters: Well_formed_prefix()'s LEX_CSTRING overload takes a character count, but DATABASE_COMMENT_MAXLEN sizes the buffers in bytes. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Yuchen Pei
ycp@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39525 Add type checks to vcol index substitution in WHERE When doing a vcol index substitution in a WHERE condition, when the vcol field is not a supertype of the vcol expression, data truncation may result in wrong results after substitution. To fix that, we add a supertype check to WHERE substitution, just like we do for GROUP BY/ORDER BY substitution. However, this fix can be too strict in some cases. For example, if the condition is WHERE <vcol_expr> > 21 where <vcol_expr> has type BIGINT and <vcol_field> has type TINYINT, the supertype check would fail, but here, data truncation would not result in wrong result, because the truncation clamps at extreme values which are -128 or 127 for TINYINT. The key is that the constant compared to is strictly inside the domain of vcol_field. To that end, we add a fallback check for WHERE substitution, that compares vcol_field type domain with the constants. A hole remains for strings with PAD SPACE collations: any const string for comparison could be a result of truncation, even if much shorter than the capacity of the vcol field. For example, for a table with (a varchar(20), vc varchar(5) as (concat('x',a)), index(vc)) vc = 'xabc' evals to 1 for a row with a = 'abc z' but concat('x',a) = 'xabc' evals to 0. The right hand side is 'xabc' could match a vc of varchar(N) for any N >= 4 because there could be arbitrary number of spaces before the truncation. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Yuchen Pei
ycp@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39525 Add type checks to vcol index substitution in WHERE When doing a vcol index substitution in a WHERE condition, when the vcol field is not a supertype of the vcol expression, data truncation may result in wrong results after substitution. To fix that, we add a supertype check to WHERE substitution, just like we do for GROUP BY/ORDER BY substitution. However, this fix can be too strict in some cases. For example, if the condition is WHERE <vcol_expr> > 21 where <vcol_expr> has type BIGINT and <vcol_field> has type TINYINT, the supertype check would fail, but here, data truncation would not result in wrong result, because the truncation clamps at extreme values which are -128 or 127 for TINYINT. The key is that the constant compared to is strictly inside the domain of vcol_field. To that end, we add a fallback check for WHERE substitution, that compares vcol_field type domain with the constants. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Sergei Petrunia
sergey@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Fix comments | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rucha Deodhar
rucha.deodhar@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41181: ASAN heap-buffer-overflow after SELECT JSON_SCHEMA_VALID Analysis: dynstr_set truncates wide/binary strings (like UCS2) using strlen(). Forcing a_res.length = je->value_len afterward creates a mismatch between the tiny allocated buffer and the large expected length, causing an ASAN memcpy overflow. Fix: Fixed by replacing dynstr_set() with dynstr_realloc() and a raw memcpy() using the explicit byte length (je->value_len). |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Mohammad Tafzeel Shams
tafzeel.shams@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-35154 : dict_sys_t::load_table() is holding exclusive dict_sys.latch for unnecessarily long time Issue: dict_load_table_one() was invoked with the exclusive dict_sys.latch held and never released it. The whole load ran inside that one critical section: reading the SYS_TABLES record, opening the .ibd file, and reading SYS_COLUMNS, SYS_VIRTUAL, SYS_INDEXES, SYS_FIELDS, the clustered index root page, and SYS_FOREIGN, as well as loading every table related by FOREIGN KEY constraints. Almost all of that is I/O. Because dict_sys.latch is the single latch that guards every dictionary lookup, one cold table open blocked every other session from opening any table, including tables that were already cached. A slow enough load could trigger the fatal semaphore wait check. Fix: Split the load into three phases. A short latched phase creates the table object and publishes it as an incomplete "stub"; the I/O runs with no latch held; a final latched phase links the FOREIGN KEY constraints. The stub's progress is held in the top two bits of dict_table_t:: n_ref_count (LOADING_DEF, LOADING_FK, LOAD_FAILED), analogous to how buf_page_t::state() combines a small lifecycle state with the buffer-fix count in one atomic. A thread that finds a loading table waits by pinning it (dict_table_t::try_pin_for_wait(), so it cannot be freed while unlatched) and acquiring dict_table_t::lock_latch in shared mode, the same latch the loader holds in exclusive mode for the duration of the load. A failed load can be torn down safely even though other threads may have already taken such a pin to wait on it: try_pin_for_wait() refuses once LOAD_FAILED is set, and the teardown path drains any pins taken earlier before freeing the stub. lock_latch is otherwise used for record-lock bookkeeping and statistics on a table; the only place the loader itself needs it for that unrelated purpose while still holding it for the load is dict_get_and_save_data_dir_path(), which skips re-acquiring the latch whenever the table is loading(), since only the loading thread can reach the table at that point. A table remains hidden (LOADING_FK) until every table related to it by FOREIGN KEY constraints has been loaded as well, and dict_sys_t:: load_table() makes them all visible in one step. The intermediate state is visible to constraint linking via dict_sys_t::find_table_fk(), so that two threads loading tables that reference each other can still link the constraint between them. Tables that InnoDB internal SQL can refer to are loaded without ever releasing the latch. The internal SQL parser is not reentrant and is serialized only by the exclusive dict_sys.latch, and it opens tables in the middle of parsing; releasing the latch there let another thread run the parser concurrently and corrupt its state. Changes: - dict_table_t : hold the progress of loading in the top bits of n_ref_count (std::atomic<uint32_t>): LOADING_DEF, LOADING_FK, LOAD_FAILED, loading(). Add the loader-only helpers start_loading(), advance_to_loading_fk(), finish_loading(), mark_load_failed(), and try_pin_for_wait() for waiters, all built on the existing lock_latch. acquire()/release() are unconditional. Add debug load_thread and is_loader(). - dict_load_table_one() : publish the table as a LOADING_DEF stub in table_non_LRU and release the latch before loading the tablespace, the columns and the indexes; reacquire it, move the table to table_LRU and advance it to LOADING_FK before loading the foreign key constraints. Failures go through dict_load_table_one_discard(). Add the hold_latch parameter and the dict_load_table_one_no_latch debug sync point. - dict_sys_t::load_table() : wait for a concurrent load of the same table via wait_for_load(); allocate the foreign key table names on a local heap, because the latch is released while draining them; in the drain loop, wait for a table whose definition is still being loaded by another thread and skip one that is only waiting for its own related tables; make all tables loaded by this invocation visible in one step, releasing each one's lock_latch. - dict_sys_t::wait_for_load() : pins the observed table via try_pin_for_wait() (returning immediately if the load already failed), releases the exclusive dict_sys.latch, blocks on the table's own lock_latch, unpins, and reacquires the latch. - dict_load_table_one_discard() : used wherever a stub must be torn down (retry after DB_SUCCESS_LOCKED_REC, column or virtual-column load failure, a corrupted index or missing FK index). Marks the stub LOAD_FAILED, releases lock_latch to wake any already-pinned waiters, drains the reference count to zero, then removes the stub. - dict_sys_t::add() : take lock_latch in exclusive mode on the loader's behalf before a loading stub becomes reachable via find_table_any(). - dict_get_and_save_data_dir_path() : skip re-acquiring lock_latch when the table is loading(), because the loading thread already holds it in exclusive mode and is the only thread that can reach the table at that point. - dict_load_hold_latch() : whether a table may be referenced by InnoDB internal SQL, and therefore must be loaded without releasing the latch: InnoDB system tables, FULLTEXT INDEX auxiliary tables and the persistent statistics tables. - dict_load_table_on_id() : copy the table name and release the SYS_TABLES page latch before calling load_table(), which may now block. Restore the cursor position only if the scan has to continue. - dict_load_foreign(), dict_load_foreigns() : add the fk_heap parameter and allocate the names appended to fk_tables on it. - dict_sys_t::find_table_any() : the previous body of find_table(), returning tables that are being loaded. Only the name is read. - dict_sys_t::find_table() : hide tables that are being loaded, both in the by-name and the by-id variant. In the by-id variant, loading is checked before any bit-field, to avoid a torn read. - dict_sys_t::find_table_fk() : like find_table(), but LOADING_FK tables are returned, so that constraints can be linked into them while holding the exclusive latch. - dict_table_can_be_evicted() : a table that is being loaded may only be freed by the thread that is loading it. - dict_foreign_add_to_cache() : resolve both sides with find_table_fk(). - btr_search_disable() : skip tables that are being loaded. Walking their indexes would race with the loading thread, which appends to that list without holding the latch. Such tables cannot have any adaptive hash index references. - create_table_info_t::create_foreign_keys() : take a temporary reference on a referenced table as soon as it is resolved, released on every exit path by a scope guard; call dict_sys.prevent_eviction() only once the constraint is actually committed to the dictionary cache, replacing the temporary reference. - create_table_info_t::create_table() : acquire a reference to the created table around the foreign key handling, and use a local heap for the names of the foreign key related tables. - row_rename_table_for_mysql() : use one local heap for both the dropped-constraint names and the foreign key table names. - assertion fix : the load path may now run without dict_sys.latch, but only in the thread that is loading the table, and a cached table is no longer necessarily fully loaded. dict_sys.locked() is relaxed to "dict_sys.locked() || table->is_loader()" in dict_load_columns(), dict_load_virtual_col(), dict_load_fields(), dict_load_indexes(), dict_index_add_to_cache(), dict_index_find_cols(), dict_index_build_internal_clust(), dict_index_build_internal_non_clust() and dict_index_build_internal_fts(). Checks of dict_table_t::cached are relaxed and reordered after the atomic loading in dict_table_add_system_columns(), dict_sys_t::add(), dict_table_rename_in_cache() and hash_insert(). - innodb.dict_load_concurrent A load is parked at dict_load_table_one_no_latch while holding no latch; another table can be loaded meanwhile, and a second opener of the same table waits. A second case checks that a table is not made visible while a table related to it by a FOREIGN KEY constraint is still being loaded by another thread. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Vladislav Vaintroub
vvaintroub@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
fix Windows build: split mariadb_private into headers-only and full parts Split mariadb_private into a headers-only part, linked to static-only plugins, and one that additionally links the server library (to fix unresolved dependencies) for module plugins. Fixes the cycle reported by CMake. Also, rocksdb_aux_lib needs server headers too. Assisted-by: Claude:claude-5-sonnet |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Aleksey Midenkov
midenok@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Refactor schema comment handling in sql_db.cc Co-authored-by: Copilot Autofix powered by AI <[email protected]> |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Oleksandr Byelkin
sanja@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41175 Fix stack-buffer-overflow in backup_log_ddl() backup_log_ddl() sized its stack log buffer assuming each identifier is at most ~40 bytes, but add_name_to_buffer() can expand each identifier character to 5 bytes when re-encoding it into my_charset_filename, so a RENAME TABLE with long, special- character names overflowed the buffer (reported under ASAN). Fixed by sizing the buffer to the true worst case per identifier, and by making add_str_to_buffer() and its callers assert on an explicit end-of-buffer pointer before writing. Co-Authored-By: Claude Sonnet 5 <[email protected]> |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Vladislav Vaintroub
vvaintroub@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
experiment: drop mariadbd link from mariadb_private on non-MSVC/AIX/Linux This was causing "install(EXPORT ...) includes target mariadb_private which requires target mariadbd that is not in any export set" on macOS, since nothing exports mariadbd. Removing it here to see whether anything actually needs it in practice - CMake's default MODULE creation flags on Apple platforms don't add -undefined dynamic_lookup (checked cmake's own Platform/Darwin.cmake), so if a plugin does reference symbols outside mysqlservices, this should fail to link rather than silently misbehave. Assisted-by: Claude:claude-5-sonnet |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Oleksandr Byelkin
sanja@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41094 KDF() aliases large iteration/width to weak 32-bit values KDF() narrowed its iteration-count and key-width arguments without checking they fit, so values differing by 2^32 aliased to the same small value and silently derived a much weaker key. Added checks that the key width fits a 16-bit unsigned value and the iteration count fits a 32-bit value before narrowing them, rejecting out-of-range values with an error instead of aliasing. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
bsrikanth-mariadb
srikanth.bondalapati@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40837 Capture sequences used only in column DEFAULT expressions Problem: A sequence referenced only in a column's DEFAULT expression (e.g. "a INT DEFAULT NEXTVAL(s1)") is opened only when a statement evaluates DEFAULT values (INSERT, LOAD DATA, etc). A plain SELECT never opens it, so the sequence never appears in thd->lex->query_tables, and Optimizer_context_recorder:: dump_sql_script() had no way to see it. The dependent table's definition was then captured without the sequence it depends on, making the captured context unusable on replay. Fix: TABLE::internal_tables already holds the sequence tables that a table's DEFAULT expressions depend on, populated whenever the table is opened regardless of statement type. dump_sql_script() now walks TABLE::internal_tables for each dumped table, opens any sequence not already open via open_and_lock_internal_tables(), and dumps each sequence's CREATE SEQUENCE and current value (via the new dump_sequence_context()/dump_sequence_current_value() factored out of the existing inline SETVAL logic) before the dependent table's own CREATE TABLE statement, so replay both in the correct order. Tested with a new MTR test in opt_context_store_ddls.test: create a sequence, create a table with a column defaulting to NEX that sequence, insert rows, then run a plain SELECT and confirm the captured optimizer context includes both the sequence's table's DDL, with the sequence appearing first. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Yuchen Pei
ycp@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39525 Add type checks to vcol index substitution in WHERE When doing a vcol index substitution in a WHERE condition, when the vcol field is not a supertype of the vcol expression, data truncation may result in wrong results after substitution. To fix that, we add a supertype check to WHERE substitution, just like we do for GROUP BY/ORDER BY substitution. However, this fix can be too strict in some cases. For example, if the condition is WHERE <vcol_expr> > 21 where <vcol_expr> has type BIGINT and <vcol_field> has type TINYINT, the supertype check would fail, but here, data truncation would not result in wrong result, because the truncation clamps at extreme values which are -128 or 127 for TINYINT. The key is that the constant compared to is strictly inside the domain of vcol_field. To that end, we add a fallback check for WHERE substitution, that compares vcol_field type domain with the constants. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Yuchen Pei
yuchen.pei@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
fixup: keep the clustered index visit under snapshot isolation innodb.lock_isolation 'table_lock' failed in the MDEV-33802 section: SELECT * FROM t FORCE INDEX (b) FOR UPDATE succeeded where ER_CHECKREAD was expected. On t(a INT PRIMARY KEY, b INT UNIQUE) the secondary index b stores (b, a), so SELECT * is covered by it, and the previous commit let a locking SELECT take the covering path under a full-scan table LOCK_X. Placing the record lock is not the only thing the clustered index visit does. With innodb_snapshot_isolation and a read view already open, lock_clust_rec_read_check_and_lock() also reads the clustered record's DB_TRX_ID and returns DB_RECORD_CHANGED when the read view cannot see it. Skipping the clustered index skips that check, so the statement silently locked a row it should have refused. The table-level lock does not substitute for the check. It gives exclusivity against concurrent transactions, whereas this reports a change that committed before the lock was taken and is invisible to an older read view. DB_TRX_ID is only stored in the clustered index record, so the check cannot be answered from a secondary index record alone. Keep visiting the clustered index whenever snapshot isolation is active and a read view is open. A plain locking SELECT that opens no read view, which is the case the optimisation targets, is unaffected. Co-Authored-By: Claude Opus 5 <[email protected]> |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Thirunarayanan Balathandayuthapani
thiru@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40324 use-of-uninitialized-value after creation of FULLTEXT table failure Problem: ======= For fulltext index, row_create_index_for_mysql() calls fts_create_index_tables(). If creating FTS auxiliary table fails, error handling performs trx->rollback() of the dictionary transaction. Rollback removes the parent table from dictionary cache and frees it. After that, convert_error_code_to_mysql() reads table->flags after table->heap. This leads to read of freed memory. Solution: ======== create_index(): Read table->flags into a local variable before calling row_create_index_for_mysql() |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Aleksey Midenkov
midenok@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41157 CREATE DATABASE COMMENT overflows db.opt comment buffer Bug #1: put_dbopt() used strmov() to copy schema_comment into a fixed DATABASE_COMMENT_MAXLEN+1 buffer. validate_comment_length() only truncates comment->length in non-strict sql_mode, leaving comment->str NUL-terminated at its original (unbounded) length. strmov() copies until the source NUL, ignoring the truncated length, overflowing the destination buffer for long comments. The fix uses strmake() bounded by comment->length instead, matching the LEX_CSTRING contract (length is authoritative, str need not be NUL-terminated at length). Bug #2: strxnmov() copied the full un-truncated comment until it ran out of buffer space mid-string with no trailing newline -- which made load_db_opt() silently discard the whole unterminated "comment=" line on the next restart, losing the comment entirely instead of just truncating it. write_db_opt() must store only the truncated comment. The fix bounds the comment copy into db.opt by the already-validated comment->length via strmake(), instead of |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Marko Mäkelä
marko.makela@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fixup! 33a015af79f0fa37134d295be5d328ec9d4da69a | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Yuchen Pei
ycp@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39525 Add type checks to vcol index substitution in WHERE When doing a vcol index substitution in a WHERE condition, when the vcol field is not a supertype of the vcol expression, data truncation may result in wrong results after substitution. To fix that, we add a supertype check to WHERE substitution, just like we do for GROUP BY/ORDER BY substitution. However, this fix can be too strict in some cases. For example, if the condition is WHERE <vcol_expr> > 21 where <vcol_expr> has type BIGINT and <vcol_field> has type TINYINT, the supertype check would fail, but here, data truncation would not result in wrong result, because the truncation clamps at extreme values which are -128 or 127 for TINYINT. The key is that the constant compared to is strictly inside the domain of vcol_field. To that end, we add a fallback check for WHERE substitution, that compares vcol_field type domain with the constants. A hole remains for strings with PAD SPACE collations: any const string for comparison could be a result of truncation, even if much shorter than the capacity of the vcol field. For example, for a table with (a varchar(20), vc varchar(5) as (concat('x',a)), index(vc)) vc = 'xabc' evals to 1 for a row with a = 'abc z' but concat('x',a) = 'xabc' evals to 0. The right hand side is 'xabc' could match a vc of varchar(N) for any N >= 4 because there could be arbitrary number of spaces before the truncation. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Yuchen Pei
ycp@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39525 Add type checks to vcol index substitution in WHERE When doing a vcol index substitution in a WHERE condition, when the vcol field is not a supertype of the vcol expression, data truncation may result in wrong results after substitution. To fix that, we add a supertype check to WHERE substitution, just like we do for GROUP BY/ORDER BY substitution. However, this fix can be too strict in some cases. For example, if the condition is WHERE <vcol_expr> > 21 where <vcol_expr> has type BIGINT and <vcol_field> has type TINYINT, the supertype check would fail, but here, data truncation would not result in wrong result, because the truncation clamps at extreme values which are -128 or 127 for TINYINT. The key is that the constant compared to is strictly inside the domain of vcol_field. To that end, we add a fallback check for WHERE substitution, that compares vcol_field type domain with the constants. A hole remains for strings with PAD SPACE collations: any const string for comparison could be a result of truncation, even if much shorter than the capacity of the vcol field. For example, for a table with (a varchar(20), vc varchar(5) as (concat('x',a)), index(vc)) vc = 'xabc' evals to 1 for a row with a = 'abc z' but concat('x',a) = 'xabc' evals to 0. The right hand side is 'xabc' could match a vc of varchar(N) for any N >= 4 because there could be arbitrary number of spaces before the truncation. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Marko Mäkelä
marko.makela@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fixup! 33a015af79f0fa37134d295be5d328ec9d4da69a | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Aleksey Midenkov
midenok@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41157 CREATE DATABASE COMMENT overflows db.opt comment buffer Bug #1: put_dbopt() used strmov() to copy schema_comment into a fixed DATABASE_COMMENT_MAXLEN+1 buffer. validate_comment_length() only truncates comment->length in non-strict sql_mode, leaving comment->str NUL-terminated at its original (unbounded) length. strmov() copies until the source NUL, ignoring the truncated length, overflowing the destination buffer for long comments. The fix uses strmake() bounded by comment->length instead, matching the LEX_CSTRING contract (length is authoritative, str need not be NUL-terminated at length). Bug #2: write_db_opt() used strxnmov() to copy the full un-truncated comment until it ran out of buffer space mid-string with no trailing newline. Which made load_db_opt() silently discard the whole unterminated "comment=" line on the next restart, losing the comment entirely instead of just truncating it. The fix bounds the comment copy into db.opt by the already-validated comment->length via strmake(), instead of relying on the source string's own NUL terminator, matching the put_dbopt() fix. Bug #3: validate_comment_length() only runs on a COMMENT clause given in the current statement. ALTER DATABASE without one instead pulls the existing comment off disk via load_db_opt(), which never bounded it. That unvalidated length then reached write_db_opt()'s own comment= copy into its stack buffer, so a legacy or hand-edited db.opt with an overlong comment= line overflowed it on ALTER DATABASE. The fix: load_db_opt() now clamps the parsed comment to DATABASE_COMMENT_MAXLEN right when it reads the "comment=" line, so every consumer (put_dbopt(), write_db_opt()'s ALTER path) always sees an already-bounded value. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||