[MDEV-23766] Optimizer tracing code is prone to producing invalid JSON Created: 2020-09-19  Updated: 2021-11-19  Resolved: 2021-11-09

Status: Closed
Project: MariaDB Server
Component/s: Optimizer
Affects Version/s: 10.4, 10.5
Fix Version/s: 10.4.23, 10.5.14, 10.6.6, 10.7.2

Type: Bug Priority: Major
Reporter: Sergei Petrunia Assignee: Sergei Krivonos (Inactive)
Resolution: Fixed Votes: 0
Labels: optimizer_trace

Issue Links:
Blocks
is blocked by MDEV-26929 Make the main testsuite runnable with... Closed
Relates
relates to MDEV-27036 Json_writer allows one to create dupl... Closed

 Description   

Consider this example patch

diff --git a/sql/sql_select.cc b/sql/sql_select.cc
 
index f0d42c32dd0..9fbae781274 100644
 
--- a/sql/sql_select.cc
 
+++ b/sql/sql_select.cc
 
@@ -1797,6 +1797,15 @@ JOIN::optimize_inner()
 
   trace_prepare.add_select_number(select_lex->select_number);
 
   Json_writer_array trace_steps(thd, "steps");
 
 
+  {
 
+    Json_writer_object trace_wrapper(thd);
 
+    Json_writer_object trace_conv(thd, "new_rewrite");
 
+    trace_conv.add("member", "member-value");
 
+    {
 
+      Json_writer_object obj(thd);
 
+      obj.add("rewrite_done", true);
 
+    }
 
+  }
 
   /*
 
     Needed in case optimizer short-cuts,
 
     set properly in make_aggr_tables_info()

It will silently produce invalid trace:

create table t1(a int);
 
insert into t1 values (1),(2);
 
set optimizer_trace=1; 
explain select * from t1;
 
select * from information_schema.optimizer_trace\G

will show this:

...
 
      "join_optimization": {
 
        "select_id": 1,
 
        "steps": [
 
          {
 
            "new_rewrite": {
 
              "member": "member-value",
 
              {
 
                "rewrite_done": true
 
              }
 
            }
 
          },

new_rewrite is an object, while it has an un-named object inside it.

How to solve this

In ideal world, one would want that JSON writer API doesn't allow to construct call sequences that produce invalid JSON. However this will require passing around a "write context" object which would be either "array member context" (which one can use to add unnamed members) or "object member context" (which one can use to add named members).

Passing the context around is inconvenient. We want to be able to write things like

    Json_writer_object trace_conv(thd);

at arbitrary place in the code.

Perhaps what we need is:

  1. A convention that by default the Optimizer trace writer is left in an unnamed context.
  2. Assert statements that will fire if one attempts to produce invalid JSON.

This MDEV is about adding the latter.

 Comments   
Comment by Sergei Krivonos (Inactive) [ 2021-10-18 ]

Two commits ready:
https://github.com/MariaDB/server/commit/cf8e78a40170118e2595e35c9c5f43aedeca91a0
https://github.com/MariaDB/server/commit/052dda61bb8d5fef271ecd091a5b5db25d57040b

Comment by Sergei Krivonos (Inactive) [ 2021-10-28 ]

MDEV-23766: Fix best_access_path by assert

test show_explain-innodb:

assert.c:0(.annobin_assert.c_end)
sql/my_json_writer.cc:62(Json_writer::start_object())
sql/my_json_writer.h:379(Json_writer_object::Json_writer_object(THD*, char const*))
sql/sql_select.cc:7463(best_access_path(JOIN*, st_join_table*, unsigned long long, POSITION const*, unsigned int, bool, double, POSITION*, POSITION*))
sql/opt_subselect.cc:3835(fix_semijoin_strategies_for_picked_join_order(JOIN*))
sql/sql_select.cc:10482(JOIN::get_best_combination())
sql/sql_select.cc:2343(JOIN::optimize_stage2())
sql/sql_select.cc:2322(JOIN::optimize_inner())
sql/sql_select.cc:1668(JOIN::optimize())
sql/sql_lex.cc:4873(st_select_lex::optimize_unflattened_subqueries(bool))
sql/opt_subselect.cc:5556(JOIN::optimize_unflattened_subqueries())
sql/sql_select.cc:2871(JOIN::optimize_stage2())
sql/sql_select.cc:2322(JOIN::optimize_inner())
sql/sql_select.cc:1668(JOIN::optimize

Comment by Sergei Krivonos (Inactive) [ 2021-10-28 ]

f7ce3a918e496a136eab036ac0aad64fea820adf

Comment by Sergei Krivonos (Inactive) [ 2021-10-28 ]

psergei,

this is how formatter use start_array and add_member:

#0 Json_writer::start_array (this=0x5555589b5d60) at /home/name/server/sql/my_json_writer.cc:81
#1 0x00005555560a7d80 in Single_line_formatting_helper::disable_and_flush (this=0x5555589b5d98) at /home/name/server/sql/my_json_writer.cc:456
#2 0x00005555560a7a82 in Single_line_formatting_helper::on_start_object (this=0x5555589b5d98) at /home/name/server/sql/my_json_writer.cc:366
#3 0x00005555560a6ec1 in Json_writer::start_object (this=0x5555589b5d60) at /home/name/server/sql/my_json_writer.cc:63
#4 0x0000555555d97d9f in Json_writer_object::Json_writer_object (this=0x7ffff0c456b0, thd=0x7fffd4000db8) at /home/name/server/sql/my_json_writer.h:366
#5 0x0000555555ed017b in JOIN::prepare

Comment by Sergei Krivonos (Inactive) [ 2021-10-28 ]

psergei,
Ready https://github.com/MariaDB/server/commit/8fb54fa006367e3a9cbe32dc1734c980cc5d5096

Comment by Sergei Krivonos (Inactive) [ 2021-11-02 ]

psergei,
Ready
https://github.com/MariaDB/server/compare/10.5...bb-10.5-MDEV-23766

Comment by Sergei Krivonos (Inactive) [ 2021-11-02 ]

psergei,
current 10.5 is unstable (test failure)
so I rebased 23766 on 10.4 https://github.com/MariaDB/server/compare/10.4...bb-10.4-MDEV-23766

Comment by Sergei Petrunia [ 2021-11-05 ]

Review input provided over email. Getting close.

Comment by Sergei Petrunia [ 2021-11-09 ]

Ok to push

Generated at Thu Feb 08 09:24:53 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.