diff options
| -rw-r--r-- | mysql-test/main/opt_trace.result | 10 | ||||
| -rw-r--r-- | mysql-test/main/opt_trace.test | 10 | ||||
| -rw-r--r-- | sql/my_json_writer.cc | 102 | ||||
| -rw-r--r-- | sql/my_json_writer.h | 52 | ||||
| -rw-r--r-- | unittest/sql/my_json_writer-t.cc | 61 |
5 files changed, 219 insertions, 16 deletions
diff --git a/mysql-test/main/opt_trace.result b/mysql-test/main/opt_trace.result index 34683d037ea..ada5a72b2fb 100644 --- a/mysql-test/main/opt_trace.result +++ b/mysql-test/main/opt_trace.result @@ -8647,5 +8647,15 @@ SELECT a FROM t1 WHERE (a,b) in (SELECT @c,@d); a DROP TABLE t1; # +# MDEV-28598: Assertion `got_name == named_item_expected()' failed in Json_writer::on_start_object +# +CREATE TABLE t1 (a int); +SET optimizer_trace=1; +INSERT INTO t1 VALUES (1),(2),(3); +SELECT * FROM t1 WHERE a IN (SELECT 1 a FROM t1 WHERE EXISTS (select 1)); +a +1 +DROP TABLE t1; +# # End of 10.4 tests # diff --git a/mysql-test/main/opt_trace.test b/mysql-test/main/opt_trace.test index 855ce11aaf5..ab30b23b743 100644 --- a/mysql-test/main/opt_trace.test +++ b/mysql-test/main/opt_trace.test @@ -654,5 +654,15 @@ SELECT a FROM t1 WHERE (a,b) in (SELECT @c,@d); DROP TABLE t1; --echo # +--echo # MDEV-28598: Assertion `got_name == named_item_expected()' failed in Json_writer::on_start_object +--echo # + +CREATE TABLE t1 (a int); +SET optimizer_trace=1; +INSERT INTO t1 VALUES (1),(2),(3); +SELECT * FROM t1 WHERE a IN (SELECT 1 a FROM t1 WHERE EXISTS (select 1)); +DROP TABLE t1; + +--echo # --echo # End of 10.4 tests --echo # diff --git a/sql/my_json_writer.cc b/sql/my_json_writer.cc index 8e35b25b822..c95e4c6d657 100644 --- a/sql/my_json_writer.cc +++ b/sql/my_json_writer.cc @@ -39,7 +39,7 @@ inline void Json_writer::on_start_object() #if !defined(NDEBUG) || defined(JSON_WRITER_UNIT_TEST) if(!fmt_helper.is_making_writer_calls()) { - VALIDITY_ASSERT(got_name == named_item_expected()); + validate_named_item_expectation(true); named_items_expectation.push_back(true); } #endif @@ -68,7 +68,7 @@ void Json_writer::start_array() #if !defined(NDEBUG) || defined(JSON_WRITER_UNIT_TEST) if(!fmt_helper.is_making_writer_calls()) { - VALIDITY_ASSERT(got_name == named_item_expected()); + validate_named_item_expectation(true); named_items_expectation.push_back(false); got_name= false; } @@ -91,9 +91,20 @@ void Json_writer::start_array() void Json_writer::end_object() { #if !defined(NDEBUG) || defined(JSON_WRITER_UNIT_TEST) - VALIDITY_ASSERT(named_item_expected()); + if (!named_item_expected()) + { + print_json_tail_to_stderr( + "end_object failed since an unnamed item is expected"); + VALIDITY_ASSERT(false); + } named_items_expectation.pop_back(); - VALIDITY_ASSERT(!got_name); + + if (got_name) + { + print_json_tail_to_stderr( + "end_object failed since got_name==true"); + VALIDITY_ASSERT(false); + } got_name= false; #endif indent_level-=INDENT_SIZE; @@ -107,7 +118,12 @@ void Json_writer::end_object() void Json_writer::end_array() { #if !defined(NDEBUG) || defined(JSON_WRITER_UNIT_TEST) - VALIDITY_ASSERT(!named_item_expected()); + if (named_item_expected()) + { + print_json_tail_to_stderr( + "end_array failed since a named item is expected"); + VALIDITY_ASSERT(false); + } named_items_expectation.pop_back(); got_name= false; #endif @@ -130,8 +146,18 @@ Json_writer& Json_writer::add_member(const char *name, size_t len) { if (!fmt_helper.on_add_member(name, len)) { +#if !defined(NDEBUG) || defined(JSON_WRITER_UNIT_TEST) // assert that we are in an object - DBUG_ASSERT(!element_started); + if (element_started) + { + char err_descr[200]; + snprintf(err_descr, sizeof(err_descr), + "failed to add_member(\"%s\", %zu) since element_started=false", + name, len); + print_json_tail_to_stderr(err_descr); + VALIDITY_ASSERT(false); + } +#endif start_element(); output.append('"'); @@ -242,8 +268,12 @@ void Json_writer::add_unquoted_str(const char* str) void Json_writer::add_unquoted_str(const char* str, size_t len) { - VALIDITY_ASSERT(fmt_helper.is_making_writer_calls() || - got_name == named_item_expected()); +#if !defined(NDEBUG) || defined(JSON_WRITER_UNIT_TEST) + if (!fmt_helper.is_making_writer_calls()) + { + validate_named_item_expectation(); + } +#endif if (on_add_str(str, len)) return; @@ -275,8 +305,12 @@ void Json_writer::add_str(const char *str) void Json_writer::add_str(const char* str, size_t num_bytes) { - VALIDITY_ASSERT(fmt_helper.is_making_writer_calls() || - got_name == named_item_expected()); +#if !defined(NDEBUG) || defined(JSON_WRITER_UNIT_TEST) + if (!fmt_helper.is_making_writer_calls()) + { + validate_named_item_expectation(); + } +#endif if (on_add_str(str, num_bytes)) return; @@ -294,6 +328,53 @@ void Json_writer::add_str(const String &str) add_str(str.ptr(), str.length()); } +#if !defined(NDEBUG) || defined(JSON_WRITER_UNIT_TEST) +void Json_writer::print_json_tail_to_stderr(const char *err_descr) +{ + size_t pos_to_print_from= + output.get_string()->length() < MAX_JSON_LEN_FOR_STDERR ? + 0 : output.get_string()->length() - MAX_JSON_LEN_FOR_STDERR; + auto str_to_print= output.get_string()->c_ptr() + pos_to_print_from; + fprintf(stderr, "JSON writer error: %s.\nJSON tail: %s\n", + err_descr, str_to_print); +} + +void Json_writer::validate_named_item_expectation(bool fix_wth_implicit_member) +{ + if (got_name != named_item_expected()) + { + if (!got_name && fix_wth_implicit_member) + { + /* + Context is expecting a named member but has an unnamed one. + Add a member with automatically assigned name to preserve + JSON document correctness + */ + add_implicit_member(); + return; + } + const char *expected= named_item_expected() ? + "named" : "unnamed"; + const char *actual= named_item_expected() ? + "unnamed" : "named"; + char err_descr[100]; + snprintf(err_descr, sizeof(err_descr), + "expected a %s member but a %s one has been added", + expected, actual); + print_json_tail_to_stderr(err_descr); + VALIDITY_ASSERT(false); + } +} + +void Json_writer::add_implicit_member() +{ + char name[50]; + sprintf(name, "implicitly_added_member_#%u", implicit_member_num); + add_member(name); + implicit_member_num++; +} +#endif + Json_writer_temp_disable::Json_writer_temp_disable(THD *thd_arg) { thd= thd_arg; @@ -308,6 +389,7 @@ bool Single_line_formatting_helper::on_add_member(const char *name, size_t len) { DBUG_ASSERT(state== INACTIVE || state == DISABLED); + if (state != DISABLED) { // remove everything from the array diff --git a/sql/my_json_writer.h b/sql/my_json_writer.h index 4b69ba70675..d6b8ade091f 100644 --- a/sql/my_json_writer.h +++ b/sql/my_json_writer.h @@ -189,7 +189,7 @@ public: str.append(c); } - const String *get_string() { return &str; } + String *get_string() { return &str; } size_t length() { return str.length(); } private: String str; @@ -296,6 +296,56 @@ private: void start_element(); void start_sub_element(); +#if !defined(NDEBUG) || defined(JSON_WRITER_UNIT_TEST) + /* + Prints the tail of JSON composed so far to stderr. + Output is limited by MAX_JSON_LEN_FOR_STDERR characters. + Format is as follows: + "JSON writer error: <description of the error>. + JSON tail: <...JSON...> + */ + void print_json_tail_to_stderr(const char *err_descr); + + enum { MAX_JSON_LEN_FOR_STDERR = 1000 }; + + /* + If the context of the JSON document is expecting a unnamed member + but a named one is added this will trigger an assertion failure. + The same works vice versa unless the "fix_wth_implicit_member" + flag is set. When fix_wth_implicit_member == true and the context is + expecting a named member but has got an unnamed one, + the function will implicitly add a JSON member and automatically assign + its name. In this case correctness of JSON document is preserved + and no assertion will trigger. + Example: + transformation": { + "select_id": 2, + { + "join_optimization": { + "select_id": 3, + "steps": [] + } + + will be converted to + transformation": { + "select_id": 2, + "implicitly_added_member_#1": { + "join_optimization": { + "select_id": 3, + "steps": [] + } + */ + void validate_named_item_expectation(bool fix_wth_implicit_member= false); + + void add_implicit_member(); + + /* + This incremental counter is used to automatically assign names + for implicitly added members + */ + unsigned int implicit_member_num= 1; +#endif + public: String_with_limit output; }; diff --git a/unittest/sql/my_json_writer-t.cc b/unittest/sql/my_json_writer-t.cc index 6398a589c33..e05dc4836ea 100644 --- a/unittest/sql/my_json_writer-t.cc +++ b/unittest/sql/my_json_writer-t.cc @@ -65,8 +65,43 @@ int main(int args, char **argv) { Json_writer w; w.start_object(); + w.add_ll(123678456); + ok(w.invalid_json, "Unnamed LL value in an object"); + } + + { + Json_writer w; + w.start_object(); w.add_ull(123); - ok(w.invalid_json, "Unnamed value in an object"); + ok(w.invalid_json, "Unnamed ULL value in an object"); + } + + { + Json_writer w; + w.start_object(); + w.add_size(567890); + ok(w.invalid_json, "Unnamed size value in an object"); + } + + { + Json_writer w; + w.start_object(); + w.add_double(123.567867); + ok(w.invalid_json, "Unnamed double value in an object"); + } + + { + Json_writer w; + w.start_object(); + w.add_bool(true); + ok(w.invalid_json, "Unnamed bool value in an object"); + } + + { + Json_writer w; + w.start_object(); + w.add_null(); + ok(w.invalid_json, "Unnamed null value in an object"); } { @@ -79,15 +114,33 @@ int main(int args, char **argv) { Json_writer w; w.start_object(); + w.add_str("some string"); + ok(w.invalid_json, "Unnamed string (const char*) in an object"); + } + + { + Json_writer w; + w.start_object(); + CHARSET_INFO cs_info{}; + String str("another string", &cs_info); + w.add_str(str); + ok(w.invalid_json, "Unnamed string (class String) in an object"); + } + + { + Json_writer w; + w.start_object(); w.start_array(); - ok(w.invalid_json, "Unnamed array in an object"); + ok(!w.invalid_json, "Implicitly added member for the unnamed array" + " in an object"); } { Json_writer w; w.start_object(); w.start_object(); - ok(w.invalid_json, "Unnamed object in an object"); + ok(!w.invalid_json, "Implicitly added member for the unnamed object" + " in an object"); } { @@ -119,8 +172,6 @@ int main(int args, char **argv) ok(w.invalid_json, "JSON array end of object"); } - - diag("Done"); return exit_status(); |