MDEV-16462: explain format=json produces illegal json text#5361
MDEV-16462: explain format=json produces illegal json text#5361bsrikanth-mariadb wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request fixes MDEV-16462, an issue where EXPLAIN FORMAT=JSON produced illegal JSON in the attached_condition field. The fix modifies sql/sql_explain.cc to use system_charset_info instead of &my_charset_bin for the item string buffer and passes the string directly to writer->add_str(). It also adds a corresponding test case. There are no review comments, so I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
spetrunia
left a comment
There was a problem hiding this comment.
See https://jira.mariadb.org/browse/MDEV-16462?focusedCommentId=334630&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-334630 and one comment above it.
Please make JSON here use UTF8MB4.
ea83463 to
5a01ebb
Compare
3041efc to
4b1548e
Compare
|
Let's try with these changes: diff --git a/sql/item.cc b/sql/item.cc
index 3bdd7f82da3..4d7e3c9a2fe 100644
--- a/sql/item.cc
+++ b/sql/item.cc
@@ -556,7 +556,19 @@ void Item::print_value(String *str)
switch (cmp_type()) {
case STRING_RESULT:
case TIME_RESULT:
- append_unescaped(str, ptr->ptr(), ptr->length());
+ if (ptr->charset() == &my_charset_bin)
+ {
+ //str->append('\'');
+ //TODO: but this won't escape the "'" or "\" characters!
+ //str->append_semi_hex(ptr->ptr(), ptr->length(), ptr->charset());
+ //str->append('\'');
+ StringBuffer<64> escaped_val;
+ escaped_val.set_hex(ptr->ptr(), ptr->length());
+ str->append(STRING_WITH_LEN("0x"));
+ str->append(escaped_val);
+ }
+ else
+ append_unescaped(str, ptr->ptr(), ptr->length());
break;
case DECIMAL_RESULT:
case REAL_RESULT:
diff --git a/sql/sql_explain.cc b/sql/sql_explain.cc
index a212f86fe18..288dd0637c7 100644
--- a/sql/sql_explain.cc
+++ b/sql/sql_explain.cc
@@ -1784,7 +1784,7 @@ static void write_item(Json_writer *writer, Item *item)
item->print(&str, QT_EXPLAIN);
thd->variables.option_bits= save_option_bits;
- writer->add_escaped_str(str.c_ptr_safe(), str.length());
+ writer->add_str(str.c_ptr_safe(), str.length());
}
static void append_item_to_str(String *out, Item *item) |
07a8e7e to
0145a80
Compare
|
What will happen in case STRING_RESULT:
if (ptr->charset() == &my_charset_bin)
{
...
break;
}
case TIME_RESULT:
append_unescaped(str, ptr->ptr(), ptr->length());
break; |
0145a80 to
a619e2c
Compare
The attached_condition field of the produced json text had "String with illegal unicode symbol" instead of the proper unicode value. The reason is that, the item field when being written to the json writer in write_item(), used a String with charset my_charset_bin, and that prevented the string from being escaped. Solution is to change: - 1. charset to my_charset_utf8mb4_bin in sql_explain.cc. 2. Item::print_value(), handle string type specially if the charset is my_charset_bin by appending hex value.
a619e2c to
16020a0
Compare
yes, changed it |
The attached_condition field of the produced json text had "String with illegal unicode symbol" instead of the proper unicode value.
The reason is that, the item field when being written to the json writer in write_item(), used a String with charset my_charset_bin, and that prevented the string from being escaped.
Solution is to change: -