maria-developers team mailing list archive
-
maria-developers team
-
Mailing list archive
-
Message #12073
Re: 6887f16b61a: MDEV-17399 Add support for JSON_TABLE.
Hi, Alexey!
There isn't much to review yet, I don't see how different parts will
work together yet. But here you are, some preliminary comments.
On Feb 06, Alexey Botchkov wrote:
> revision-id: 6887f16b61a (mariadb-10.5.0-153-g6887f16b61a)
> parent(s): 74f76206369
> author: Alexey Botchkov <holyfoot@xxxxxxxxxxx>
> committer: Alexey Botchkov <holyfoot@xxxxxxxxxxx>
> timestamp: 2020-02-04 17:12:55 +0400
> message:
>
> MDEV-17399 Add support for JSON_TABLE.
>
> Syntax for the JSON_TABLE added.
Standard JSON_TABLE syntax is quite complex.
You are doing a subset of it, I presume.
It would be good to specify here, in the comment, what syntax exactly
are you implementing.
> ---
> sql/lex.h | 5 ++
> sql/sql_digest.cc | 1 +
> sql/sql_lex.h | 3 +
> sql/sql_yacc.yy | 164 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> sql/table.cc | 1 +
> sql/table.h | 2 +
> sql/table_function.h | 89 ++++++++++++++++++++++++++++
> 7 files changed, 264 insertions(+), 1 deletion(-)
>
> diff --git a/sql/lex.h b/sql/lex.h
> index f3fc1513369..774f9c1517d 100644
> --- a/sql/lex.h
> +++ b/sql/lex.h
> @@ -208,6 +208,7 @@ static SYMBOL symbols[] = {
> { "ELSE", SYM(ELSE)},
> { "ELSEIF", SYM(ELSEIF_MARIADB_SYM)},
> { "ELSIF", SYM(ELSIF_MARIADB_SYM)},
> + { "EMPTY", SYM(EMPTY_SYM)},
> { "ENABLE", SYM(ENABLE_SYM)},
> { "ENCLOSED", SYM(ENCLOSED)},
> { "END", SYM(END)},
> @@ -414,6 +415,7 @@ static SYMBOL symbols[] = {
> { "NATIONAL", SYM(NATIONAL_SYM)},
> { "NATURAL", SYM(NATURAL)},
> { "NCHAR", SYM(NCHAR_SYM)},
> + { "NESTED", SYM(NESTED_SYM)},
> { "NEVER", SYM(NEVER_SYM)},
> { "NEW", SYM(NEW_SYM)},
> { "NEXT", SYM(NEXT_SYM)},
> @@ -448,6 +450,7 @@ static SYMBOL symbols[] = {
> { "OPTIONALLY", SYM(OPTIONALLY)},
> { "OR", SYM(OR_SYM)},
> { "ORDER", SYM(ORDER_SYM)},
> + { "ORDINALITY", SYM(ORDINALITY_SYM)},
> { "OTHERS", SYM(OTHERS_MARIADB_SYM)},
> { "OUT", SYM(OUT_SYM)},
> { "OUTER", SYM(OUTER)},
> @@ -460,6 +463,7 @@ static SYMBOL symbols[] = {
> { "PAGE_CHECKSUM", SYM(PAGE_CHECKSUM_SYM)},
> { "PARSER", SYM(PARSER_SYM)},
> { "PARSE_VCOL_EXPR", SYM(PARSE_VCOL_EXPR_SYM)},
> + { "PATH", SYM(PATH_SYM)},
> { "PERIOD", SYM(PERIOD_SYM)},
> { "PARTIAL", SYM(PARTIAL)},
> { "PARTITION", SYM(PARTITION_SYM)},
> @@ -742,6 +746,7 @@ static SYMBOL sql_functions[] = {
> { "FIRST_VALUE", SYM(FIRST_VALUE_SYM)},
> { "GROUP_CONCAT", SYM(GROUP_CONCAT_SYM)},
> { "JSON_ARRAYAGG", SYM(JSON_ARRAYAGG_SYM)},
> + { "JSON_TABLE", SYM(JSON_TABLE_SYM)},
> { "JSON_OBJECTAGG", SYM(JSON_OBJECTAGG_SYM)},
> { "LAG", SYM(LAG_SYM)},
> { "LEAD", SYM(LEAD_SYM)},
you've added them all as reserved words, is it intentonal?
> diff --git a/sql/sql_lex.h b/sql/sql_lex.h
> index 308bebd9fa1..591de1e1583 100644
> --- a/sql/sql_lex.h
> +++ b/sql/sql_lex.h
> @@ -33,6 +33,7 @@
> #include "sql_tvc.h"
> #include "item.h"
> #include "sql_limit.h" // Select_limit_counters
> +#include "table_function.h" // Json_table_column
>
> /* Used for flags of nesting constructs */
> #define SELECT_NESTING_MAP_SIZE 64
> @@ -3249,6 +3250,8 @@ struct LEX: public Query_tables_list
> SQL_I_List<ORDER> proc_list;
> SQL_I_List<TABLE_LIST> auxiliary_table_list, save_list;
> Column_definition *last_field;
> + Json_table_column *cur_json_table_column, *json_table_column_nest;
> + Table_function_json_table *json_table;
Did you have to put everything in LEX?
It's pretty big already and it's never fully used, its members allocate
memory all together at the same time, but used for different parts of
the grammar, different statements.
Could you try to pass that through the parser stack?
> Item_sum *in_sum_func;
> udf_func udf;
> HA_CHECK_OPT check_opt; // check/repair options
> diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy
> index c00ba42846a..5ef1bbd0721 100644
> --- a/sql/sql_yacc.yy
> +++ b/sql/sql_yacc.yy
> @@ -207,6 +208,8 @@ void _CONCAT_UNDERSCORED(turn_parser_debug_on,yyparse)()
> Lex_for_loop_st for_loop;
> Lex_for_loop_bounds_st for_loop_bounds;
> Lex_trim_st trim;
> + Json_table_column::On_response json_on_response;
> +
> vers_history_point_t vers_history_point;
> struct
> {
> @@ -225,6 +228,7 @@ void _CONCAT_UNDERSCORED(turn_parser_debug_on,yyparse)()
>
> /* pointers */
> Create_field *create_field;
> + Json_table_column *json_table_column;
> Spvar_definition *spvar_definition;
> Row_definition_list *spvar_definition_list;
> const Type_handler *type_handler;
> @@ -308,6 +312,7 @@ void _CONCAT_UNDERSCORED(turn_parser_debug_on,yyparse)()
> enum vers_kind_t vers_range_unit;
> enum Column_definition::enum_column_versioning vers_column_versioning;
> enum plsql_cursor_attr_t plsql_cursor_attr;
> + enum Json_table_column::enum_on_type json_on_type;
> }
>
> %{
> @@ -489,6 +494,7 @@ End SQL_MODE_ORACLE_SPECIFIC */
> %token <kwd> ELSEIF_MARIADB_SYM
> %token <kwd> ELSE /* SQL-2003-R */
> %token <kwd> ELSIF_ORACLE_SYM /* PLSQL-R */
> +%token <kwd> EMPTY_SYM /* SQL-2016-R */
> %token <kwd> ENCLOSED
> %token <kwd> ESCAPED
> %token <kwd> EXCEPT_SYM /* SQL-2003-R */
> @@ -506,6 +512,7 @@ End SQL_MODE_ORACLE_SPECIFIC */
> %token <kwd> GRANT /* SQL-2003-R */
> %token <kwd> GROUP_CONCAT_SYM
> %token <rwd> JSON_ARRAYAGG_SYM
> +%token <rwd> JSON_TABLE_SYM
> %token <rwd> JSON_OBJECTAGG_SYM
> %token <kwd> GROUP_SYM /* SQL-2003-R */
> %token <kwd> HAVING /* SQL-2003-R */
> @@ -564,6 +571,7 @@ End SQL_MODE_ORACLE_SPECIFIC */
> %token <kwd> MOD_SYM /* SQL-2003-N */
> %token <kwd> NATURAL /* SQL-2003-R */
> %token <kwd> NEG
> +%token <kwd> NESTED_SYM /* SQL-2003-N */
No, NESTED is not in the SQL:2003 list of non-reserved words.
Please specify a correct comment tag for every keyword you're adding here.
> %token <kwd> NOT_SYM /* SQL-2003-R */
> %token <kwd> NO_WRITE_TO_BINLOG
> %token <kwd> NOW_SYM
> @@ -575,6 +583,7 @@ End SQL_MODE_ORACLE_SPECIFIC */
> %token <kwd> OPTIMIZE
> %token <kwd> OPTIONALLY
> %token <kwd> ORDER_SYM /* SQL-2003-R */
> +%token <kwd> ORDINALITY_SYM /* SQL-2003-N */
> %token <kwd> OR_SYM /* SQL-2003-R */
> %token <kwd> OTHERS_ORACLE_SYM /* SQL-2011-N, PLSQL-R */
> %token <kwd> OUTER
> @@ -585,6 +594,7 @@ End SQL_MODE_ORACLE_SPECIFIC */
> %token <kwd> PAGE_CHECKSUM_SYM
> %token <kwd> PARSE_VCOL_EXPR_SYM
> %token <kwd> PARTITION_SYM /* SQL-2003-R */
> +%token <kwd> PATH_SYM /* SQL-2003-N */
> %token <kwd> PERCENTILE_CONT_SYM
> %token <kwd> PERCENTILE_DISC_SYM
> %token <kwd> PERCENT_RANK_SYM
> @@ -1336,12 +1346,16 @@ End SQL_MODE_ORACLE_SPECIFIC */
>
> %type <type_handler> int_type real_type
>
> +%type <json_on_type> json_empty_or_error
> +%type <json_on_response> json_on_response
> +
> %type <Lex_field_type> type_with_opt_collate field_type
> field_type_numeric
> field_type_string
> field_type_lob
> field_type_temporal
> field_type_misc
> + json_table_field_type
>
> %type <Lex_dyncol_type> opt_dyncol_type dyncol_type
> numeric_dyncol_type temporal_dyncol_type string_dyncol_type
> @@ -1487,7 +1501,7 @@ End SQL_MODE_ORACLE_SPECIFIC */
> table_primary_derived table_primary_derived_opt_parens
> derived_table_list table_reference_list_parens
> nested_table_reference_list join_table_parens
> - update_table_list
> + update_table_list table_function
> %type <date_time_type> date_time_type;
> %type <interval> interval
>
> @@ -1642,6 +1656,9 @@ End SQL_MODE_ORACLE_SPECIFIC */
> opt_delete_gtid_domain
> asrow_attribute
> opt_constraint_no_id
> + json_table_columns_clause json_table_columns_list json_table_column
> + json_table_column_type json_opt_on_empty_or_error
> +
>
> %type <NONE> call sp_proc_stmts sp_proc_stmts1 sp_proc_stmt
> %type <NONE> sp_if_then_statements sp_case_then_statements
> @@ -11320,6 +11337,150 @@ join_table_list:
> derived_table_list { MYSQL_YYABORT_UNLESS($$=$1); }
> ;
>
> +json_table_columns_clause:
> + COLUMNS '(' json_table_columns_list ')'
> + {}
> + ;
> +
> +json_table_columns_list:
> + json_table_column
> + | json_table_columns_list ',' json_table_column
> + {}
> + ;
> +
> +json_table_column:
> + ident
> + {
> + LEX *lex=Lex;
> + Create_field *f= new (thd->mem_root) Create_field();
> +
> + if (unlikely(check_string_char_length(&$1, 0, NAME_CHAR_LEN,
> + system_charset_info, 1)))
> + my_yyabort_error((ER_TOO_LONG_IDENT, MYF(0), $1.str));
> +
> + lex->cur_json_table_column=
> + new (thd->mem_root) Json_table_column(f, lex->json_table_column_nest);
> +
> + if (unlikely(!f || !lex->cur_json_table_column))
> + MYSQL_YYABORT;
> +
> + lex->init_last_field(f, &$1, NULL);
> + }
> + json_table_column_type
> + {
> + LEX *lex=Lex;
> + lex->json_table->m_columns.push_back(lex->cur_json_table_column,
> + thd->mem_root);
> + }
> + ;
> +
> +json_table_column_type:
> + FOR_SYM ORDINALITY_SYM
> + {
> + Lex->cur_json_table_column->set(Json_table_column::FOR_ORDINALITY);
> + }
> + | json_table_field_type PATH_SYM TEXT_STRING_sys
> + json_opt_on_empty_or_error
> + {
> + Lex->last_field->set_attributes(thd, $1, Lex->charset,
> + COLUMN_DEFINITION_TABLE_FIELD);
> + Lex->cur_json_table_column->set(Json_table_column::PATH, &$3);
> + }
> + | json_table_field_type EXISTS PATH_SYM TEXT_STRING_sys
> + {
> + Lex->last_field->set_attributes(thd, $1, Lex->charset,
> + COLUMN_DEFINITION_TABLE_FIELD);
> + Lex->cur_json_table_column->set(Json_table_column::EXISTS_PATH, &$4);
> + }
> + | NESTED_SYM PATH_SYM TEXT_STRING_sys
> + {
> + LEX *lex=Lex;
> + lex->cur_json_table_column->set(Json_table_column::NESTED_PATH);
> + lex->json_table_column_nest= lex->cur_json_table_column;
> + }
> + json_table_columns_clause
> + {
> + LEX *lex=Lex;
> + lex->cur_json_table_column= lex->json_table_column_nest;
> + lex->json_table_column_nest= lex->cur_json_table_column->m_nest;
> + }
> + ;
> +
> +json_table_field_type:
> + field_type_numeric
> + | field_type_temporal
> + | field_type_string
> + ;
> +
> +json_opt_on_empty_or_error:
> + /* none */
> + {}
> + | json_on_response ON json_empty_or_error json_opt_on_empty_or_error
> + {
> + if ($3 == Json_table_column::ON_EMPTY)
> + {
> + Lex->cur_json_table_column->m_on_empty= $1;
> + }
> + else /* ON_ERROR */
> + {
> + Lex->cur_json_table_column->m_on_error= $1;
> + }
> + }
> + ;
> +
> +json_on_response:
> + ERROR_SYM
> + {
> + $$.m_response= Json_table_column::RESPONSE_ERROR;
> + }
> + | NULL_SYM
> + {
> + $$.m_response= Json_table_column::RESPONSE_NULL;
> + }
> + | DEFAULT TEXT_STRING_sys
> + {
> + $$.m_response= Json_table_column::RESPONSE_DEFAULT;
> + $$.m_default= $1;
> + }
> + ;
> +
> +json_empty_or_error:
> + ERROR_SYM
> + {
> + $$= Json_table_column::ON_EMPTY;
> + }
> + | EMPTY_SYM
> + {
> + $$= Json_table_column::ON_ERROR;
> + }
> + ;
> +
> +table_function:
> + JSON_TABLE_SYM '(' expr ',' TEXT_STRING_sys
difficult to see what you're parsing without a syntax specs in the
commit comment.
> + {
> + Table_function_json_table *jt=
> + new (thd->mem_root) Table_function_json_table($3, &$5);
> + if (unlikely(!jt))
> + MYSQL_YYABORT;
> + Lex->json_table= jt;
> + Lex->json_table_column_nest= NULL;
> + }
> + json_table_columns_clause ')' AS ident_table_alias
> + {
> +LEX_CSTRING tn;
> + SELECT_LEX *sel= Select;
> + sel->table_join_options= 0;
> +tn.str= "t1";
> +tn.length= 2;
what's that? just to make the code compile for now?
why not to use $10 - this is your ident_table_alias, currently it's
ignored.
> + if (!($$= Select->add_table_to_list(thd,
> + new (thd->mem_root) Table_ident(&tn), &tn,
> + Select->get_table_join_options(),
> + YYPS->m_lock_type,
> + YYPS->m_mdl_type)))
> + MYSQL_YYABORT;
> + }
> + ;
> +
> /*
> The ODBC escape syntax for Outer Join is: '{' OJ join_table '}'
> The parser does not define OJ as a token, any ident is accepted
> @@ -11527,6 +11688,7 @@ table_factor:
> $$= $1;
> }
> | table_reference_list_parens { $$= $1; }
> + | table_function { $$= $1; }
> ;
>
> table_primary_ident_opt_parens:
Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx
