Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-23766

Optimizer tracing code is prone to producing invalid JSON

Details

    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.

      Attachments

        Issue Links

          Activity

            sergei.krivonos Sergei Krivonos (Inactive) added a comment - psergei , Ready https://github.com/MariaDB/server/commit/8fb54fa006367e3a9cbe32dc1734c980cc5d5096
            sergei.krivonos Sergei Krivonos (Inactive) added a comment - psergei , Ready https://github.com/MariaDB/server/compare/10.5...bb-10.5-MDEV-23766

            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

            sergei.krivonos Sergei Krivonos (Inactive) added a comment - 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

            Review input provided over email. Getting close.

            psergei Sergei Petrunia added a comment - Review input provided over email. Getting close.

            Ok to push

            psergei Sergei Petrunia added a comment - Ok to push

            People

              sergei.krivonos Sergei Krivonos (Inactive)
              psergei Sergei Petrunia
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.