Hi, Sergei! On Tue, Apr 7, 2020 at 3:03 PM Sergei Golubchik wrote:

Hi, Aleksey!

On Apr 07, Aleksey Midenkov wrote:

...

revision-id: 9941c6a3179 (mariadb-10.5.2-164-g9941c6a3179) parent(s): 920c3c6b237 author: Aleksey Midenkov committer: Aleksey Midenkov 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@mariadb.org

-- All the best, Aleksey Midenkov @midenok

Hi, Sergei! I think there should be hard limit on maximum partitions created, as there can be manual timestamp jump. F.ex. timestamp jump for 20 years creates >170000 hourly partitions. What the limit should be? I'd better refuse to create more than 100 partitions. On Thu, Apr 9, 2020 at 9:25 PM Sergei Golubchik wrote:

Hi, Aleksey!

On Apr 09, Aleksey Midenkov wrote:

...

...

...

+# +# 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?

Not to me. In my opinion it looks 1) confusing

Not more confusing than say "create table" and "create user". It is natural feature of verbs to be applied to different nouns.

2) hackish, a.k.a. "they didn't want to introduce a new keyword, so picked an existing one that looked at least remotely relevant"

I don't think it looks like a hack. Partitions number is auto-incremented, isn't it? Besides, "auto-increment" is best sounding feature name: "auto-create" sounds inconvenient, "auto-add" sounds even more clumsy. But to avoid long dispute on this rather minor topic I removed AUTO_INCREMENT keyword.

...

To be continued...

I've attached a proof-of-concept patch that adds a partition using fallback-and-retry mechanism of open_table().

Thanks! Here is the final part of my reply to your review: On Tue, Apr 7, 2020 at 3:03 PM Sergei Golubchik wrote:

...

...

+set timestamp= unix_timestamp('2000-01-01 00:00:00'); +affected rows: 0 +create or replace table t1 (x int) with system versioning +partition by system_time interval 1 hour auto_increment ( +partition p1 history, +partition p3 history, +partition pn current); +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 1 HOUR STARTS TIMESTAMP'2000-01-01 00:00:00' AUTO_INCREMENT +(PARTITION `p1` HISTORY ENGINE = DEFAULT_ENGINE, + PARTITION `p3` HISTORY ENGINE = DEFAULT_ENGINE, + PARTITION `pn` CURRENT ENGINE = DEFAULT_ENGINE) +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 INTERVAL 1 HOUR STARTS TIMESTAMP'2000-01-01 00:00:00' AUTO_INCREMENT +(PARTITION `p1` HISTORY ENGINE = DEFAULT_ENGINE, + PARTITION `p3` HISTORY ENGINE = DEFAULT_ENGINE, + PARTITION `pn` CURRENT ENGINE = DEFAULT_ENGINE) +affected rows: 1 +set timestamp= unix_timestamp('2000-01-01 01:00:00'); +affected rows: 0 +update t1 set x= x + 3; +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 1 HOUR STARTS TIMESTAMP'2000-01-01 00:00:00' AUTO_INCREMENT +(PARTITION `p1` HISTORY ENGINE = DEFAULT_ENGINE, + PARTITION `p3` HISTORY ENGINE = DEFAULT_ENGINE, + PARTITION `p2` HISTORY ENGINE = DEFAULT_ENGINE, + PARTITION `pn` CURRENT ENGINE = DEFAULT_ENGINE) +affected rows: 1 +set timestamp= unix_timestamp('2000-01-01 02:00:00'); +affected rows: 0 +update t1 set x= x + 4; +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 1 HOUR STARTS TIMESTAMP'2000-01-01 00:00:00' AUTO_INCREMENT +(PARTITION `p1` HISTORY ENGINE = DEFAULT_ENGINE, + PARTITION `p3` HISTORY ENGINE = DEFAULT_ENGINE, + PARTITION `p2` HISTORY ENGINE = DEFAULT_ENGINE, + PARTITION `p4` HISTORY ENGINE = DEFAULT_ENGINE, + PARTITION `pn` CURRENT ENGINE = DEFAULT_ENGINE) +affected rows: 1 +lock tables t1 write; +affected rows: 0 +set timestamp= unix_timestamp('2000-01-01 03:00:00'); +affected rows: 0 +update t1 set x= x + 5; +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 1 HOUR STARTS TIMESTAMP'2000-01-01 00:00:00' AUTO_INCREMENT +(PARTITION `p1` HISTORY ENGINE = DEFAULT_ENGINE, + PARTITION `p3` HISTORY ENGINE = DEFAULT_ENGINE, + PARTITION `p2` HISTORY ENGINE = DEFAULT_ENGINE, + PARTITION `p4` HISTORY ENGINE = DEFAULT_ENGINE, + PARTITION `p5` HISTORY ENGINE = DEFAULT_ENGINE, + PARTITION `pn` CURRENT ENGINE = DEFAULT_ENGINE) +affected rows: 1 +unlock tables; +affected rows: 0 +# Test VIEW, LOCK TABLES +set timestamp= unix_timestamp('2000-01-01 00:00:00'); +affected rows: 0 +create or replace table t1 (x int) with system versioning +partition by system_time interval 1 hour auto_increment; +affected rows: 0 +create or replace view v1 as select * from t1; +affected rows: 0 +insert into v1 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 1 HOUR STARTS TIMESTAMP'2000-01-01 00:00:00' AUTO_INCREMENT +PARTITIONS 3 +affected rows: 1 +lock tables t1 write; +affected rows: 0 +set timestamp= unix_timestamp('2000-01-01 01:00:00'); +affected rows: 0 +update t1 set x= x + 3; +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 1 HOUR STARTS TIMESTAMP'2000-01-01 00:00:00' AUTO_INCREMENT +PARTITIONS 4 +affected rows: 1 +unlock tables; +affected rows: 0 +drop view v1; +affected rows: 0 +drop tables t1; +affected rows: 0 +# Multiple increments in single command +set timestamp= unix_timestamp('2000-01-01 00:00:00'); +affected rows: 0 +create or replace table t1 (x int) with system versioning +partition by system_time interval 1 hour auto_increment partitions 3; +affected rows: 0 +create or replace table t2 (y int) with system versioning +partition by system_time interval 1 hour auto_increment partitions 4; +affected rows: 0 +insert into t1 values (1); +affected rows: 1 +insert into t2 values (2); +affected rows: 1 +set timestamp= unix_timestamp('2000-01-01 01:00:00'); +affected rows: 0 +update t1, t2 set x= x + 1, y= y + 1; +affected rows: 2 +info: Rows matched: 2 Changed: 2 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 1 HOUR STARTS TIMESTAMP'2000-01-01 00:00:00' AUTO_INCREMENT +PARTITIONS 4 +affected rows: 1 +show create table t2; +Table Create Table +t2 CREATE TABLE `t2` ( + `y` int(11) DEFAULT NULL +) ENGINE=DEFAULT_ENGINE DEFAULT CHARSET=latin1 WITH SYSTEM VERSIONING + PARTITION BY SYSTEM_TIME INTERVAL 1 HOUR 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, t2 set x= x + 1, y= y + 1; +affected rows: 2 +info: Rows matched: 2 Changed: 2 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 1 HOUR STARTS TIMESTAMP'2000-01-01 00:00:00' AUTO_INCREMENT +PARTITIONS 5 +affected rows: 1 +show create table t2; +Table Create Table +t2 CREATE TABLE `t2` ( + `y` int(11) DEFAULT NULL +) ENGINE=DEFAULT_ENGINE DEFAULT CHARSET=latin1 WITH SYSTEM VERSIONING + PARTITION BY SYSTEM_TIME INTERVAL 1 HOUR STARTS TIMESTAMP'2000-01-01 00:00:00' AUTO_INCREMENT +PARTITIONS 5 +affected rows: 1 +drop tables t1, t2; +affected rows: 0 +# PS, SP, LOCK TABLES +set timestamp= unix_timestamp('2000-01-01 00:00:00'); +affected rows: 0 +create or replace table t1 (x int) with system versioning +partition by system_time interval 1 hour auto_increment; +affected rows: 0 +execute immediate '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 1 HOUR STARTS TIMESTAMP'2000-01-01 00:00:00' AUTO_INCREMENT +PARTITIONS 3 +affected rows: 1 +prepare s from 'update t1 set x= x + 6'; +affected rows: 0 +info: Statement prepared +set timestamp= unix_timestamp('2000-01-01 01:00:00'); +affected rows: 0 +execute s; +affected rows: 1 +info: Rows matched: 1 Changed: 1 Inserted: 1 Warnings: 0 +execute s; +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 1 HOUR STARTS TIMESTAMP'2000-01-01 00:00:00' AUTO_INCREMENT +PARTITIONS 4 +affected rows: 1 +lock tables t1 write; +affected rows: 0 +set timestamp= unix_timestamp('2000-01-01 02:00:00'); +affected rows: 0 +execute s; +affected rows: 1 +info: Rows matched: 1 Changed: 1 Inserted: 1 Warnings: 0 +execute s; +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 1 HOUR STARTS TIMESTAMP'2000-01-01 00:00:00' AUTO_INCREMENT +PARTITIONS 5

add a test where timestamp is incremented by, say, 24 hours, please

Added.

...

+affected rows: 1 +unlock tables; +affected rows: 0 +drop prepare s; +affected rows: 0 +create procedure sp() update t1 set x= x + 7; +affected rows: 0 +set timestamp= unix_timestamp('2000-01-01 03:00:00'); +affected rows: 0 +call sp; +affected rows: 1 +call sp; +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 1 HOUR STARTS TIMESTAMP'2000-01-01 00:00:00' AUTO_INCREMENT +PARTITIONS 6 +affected rows: 1 +lock tables t1 write; +affected rows: 0 +set timestamp= unix_timestamp('2000-01-01 04:00:00'); +affected rows: 0 +call sp; +affected rows: 1 +call sp; +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 1 HOUR STARTS TIMESTAMP'2000-01-01 00:00:00' AUTO_INCREMENT +PARTITIONS 7 +affected rows: 1 +unlock tables; +affected rows: 0 +drop procedure sp; +affected rows: 0 +# Complex table +set timestamp= unix_timestamp('2000-01-01 00:00:00'); +affected rows: 0 +create or replace table t1 ( +x int primary key auto_increment, +t timestamp(6) default '2001-11-11 11:11:11', +b blob(4096) compressed null, +c varchar(1033) character set utf8 not null, +u int unique, +m enum('a', 'b', 'c') not null default 'a' comment 'absolute', +i1 tinyint, i2 smallint, i3 bigint, +index three(i1, i2, i3), +v1 timestamp(6) generated always as (t + interval 1 day), +v2 timestamp(6) generated always as (t + interval 1 month) stored, +s timestamp(6) as row start, +e timestamp(6) as row end, +period for system_time (s, e), +ps date, pe date, +period for app_time (ps, pe), +constraint check_constr check (u > -1)) +with system versioning default charset=ucs2 +partition by system_time interval 1 hour auto_increment ( +partition p2 history, +partition pn current); +affected rows: 0 +show create table t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `x` int(11) NOT NULL AUTO_INCREMENT, + `t` timestamp(6) NOT NULL DEFAULT '2001-11-11 11:11:11.000000', + `b` blob /*!100301 COMPRESSED*/ DEFAULT NULL, + `c` varchar(1033) CHARACTER SET utf8 NOT NULL, + `u` int(11) DEFAULT NULL, + `m` enum('a','b','c') NOT NULL DEFAULT 'a' COMMENT 'absolute', + `i1` tinyint(4) DEFAULT NULL, + `i2` smallint(6) DEFAULT NULL, + `i3` bigint(20) DEFAULT NULL, + `v1` timestamp(6) GENERATED ALWAYS AS (`t` + interval 1 day) VIRTUAL, + `v2` timestamp(6) GENERATED ALWAYS AS (`t` + interval 1 month) STORED, + `s` timestamp(6) GENERATED ALWAYS AS ROW START, + `e` timestamp(6) GENERATED ALWAYS AS ROW END, + `ps` date NOT NULL, + `pe` date NOT NULL, + PRIMARY KEY (`x`,`e`), + UNIQUE KEY `u` (`u`,`e`), + KEY `three` (`i1`,`i2`,`i3`), + PERIOD FOR SYSTEM_TIME (`s`, `e`), + PERIOD FOR `app_time` (`ps`, `pe`), + CONSTRAINT `check_constr` CHECK (`u` > -1) +) ENGINE=DEFAULT_ENGINE DEFAULT CHARSET=ucs2 WITH SYSTEM VERSIONING + PARTITION BY SYSTEM_TIME INTERVAL 1 HOUR STARTS TIMESTAMP'2000-01-01 00:00:00' AUTO_INCREMENT +(PARTITION `p2` HISTORY ENGINE = DEFAULT_ENGINE, + PARTITION `pn` CURRENT ENGINE = DEFAULT_ENGINE) +affected rows: 1 +insert into t1 (x, c, u, i1, i2, i3, ps, pe) +values (1, 'cc', 0, 1, 2, 3, '1999-01-01', '2000-01-01'); +affected rows: 1 +show create table t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `x` int(11) NOT NULL AUTO_INCREMENT, + `t` timestamp(6) NOT NULL DEFAULT '2001-11-11 11:11:11.000000', + `b` blob /*!100301 COMPRESSED*/ DEFAULT NULL, + `c` varchar(1033) CHARACTER SET utf8 NOT NULL, + `u` int(11) DEFAULT NULL, + `m` enum('a','b','c') NOT NULL DEFAULT 'a' COMMENT 'absolute', + `i1` tinyint(4) DEFAULT NULL, + `i2` smallint(6) DEFAULT NULL, + `i3` bigint(20) DEFAULT NULL, + `v1` timestamp(6) GENERATED ALWAYS AS (`t` + interval 1 day) VIRTUAL, + `v2` timestamp(6) GENERATED ALWAYS AS (`t` + interval 1 month) STORED, + `s` timestamp(6) GENERATED ALWAYS AS ROW START, + `e` timestamp(6) GENERATED ALWAYS AS ROW END, + `ps` date NOT NULL, + `pe` date NOT NULL, + PRIMARY KEY (`x`,`e`), + UNIQUE KEY `u` (`u`,`e`), + KEY `three` (`i1`,`i2`,`i3`), + PERIOD FOR SYSTEM_TIME (`s`, `e`), + PERIOD FOR `app_time` (`ps`, `pe`), + CONSTRAINT `check_constr` CHECK (`u` > -1) +) ENGINE=DEFAULT_ENGINE AUTO_INCREMENT=2 DEFAULT CHARSET=ucs2 WITH SYSTEM VERSIONING + PARTITION BY SYSTEM_TIME INTERVAL 1 HOUR STARTS TIMESTAMP'2000-01-01 00:00:00' AUTO_INCREMENT +(PARTITION `p2` HISTORY ENGINE = DEFAULT_ENGINE, + PARTITION `p1` HISTORY ENGINE = DEFAULT_ENGINE, + PARTITION `pn` CURRENT ENGINE = DEFAULT_ENGINE) +affected rows: 1 +set timestamp= unix_timestamp('2000-01-01 01:00:00'); +affected rows: 0 +update t1 set x= x + 8; +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) NOT NULL AUTO_INCREMENT, + `t` timestamp(6) NOT NULL DEFAULT '2001-11-11 11:11:11.000000', + `b` blob /*!100301 COMPRESSED*/ DEFAULT NULL, + `c` varchar(1033) CHARACTER SET utf8 NOT NULL, + `u` int(11) DEFAULT NULL, + `m` enum('a','b','c') NOT NULL DEFAULT 'a' COMMENT 'absolute', + `i1` tinyint(4) DEFAULT NULL, + `i2` smallint(6) DEFAULT NULL, + `i3` bigint(20) DEFAULT NULL, + `v1` timestamp(6) GENERATED ALWAYS AS (`t` + interval 1 day) VIRTUAL, + `v2` timestamp(6) GENERATED ALWAYS AS (`t` + interval 1 month) STORED, + `s` timestamp(6) GENERATED ALWAYS AS ROW START, + `e` timestamp(6) GENERATED ALWAYS AS ROW END, + `ps` date NOT NULL, + `pe` date NOT NULL, + PRIMARY KEY (`x`,`e`), + UNIQUE KEY `u` (`u`,`e`), + KEY `three` (`i1`,`i2`,`i3`), + PERIOD FOR SYSTEM_TIME (`s`, `e`), + PERIOD FOR `app_time` (`ps`, `pe`), + CONSTRAINT `check_constr` CHECK (`u` > -1) +) ENGINE=DEFAULT_ENGINE AUTO_INCREMENT=10 DEFAULT CHARSET=ucs2 WITH SYSTEM VERSIONING + PARTITION BY SYSTEM_TIME INTERVAL 1 HOUR STARTS TIMESTAMP'2000-01-01 00:00:00' AUTO_INCREMENT +(PARTITION `p2` HISTORY ENGINE = DEFAULT_ENGINE, + PARTITION `p1` HISTORY ENGINE = DEFAULT_ENGINE, + PARTITION `p3` HISTORY ENGINE = DEFAULT_ENGINE, + PARTITION `pn` CURRENT ENGINE = DEFAULT_ENGINE) +affected rows: 1 +# Concurrent DML +set timestamp= unix_timestamp('2000-01-01 00:00:00'); +create or replace table t1 (x int) with system versioning +partition by system_time interval 1 hour auto_increment; +insert into t1 values (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 1 HOUR STARTS TIMESTAMP'2000-01-01 00:00:00' AUTO_INCREMENT +PARTITIONS 3 +connect con8, localhost, root; +connect con7, localhost, root; +connect con6, localhost, root; +connect con5, localhost, root; +connect con4, localhost, root; +connect con3, localhost, root; +connect con2, localhost, root; +connect con1, localhost, root; +set timestamp= unix_timestamp('2000-01-01 01:00:00'); +update t1 set x= x + 10; +connection con2; +set timestamp= unix_timestamp('2000-01-01 01:00:00'); +update t1 set x= x + 20; +connection con3; +set timestamp= unix_timestamp('2000-01-01 01:00:00'); +update t1 set x= x + 30; +connection con4; +set timestamp= unix_timestamp('2000-01-01 01:00:00'); +update t1 set x= x + 40; +connection con5; +set timestamp= unix_timestamp('2000-01-01 01:00:00'); +update t1 set x= x + 50; +connection con6; +set timestamp= unix_timestamp('2000-01-01 01:00:00'); +update t1 set x= x + 60; +connection con7; +set timestamp= unix_timestamp('2000-01-01 01:00:00'); +update t1 set x= x + 70; +connection con8; +set timestamp= unix_timestamp('2000-01-01 01:00:00'); +update t1 set x= x + 80; +connection con1; +disconnect con1; +connection con2; +disconnect con2; +connection con3; +disconnect con3; +connection con4; +disconnect con4; +connection con5; +disconnect con5; +connection con6; +disconnect con6; +connection con7; +disconnect con7; +disconnect con8; +connection default; +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 1 HOUR STARTS TIMESTAMP'2000-01-01 00:00:00' AUTO_INCREMENT +PARTITIONS 4 +drop tables t1; +create or replace table t1 (x int) with system versioning engine innodb +partition by system_time interval 1 hour auto; +start transaction; +select * from t1; +x +connect con1, localhost, root; +set lock_wait_timeout= 1; +insert into t1 values (1); +Warnings: +Warning 4114 Versioned table `test`.`t1`: last HISTORY partition (`p0`) is out of INTERVAL, need more HISTORY partitions +Error 1205 Lock wait timeout exceeded; try restarting transaction +Warning 4171 Auto-increment history partition: alter partition table failed +Warning 4171 Versioned table `test`.`t1`: adding HISTORY partition failed with error 0, see error log for details

"error 0" is strange and "see error log for details" isn't very user-friendly, a user might not have access to the error log at all

Removed this from message. All error messages are actually pushed to warning stack.

...

+select * from t1; +x +1

I don't understand, there was an error above, why did insert succeed?

The history is miscellaneous facility and should not disrupt main business process. Now as the locking is done by Open_table_context this principle is violated.

...

+disconnect con1; +connection default; +drop table t1; diff --git a/mysql-test/suite/versioning/r/rpl.result b/mysql-test/suite/versioning/r/rpl.result index 627f3991499..68113190889 100644 --- a/mysql-test/suite/versioning/r/rpl.result +++ b/mysql-test/suite/versioning/r/rpl.result @@ -164,4 +164,65 @@ update t1 set i = 0; connection slave; connection master; drop table t1; +# +# MDEV-17554 Auto-create new partition for system versioned tables with history partitioned by INTERVAL/LIMIT +# +set timestamp= unix_timestamp('2000-01-01 00:00:00'); +create or replace table t1 (x int) with system versioning +partition by system_time interval 1 hour auto_increment; +insert t1 values (); +set timestamp= unix_timestamp('2000-01-01 01:00:00'); +delete from t1; +show create table t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `x` int(11) DEFAULT NULL +) ENGINE=ENGINE DEFAULT CHARSET=latin1 WITH SYSTEM VERSIONING + PARTITION BY SYSTEM_TIME INTERVAL 1 HOUR STARTS TIMESTAMP'2000-01-01 00:00:00' AUTO_INCREMENT +PARTITIONS 4 +connection slave; +show create table t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `x` int(11) DEFAULT NULL +) ENGINE=ENGINE DEFAULT CHARSET=latin1 WITH SYSTEM VERSIONING + PARTITION BY SYSTEM_TIME INTERVAL 1 HOUR STARTS TIMESTAMP'2000-01-01 00:00:00' AUTO_INCREMENT +PARTITIONS 4 +connection master; +drop table t1; +# +# MENT-685 DML events for auto-partitioned tables are written into binary log twice +#

the test below doesn't seem to match the description above

Fixed.

...

+create table t1 (x int) partition by hash (x); +alter table t1 add partition partitions 1 auto_increment; +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 +drop table t1; +create table t1 (x int) with system versioning +partition by system_time limit 1000 auto +(partition p1 history, partition pn current); +insert into t1 values (1); +show create table t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `x` int(11) DEFAULT NULL +) ENGINE=ENGINE DEFAULT CHARSET=latin1 WITH SYSTEM VERSIONING + PARTITION BY SYSTEM_TIME LIMIT 1000 AUTO_INCREMENT +(PARTITION `p1` HISTORY ENGINE = ENGINE, + PARTITION `p2` HISTORY ENGINE = ENGINE, + PARTITION `pn` CURRENT ENGINE = ENGINE) +connection slave; +show create table t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `x` int(11) DEFAULT NULL +) ENGINE=ENGINE DEFAULT CHARSET=latin1 WITH SYSTEM VERSIONING + PARTITION BY SYSTEM_TIME LIMIT 1000 AUTO_INCREMENT +(PARTITION `p1` HISTORY ENGINE = ENGINE, + PARTITION `p2` HISTORY ENGINE = ENGINE, + PARTITION `pn` CURRENT ENGINE = ENGINE) +select * from t1; +x +1 +connection master; +drop table t1; include/rpl_end.inc diff --git a/sql/handler.cc b/sql/handler.cc index c6ecc9566d8..c86f96b9689 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -1512,7 +1512,7 @@ int ha_commit_trans(THD *thd, bool all) DBUG_ASSERT(thd->transaction.stmt.ha_list == NULL || trans == &thd->transaction.stmt);

- if (thd->in_sub_stmt) + if (thd->in_sub_stmt & ~SUB_STMT_AUTO_HIST)

1. why? That is, why do you call ha_commit_trans() when adding a partition?

close_tables_for_reopen() requires thd->transaction.stmt.is_empty(). AFAICS auto-create does statement commit, not transaction commit.

2. please add a test with insert under start transaction. what should happen in this case?

Added. It just works.

...

{ DBUG_ASSERT(0); /* diff --git a/sql/partition_info.cc b/sql/partition_info.cc index f4b7260f8b0..617c839721b 100644 --- a/sql/partition_info.cc +++ b/sql/partition_info.cc @@ -848,29 +850,289 @@ void partition_info::vers_set_hist_part(THD *thd) else vers_info->hist_part= next; } + } + else if (vers_info->interval.is_set()) + { + if (vers_info->hist_part->range_value <= thd->query_start()) + { + partition_element *next= NULL; + bool error= true; + List_iterator it(partitions); + while (next != vers_info->hist_part) + next= it++; + + while ((next= it++) != vers_info->now_part) + { + vers_info->hist_part= next; + if (next->range_value > thd->query_start()) + { + error= false; + break; + } + } + if (error) + my_error(WARN_VERS_PART_FULL, MYF(ME_WARNING|ME_ERROR_LOG), + table->s->db.str, table->s->table_name.str, + vers_info->hist_part->partition_name, "INTERVAL"); + } + } + + if (!vers_info->auto_inc || + vers_info->hist_part->id + VERS_MIN_EMPTY < vers_info->now_part->id) return; + + switch (thd->lex->sql_command) + { + case SQLCOM_DELETE: + if (thd->lex->last_table()->vers_conditions.delete_history) + break; + /* fallthrough */ + case SQLCOM_UPDATE: + case SQLCOM_INSERT: + case SQLCOM_INSERT_SELECT: + case SQLCOM_LOAD: + case SQLCOM_REPLACE: + case SQLCOM_REPLACE_SELECT: + case SQLCOM_DELETE_MULTI: + case SQLCOM_UPDATE_MULTI:

it's rather fragile to check for specific sql statements. why not to look at the table lock instead? (with a special check for delete history)

We have to restrict auto-creation to history generating DML only. Other commands should not be affected by wait for MDL_EXCLUSIVE. SQLCOM_INSERT, SQLCOM_INSERT_SELECT and SQLCOM_LOAD should be removed from the list. And when ALTER does write-lock it's obvious we don't do auto-creation.

Ok, it's a bug. Please add a test with multi-update, where a partitioned table is *not* updated. Like

update t1, tpart set t1.x=5 where t1.y=tpart.z;

Yes, it's difficult. The other cases are when UPDATE updates non-versioned field or updates to same values. I don't think we must handle these.

here a new partition should clearly not be created. also, a simpler example (multi-update is difficult):

insert t1 select * from tpart;

add both, please.

Added.

...

+ { + TABLE_SHARE *share; + List_iterator_fast it(thd->vers_auto_part_tables); + while ((share= it++)) + { + if (table->s == share) + break; + } + if (share) + break; + /* Prevent spawning multiple instances of vers_add_auto_parts() */ + bool altering; + mysql_mutex_lock(&table->s->LOCK_share); + altering= table->s->vers_auto_part; + if (!altering) + table->s->vers_auto_part= true; + mysql_mutex_unlock(&table->s->LOCK_share); + if (altering) + break;

what happens if you're altering already? logically this thread should wait. Where does it do it?

This code was removed.

...

+ if (thd->vers_auto_part_tables.push_back(table->s)) + { + my_error(ER_OUT_OF_RESOURCES, MYF(0)); + } + } + default:; } +}

- if (vers_info->interval.is_set()) - { - if (vers_info->hist_part->range_value > thd->query_start()) - return;

- partition_element *next= NULL; - List_iterator it(partitions); - while (next != vers_info->hist_part) - next= it++; +/** + @brief Run fast_alter_partition_table() to add new history partitions + for tables requiring them. +*/ +void vers_add_auto_parts(THD *thd) +{ + HA_CREATE_INFO create_info; + Alter_info alter_info; + String query; + TABLE_LIST *table_list= NULL; + partition_info *save_part_info= thd->work_part_info; + Query_tables_list save_query_tables; + Reprepare_observer *save_reprepare_observer= thd->m_reprepare_observer; + Diagnostics_area new_stmt_da(thd->query_id, false, true); + Diagnostics_area *save_stmt_da= thd->get_stmt_da(); + bool save_no_write_to_binlog= thd->lex->no_write_to_binlog; + const CSET_STRING save_query= thd->query_string; + thd->m_reprepare_observer= NULL; + thd->lex->reset_n_backup_query_tables_list(&save_query_tables); + thd->in_sub_stmt|= SUB_STMT_AUTO_HIST; + thd->lex->no_write_to_binlog= !thd->is_current_stmt_binlog_format_row(); + TABLE_LIST *tl; + + DBUG_ASSERT(!thd->vers_auto_part_tables.is_empty()); + + for (TABLE_SHARE &share: thd->vers_auto_part_tables) + { + tl= (TABLE_LIST *) thd->alloc(sizeof(TABLE_LIST)); + tl->init_one_table(&share.db, &share.table_name, NULL, TL_READ_NO_INSERT); + tl->open_type= OT_BASE_ONLY; + tl->i_s_requested_object= OPEN_TABLE_ONLY; + tl->next_global= table_list; + table_list= tl; + } + + /* NB: mysql_execute_command() can be recursive because of PS/SP. + Don't duplicate any processing including error messages. */ + thd->vers_auto_part_tables.empty(); + + DBUG_ASSERT(!thd->is_error()); + /* NB: we have to preserve m_affected_rows, m_row_count_func, m_last_insert_id, etc */ + thd->set_stmt_da(&new_stmt_da); + new_stmt_da.set_overwrite_status(true); + + DDL_options_st ddl_opts_none; + ddl_opts_none.init(); + if (open_and_lock_tables(thd, ddl_opts_none, table_list, false, 0)) + goto open_err; + + for (tl= table_list; tl; tl= tl->next_global) + { + TABLE *table= tl->table; + DBUG_ASSERT(table); + DBUG_ASSERT(table->s->get_table_ref_type() == TABLE_REF_BASE_TABLE); + DBUG_ASSERT(table->versioned()); + DBUG_ASSERT(table->part_info); + DBUG_ASSERT(table->part_info->vers_info); + alter_info.reset(); + alter_info.partition_flags= ALTER_PARTITION_ADD|ALTER_PARTITION_AUTO_HIST; + create_info.init(); + create_info.alter_info= &alter_info; + Alter_table_ctx alter_ctx(thd, tl, 1, &table->s->db, &table->s->table_name); + + if (thd->mdl_context.upgrade_shared_lock(table->mdl_ticket, + MDL_SHARED_NO_WRITE, + thd->variables.lock_wait_timeout)) + goto exit; + + create_info.db_type= table->s->db_type(); + create_info.options|= HA_VERSIONED_TABLE; + DBUG_ASSERT(create_info.db_type); + + create_info.vers_info.set_start(table->s->vers_start_field()->field_name); + create_info.vers_info.set_end(table->s->vers_end_field()->field_name); + + partition_info *part_info= new partition_info(); + if (unlikely(!part_info)) + { + my_error(ER_OUT_OF_RESOURCES, MYF(0)); + goto exit; + } + part_info->use_default_num_partitions= false; + part_info->use_default_num_subpartitions= false; + part_info->num_parts= 1; + part_info->num_subparts= table->part_info->num_subparts; + part_info->subpart_type= table->part_info->subpart_type; + if (unlikely(part_info->vers_init_info(thd))) + { + my_error(ER_OUT_OF_RESOURCES, MYF(0)); + goto exit; + } + /* Choose first non-occupied name suffix */ + uint32 suffix= table->part_info->num_parts - 1; + DBUG_ASSERT(suffix > 0); + char part_name[MAX_PART_NAME_SIZE + 1]; + if (make_partition_name(part_name, suffix)) + { +vers_make_name_err: + push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN, + WARN_VERS_HIST_PART_ERROR, + "Auto-increment history partition: " + "name generation failed for suffix %d", suffix); + my_error(WARN_VERS_HIST_PART_ERROR, MYF(ME_WARNING), + table->s->db.str, table->s->table_name.str, 0); + goto exit; + } + List_iterator_fast it(table->part_info->partitions); + partition_element *el; + while ((el= it++)) + { + if (0 == my_strcasecmp(&my_charset_latin1, el->partition_name, part_name)) + { + if (make_partition_name(part_name, ++suffix)) + goto vers_make_name_err; + it.rewind(); + } + }

- while ((next= it++) != vers_info->now_part) + // NB: set_ok_status() requires DA_EMPTY + thd->get_stmt_da()->reset_diagnostics_area(); + + thd->work_part_info= part_info; + if (part_info->set_up_defaults_for_partitioning(thd, table->file, + NULL, suffix + 1)) { - vers_info->hist_part= next; - if (next->range_value > thd->query_start()) - return; + push_warning(thd, Sql_condition::WARN_LEVEL_WARN, + WARN_VERS_HIST_PART_ERROR, + "Auto-increment history partition: " + "setting up defaults failed"); + goto exit; + } + bool partition_changed= false; + bool fast_alter_partition= false; + if (prep_alter_part_table(thd, table, &alter_info, &create_info, + &partition_changed, &fast_alter_partition)) + { + push_warning(thd, Sql_condition::WARN_LEVEL_WARN, WARN_VERS_HIST_PART_ERROR, + "Auto-increment history partition: " + "alter partitition prepare failed"); + goto exit; + } + if (!fast_alter_partition) + { + push_warning(thd, Sql_condition::WARN_LEVEL_WARN, WARN_VERS_HIST_PART_ERROR, + "Auto-increment history partition: " + "fast alter partitition is not possible"); + my_error(WARN_VERS_HIST_PART_ERROR, MYF(ME_WARNING), + table->s->db.str, table->s->table_name.str, 0); + goto exit; + } + DBUG_ASSERT(partition_changed); + if (mysql_prepare_alter_table(thd, table, &create_info, &alter_info, + &alter_ctx)) + { + push_warning(thd, Sql_condition::WARN_LEVEL_WARN, WARN_VERS_HIST_PART_ERROR, + "Auto-increment history partition: " + "alter prepare failed"); + goto exit; + } + + // Forge query string for rpl logging + if (!thd->lex->no_write_to_binlog) + { + query.set(STRING_WITH_LEN("ALTER TABLE `"), &my_charset_latin1); + + if (query.append(table->s->db) || + query.append(STRING_WITH_LEN("`.`")) || + query.append(table->s->table_name) || + query.append("` ADD PARTITION (PARTITION `") || + query.append(part_name) || + query.append("` HISTORY) AUTO_INCREMENT")) + { + my_error(ER_OUT_OF_RESOURCES, MYF(ME_ERROR_LOG)); + goto exit; + } + CSET_STRING qs(query.c_ptr(), query.length(), &my_charset_latin1); + thd->set_query(qs); + } + + if (fast_alter_partition_table(thd, table, &alter_info, &create_info, + tl, &table->s->db, &table->s->table_name)) + { + push_warning(thd, Sql_condition::WARN_LEVEL_WARN, WARN_VERS_HIST_PART_ERROR, + "Auto-increment history partition: " + "alter partition table failed"); + my_error(WARN_VERS_HIST_PART_ERROR, MYF(ME_WARNING), + tl->db.str, tl->table_name.str, 0); } - my_error(WARN_VERS_PART_FULL, MYF(ME_WARNING|ME_ERROR_LOG), - table->s->db.str, table->s->table_name.str, - vers_info->hist_part->partition_name, "INTERVAL"); } + + if (!thd->transaction.stmt.is_empty()) + trans_commit_stmt(thd); + +exit: + // If we failed with error allow non-processed tables to be processed next time + if (tl) + while ((tl= tl->next_global)) + tl->table->s->vers_auto_part= false; + close_thread_tables(thd); +open_err: + thd->work_part_info= save_part_info; + thd->m_reprepare_observer= save_reprepare_observer; + thd->lex->restore_backup_query_tables_list(&save_query_tables); + thd->in_sub_stmt&= ~SUB_STMT_AUTO_HIST; + if (!new_stmt_da.is_warning_info_empty()) + save_stmt_da->copy_sql_conditions_from_wi(thd, new_stmt_da.get_warning_info()); + thd->set_stmt_da(save_stmt_da); + thd->lex->no_write_to_binlog= save_no_write_to_binlog; + thd->set_query(save_query); }

diff --git a/sql/partition_info.h b/sql/partition_info.h index eb8e53a381a..d02eecea073 100644 --- a/sql/partition_info.h +++ b/sql/partition_info.h @@ -34,10 +34,19 @@ typedef bool (*check_constants_func)(THD *thd, partition_info *part_info);

struct st_ddl_log_memory_entry;

+ +/* Auto-create history partition configuration */ +static const uint VERS_MIN_EMPTY= 1;

No, this doesn't work. One can easily set @@timestamp to do a multi-hour jump and no fixed value of VERS_MIN_EMPTY will help. You need to add partitions before the statement, as I wrote in my previous review.

Fixed with the help of your PoC.

...

+static const uint VERS_MIN_INTERVAL= 3600; // seconds +static const uint VERS_MIN_LIMIT= 1000; +static const uint VERS_ERROR_TIMEOUT= 300; // seconds + + struct Vers_part_info : public Sql_alloc { Vers_part_info() : limit(0), + auto_inc(false), now_part(NULL), hist_part(NULL) { diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index 7cc1faea79b..4be3342e78e 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -7521,14 +7533,19 @@ add_partition_rule:

add_part_extra: /* empty */ - | '(' part_def_list ')' + | '(' part_def_list ')' opt_vers_auto_inc { LEX *lex= Lex; lex->part_info->num_parts= lex->part_info->partitions.elements; + if ($4) + lex->alter_info.partition_flags|= ALTER_PARTITION_AUTO_HIST; } - | PARTITIONS_SYM real_ulong_num + | PARTITIONS_SYM real_ulong_num opt_vers_auto_inc { - Lex->part_info->num_parts= $2; + LEX *lex= Lex;

^^^ this pattern is obsolete for, like, 10 years. It does no harm, but adds no value either.

Fixed.

...

+ lex->part_info->num_parts= $2; + if ($3) + lex->alter_info.partition_flags|= ALTER_PARTITION_AUTO_HIST; } ;

diff --git a/storage/mroonga/mrn_table.cpp b/storage/mroonga/mrn_table.cpp index b10668cfcce..6458402f572 100644 --- a/storage/mroonga/mrn_table.cpp +++ b/storage/mroonga/mrn_table.cpp @@ -932,7 +932,7 @@ MRN_SHARE *mrn_get_share(const char *table_name, TABLE *table, int *error) share->wrap_key_info = NULL; share->wrap_primary_key = MAX_KEY; } - memcpy(wrap_table_share, table->s, sizeof(*wrap_table_share)); + memcpy((void *)wrap_table_share, (void *)table->s, sizeof(*wrap_table_share));

why is that?

I had compilation error on release build.

...

mrn_init_sql_alloc(current_thd, &(wrap_table_share->mem_root)); wrap_table_share->keys = share->wrap_keys; wrap_table_share->key_info = share->wrap_key_info;

Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org

-- All the best, Aleksey Midenkov @midenok