maria-developers team mailing list archive

[Alexander Barkov <bar@xxxxxxxxxxx>]

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;
           }