Hello Sanja,
On 04/10/2018 03:37 PM, Oleksandr Byelkin wrote: > Am 10.04.2018 um 10:58 schrieb Alexander Barkov: >> Hello Sanja, >> >> >> I reviewed your recent changes in "10.3-MDEV-11953" >> (and the attached additional patch for sql_yacc_ora.yy) >> >> I have some proposals: >> >> >> >> 1. Can you please move huge pieces of the code from sql_yacc.yy to LEX >> or other relevant classes? >> >> It makes the grammar much more readable (patches are aslo much more >> readable). >> >> I'd move the relevant pieces of the code to LEX as a separate patch, >> even before fixing the grammar. > OK >> >> 2. You're adding too many main_select_push() and pop_select(). >> Please move them to upper level rules (it should be possible in many >> cases). > Impossible It's possible. I easily removed 15 push/pop pairs. But it does not look nice either. It's easier to read when all "create" alternatives reside in the same "create" rule. Ok. I won't insist on that. Just adding "expr" with push/pop should be good enough. >> >> Add new helper rules when needed. >> For example, this piece of code repeats many times: >> >> + { >> + if (Lex->main_select_push()) >> + MYSQL_YYABORT; >> + } >> + expr >> + { >> + Lex->pop_select(); //main select >> + $$= $3; >> >> It deserved a rule, say, expr_with_select_push_pop. >> You can find a better name :) >> > OK >> - Serg and I spent a lot of time working on this task: >> MDEV-8909 union parser cleanup >> (and its 13 dependency tasks, and 3 related tasks, >> see the "Issue links" section in MDEV). >> >> We think that it should be the parser who disallows bad grammar, instead >> of post-analysis with raising errors like >> ER_CANT_USE_OPTION_HERE. >> Please keep using the same approach. > The task did not made parser recognizing brackets, and I have no ideas > how to return parser errors when all SELECT parsed in the same way in > difference from the previous parser which could recognize only one level > of SELECTs. Can you give an example of a query that is not parsed by the current 10.3 parser, but is parsed in 10.3-MDEV-11953 ? Thanks. >> Thanks! >> > [skip]
diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index d4122fe..e75a1a6 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -1749,6 +1749,7 @@ bool my_yyoverflow(short **a, YYSTYPE **b, size_t *yystacksize); %type <object_ddl_options> create_or_replace + create_or_replace_select_push opt_if_not_exists opt_if_exists @@ -1932,7 +1933,7 @@ bool my_yyoverflow(short **a, YYSTYPE **b, size_t *yystacksize); %type <NONE> analyze_stmt_command - query verb_clause create change select do drop insert replace insert2 + query verb_clause create create2 create3 change select do drop insert replace insert2 insert_values update delete truncate rename compound_statement show describe load alter optimize keycache preload flush reset purge begin commit rollback savepoint release @@ -2567,12 +2568,24 @@ connection_name: /* create a table */ create: - create_or_replace opt_temporary TABLE_SYM opt_if_not_exists + create2 { Lex->pop_select(); } + | create3 + ; + +create_or_replace_select_push: + create_or_replace + { + $$= $1; + if (Lex->main_select_push()) + MYSQL_YYABORT; + } + ; + +create2: + create_or_replace_select_push opt_temporary TABLE_SYM opt_if_not_exists { LEX *lex= thd->lex; lex->create_info.init(); - if (lex->main_select_push()) - MYSQL_YYABORT; lex->current_select->parsing_place= BEFORE_OPT_LIST; if (lex->set_command_with_check(SQLCOM_CREATE_TABLE, $2, $1 | $4)) MYSQL_YYABORT; @@ -2609,13 +2622,10 @@ create: $6->table.str); } create_table_set_open_action_and_adjust_tables(lex); - Lex->pop_select(); //main select } - | create_or_replace opt_temporary SEQUENCE_SYM opt_if_not_exists table_ident + | create_or_replace_select_push opt_temporary SEQUENCE_SYM opt_if_not_exists table_ident { LEX *lex= thd->lex; - if (Lex->main_select_push()) - MYSQL_YYABORT; lex->create_info.init(); if (lex->set_command_with_check(SQLCOM_CREATE_SEQUENCE, $2, $1 | $4)) MYSQL_YYABORT; @@ -2670,12 +2680,9 @@ create: $5->table.str); } create_table_set_open_action_and_adjust_tables(lex); - Lex->pop_select(); //main select } - | create_or_replace opt_unique INDEX_SYM opt_if_not_exists + | create_or_replace_select_push opt_unique INDEX_SYM opt_if_not_exists { - if (Lex->main_select_push()) - MYSQL_YYABORT; } ident opt_key_algorithm_clause @@ -2689,12 +2696,9 @@ create: '(' key_list ')' opt_lock_wait_timeout normal_key_options opt_index_lock_algorithm { - Lex->pop_select(); //main select } - | create_or_replace fulltext INDEX_SYM + | create_or_replace_select_push fulltext INDEX_SYM { - if (Lex->main_select_push()) - MYSQL_YYABORT; } opt_if_not_exists ident ON table_ident @@ -2707,12 +2711,9 @@ create: '(' key_list ')' opt_lock_wait_timeout fulltext_key_options opt_index_lock_algorithm { - Lex->pop_select(); //main select } - | create_or_replace spatial INDEX_SYM + | create_or_replace_select_push spatial INDEX_SYM { - if (Lex->main_select_push()) - MYSQL_YYABORT; } opt_if_not_exists ident ON table_ident @@ -2725,14 +2726,11 @@ create: '(' key_list ')' opt_lock_wait_timeout spatial_key_options opt_index_lock_algorithm { - Lex->pop_select(); //main select } - | create_or_replace DATABASE opt_if_not_exists ident + | create_or_replace_select_push DATABASE opt_if_not_exists ident { Lex->create_info.default_table_charset= NULL; Lex->create_info.used_fields= 0; - if (Lex->main_select_push()) - MYSQL_YYABORT; } opt_create_database_options { @@ -2740,105 +2738,80 @@ create: if (lex->set_command_with_check(SQLCOM_CREATE_DB, 0, $1 | $3)) MYSQL_YYABORT; lex->name= $4; - Lex->pop_select(); //main select } - | create_or_replace definer_opt opt_view_suid VIEW_SYM + | create_or_replace_select_push definer_opt opt_view_suid VIEW_SYM opt_if_not_exists table_ident { - if (Lex->main_select_push()) - MYSQL_YYABORT; if (Lex->add_create_view(thd, $1 | $5, DTYPE_ALGORITHM_UNDEFINED, $3, $6)) MYSQL_YYABORT; } view_list_opt AS view_select { - Lex->pop_select(); //main select } - | create_or_replace view_algorithm definer_opt opt_view_suid VIEW_SYM + | create_or_replace_select_push view_algorithm definer_opt opt_view_suid VIEW_SYM opt_if_not_exists table_ident { if (Lex->add_create_view(thd, $1 | $6, $2, $4, $7)) MYSQL_YYABORT; - if (Lex->main_select_push()) - MYSQL_YYABORT; } view_list_opt AS view_select { - Lex->pop_select(); //main select } - | create_or_replace definer_opt TRIGGER_SYM + | create_or_replace_select_push definer_opt TRIGGER_SYM { - if (Lex->main_select_push()) - MYSQL_YYABORT; Lex->create_info.set($1); } trigger_tail { - Lex->pop_select(); //main select } - | create_or_replace definer_opt PROCEDURE_SYM + | create_or_replace_select_push definer_opt PROCEDURE_SYM { - if (Lex->main_select_push()) - MYSQL_YYABORT; Lex->create_info.set($1); } sp_tail { - Lex->pop_select(); //main select } - | create_or_replace definer_opt EVENT_SYM + | create_or_replace_select_push definer_opt EVENT_SYM { - if (Lex->main_select_push()) - MYSQL_YYABORT; Lex->create_info.set($1); } event_tail { - Lex->pop_select(); //main select } - | create_or_replace definer FUNCTION_SYM + | create_or_replace_select_push definer FUNCTION_SYM { - if (Lex->main_select_push()) - MYSQL_YYABORT; Lex->create_info.set($1); } sf_tail { - Lex->pop_select(); //main select } - | create_or_replace definer AGGREGATE_SYM FUNCTION_SYM + | create_or_replace_select_push definer AGGREGATE_SYM FUNCTION_SYM { - if (Lex->main_select_push()) - MYSQL_YYABORT; Lex->create_info.set($1); } sf_tail_aggregate { - Lex->pop_select(); //main select } - | create_or_replace no_definer FUNCTION_SYM + | create_or_replace_select_push no_definer FUNCTION_SYM { - if (Lex->main_select_push()) - MYSQL_YYABORT; Lex->create_info.set($1); } create_function_tail { - Lex->pop_select(); //main select } - | create_or_replace no_definer AGGREGATE_SYM FUNCTION_SYM + | create_or_replace_select_push no_definer AGGREGATE_SYM FUNCTION_SYM { - if (Lex->main_select_push()) - MYSQL_YYABORT; Lex->create_info.set($1); } create_aggregate_function_tail { - Lex->pop_select(); //main select } - | create_or_replace USER_SYM opt_if_not_exists clear_privileges grant_list + ; + +create3: + create_or_replace USER_SYM opt_if_not_exists clear_privileges grant_list opt_require_clause opt_resource_options { if (Lex->set_command_with_check(SQLCOM_CREATE_USER, $1 | $3)) @@ -2850,7 +2823,7 @@ create: if (Lex->set_command_with_check(SQLCOM_CREATE_ROLE, $1 | $3)) MYSQL_YYABORT; } - | CREATE LOGFILE_SYM GROUP_SYM logfile_group_info + | CREATE LOGFILE_SYM GROUP_SYM logfile_group_info { Lex->alter_tablespace_info->ts_cmd_type= CREATE_LOGFILE_GROUP; }
_______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp