maria-developers team mailing list archive

[Aleksey Midenkov <midenok@xxxxxxxxx>]

Hi, Sergei!

On Tue, Apr 7, 2020 at 3:03 PM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
>
> Hi, Aleksey!
>
> On Apr 07, Aleksey Midenkov wrote:
> > revision-id: 9941c6a3179 (mariadb-10.5.2-164-g9941c6a3179)
> > parent(s): 920c3c6b237
> > author: Aleksey Midenkov <midenok@xxxxxxxxx>
> > committer: Aleksey Midenkov <midenok@xxxxxxxxx>
> > timestamp: 2020-04-06 08:05:43 +0300
> > message:
> >
> > MDEV-17554 Auto-create new partition for system versioned tables with history partitioned by INTERVAL/LIMIT
>
> See some comment below, please
>
> > diff --git a/mysql-test/suite/versioning/common.inc b/mysql-test/suite/versioning/common.inc
> > index 355b571e5a0..b35a5138015 100644
> > --- a/mysql-test/suite/versioning/common.inc
> > +++ b/mysql-test/suite/versioning/common.inc
> > @@ -6,6 +6,7 @@ if (!$TEST_VERSIONING_SO)
> >  source include/have_innodb.inc;
> >
> >  set @@session.time_zone='+00:00';
> > +set @@global.time_zone='+00:00';
>
> Why is that? I understand you might've needed it when a partition was added
> in a separate thread, but why now?

There is concurrency case in partition.test it is needed for other connections.

>
> >  select ifnull(max(transaction_id), 0) into @start_trx_id from mysql.transaction_registry;
> >  set @test_start=now(6);
> >
> > diff --git a/mysql-test/suite/versioning/r/delete_history.result b/mysql-test/suite/versioning/r/delete_history.result
> > index cb865a835b3..2e4a2bf9974 100644
> > --- a/mysql-test/suite/versioning/r/delete_history.result
> > +++ b/mysql-test/suite/versioning/r/delete_history.result
> > @@ -154,3 +154,18 @@ select * from t1;
> >  a
> >  1
> >  drop table t1;
> > +#
> > +# MDEV-17554 Auto-create new partition for system versioned tables with history partitioned by INTERVAL/LIMIT
> > +#
> > +# Don't auto-create new partition on DELETE HISTORY:
> > +create or replace table t (a int) with system versioning
> > +partition by system_time limit 1000 auto_increment;
>
> this looks like a hack, I think we need a dedicated syntax for that.
> but I couldn't think of anything good now.
>
> ok, I see that you allow both AUTO_INCREMENT and AUTO.
> May be better just to use AUTO?

AUTO_INCREMENT looks more explanatory. Isn't it?

>
> > +delete history from t;
> > +show create table t;
> > +Table        Create Table
> > +t    CREATE TABLE `t` (
> > +  `a` int(11) DEFAULT NULL
> > +) ENGINE=DEFAULT_ENGINE DEFAULT CHARSET=latin1 WITH SYSTEM VERSIONING
> > + PARTITION BY SYSTEM_TIME LIMIT 1000 AUTO_INCREMENT
> > +PARTITIONS 2
> > +drop table t;
> > diff --git a/mysql-test/suite/versioning/r/partition.result b/mysql-test/suite/versioning/r/partition.result
> > index a7047cbd11b..660d2c81961 100644
> > --- a/mysql-test/suite/versioning/r/partition.result
> > +++ b/mysql-test/suite/versioning/r/partition.result
> > @@ -289,11 +291,27 @@ x
> >  6
> >  7
> >  8
> > -## rotation by INTERVAL
> > +# Auto-create history partitions
> > +create or replace table t1 (x int) with system versioning
> > +partition by system_time limit 1000 auto_increment;
> > +lock tables t1 write;
> > +insert into t1 values (1);
> > +update t1 set x= x + 1;
> > +unlock tables;
> > +show create table t1;
> > +Table        Create Table
> > +t1   CREATE TABLE `t1` (
> > +  `x` int(11) DEFAULT NULL
> > +) ENGINE=DEFAULT_ENGINE DEFAULT CHARSET=latin1 WITH SYSTEM VERSIONING
> > + PARTITION BY SYSTEM_TIME LIMIT 1000 AUTO_INCREMENT
> > +PARTITIONS 3
>
> this test needs a comment, I don't understand what's happening here.
> why lock tables?
>

It's just another case for LOCK TABLES. Moved it to better place.

> > +#
> > +# Rotation by INTERVAL
> > +#
> >  create or replace table t1 (x int)
> >  with system versioning
> >  partition by system_time interval 0 second partitions 3;
> > -ERROR HY000: Wrong parameters for partitioned `t1`: wrong value for 'INTERVAL'
> > +ERROR HY000: Wrong parameters for partitioned `t1`: wrong value for INTERVAL
> >  create table t1 (i int) with system versioning
> >  partition by system_time interval 6 day limit 98;
> >  ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'limit 98' at line 2
> > @@ -302,7 +320,7 @@ partition by system_time interval 10 year partitions 3;
> >  ERROR 22003: TIMESTAMP value is out of range in 'INTERVAL'
> >  # INTERVAL and ALTER TABLE
> >  create or replace table t1 (i int) with system versioning
> > -partition by system_time interval 1 hour;
> > +partition by system_time interval 59 minute;
>
> why?

Leftover from old patch. Reverted.

>
> >  set @ts=(select partition_description from information_schema.partitions
> >  where table_schema='test' and table_name='t1' and partition_name='p0');
> >  alter table t1 add column b int;
> > @@ -353,28 +371,51 @@ Warning 4114    Versioned table `test`.`t1`: last HISTORY partition (`p1`) is out o
> >  delete from t1;
> >  Warnings:
> >  Warning      4114    Versioned table `test`.`t1`: last HISTORY partition (`p1`) is out of INTERVAL, need more HISTORY partitions
> > -select subpartition_name,partition_description,table_rows from information_schema.partitions where table_schema='test' and table_name='t1';
> > +select subpartition_name,partition_description from information_schema.partitions where table_schema='test' and table_name='t1';
>
> why?

Leftover from old patch. Reverted.

>
> > -subpartition_name    partition_description   table_rows
> > -p1sp0        2001-02-04 00:00:00     1
> > -p1sp1        2001-02-04 00:00:00     1
> > -pnsp0        CURRENT 0
> > -pnsp1        CURRENT 0
> > +subpartition_name    partition_description
> > +p1sp0        2001-02-04 00:00:00
> > +p1sp1        2001-02-04 00:00:00
> > +pnsp0        CURRENT
> > +pnsp1        CURRENT
> > +select * from t1 partition (p1);
> > +i
> > +1
> > +2
> >  set timestamp=unix_timestamp('2001-02-04 10:20:55');
> >  alter table t1 add partition (partition p0 history, partition p2 history);
> >  set timestamp=unix_timestamp('2001-02-04 10:30:00');
> >  insert t1 values (4),(5);
> >  set timestamp=unix_timestamp('2001-02-04 10:30:10');
> >  update t1 set i=6 where i=5;
> > -select subpartition_name,partition_description,table_rows from information_schema.partitions where table_schema='test' and table_name='t1';
> > -subpartition_name    partition_description   table_rows
> > -p1sp0        2001-02-04 00:00:00     1
> > -p1sp1        2001-02-04 00:00:00     0
> > -p0sp0        2001-02-05 00:00:00     1
> > -p0sp1        2001-02-05 00:00:00     1
> > -p2sp0        2001-02-06 00:00:00     0
> > -p2sp1        2001-02-06 00:00:00     0
> > -pnsp0        CURRENT 0
> > -pnsp1        CURRENT 2
> > +select subpartition_name, partition_description from information_schema.partitions where table_schema='test' and table_name='t1';
> > +subpartition_name    partition_description
> > +p1sp0        2001-02-04 00:00:00
> > +p1sp1        2001-02-04 00:00:00
> > +p0sp0        2001-02-05 00:00:00
> > +p0sp1        2001-02-05 00:00:00
> > +p2sp0        2001-02-06 00:00:00
> > +p2sp1        2001-02-06 00:00:00
> > +pnsp0        CURRENT
> > +pnsp1        CURRENT
> > +select * from t1 partition (p1);
> > +i
> > +1
> > +select * from t1 partition (p0);
> > +i
> > +5
> > +2
> > +select * from t1 partition (p2);
> > +i
> > +alter table t1 rebuild partition p0, p1, p2;
> > +select * from t1 partition (p1);
> > +i
> > +1
> > +select * from t1 partition (p0);
> > +i
> > +5
> > +2
> > +select * from t1 partition (p2);
> > +i
> >  ## pruning check
> >  set @ts=(select partition_description from information_schema.partitions
> >  where table_schema='test' and table_name='t1' and partition_name='p0' limit 1);
> > @@ -1044,3 +1085,568 @@ t1    CREATE TABLE `t1` (
> >   PARTITION BY SYSTEM_TIME
> >  PARTITIONS 8
> >  drop tables t1;
> > +#
> > +# MDEV-17554 Auto-create new partition for system versioned tables with history partitioned by INTERVAL/LIMIT
> > +#
> > +create or replace table t1 (x int) with system versioning
> > +partition by system_time limit 999 auto_increment;
> > +ERROR HY000: Wrong parameters for partitioned `t1`: wrong value for LIMIT (< 1000)
> > +create or replace table t1 (x int) with system versioning
> > +partition by system_time interval 3599 second auto_increment;
> > +ERROR HY000: Wrong parameters for partitioned `t1`: wrong value for INTERVAL (< 1 HOUR)
>
> no arbitrary limitations, please

This was already fixed.

>
> > +create or replace table t1 (x int) with system versioning
> > +partition by system_time limit 1000 auto_increment;
> > +affected rows: 0
> > +show create table t1;
> > +Table        Create Table
> > +t1   CREATE TABLE `t1` (
> > +  `x` int(11) DEFAULT NULL
> > +) ENGINE=DEFAULT_ENGINE DEFAULT CHARSET=latin1 WITH SYSTEM VERSIONING
> > + PARTITION BY SYSTEM_TIME LIMIT 1000 AUTO_INCREMENT
> > +PARTITIONS 2
> > +affected rows: 1
> > +insert into t1 values (1);
> > +affected rows: 1
> > +show create table t1;
> > +Table        Create Table
> > +t1   CREATE TABLE `t1` (
> > +  `x` int(11) DEFAULT NULL
> > +) ENGINE=DEFAULT_ENGINE DEFAULT CHARSET=latin1 WITH SYSTEM VERSIONING
> > + PARTITION BY SYSTEM_TIME LIMIT 1000 AUTO_INCREMENT
> > +PARTITIONS 3
> > +affected rows: 1
> > +# Increment from 2 to 5
> > +create or replace table t1 (x int) with system versioning
> > +partition by system_time interval 3600 second
> > +starts '2000-01-01 00:00:00' auto_increment;
> > +affected rows: 0
> > +show create table t1;
> > +Table        Create Table
> > +t1   CREATE TABLE `t1` (
> > +  `x` int(11) DEFAULT NULL
> > +) ENGINE=DEFAULT_ENGINE DEFAULT CHARSET=latin1 WITH SYSTEM VERSIONING
> > + PARTITION BY SYSTEM_TIME INTERVAL 3600 SECOND STARTS TIMESTAMP'2000-01-01 00:00:00' AUTO_INCREMENT
> > +PARTITIONS 2
> > +affected rows: 1
> > +set timestamp= unix_timestamp('2000-01-01 00:00:00');
> > +affected rows: 0
> > +insert into t1 values (1);
> > +affected rows: 1
> > +show create table t1;
> > +Table        Create Table
> > +t1   CREATE TABLE `t1` (
> > +  `x` int(11) DEFAULT NULL
> > +) ENGINE=DEFAULT_ENGINE DEFAULT CHARSET=latin1 WITH SYSTEM VERSIONING
> > + PARTITION BY SYSTEM_TIME INTERVAL 3600 SECOND STARTS TIMESTAMP'2000-01-01 00:00:00' AUTO_INCREMENT
> > +PARTITIONS 3
> > +affected rows: 1
> > +set timestamp= unix_timestamp('2000-01-01 01:00:00');
> > +affected rows: 0
> > +update t1 set x= x + 1;
> > +affected rows: 1
> > +info: Rows matched: 1  Changed: 1  Inserted: 1  Warnings: 0
> > +show create table t1;
> > +Table        Create Table
> > +t1   CREATE TABLE `t1` (
> > +  `x` int(11) DEFAULT NULL
> > +) ENGINE=DEFAULT_ENGINE DEFAULT CHARSET=latin1 WITH SYSTEM VERSIONING
> > + PARTITION BY SYSTEM_TIME INTERVAL 3600 SECOND STARTS TIMESTAMP'2000-01-01 00:00:00' AUTO_INCREMENT
> > +PARTITIONS 4
> > +affected rows: 1
> > +set timestamp= unix_timestamp('2000-01-01 02:00:00');
> > +affected rows: 0
> > +update t1 set x= x + 2;
> > +affected rows: 1
> > +info: Rows matched: 1  Changed: 1  Inserted: 1  Warnings: 0
> > +show create table t1;
> > +Table        Create Table
> > +t1   CREATE TABLE `t1` (
> > +  `x` int(11) DEFAULT NULL
> > +) ENGINE=DEFAULT_ENGINE DEFAULT CHARSET=latin1 WITH SYSTEM VERSIONING
> > + PARTITION BY SYSTEM_TIME INTERVAL 3600 SECOND STARTS TIMESTAMP'2000-01-01 00:00:00' AUTO_INCREMENT
> > +PARTITIONS 5
> > +affected rows: 1
> > +# Increment from 3 to 6, manual names, LOCK TABLES
>
> again, I think this is overcomplication and overengineering.
> I don't see any need for mixing automatic and manual partition names.
>

There are two reasons for this. First one is
https://jira.mariadb.org/browse/MDEV-22155

Second one is user must be able to drop arbitrary partitions and
continue benefit from auto-creation.

To be continued...

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



--
All the best,

Aleksey Midenkov
@midenok