maria-developers team mailing list archive

[Sergey Petrunia <sergey@xxxxxxxxxxx>]

Hi Oleg,

> commit 53d4d2852463714678b6e753ecffc1ecdde13891
> Author: Oleg Smirnov <olernov@xxxxxxxxx>
> Date:   Thu Jan 13 19:24:22 2022 +0300
>     
>     MDEV-10000 Add support for FORMAT=JSON
>     
>     Makes possible calls to SHOW EXPLAIN and EXPLAIN FOR CONNECTION
>     providing output in JSON format:
>     - SHOW EXPLAIN FORMAT=JSON FOR $con
>     - EXPLAIN FORMAT=JSON FOR CONNECTION $con
> 

The patch is mostly ok. I have a few cosmetic comments.

Please follow the coding style and make sure the lines are no longer than 80
chars.

> diff --git a/sql/sql_show.h b/sql/sql_show.h
> index 3d7a4d1146c..8c8ae416699 100644
> --- a/sql/sql_show.h
> +++ b/sql/sql_show.h
> @@ -164,6 +164,9 @@ class Show_explain_request : public Apc_target::Apc_call
>    THD *target_thd;  /* thd that we're running SHOW EXPLAIN for */
>    THD *request_thd; /* thd that run SHOW EXPLAIN command */
>    
> +  bool is_json_format= false; /* set to TRUE if you need the result in JSON 
> +                              format, FALSE - in traditional tabular */
> +
Multi-line comments must precede the variable:

   /*
     Comment here...
   */
   bool is_json_format;

>    /* If true, there was some error when producing EXPLAIN output. */
>    bool failed_to_produce;
>     
> diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy
> index 74757970411..a7188762cbd 100644
> --- a/sql/sql_yacc.yy
> +++ b/sql/sql_yacc.yy
> @@ -13945,12 +13945,13 @@ show_param:
>              Lex->spname= $3;
>              Lex->sql_command = SQLCOM_SHOW_CREATE_EVENT;
>            }
> -        | describe_command FOR_SYM expr
> +        | describe_command opt_format_json FOR_SYM expr
>            {
>              Lex->sql_command= SQLCOM_SHOW_EXPLAIN;
> -            if (unlikely(prepare_schema_table(thd, Lex, 0, SCH_EXPLAIN)))
> +            if (unlikely(prepare_schema_table(thd, Lex, 0, 
> +			      Lex->explain_json ? SCH_EXPLAIN_JSON : SCH_EXPLAIN_TABULAR)))
>                MYSQL_YYABORT;
> -            add_value_to_list(thd, $3);
> +            add_value_to_list(thd, $4);
>            }
>          | IDENT_sys remember_tok_start wild_and_where
>             {
> @@ -14123,12 +14124,18 @@ opt_describe_column:
>          ;

Please fix identation of this rule to match of identation of others.

Please also add a comment about the alternative syntax, both here and in the
SHOW EXPLAIN rule.

>  explain_for_connection:
> -          describe_command FOR_SYM CONNECTION_SYM expr
> +          describe_command opt_format_json FOR_SYM CONNECTION_SYM expr
>            {
> +            Lex->wild=0;
> +            Lex->ident= null_clex_str;
> +            mysql_init_select(Lex);
> +            Lex->current_select->parsing_place= SELECT_LIST;
> +            Lex->create_info.init();
>              Lex->sql_command= SQLCOM_SHOW_EXPLAIN;
> -            if (unlikely(prepare_schema_table(thd, Lex, 0, SCH_EXPLAIN)))
> +            if (unlikely(prepare_schema_table(thd, Lex, 0, 
> +			    Lex->explain_json ? SCH_EXPLAIN_JSON : SCH_EXPLAIN_TABULAR)))
>                MYSQL_YYABORT;
> -            add_value_to_list(thd, $4);
> +            add_value_to_list(thd, $5);
>            }
>  		;
 
BR
 Sergei
-- 
Sergei Petrunia, Software Developer
MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net