From 5ac4350c981051f9ff518a051067b3a045278e99 Mon Sep 17 00:00:00 2001 From: bsrikanth-mariadb Date: Thu, 23 Jul 2026 14:30:42 +0530 Subject: [PATCH] MDEV-40383:innodb_gis.point_basic fails on replay There are 2 problems: - 1. The REPLACE statement that is recorded doesn't store the value of geometry type field correctly. 2. The table definition that got recorded has fields with non-null constraint, and no default value is specified. Also, the "REPLACE INTO" statement that gets stored in the context, doesn't have any value specified for these non-null fields. Solution is to: - 1. When using REPLACE INTO statement, store all the non-numeric values in HEX, whenever conversion from field's charset to output's charset is lossy. 2. Instead of storing only the column values that were projected in the query, store all the non-virtual column values into the recorded REPLACE INTO statement. Implementation details: - 1. Introduce a new method is_charset_conversion_lossless() in filesort.cc, to check if the output charset to which field's data is being written to, results in a lossless conversion. If so, non-numeric values being witten using REPLACE INTO statement are stored in string representation, else they are converted to HEX. 2. From join_read_const(), and join_read_system() methods in sql_select.cc re-read the const row for all the non-virtual fields in the table. Similarly, for min/max optimization in opt_sum_query() of opt)_sum.cc, include all the non-virtual fields to be dumped into the "REPLACE INTO" statement. After the row is re-read and recorded, restore the table->read_set, table->status, and the const row, to the value that was before with the help of widen_read_set_no_vcols() method. --- .../main/opt_context_replay_basic.result | 101 ++++++++++++++++ mysql-test/main/opt_context_replay_basic.test | 113 ++++++++++++++++++ sql/filesort.cc | 44 ++++++- sql/opt_context_store_replay.cc | 84 +++++++++++++ sql/opt_context_store_replay.h | 15 ++- sql/opt_sum.cc | 27 ++++- sql/sql_select.cc | 14 +-- 7 files changed, 378 insertions(+), 20 deletions(-) diff --git a/mysql-test/main/opt_context_replay_basic.result b/mysql-test/main/opt_context_replay_basic.result index f2cca086e4dbc..3ccdc8803791a 100644 --- a/mysql-test/main/opt_context_replay_basic.result +++ b/mysql-test/main/opt_context_replay_basic.result @@ -568,5 +568,106 @@ nv 11 set optimizer_replay_context=''; drop table s1; +# +# MDEV-40383: innodb_gis.point_basic fails on replay +# +CREATE TABLE t1 ( +a INT NOT NULL, +p POINT NOT NULL, +l LINESTRING NOT NULL, +g GEOMETRY NOT NULL, +PRIMARY KEY(p), +SPATIAL KEY `idx2` (p), +SPATIAL KEY `idx3` (l), +SPATIAL KEY `idx4` (g) +); +INSERT INTO t1 VALUES( +1, ST_GeomFromText('POINT(10 10)'), +ST_GeomFromText('LINESTRING(1 1, 5 5, 10 10)'), +ST_GeomFromText('POLYGON((30 30, 40 40, 50 50, 30 50, 30 40, 30 30))')); +INSERT INTO t1 VALUES( +2, ST_GeomFromText('POINT(20 20)'), +ST_GeomFromText('LINESTRING(2 3, 7 8, 9 10, 15 16)'), +ST_GeomFromText('POLYGON((10 30, 30 40, 40 50, 40 30, 30 20, 10 30))')); +set optimizer_record_context=1; +EXPLAIN SELECT a, ST_AsText(p) FROM t1 WHERE a = 2 AND p = ST_GeomFromText('POINT(20 20)'); +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 const PRIMARY,idx2 PRIMARY 27 const 1 +select context into dumpfile "../../tmp/dump1.sql" +from information_schema.optimizer_context; +set optimizer_record_context=0; +drop table t1; +set optimizer_replay_context='opt_context'; +# Same query as above, must have same explain: +EXPLAIN SELECT a, ST_AsText(p) FROM t1 WHERE a = 2 AND p = ST_GeomFromText('POINT(20 20)'); +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 const PRIMARY,idx2 PRIMARY 27 const 1 +set optimizer_replay_context=''; +SELECT a, ST_AsText(p), ST_AsText(l), ST_AsText(g) FROM t1; +a ST_AsText(p) ST_AsText(l) ST_AsText(g) +2 POINT(20 20) LINESTRING(2 3,7 8,9 10,15 16) POLYGON((10 30,30 40,40 50,40 30,30 20,10 30)) +# +# MIN/MAX recording with geometry fields in the table +# +INSERT INTO t1 VALUES( +1, ST_GeomFromText('POINT(10 10)'), +ST_GeomFromText('LINESTRING(1 1, 5 5, 10 10)'), +ST_GeomFromText('POLYGON((30 30, 40 40, 50 50, 30 50, 30 40, 30 30))')); +alter table t1 add index(a); +select a from t1; +a +1 +2 +SELECT MIN(a) FROM t1; +MIN(a) +1 +set optimizer_record_context=1; +EXPLAIN SELECT MIN(a) FROM t1; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE NULL NULL NULL NULL NULL NULL NULL Select tables optimized away +select context into dumpfile "../../tmp/dump1.sql" +from information_schema.optimizer_context; +set optimizer_record_context=0; +drop table t1; +set optimizer_replay_context='opt_context'; +# Same query as above, must have same explain: +EXPLAIN SELECT MIN(a) FROM t1; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE NULL NULL NULL NULL NULL NULL NULL Select tables optimized away +set optimizer_replay_context=''; +SELECT a, ST_AsText(p), ST_AsText(l), ST_AsText(g) FROM t1; +a ST_AsText(p) ST_AsText(l) ST_AsText(g) +1 POINT(10 10) LINESTRING(1 1,5 5,10 10) POLYGON((30 30,40 40,50 50,30 50,30 40,30 30)) +drop table t1; +# +# MIN/MAX recording with a virtual column present. +# +CREATE TABLE t1 ( +a INT NOT NULL, +b INT NOT NULL, +v INT AS (a + 100) VIRTUAL, +KEY(a) +) ENGINE=MyISAM; +INSERT INTO t1 (a,b) VALUES (1,10),(2,20),(3,30),(1,40); +set optimizer_record_context=1; +EXPLAIN SELECT MIN(a) FROM t1; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE NULL NULL NULL NULL NULL NULL NULL Select tables optimized away +select context into dumpfile "../../tmp/dump1.sql" +from information_schema.optimizer_context; +set optimizer_record_context=0; +drop table t1; +set optimizer_replay_context='opt_context'; +# Same query as above, must have same explain: +EXPLAIN SELECT MIN(a) FROM t1; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE NULL NULL NULL NULL NULL NULL NULL Select tables optimized away +set optimizer_replay_context=''; +# MIN(a) row is (1,10); the non-indexed NOT NULL column b must be +# captured (not defaulted to 0), and v must be recomputed as a+100=101: +SELECT a, b, v FROM t1; +a b v +1 10 101 +drop table t1; # End of 13.1 tests drop database db1; diff --git a/mysql-test/main/opt_context_replay_basic.test b/mysql-test/main/opt_context_replay_basic.test index 961609f376e4e..f45f8fd6d517b 100644 --- a/mysql-test/main/opt_context_replay_basic.test +++ b/mysql-test/main/opt_context_replay_basic.test @@ -376,6 +376,119 @@ select lastval(s1) as nv; set optimizer_replay_context=''; --remove_file "$MYSQLTEST_VARDIR/tmp/dump1.sql" drop table s1; + +--echo # +--echo # MDEV-40383: innodb_gis.point_basic fails on replay +--echo # + +CREATE TABLE t1 ( + a INT NOT NULL, + p POINT NOT NULL, + l LINESTRING NOT NULL, + g GEOMETRY NOT NULL, + PRIMARY KEY(p), + SPATIAL KEY `idx2` (p), + SPATIAL KEY `idx3` (l), + SPATIAL KEY `idx4` (g) +); + +INSERT INTO t1 VALUES( +1, ST_GeomFromText('POINT(10 10)'), +ST_GeomFromText('LINESTRING(1 1, 5 5, 10 10)'), +ST_GeomFromText('POLYGON((30 30, 40 40, 50 50, 30 50, 30 40, 30 30))')); + +INSERT INTO t1 VALUES( +2, ST_GeomFromText('POINT(20 20)'), +ST_GeomFromText('LINESTRING(2 3, 7 8, 9 10, 15 16)'), +ST_GeomFromText('POLYGON((10 30, 30 40, 40 50, 40 30, 30 20, 10 30))')); + +set optimizer_record_context=1; +EXPLAIN SELECT a, ST_AsText(p) FROM t1 WHERE a = 2 AND p = ST_GeomFromText('POINT(20 20)'); +select context into dumpfile "../../tmp/dump1.sql" +from information_schema.optimizer_context; +set optimizer_record_context=0; +drop table t1; +--disable_query_log +--disable_result_log +--source "$MYSQLTEST_VARDIR/tmp/dump1.sql" +--enable_query_log +--enable_result_log +set optimizer_replay_context='opt_context'; +--echo # Same query as above, must have same explain: +EXPLAIN SELECT a, ST_AsText(p) FROM t1 WHERE a = 2 AND p = ST_GeomFromText('POINT(20 20)'); + +set optimizer_replay_context=''; +--remove_file "$MYSQLTEST_VARDIR/tmp/dump1.sql" +SELECT a, ST_AsText(p), ST_AsText(l), ST_AsText(g) FROM t1; + +--echo # +--echo # MIN/MAX recording with geometry fields in the table +--echo # +INSERT INTO t1 VALUES( +1, ST_GeomFromText('POINT(10 10)'), +ST_GeomFromText('LINESTRING(1 1, 5 5, 10 10)'), +ST_GeomFromText('POLYGON((30 30, 40 40, 50 50, 30 50, 30 40, 30 30))')); + +alter table t1 add index(a); + +select a from t1; +SELECT MIN(a) FROM t1; + +set optimizer_record_context=1; +EXPLAIN SELECT MIN(a) FROM t1; + +select context into dumpfile "../../tmp/dump1.sql" +from information_schema.optimizer_context; + +set optimizer_record_context=0; +drop table t1; +--disable_query_log +--disable_result_log +--source "$MYSQLTEST_VARDIR/tmp/dump1.sql" +--enable_query_log +--enable_result_log +set optimizer_replay_context='opt_context'; +--echo # Same query as above, must have same explain: +EXPLAIN SELECT MIN(a) FROM t1; + +set optimizer_replay_context=''; +--remove_file "$MYSQLTEST_VARDIR/tmp/dump1.sql" +SELECT a, ST_AsText(p), ST_AsText(l), ST_AsText(g) FROM t1; +drop table t1; + +--echo # +--echo # MIN/MAX recording with a virtual column present. +--echo # +CREATE TABLE t1 ( + a INT NOT NULL, + b INT NOT NULL, + v INT AS (a + 100) VIRTUAL, + KEY(a) +) ENGINE=MyISAM; +INSERT INTO t1 (a,b) VALUES (1,10),(2,20),(3,30),(1,40); + +set optimizer_record_context=1; +EXPLAIN SELECT MIN(a) FROM t1; +select context into dumpfile "../../tmp/dump1.sql" +from information_schema.optimizer_context; +set optimizer_record_context=0; +drop table t1; +--disable_query_log +--disable_result_log +--source "$MYSQLTEST_VARDIR/tmp/dump1.sql" +--enable_query_log +--enable_result_log +set optimizer_replay_context='opt_context'; +--echo # Same query as above, must have same explain: +EXPLAIN SELECT MIN(a) FROM t1; + +set optimizer_replay_context=''; +--remove_file "$MYSQLTEST_VARDIR/tmp/dump1.sql" +--echo # MIN(a) row is (1,10); the non-indexed NOT NULL column b must be +--echo # captured (not defaulted to 0), and v must be recomputed as a+100=101: +SELECT a, b, v FROM t1; +drop table t1; + --echo # End of 13.1 tests drop database db1; diff --git a/sql/filesort.cc b/sql/filesort.cc index 0b8fbfb83feeb..e5c31f7eeaa69 100644 --- a/sql/filesort.cc +++ b/sql/filesort.cc @@ -3024,6 +3024,25 @@ static uint make_packed_sortkey(Sort_param *param, uchar *to) return length; } +static bool is_charset_conversion_lossless(const CHARSET_INFO *from_cs, + const CHARSET_INFO *to_cs) +{ + if (to_cs == &my_charset_bin) + return true; // binary swallows any bytes + if (from_cs == &my_charset_bin) + return false; // arbitrary bytes ⊄ text charset + if (my_charset_same(from_cs, to_cs)) + return true; // same repertoire family + if (from_cs->state & MY_CS_PUREASCII) // ASCII-only source... + return my_charset_is_ascii_based(to_cs); // ...into any ASCII-based target + if ((to_cs->state & + MY_CS_UNICODE) && // Unicode target covers everything,M N?'b; + (to_cs->state & + MY_CS_UNICODE_SUPPLEMENT)) // incl. non-BMP (so utf8mb4/utf16/utf32, + return true; // but NOT plain utf8mb3/ucs2) + return false; // unknown → treat as possibly lossy +} + /* @brief Format the row record and store it in the output @@ -3148,12 +3167,25 @@ void format_and_store_row(TABLE *table, const uchar *rec, bool print_names, } field->val_str(&tmp); } - if (require_quote) - output.append('\''); - output.append_for_single_quote_opt_convert(tmp.ptr(), tmp.length(), - field->charset()); - if (require_quote) - output.append('\''); + /* + Emit non-empty values as a hex literal whenever converting field's + charset to the output charset conversion is lossy; otherwise emit the + charset-converted value, quoted only when the type requires it. + */ + if (require_quote && tmp.length() && + !is_charset_conversion_lossless(tmp.charset(), output.charset())) + { + output.append(STRING_WITH_LEN("0x")); + output.append_hex(tmp.ptr(), tmp.length()); + } + else + { + if (require_quote) + output.append('\''); + output.append_for_single_quote_opt_convert(tmp); + if (require_quote) + output.append('\''); + } } } output.append(')'); diff --git a/sql/opt_context_store_replay.cc b/sql/opt_context_store_replay.cc index e69f3dca3c468..2a59dedb81753 100644 --- a/sql/opt_context_store_replay.cc +++ b/sql/opt_context_store_replay.cc @@ -2371,3 +2371,87 @@ void clean_captured_ctx(THD *thd) delete thd->captured_opt_ctx; thd->captured_opt_ctx= nullptr; } + +/* + Point table->read_set at a private bitmap (table->tmp_set) covering every + stored column, so that a subsequent read/record captures the full row. + + Virtual columns are excluded: they cannot be assigned in REPLACE INTO and are + recomputed on read. We copy s->all_set into the per-table tmp_set rather than + aliasing s->all_set directly -- s->all_set is shared across the whole + TABLE_SHARE and must never be mutated. + + Precondition: table->tmp_set must stay free for the caller's use until + read_set is restored. This holds on the const-row and MIN/MAX read paths + precisely because we clear the virtual-column bits: with those bits unset, + TABLE::update_virtual_fields(VCOL_UPDATE_FOR_READ) skips them during the read + and so never reuses tmp_set as its own scratch. A caller on a path that + evaluates virtual columns into tmp_set would corrupt the widened read_set. + + @return the previous read_set, which the caller must restore (via + column_bitmaps_set) once the row has been read and recorded. +*/ +MY_BITMAP *widen_read_set_no_vcols(TABLE *table) +{ + MY_BITMAP *saved_read_set= table->read_set; + bitmap_copy(&table->tmp_set, &table->s->all_set); + for (Field **pfield= table->field; *pfield; pfield++) + { + /* virtual columns need not be stored. */ + if ((*pfield)->vcol_info) + bitmap_clear_bit(&table->tmp_set, (*pfield)->field_index); + } + table->column_bitmaps_set(&table->tmp_set, table->write_set); + return saved_read_set; +} + +/* + Re-read a const/system table row with ALL but Virtual columns and + record it for the optimizer context. + + join_read_system()/join_read_const() only fetch the columns present in + table->read_set. That's sufficient for execution, but when we record the + const row for replay it yields an incomplete REPLACE INTO -- columns that + are NOT NULL and have no default then depend on a relaxed sql_mode (see + Optimizer_context_recorder::record_table_row()). Widen read_set to all + columns, re-read the single row, record it, then restore read_set so the + chosen plan is not disturbed. + + The caller must have cached the row the optimizer actually used in + record[1] before calling this. We re-read the full row into record[0] only + to record it, then unconditionally restore record[0] from record[1] so that + recording leaves execution's record[0] byte-for-byte identical to the + non-recording case -- regardless of whether the re-read found a row. +*/ +void record_const_row_full(JOIN_TAB *tab, bool is_system) +{ + TABLE *table= tab->table; + Optimizer_context_recorder *rec= tab->join->thd->opt_ctx_recorder; + + uint saved_status= table->status; + MY_BITMAP *saved_read_set= widen_read_set_no_vcols(table); + + int error; + /* + Re-read the single const row with the widened read_set, mirroring how the + optimizer originally fetched it: a system table (join_read_system) has at + most one row, read via the primary key; a const eq_ref table + (join_read_const) is fetched by an exact lookup on tab->ref. + */ + if (is_system) + error= table->file->ha_read_first_row(table->record[0], + table->s->primary_key); + else + error= table->file->ha_index_read_idx_map( + table->record[0], tab->ref.key, (uchar *) tab->ref.key_buff, + make_prev_keypart_map(tab->ref.key_parts), HA_READ_KEY_EXACT); + + if (likely(!error)) + rec->record_current_table_row(table); // records the full record[0] + + /* Recording must not perturb the row execution uses: restore it. */ + restore_record(table, record[1]); + + table->column_bitmaps_set(saved_read_set, table->write_set); + table->status= saved_status; +} diff --git a/sql/opt_context_store_replay.h b/sql/opt_context_store_replay.h index 55b587dc2a166..929484e3b5ffe 100644 --- a/sql/opt_context_store_replay.h +++ b/sql/opt_context_store_replay.h @@ -25,6 +25,7 @@ class Item_subselect; class Json_writer; class Json_writer_object; +using JOIN_TAB= struct st_join_table; /*************************************************************************** * Part 1: APIs for recording Optimizer Context. @@ -66,11 +67,6 @@ class Optimizer_context_recorder const KEY_PART_INFO *key_part, uint keynr, const key_range *min_range, const key_range *max_range, ha_rows records); - void record_const_table_row(TABLE *tbl) - { - /* use table->record[1] */ - record_table_row(tbl, 1); - } void record_current_table_row(TABLE *tbl) { /* use table->record[0] */ @@ -140,6 +136,15 @@ int fill_optimizer_context_capture_info(THD *thd, TABLE_LIST *tables, Item *); void clean_captured_ctx(THD *thd); +/* + Widen read_set to all stored (non-virtual) columns via table->tmp_set, so the + next read/record captures the full row; returns the previous read_set for the + caller to restore. See the definition for the tmp_set precondition. +*/ +MY_BITMAP *widen_read_set_no_vcols(TABLE *table); + +void record_const_row_full(JOIN_TAB *tab, bool is_system); + /*************************************************************************** * Part 3: APIs for loading previously saved Optimizer Context and replaying * it: making the optimizer work as if the environment was like it has been diff --git a/sql/opt_sum.cc b/sql/opt_sum.cc index eca59bbc68ec3..90aa620ebac50 100644 --- a/sql/opt_sum.cc +++ b/sql/opt_sum.cc @@ -428,6 +428,23 @@ int opt_sum_query(THD *thd, error= 0; table->file->info_push(INFO_KIND_FORCE_LIMIT_BEGIN, &info_limit); + /* + While recording optimizer context we want record_current_table_row() + to emit a complete REPLACE INTO, so widen read_set to all stored + columns. format_and_store_row() picks the columns to print from + read_set, so this matters even for a clustered primary key (where the + whole row is already materialized); for a covering secondary index it + additionally forces the full row to be read, since keyread was + suppressed in find_key_for_maxmin(). + + Only the non-const branch reads the row here. A const table's row was + read into record[0] earlier and recorded (as a full row) by + record_const_row_full() on the const-table read path, so we neither + widen nor re-read it here. + */ + MY_BITMAP *saved_read_set= NULL; + if (thd->opt_ctx_recorder && !table->const_table) + saved_read_set= widen_read_set_no_vcols(table); if (!table->const_table) { if (likely(!(error= table->file->ha_index_init((uint) ref.key, @@ -448,6 +465,8 @@ int opt_sum_query(THD *thd, if (Optimizer_context_recorder *rec= thd->opt_ctx_recorder) rec->record_current_table_row(table); } + if (saved_read_set) + table->column_bitmaps_set(saved_read_set, table->write_set); if (!table->const_table) { table->file->ha_end_keyread(); @@ -1026,7 +1045,13 @@ static bool find_key_for_maxmin(bool max_fl, TABLE_REF *ref, The following test is false when the key in the key tree is converted (for example to upper case) */ - if (field->part_of_key.is_set(idx)) + /* + When recording optimizer context we need the whole row (see + opt_sum_query()), so do not switch to index-only reads -- otherwise + only the MIN/MAX column would be materialized into record[0]. + */ + if (field->part_of_key.is_set(idx) && + !table->in_use->opt_ctx_recorder) table->file->ha_start_keyread(idx); *reverse= part->key_part_flag & HA_REVERSE_SORT ? true : false; DBUG_RETURN(TRUE); diff --git a/sql/sql_select.cc b/sql/sql_select.cc index a82fb09ab0832..fde2790e57916 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -25066,7 +25066,6 @@ join_read_const_table(THD *thd, JOIN_TAB *tab, POSITION *pos) DBUG_RETURN(0); } - /** Read a constant table when there is at most one matching row, using a table scan. @@ -25095,9 +25094,9 @@ join_read_system(JOIN_TAB *tab) empty_record(table); // Make empty record return -1; } - store_record(table,record[1]); - if (Optimizer_context_recorder *rec= tab->join->thd->opt_ctx_recorder) - rec->record_const_table_row(table); + store_record(table, record[1]); // cache the row the optimizer used + if (tab->join->thd->opt_ctx_recorder) + record_const_row_full(tab, true); // re-read full row, record, restore } else if (!table->status) // Only happens with left join restore_record(table,record[1]); // restore old record @@ -25152,10 +25151,9 @@ join_read_const(JOIN_TAB *tab) return report_error(table, error); return -1; } - store_record(table,record[1]); - - if (Optimizer_context_recorder *rec= tab->join->thd->opt_ctx_recorder) - rec->record_const_table_row(table); + store_record(table, record[1]); // cache the row the optimizer used + if (tab->join->thd->opt_ctx_recorder) + record_const_row_full(tab, false); // re-read full row, record, restore } else if (!(table->status & ~STATUS_NULL_ROW)) // Only happens with left join {