maria-developers team mailing list archive

[Aleksey Midenkov <midenok@xxxxxxxxx>]

Sergei,

On Wed, Sep 1, 2021 at 8:21 PM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
>
> Hi, Aleksey!
>
> On Sep 01, Aleksey Midenkov wrote:
> > revision-id: dc2ace70f1b (mariadb-10.6.1-68-gdc2ace70f1b)
> > parent(s): 19fbfab084f
> > author: Aleksey Midenkov
> > committer: Aleksey Midenkov
> > timestamp: 2021-08-30 22:37:08 +0300
> > message:
> >
> > Syntax with MIGRATE keyword
> >
> > ALTER TABLE tbl_name
> >     [alter_option [, alter_option] ...] |
> >     [partition_options]
> >
> > partition_option: {
> >     ...
> >     | MIGRATE [OUT] PARTITION partition_name TO [TABLE] tbl_name
> > }
> >
> > Examples:
> >
> >     ALTER TABLE t1 MIGRATE PARTITION p1 TO tp1;
> >     ALTER TABLE t1 MIGRATE PARTITION p2 TO TABLE tp2;
> >     ALTER TABLE t1 MIGRATE OUT PARTITION p3 TO TABLE tp3;
> >
> > diff --git a/mysql-test/suite/parts/r/alter_table.result b/mysql-test/suite/parts/r/alter_table.result
> > index bc1bf661d4c..7a2d45c51c1 100644
> > --- a/mysql-test/suite/parts/r/alter_table.result
> > +++ b/mysql-test/suite/parts/r/alter_table.result
> > @@ -258,9 +258,46 @@ show grants for current_user;
> >  Grants for alan@%
> >  GRANT USAGE ON *.* TO `alan`@`%`
> >  GRANT INSERT, CREATE, DROP, ALTER ON `test`.* TO `alan`@`%`
> > -alter table t1 extract partition p1 as table tp1;
> > +alter table t1 migrate partition p1 to table tp1;
> >  disconnect alan;
> >  connection default;
> >  drop database EXISTENT;
> >  drop user alan;
> >  drop tables t1, tp1, tp2, tp4;
> > +# Cunning syntax
>
> what's so cunning here?
>
> > +create or replace table t1 (x int)
> > +partition by range(x) (
> > diff --git a/sql/handler.h b/sql/handler.h
> > index c455625aafe..7ac9e929ecc 100644
> > --- a/sql/handler.h
> > +++ b/sql/handler.h
> > @@ -824,7 +824,7 @@ typedef bool Log_func(THD*, TABLE*, bool, const uchar*, const uchar*);
> >  #define ALTER_PARTITION_TRUNCATE    (1ULL << 11)
> >  // Set for REORGANIZE PARTITION
> >  #define ALTER_PARTITION_TABLE_REORG           (1ULL << 12)
> > -#define ALTER_PARTITION_EXTRACT    (1ULL << 13)
> > +#define ALTER_PARTITION_MIGRATE_OUT    (1ULL << 13)
> >
> >  /*
> >    This is master database for most of system tables. However there
> > diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy
> > index 49dd244f2ff..e1be6ea9557 100644
> > --- a/sql/sql_yacc.yy
> > +++ b/sql/sql_yacc.yy
> > @@ -7492,6 +7492,16 @@ ident_or_empty:
> >          | ident
> >          ;
> >
> > +move_out_partition:
> > +        MIGRATE_SYM PARTITION_SYM
> > +        | MIGRATE_SYM OUT_SYM PARTITION_SYM
>
> Where does this OUT_SYM came out from?
> It doesn't add any values, it doesn't make the syntax more natural.
> Why did you add it?

I think that helps to make syntax more natural if we do MIGRATE
PARTITION in both directions.
If it is written MIGRATE OUT or MIGRATE IN that is easier to
understand what's going on. FROM/TO in the end is not helping much
because it is harder to notice. Partition specification presence or
absence: not so explicit to understand quickly.

>
> > +        ;
> > +
> > +to_table:
> > +        TO_SYM TABLE_SYM
> > +        | TO_SYM
>
> Let's keep the TABLE_SYM. Saying
>
>    MIGRATE PARTITION p1 TO p2
>
> is very vague, the syntax should explicitly say that it migrates a
> PARTITION to a TABLE. The statement mentions many different types of
> objects, if should be clear about what object type every identifier
> refers to.

That is a shortcut for easier typing and that is precious for
command-line work! If some names can make misunderstanding one can
always write

MIGRATE PARTITION p1 TO TABLE p2

Please do not restrict user freedom to choose between verbosity and
brevity. I guess you have a preference for perl over python (and the
democracy over dictatorship).

>
> > +        ;
> > +
> >  alter_commands:
> >            /* empty */
> >          | DISCARD TABLESPACE
> > @@ -7619,15 +7629,15 @@ alter_commands:
> >                MYSQL_YYABORT;
> >              Lex->alter_info.partition_flags|= ALTER_PARTITION_EXCHANGE;
> >            }
> > -        | EXTRACT_SYM PARTITION_SYM alt_part_name_item
> > -          AS TABLE_SYM table_ident have_partitioning
> > +        | move_out_partition alt_part_name_item
> > +          to_table table_ident have_partitioning
>
> please, let it be just
>
>              MIGRATE_SYM PARTITION_SYM alt_part_name_item
>              TO_SYM TABLE_SYM table_ident have_partitioning
>
> >            {
> > -            if (Lex->stmt_alter_table($6))
> > +            if (Lex->stmt_alter_table($4))
> >                MYSQL_YYABORT;
> >              Lex->m_sql_cmd= new (thd->mem_root) Sql_cmd_alter_table();
> >              if (unlikely(Lex->m_sql_cmd == NULL))
> >                MYSQL_YYABORT;
> > -            Lex->alter_info.partition_flags|= ALTER_PARTITION_EXTRACT;
> > +            Lex->alter_info.partition_flags|= ALTER_PARTITION_MIGRATE_OUT;
> >            }
> >          ;
>
> Regards,
> Sergei
> VP of MariaDB Server Engineering
> and security@xxxxxxxxxxx



-- 
All the best,

Aleksey Midenkov
@midenok