maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Michael!

On Sep 02, Michael Widenius wrote:
> revision-id: b48dfdedd59 (mariadb-10.6.1-64-gb48dfdedd59)
> parent(s): 5f0b6b017a8
> author: Michael Widenius
> committer: Michael Widenius
> timestamp: 2021-08-24 11:22:29 +0300
> message:
> 
> MDEV-26307 multi-source-replication support mysql syntax(for channel)
> 
> diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy
> index 0c1c68cd644..704812e5dd8 100644
> --- a/sql/sql_yacc.yy
> +++ b/sql/sql_yacc.yy
> @@ -2307,6 +2309,34 @@ connection_name:
>           }
>           ;
>  
> +optional_for_channel:
> +        /* empty */
> +          {
> +            /*do nothing */
> +          }
> +        | for_channel
> +
> +        ;
> +
> +for_channel:
> +        FOR_SYM CHANNEL_SYM TEXT_STRING_sys
> +        {
> +          if (Lex->mi.connection_name.str != NULL)
> +          {
> +            my_yyabort_error((ER_WRONG_ARGUMENTS, MYF(0), "CONNECTION_NAME AND FOR CHANNEL CAN NOT BE SPECIFIED AT THE SAME TIME)"));

No, please, no LONG ENGLISH MESSAGES IN UPPERCASE WITH STRAY PARENTHESYS)
embedded as a part of the error message.

In your tests it looks like

ERROR HY000: Incorrect arguments to CONNECTION_NAME AND FOR CHANNEL CAN NOT BE SPECIFIED AT THE SAME TIME)

or, for example,

ERROR HY000: Felaktiga argument till CONNECTION_NAME AND FOR CHANNEL CAN NOT BE SPECIFIED AT THE SAME TIME)

The correct approach is to have the parser ensure the syntax is correct.
Meaning, it should be

    START_SYM SLAVE optional_connection_name slave_thread_opts
  | START_SYM SLAVE slave_thread_opts optional_for_channel

the incorrect but still kind of acceptable approach is to fake this and
check in the code, like you did, but use my_parse_error, not ER_WRONG_ARGUMENTS

> +          }
> +          else
> +          {
> +            Lex->mi.connection_name= $3;
> +#ifdef HAVE_REPLICATION
> +           if (unlikely(check_master_connection_name(&$3)))
> +              my_yyabort_error((ER_WRONG_ARGUMENTS, MYF(0), "MASTER_CONNECTION_NAME"));
> +#endif
> +          }
> +
> +          }
> +          ;
> +
>  /* create a table */
>  
>  create:
> @@ -8030,7 +8060,7 @@ opt_to:
>          ;
>  
>  slave:
> -          START_SYM SLAVE optional_connection_name slave_thread_opts
> +          START_SYM SLAVE optional_connection_name slave_thread_opts optional_for_channel
>            {
>              LEX *lex=Lex;
>              lex->sql_command = SQLCOM_SLAVE_START;

Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx