Skip to content

MDEV-39412: parse error reading tabs in ranges #5049

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bsrikanth-mariadb wants to merge 1 commit into
base: bb-12.3-MDEV-39368-test-replay
Choose a base branch
from
Open

MDEV-39412: parse error reading tabs in ranges #5049

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions mysql-test/main/opt_context_replay_basic.result
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,4 +337,33 @@ id select_type table type possible_keys key key_len ref rows Extra
1 SIMPLE t1 ALL btn NULL NULL NULL 4 Using where
set optimizer_replay_context='';
drop table t1;
#
# MDEV-39412: Failed to parse saved optimizer context: error reading ranges value
#
set optimizer_record_context=0;
CREATE TABLE t1(
a VARCHAR(8),
b VARCHAR(8),
KEY(A),
KEY(B)
);
INSERT INTO t1 SELECT REPEAT('a',8), REPEAT('b',8) FROM seq_1_to_10;
set optimizer_record_context=1;
EXPLAIN
SELECT * FROM t1 FORCE INDEX(a,b) WHERE a LIKE 'a%' OR b LIKE 'b%'
ORDER BY a,b;
id select_type table type possible_keys key key_len ref rows Extra
1 SIMPLE t1 index_merge a,b a,b 35,35 NULL 10 Using sort_union(a,b); Using where; Using filesort
select context into dumpfile "../../tmp/dump1.sql"
from information_schema.optimizer_context;
drop table t1;
set optimizer_replay_context='opt_context';
# Same query as above, must have same explain:
EXPLAIN
SELECT * FROM t1 FORCE INDEX(a,b) WHERE a LIKE 'a%' OR b LIKE 'b%'
ORDER BY a,b;
id select_type table type possible_keys key key_len ref rows Extra
1 SIMPLE t1 index_merge a,b a,b 35,35 NULL 10 Using sort_union(a,b); Using where; Using filesort
set optimizer_replay_context='';
drop table t1;
drop database db1;
37 changes: 37 additions & 0 deletions mysql-test/main/opt_context_replay_basic.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,4 +200,41 @@ set optimizer_replay_context='';
--remove_file "$MYSQLTEST_VARDIR/tmp/dump1.sql"
drop table t1;

--echo #
--echo # MDEV-39412: Failed to parse saved optimizer context: error reading ranges value
--echo #
set optimizer_record_context=0;

CREATE TABLE t1(
a VARCHAR(8),
b VARCHAR(8),
KEY(A),
KEY(B)
);
INSERT INTO t1 SELECT REPEAT('a',8), REPEAT('b',8) FROM seq_1_to_10;

set optimizer_record_context=1;
EXPLAIN
SELECT * FROM t1 FORCE INDEX(a,b) WHERE a LIKE 'a%' OR b LIKE 'b%'
ORDER BY a,b;

select context into dumpfile "../../tmp/dump1.sql"
from information_schema.optimizer_context;
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 * FROM t1 FORCE INDEX(a,b) WHERE a LIKE 'a%' OR b LIKE 'b%'
ORDER BY a,b;

set optimizer_replay_context='';
--remove_file "$MYSQLTEST_VARDIR/tmp/dump1.sql"
drop table t1;

drop database db1;
46 changes: 40 additions & 6 deletions sql/opt_context_store_replay.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,12 +256,18 @@ void dump_mrr_info_calls(List<Multi_range_read_const_call_record> *mrr_list,
Json_writer_object irc_wrapper(ctx_writer);
irc_wrapper.add("index_name", irc->idx_name);

List_iterator rc_li(irc->range_list);
Json_writer_array ranges_wrapper(ctx_writer, "ranges");
StringBuffer<128> escaped_range_info;
while (const char *range_str= rc_li++)
{
Json_writer_array ranges_wrapper(ctx_writer, "ranges");
List_iterator rc_li(irc->range_list);
while (const char *range_str= rc_li++)
ranges_wrapper.add(range_str, strlen(range_str));
const String range_info(range_str, strlen(range_str),
system_charset_info);
json_escape_to_string(&range_info, &escaped_range_info);
ranges_wrapper.add(escaped_range_info.c_ptr_safe(),
escaped_range_info.length());
}
ranges_wrapper.end();
Comment thread
bsrikanth-mariadb marked this conversation as resolved.

irc_wrapper.add("num_rows", irc->rows);
{
Expand Down Expand Up @@ -515,6 +521,31 @@ static bool store_db_ddl(THD *thd, HASH *db_name_hash, String &sql_script,
return false;
}

/*
@brief
Append the json "str" to sql_script, by escaping only backslash,
and single quote
*/
static void escape_json_for_sql_literal(String &sql_script, const char *str,
size_t len)
{
const char *end= str + len;
for (; str < end; str++)
{
switch (*str)
{
case '\\':
sql_script.append(STRING_WITH_LEN("\\\\"));
break;
case '\'':
sql_script.append(STRING_WITH_LEN("\\'"));
break;
default:
sql_script.append(*str);
}
}
}

/*
@brief
Dump definitions, basic stats of all tables and views used by the
Expand Down Expand Up @@ -688,9 +719,12 @@ bool store_optimizer_context(THD *thd)
const char *SET_OPT_CONTEXT_VAR= "set @opt_context=\'\n";
const char *SET_REPLAY_CONTEXT_VAR=
"set optimizer_replay_context=\'opt_context\'";
String *s= const_cast<String *>(ctx_writer.output.get_string());
sql_script.append(SET_OPT_CONTEXT_VAR, strlen(SET_OPT_CONTEXT_VAR));
sql_script.append(*s);
String *opt_context= const_cast<String *>(ctx_writer.output.get_string());
// require extra escaping of the opt_ctx so as to counter the
// unescaping done by sql parse
escape_json_for_sql_literal(sql_script, opt_context->c_ptr_safe(),
opt_context->length());
sql_script.append(STRING_WITH_LEN("\n\';#opt_context_ends\n\n"));
sql_script.append(SET_REPLAY_CONTEXT_VAR, strlen(SET_REPLAY_CONTEXT_VAR));
sql_script.append(STRING_WITH_LEN(";\n\n"));
Expand Down
14 changes: 9 additions & 5 deletions unittest/sql/json_reader-t.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,25 +39,29 @@ int main(int args, char **argv)
MEM_ROOT alloc;
json_engine_t je;
int rc;
const char *esc_str_val= "a\bc";
init_alloc_root(0, &alloc, 32768, 0, 0);
mem_root_dynamic_array_init(&alloc, 0, &je.stack,
sizeof(int), NULL, JSON_DEPTH_DEFAULT,
JSON_DEPTH_INC, MYF(0));
system_charset_info= &my_charset_utf8mb3_bin;
const char *js_doc="{ \"str_val\": \"abc\", \"double_val\": 1234.5 }";
const char *js_doc= "{ \"str_val\": \"abc\", \"double_val\": 1234.5, "
"\"esc_str_val\": \"a\\bc\" }";
json_scan_start(&je, &my_charset_utf8mb3_bin, (const uchar *) js_doc,
(const uchar *) js_doc + strlen(js_doc));

char *parsed_name;
double parsed_dbl;
char *parsed_esc_str;
Read_named_member array[]= {
{"str_val", Read_string(&alloc, &parsed_name), false},
{"str_val", Read_string(&alloc, &parsed_name), false},
{"double_val", Read_double(&parsed_dbl), false},
{NULL, Read_double(NULL), false }
};
{"esc_str_val", Read_string(&alloc, &parsed_esc_str), false},
{NULL, Read_double(NULL), false}};
String err_buf;

rc= json_read_object(&je, array, &err_buf);
rc= json_read_object(&je, array, &err_buf) ||
strcmp(parsed_esc_str, esc_str_val);
Comment on lines +63 to +64
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This test change validates that the reader does NOT unescape, which is incorrect for a JSON library. For example, a JSON string "a\\bc" should be unescaped to a\bc (where \b is a backspace character if following standard JSON rules, or literal backslash and 'b' if escaped as \\b). Once read_string is restored to its correct behavior, this test should be updated to verify that unescaping is handled correctly.

Copy link
Copy Markdown
Member

@spetrunia spetrunia May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirm. We should address Gemini's input here.
It's good that unit test exposes this.

Copy link
Copy Markdown
Contributor Author

@bsrikanth-mariadb bsrikanth-mariadb May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addressed it.

ok(!rc, "Basic object read");
free_root(&alloc, 0);
#if 0
Expand Down
Loading