- developers - lists.mariadb.org

Re: [Maria-developers] 8742d176bc2: Added support for more functions when using partitioned S3 tables
by Sergei Golubchik 18 Apr '20

18 Apr '20

Hi, Michael! On Apr 13, Michael Widenius wrote: > revision-id: 8742d176bc2 (mariadb-10.5.2-127-g8742d176bc2) > parent(s): bf32018be96 > author: Michael Widenius <monty(a)mariadb.com> > committer: Michael Widenius <monty(a)mariadb.com> > timestamp: 2020-04-09 16:41:42 +0300 > message: > > Added support for more functions when using partitioned S3 tables > > MDEV-22088 S3 partitioning support > > All ALTER PARTITION commands should now work on S3 tables except > > REBUILD PARTITION > TRUNCATE PARTITION > REORGANIZE PARTITION > > In addition, PARTIONED S3 TABLES can also be replicated. > This is achived by storing the partition tables .frm and .par file on S3 > for partitioned shared (S3) tables. > > The discovery methods are enchanced by allowing engines that supports > discovery to also support of the partitioned tables .frm and .par file > > Things in more detail > > - The .frm and .par files of partitioned tables are stored in S3 and kept > in sync. > - Added hton callback create_partitioning_metadata to inform handler > that metadata for a partitoned file has changed > - Added back handler::discover_check_version() to be able to check if > a table's or a part table's definition has changed. > - Added handler::check_if_updates_are_ignored(). Needed for partitioning. > - Renamed rebind() -> rebind_psi(), as it was before. > - Changed CHF_xxx hadnler flags to an enum > - Changed some checks from using table->file->ht to use > table->file->partition_ht() to get discovery to work with partitioning. > - If TABLE_SHARE::init_from_binary_frm_image() fails, ensure that we > don't leave any .frm or .par files around. > - Fixed that writefrm() doesn't leave unusable .frm files around > - Appended extension to path for writefrm() to be able to reuse to function > for creating .par files. > - Added DBUG_PUSH("") to a a few functions that caused a lot of not > critical tracing. > > diff --git a/mysql-test/suite/s3/replication_partition.test b/mysql-test/suite/s3/replication_partition.test > new file mode 100644 > index 00000000000..8a177f8f075 > --- /dev/null > +++ b/mysql-test/suite/s3/replication_partition.test > @@ -0,0 +1,170 @@ > +--source include/have_s3.inc > +--source include/have_partition.inc > +--source include/master-slave.inc master-slave should always be included last, after all other have_xxx includes. > +--source include/have_binlog_format_mixed.inc > +--source include/have_innodb.inc > +--source include/have_sequence.inc > +--source create_database.inc > + > +connection slave; > +let $MYSQLD_DATADIR= `select @@datadir`; > +--replace_result $database database > +--eval use $database > +connection master; > + > +--echo # > +--echo # Check replication of parititioned S3 tables > +--echo # > + > +CREATE TABLE t1 ( > + c1 INT DEFAULT NULL > +) ENGINE=Aria > + PARTITION BY HASH (c1) > + PARTITIONS 3; > +INSERT INTO t1 VALUE (1), (2), (101), (102), (201), (202); > +ALTER TABLE t1 ENGINE=S3; > +ALTER TABLE t1 ADD PARTITION PARTITIONS 6; > +select sum(c1) from t1; > +ALTER TABLE t1 ADD COLUMN c INT; > +select sum(c1) from t1; > +sync_slave_with_master; > +show create table t1; > +select sum(c1) from t1; > +connection master; > +drop table t1; > + > +--echo # > +--echo # Checking that the slave is keeping in sync with changed partitions > +--echo # > + > +CREATE TABLE t1 ( > + c1 int primary key, > + c2 int DEFAULT NULL > +) ENGINE=InnoDB > + PARTITION BY RANGE (c1) > + (PARTITION p1 VALUES LESS THAN (200), > + PARTITION p2 VALUES LESS THAN (300), > + PARTITION p3 VALUES LESS THAN (400)); > +insert into t1 select seq*100,seq*100 from seq_1_to_3; > +alter table t1 engine=S3; > +show create table t1; > + > +sync_slave_with_master; > +select sum(c1) from t1; > +--file_exists $MYSQLD_DATADIR/$database/t1.frm > +--file_exists $MYSQLD_DATADIR/$database/t1.par > +stop slave; > +connection master; > +ALTER TABLE t1 ADD PARTITION (PARTITION p4 VALUES LESS THAN (500)); > +connection slave; > +show create table t1; > +select sum(c1) from t1; > +start slave; > +connection master; > +sync_slave_with_master; > +select sum(c1)+0 from t1; > +stop slave; > + > +# .frm amd .par files should not exists on the salve as it has just seen the > +# ALTER TABLE which cased the removal of the .frm and .par files. The table > +# from the above "select sum()" came from table cache and was used as it's > +# id matches the one in S3 > +--error 1 > +--file_exists $MYSQLD_DATADIR/$database/t1.frm > +--error 1 > +--file_exists $MYSQLD_DATADIR/$database/t1.par > +# Flushing the table cache will force the .frm and .par files to be > +# re-generated > +flush tables; > +select sum(c1)+0 from t1; > +--file_exists $MYSQLD_DATADIR/$database/t1.frm > +--file_exists $MYSQLD_DATADIR/$database/t1.par > + > +connection master; > +drop table t1; > +connection slave; > +--file_exists $MYSQLD_DATADIR/$database/t1.par > +--replace_result $database database > +--error ER_NO_SUCH_TABLE > +select sum(c1) from t1; > +--error 1 > +--file_exists $MYSQLD_DATADIR/$database/t1.par > +start slave; > +connection master; > + > +--echo # > +--echo # Check altering partitioned table to S3 and back > +--echo # Checks also rename partitoned table and drop partition > +--echo # > + > +CREATE TABLE t2 ( > + c1 int primary key, > + c2 int DEFAULT NULL > +) ENGINE=InnoDB > + PARTITION BY RANGE (c1) > + (PARTITION p1 VALUES LESS THAN (200), > + PARTITION p2 VALUES LESS THAN (300), > + PARTITION p3 VALUES LESS THAN (400)); > +insert into t2 select seq*100,seq*100 from seq_1_to_3; > +alter table t2 engine=S3; > +rename table t2 to t1; > +alter table t1 drop partition p1; > +sync_slave_with_master; > +select sum(c1) from t1; > +connection master; > +alter table t1 engine=innodb; > +sync_slave_with_master; > +select sum(c1) from t1; > +connection master; > +drop table t1; > + > +--echo # > +--echo # Check that slaves ignores changes to S3 tables. > +--echo # > + > +connection master; > +CREATE TABLE t1 ( > + c1 int primary key, > + c2 int DEFAULT NULL > +) ENGINE=InnoDB > + PARTITION BY RANGE (c1) > + (PARTITION p1 VALUES LESS THAN (200), > + PARTITION p2 VALUES LESS THAN (300), > + PARTITION p3 VALUES LESS THAN (400)); > +insert into t1 select seq*100,seq*100 from seq_1_to_3; > +create table t2 like t1; > +alter table t2 remove partitioning; > +insert into t2 values (450,450); > +sync_slave_with_master; > +stop slave; > +connection master; > +alter table t1 engine=s3; > +alter table t2 engine=s3; > +ALTER TABLE t1 ADD PARTITION (PARTITION p4 VALUES LESS THAN (500)); > +alter table t1 exchange partition p4 with table t2; > +select count(*) from t1; > +drop table t1,t2; > +connection slave; > +start slave; > +connection master; > +sync_slave_with_master; > +--replace_result $database database > +--error ER_NO_SUCH_TABLE > +select sum(c1) from t1; > +connection master; > + > +--echo # > +--echo # Check slave binary log > +--echo # > + > +sync_slave_with_master; > +--let $binlog_database=$database > +--source include/show_binlog_events.inc > +connection master; > + > +--echo # > +--echo # clean up > +--echo # > +--source drop_database.inc > +sync_slave_with_master; > +--source include/rpl_end.inc > diff --git a/mysys/my_symlink.c b/mysys/my_symlink.c > index cbee78a7f5c..323ae69a39c 100644 > --- a/mysys/my_symlink.c > +++ b/mysys/my_symlink.c > @@ -154,7 +154,8 @@ int my_realpath(char *to, const char *filename, myf MyFlags) > original name but will at least be able to resolve paths that starts > with '.'. > */ > - DBUG_PRINT("error",("realpath failed with errno: %d", errno)); > + if (MyFlags) > + DBUG_PRINT("error",("realpath failed with errno: %d", errno)); This is kind of against the concept of dbug. dbug's idea is that one should not edit the code every time, but put DBUG points one and enable/disable them at run-time. In cases like that you can specify precisely what you want to see in the trace with complicated command-line --debug string. But I usually just let it log everything and then use dbug/remove_function_from_trace.pl script. > my_errno=errno; > if (MyFlags & MY_WME) > my_error(EE_REALPATH, MYF(0), filename, my_errno); > diff --git a/sql/discover.cc b/sql/discover.cc > index e49a2a3b0c0..7f3fe73c155 100644 > --- a/sql/discover.cc > +++ b/sql/discover.cc > @@ -99,23 +99,23 @@ int readfrm(const char *name, const uchar **frmdata, size_t *len) > > > /* > - Write the content of a frm data pointer > - to a frm file. > + Write the content of a frm data pointer to a frm or par file. really? writefrm() writes to a .par file? may be rename it to writefile() then? > > - @param path path to table-file "db/name" > - @param frmdata frm data > - @param len length of the frmdata > + @param path full path to table-file "db/name.frm" or .par > + @param db Database name. Only used for my_error() > + @param table Table name. Only used for my_error() > + @param data data to write to file > + @param len length of the data > > @retval > 0 ok > @retval > - 2 Could not write file > + <> 0 Could not write file. In this case the file is not created > */ > > int writefrm(const char *path, const char *db, const char *table, > - bool tmp_table, const uchar *frmdata, size_t len) > + bool tmp_table, const uchar *data, size_t len) > { > - char file_name[FN_REFLEN+1]; > int error; > int create_flags= O_RDWR | O_TRUNC; > DBUG_ENTER("writefrm"); > diff --git a/sql/ha_partition.cc b/sql/ha_partition.cc > index 0ec1f2138ab..b8b5085f389 100644 > --- a/sql/ha_partition.cc > +++ b/sql/ha_partition.cc > @@ -629,7 +629,8 @@ int ha_partition::rename_table(const char *from, const char *to) > > SYNOPSIS > create_partitioning_metadata() > - name Full path of table name > + path Full path of new table name > + old_p Full path of old table name This is a rather meaningless phrase, there cannot be a "path" of a "name" May be, write "A path to the new/old frm file but without the extension" instead? > create_info Create info generated for CREATE TABLE > > RETURN VALUE > @@ -678,6 +681,19 @@ int ha_partition::create_partitioning_metadata(const char *path, > DBUG_RETURN(1); > } > } > + > + /* m_part_info is only NULL when we failed to create a partition table */ How can m_part_info be NULL here? create_handler_file() dereferences m_part_info unconditionally, if m_part_info would've been NULL it'll crash. > + if (m_part_info) > + { > + part= m_part_info->partitions.head(); > + if ((part->engine_type)->create_partitioning_metadata && > + ((part->engine_type)->create_partitioning_metadata)(path, old_path, > + action_flag)) > + { > + my_error(ER_CANT_CREATE_HANDLER_FILE, MYF(0)); > + DBUG_RETURN(1); > + } > + } > DBUG_RETURN(0); > } > > @@ -1604,6 +1620,7 @@ int ha_partition::prepare_new_partition(TABLE *tbl, > > if (!(file->ht->flags & HTON_CAN_READ_CONNECT_STRING_IN_PARTITION)) > tbl->s->connect_string= p_elem->connect_string; > + create_info->options|= HA_CREATE_TMP_ALTER; This looks wrong. The partition isn't created as a temporary file. HA_CREATE_TMP_ALTER is used for autiding and perfschema to distinguish between normal tables, temporary tables and these alter-tmp-tables. I see that you've hijacked this flag and check it inside s3, but it wasn't designed for that, so change s3 instead, not what the flag means. I don't think you need that if inside s3 at all, if hton flag says HTON_TEMPORARY_NOT_SUPPORTED, then s3 won't be asked to create a temporary table and you don't need a check for that in create. > if ((error= file->ha_create(part_name, tbl, create_info))) > { > /* > @@ -2361,14 +2379,20 @@ uint ha_partition::del_ren_table(const char *from, const char *to) > const char *to_path= NULL; > uint i; > handler **file, **abort_file; > + THD *thd= ha_thd(); > DBUG_ENTER("ha_partition::del_ren_table"); > > - if (get_from_handler_file(from, ha_thd()->mem_root, false)) > - DBUG_RETURN(TRUE); > + if (get_from_handler_file(from, thd->mem_root, false)) > + DBUG_RETURN(my_errno ? my_errno : ENOENT); > DBUG_ASSERT(m_file_buffer); > DBUG_PRINT("enter", ("from: (%s) to: (%s)", from, to ? to : "(nil)")); > name_buffer_ptr= m_name_buffer_ptr; > + > file= m_file; > + /* The command should be logged with IF EXISTS if using a shared table */ > + if (m_file[0]->ht->flags & HTON_TABLE_MAY_NOT_EXIST_ON_SLAVE) > + thd->replication_flags|= OPTION_IF_EXISTS; I don't think this and the whole thd->replication_flags is needed, because in the sql_table.cc you can check directly table->file->partition_ht()->flags && HTON_TABLE_MAY_NOT_EXIST_ON_SLAVE > + > if (to == NULL) > { > /* > @@ -2424,7 +2453,35 @@ uint ha_partition::del_ren_table(const char *from, const char *to) > goto rename_error; > } > } > + > + /* Update .par file in the handlers that supports it */ > + if ((*m_file)->ht->create_partitioning_metadata) > + { > + if (to == NULL) > + error= (*m_file)->ht->create_partitioning_metadata(NULL, from, > + CHF_DELETE_FLAG); > + else > + error= (*m_file)->ht->create_partitioning_metadata(to, from, > + CHF_RENAME_FLAG); why not just error= (*m_file)->ht->create_partitioning_metadata(to, from); the engine can distingush CREATE from RENAME from DELETE by looking at whether 'to' or `from` is NULL. > + DBUG_EXECUTE_IF("failed_create_partitioning_metadata", > + { my_message_sql(ER_OUT_OF_RESOURCES,"Simulated crash",MYF(0)); > + error= 1; > + }); > + if (error) > + { > + if (to) > + { > + (void) handler::rename_table(to, from); > + (void) (*m_file)->ht->create_partitioning_metadata(from, to, > + CHF_RENAME_FLAG); > + goto rename_error; > + } > + else > + save_error=error; > + } > + } > DBUG_RETURN(save_error); > + > rename_error: > name_buffer_ptr= m_name_buffer_ptr; > for (abort_file= file, file= m_file; file < abort_file; file++) > @@ -3729,6 +3787,16 @@ int ha_partition::rebind() > #endif /* HAVE_M_PSI_PER_PARTITION */ > > > +/* > + Check if the table definition has changed for the part tables > + We use the first partition for the check. > +*/ > + > +int ha_partition::discover_check_version() > +{ > + return m_file[0]->discover_check_version(); > +} 1. generally, you have to do it for every open partition, not just for the first one. 2. m_file[0] partition is not necessarily open, you need to use the first open partition here (and everywhere) > + > /** > Clone the open and locked partitioning handler. > > @@ -11382,6 +11450,12 @@ int ha_partition::end_bulk_delete() > } > > > +bool ha_partition::check_if_updates_are_ignored(const char *op) const > +{ > + return (handler::check_if_updates_are_ignored(op) || > + ha_check_if_updates_are_ignored(table->in_use, partition_ht(), op)); no you don't need this virtual method at all, simply do - if (ha_check_if_updates_are_ignored(ha_thd(), ht, "DROP")) + if (ha_check_if_updates_are_ignored(ha_thd(), partition_ht(), "DROP")) that's why partition_ht() was created in the first place. > +} > + > /** > Perform initialization for a direct update request. > > diff --git a/sql/handler.h b/sql/handler.h > index 8c45c64bec8..eadbf28229c 100644 > --- a/sql/handler.h > +++ b/sql/handler.h > @@ -3183,7 +3200,10 @@ class handler :public Sql_alloc > > public: > virtual void unbind_psi(); > - virtual int rebind(); > + virtual void rebind_psi(); this doesn't have to be virtual anymore, because you moved the check into the virtual discover_check_version() > + /* Return error if definition doesn't match for already opened table */ > + virtual int discover_check_version() { return 0; } > + > /** > Put the handler in 'batch' mode when collecting > table io instrumented events. > diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc > index 53ccdb5d4c3..b1756b83056 100644 > --- a/sql/sql_parse.cc > +++ b/sql/sql_parse.cc > @@ -5906,6 +5906,8 @@ mysql_execute_command(THD *thd) > case SQLCOM_CALL: > case SQLCOM_REVOKE: > case SQLCOM_GRANT: > + if (thd->variables.option_bits & OPTION_IF_EXISTS) > + lex->create_info.set(DDL_options_st::OPT_IF_EXISTS); 1. please remove the same (now redundant) code from Sql_cmd_alter_table::execute() 2. better put it only for the relevant case. Like this case SQLCOM_ALTER_TABLE: if (thd->variables.option_bits & OPTION_IF_EXISTS) lex->create_info.set(DDL_options_st::OPT_IF_EXISTS); /* fall through */ case SQLCOM_ANALYZE_TABLE: ... > DBUG_ASSERT(lex->m_sql_cmd != NULL); > res= lex->m_sql_cmd->execute(thd); > DBUG_PRINT("result", ("res: %d killed: %d is_error: %d", > diff --git a/sql/sql_partition.cc b/sql/sql_partition.cc > index 7385f9059e1..351464939e2 100644 > --- a/sql/sql_partition.cc > +++ b/sql/sql_partition.cc > @@ -7057,6 +7059,10 @@ uint fast_alter_partition_table(THD *thd, TABLE *table, > lpt->pack_frm_data= NULL; > lpt->pack_frm_len= 0; > > + /* Add IF EXISTS to binlog if shared table */ > + if (table->file->partition_ht()->flags & HTON_TABLE_MAY_NOT_EXIST_ON_SLAVE) > + thd->variables.option_bits|= OPTION_IF_EXISTS; > + No need to, you already have that in mysql_alter_table(). Just fix the check there, like this: if (!if_exists && - (table->s->db_type()->flags & HTON_TABLE_MAY_NOT_EXIST_ON_SLAVE)) + (table->file->partition_ht()->flags & HTON_TABLE_MAY_NOT_EXIST_ON_SLAVE)) > if (table->file->alter_table_flags(alter_info->flags) & > HA_PARTITION_ONE_PHASE) > { > diff --git a/sql/sql_partition_admin.cc b/sql/sql_partition_admin.cc > index ed77c0938f3..d29c014bdc2 100644 > --- a/sql/sql_partition_admin.cc > +++ b/sql/sql_partition_admin.cc > @@ -529,14 +530,46 @@ bool Sql_cmd_alter_table_exchange_partition:: > table_list->mdl_request.set_type(MDL_SHARED_NO_WRITE); > if (unlikely(open_tables(thd, &table_list, &table_counter, 0, > &alter_prelocking_strategy))) > + { > + if (thd->lex->if_exists() && > + thd->get_stmt_da()->sql_errno() == ER_NO_SUCH_TABLE) > + { > + /* > + ALTER TABLE IF EXISTS was used on not existing table > + We have to log the query on a slave as the table may be a shared one > + from the master and we need to ensure that the next slave can see > + the statement as this slave may not have the table shared > + */ > + thd->clear_error(); > + if (thd->slave_thread && > + write_bin_log(thd, true, thd->query(), thd->query_length())) > + DBUG_RETURN(true); > + my_ok(thd); > + DBUG_RETURN(false); > + } I don't like that. There are many Sql_cmd_alter_table* classes and lots of duplicated code. Okay, you've put your OPTION_IF_EXISTS check into the big switch in the mysql_execute_command() and thus avoided duplicating that code. But this if() is also in mysql_alter_table() (and may be more?) I'm thinking that doing many Sql_cmd_alter_table* classes was a bad idea which leads to code duplication. How can we fix that? > DBUG_RETURN(true); > + } > > part_table= table_list->table; > swap_table= swap_table_list->table; > > + if (part_table->file->check_if_updates_are_ignored("ALTER")) > + { > + if (thd->slave_thread && > + write_bin_log_with_if_exists(thd, true, false, true)) > + DBUG_RETURN(true); > + my_ok(thd); > + DBUG_RETURN(false); > + } another duplicated one > + > if (unlikely(check_exchange_partition(swap_table, part_table))) > DBUG_RETURN(TRUE); > > + /* Add IF EXISTS to binlog if shared table */ > + if (part_table->file->partition_ht()->flags & > + HTON_TABLE_MAY_NOT_EXIST_ON_SLAVE) > + force_if_exists= 1; and again > + > /* set lock pruning on first table */ > partition_name= alter_info->partition_names.head(); > if (unlikely(table_list->table->part_info-> > @@ -784,16 +821,51 @@ bool Sql_cmd_alter_table_truncate_partition::execute(THD *thd) > #endif /* WITH_WSREP */ > > if (open_tables(thd, &first_table, &table_counter, 0)) > - DBUG_RETURN(true); > + { > + if (thd->lex->if_exists() && > + thd->get_stmt_da()->sql_errno() == ER_NO_SUCH_TABLE) > + { > + /* > + ALTER TABLE IF EXISTS was used on not existing table > + We have to log the query on a slave as the table may be a shared one > + from the master and we need to ensure that the next slave can see > + the statement as this slave may not have the table shared > + */ > + thd->clear_error(); > + if (thd->slave_thread && > + write_bin_log(thd, true, thd->query(), thd->query_length())) > + DBUG_RETURN(TRUE); > + my_ok(thd); > + DBUG_RETURN(FALSE); > + } > + DBUG_RETURN(TRUE); > + } see? > > - if (!first_table->table || first_table->view || > - first_table->table->s->db_type() != partition_hton) > + if (!first_table->table || first_table->view) > { > my_error(ER_PARTITION_MGMT_ON_NONPARTITIONED, MYF(0)); > DBUG_RETURN(TRUE); > } > > - > + if (first_table->table->file->check_if_updates_are_ignored("ALTER")) > + { > + if (thd->slave_thread && > + write_bin_log_with_if_exists(thd, true, false, 1)) > + DBUG_RETURN(true); > + my_ok(thd); > + DBUG_RETURN(false); > + } > + > + if (first_table->table->s->db_type() != partition_hton) > + { > + my_error(ER_PARTITION_MGMT_ON_NONPARTITIONED, MYF(0)); > + DBUG_RETURN(TRUE); > + } why did you split the check for ER_PARTITION_MGMT_ON_NONPARTITIONED? > + > + if (first_table->table->file->partition_ht()->flags & > + HTON_TABLE_MAY_NOT_EXIST_ON_SLAVE) > + force_if_exists= 1; > + > /* > Prune all, but named partitions, > to avoid excessive calls to external_lock(). > diff --git a/sql/table.cc b/sql/table.cc > index fe096835144..c6b42b7ba36 100644 > --- a/sql/table.cc > +++ b/sql/table.cc > @@ -25,7 +25,8 @@ > // primary_key_name > #include "sql_parse.h" // free_items > #include "strfunc.h" // unhex_type2 > -#include "sql_partition.h" // mysql_unpack_partition, > +#include "ha_partition.h" // PART_EXT > + // mysql_unpack_partition, also included below > // fix_partition_func, partition_info > #include "sql_base.h" > #include "create_options.h" > @@ -42,6 +43,7 @@ > #include "rpl_filter.h" > #include "sql_cte.h" > #include "ha_sequence.h" > +#include "ha_partition.h" also included above > #include "sql_show.h" > #include "opt_trace.h" > > @@ -620,6 +622,9 @@ enum open_frm_error open_table_def(THD *thd, TABLE_SHARE *share, uint flags) > { > DBUG_ASSERT(flags & GTS_TABLE); > DBUG_ASSERT(flags & GTS_USE_DISCOVERY); > + /* Delete .frm and .par files */ > + mysql_file_delete_with_symlink(key_file_frm, path, "", MYF(0)); > + strxmov(path, share->normalized_path.str, PAR_EXT, NullS); > mysql_file_delete_with_symlink(key_file_frm, path, "", MYF(0)); no need to strxmov, this third "" argument is the file extension, you can directly do mysql_file_delete_with_symlink(key_file_frm, share->normalized_path.str, PAR_EXT, MYF(0)); also it should be key_file_partition_ddl_log not key_file_frm (it's not a ddl log, but sql_table.cc uses key_file_partition_ddl_log for par files) > file= -1; > } > @@ -1679,12 +1687,13 @@ class Field_data_type_info_array > > 42..46 are unused since 5.0 (were for RAID support) > Also, there're few unused bytes in forminfo. > - > */ > > int TABLE_SHARE::init_from_binary_frm_image(THD *thd, bool write, > const uchar *frm_image, > - size_t frm_length) > + size_t frm_length, > + const uchar *par_image, > + size_t par_length) I don't think writing .par file belongs here, why not to do it in the caller? Or, better, don't send .par file to S3 and don't discover it, it'll make everything much simpler. .par can be created from the .frm, see sql_table.cc: file= mysql_create_frm_image(thd, orig_db, orig_table_name, create_info, alter_info, create_table_mode, key_info, key_count, frm); ... if (file->ha_create_partitioning_metadata(path, NULL, CHF_CREATE_FLAG)) goto err; > { > TABLE_SHARE *share= this; > uint new_frm_ver, field_pack_length, new_field_pack_flag; > @@ -1715,24 +1724,31 @@ int TABLE_SHARE::init_from_binary_frm_image(THD *thd, bool write, > uint len; > uint ext_key_parts= 0; > plugin_ref se_plugin= 0; > - bool vers_can_native= false; > + bool vers_can_native= false, frm_created= 0; > Field_data_type_info_array field_data_type_info_array; > - > MEM_ROOT *old_root= thd->mem_root; > Virtual_column_info **table_check_constraints; > extra2_fields extra2; > - > DBUG_ENTER("TABLE_SHARE::init_from_binary_frm_image"); > > keyinfo= &first_keyinfo; > thd->mem_root= &share->mem_root; > > - if (write && write_frm_image(frm_image, frm_length)) > - goto err; > - > if (frm_length < FRM_HEADER_SIZE + FRM_FORMINFO_SIZE) > goto err; > > + if (write) > + { > +#ifdef WITH_PARTITION_STORAGE_ENGINE > + if (par_image) > + if (write_par_image(par_image, par_length)) > + goto err; > +#endif > + frm_created= 1; > + if (write_frm_image(frm_image, frm_length)) > + goto err; > + } why not to create it at the end? then you won't need to delete it on an error. > + > share->frm_version= frm_image[2]; > /* > Check if .frm file created by MySQL 5.0. In this case we want to > diff --git a/storage/maria/ha_maria.cc b/storage/maria/ha_maria.cc > index ed9c5404a7f..1f99fd9d895 100644 > --- a/storage/maria/ha_maria.cc > +++ b/storage/maria/ha_maria.cc > @@ -2724,7 +2724,7 @@ void ha_maria::drop_table(const char *name) > { > DBUG_ASSERT(file->s->temporary); > (void) ha_close(); > - (void) maria_delete_table_files(name, 1, 0); > + (void) maria_delete_table_files(name, 1, MY_WME); MYF(MY_WME) as always? > } > > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 1

Re: [Maria-developers] 9243921c84b: Make all #sql temporary table names uniform
by Sergei Golubchik 18 Apr '20

18 Apr '20

Hi, Michael! On Apr 13, Michael Widenius wrote: > revision-id: 9243921c84b (mariadb-10.5.2-128-g9243921c84b) > parent(s): 8742d176bc2 > author: Michael Widenius <monty(a)mariadb.com> > committer: Michael Widenius <monty(a)mariadb.com> > timestamp: 2020-04-09 22:02:06 +0300 > message: > > Make all #sql temporary table names uniform > > The reason for this is to make all temporary file names similar and > also to be able to figure out from where a #sql-xxx name orginates. > > New format is for most cases: > #sql-name-current_pid-thread_id[-increment] > Where name is one of subselect, alter, exchange, temptable or backup > > The execptions are: typo, "exceptions" > ALTER PARTITION shadow files: > #sql-shadow-'original_table_name' Please, add a thread_id here at the end. normally MDL should ensure that no two threads can have a shadow for the same table at the same time, but we have enough bugs as it is to introduce another vector when two threads can overwrite each other temp files. > Names from the temp pool: > #sql-name-current_pid-pool_number Would you mind if I drop temp pool completely in 10.5? It was added by Jeremy in January 2001 with the comment Added --temp-pool option to mysqld. This will cause temporary files created to use a small set of filenames, to try and avoid problems in the Linux kernel. And I doubt it's still an issue in 2020. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

3 4

Re: [Maria-developers] faf8de3aa98: Fixed some assert crashes related to keyread.
by Sergei Golubchik 18 Apr '20

18 Apr '20

Hi, Michael! On Apr 13, Michael Widenius wrote: > revision-id: faf8de3aa98 (mariadb-10.5.2-121-gfaf8de3aa98) > parent(s): 3cbe15bd78c > author: Michael Widenius <monty(a)mariadb.com> > committer: Michael Widenius <monty(a)mariadb.com> > timestamp: 2020-04-09 01:37:02 +0300 > message: > > Fixed some assert crashes related to keyread. > > - MDEV-22062 Assertion `!table->file->keyread_enabled()' failed in > close_thread_table() > - MDEV-22077 table->no_keyread .. failed in join_read_first() > > diff --git a/mysql-test/main/keyread.test b/mysql-test/main/keyread.test > index d9d3002d392..76d0f5674dd 100644 > --- a/mysql-test/main/keyread.test > +++ b/mysql-test/main/keyread.test > @@ -8,3 +8,14 @@ create view v1 as select * from t1 where f2 = 1; > select distinct f1 from v1; > drop view v1; > drop table t1; > + > +# > +# MDEV-22062 Assertion `!table->file->keyread_enabled()' failed in > +# close_thread_table > +# > + > +CREATE TABLE t1 (a INT NOT NULL, UNIQUE(a)) ENGINE=InnoDB; > +INSERT INTO t1 VALUES (1),(2); > +DELETE FROM t1 ORDER BY a LIMIT 1; > +SELECT * FROM t1; > +DROP TABLE t1; where's a test case for MDEV-22077? > diff --git a/sql/sql_delete.cc b/sql/sql_delete.cc > index 19eabbb053c..2fc0de4345f 100644 > --- a/sql/sql_delete.cc > +++ b/sql/sql_delete.cc > @@ -547,10 +547,12 @@ bool mysql_delete(THD *thd, TABLE_LIST *table_list, COND *conds, > else > { > ha_rows scanned_limit= query_plan.scanned_rows; > + table->no_keyread= 1; > query_plan.index= get_index_for_order(order, table, select, limit, > &scanned_limit, > &query_plan.using_filesort, > &reverse); > + table->no_keyread= 0; why? > if (!query_plan.using_filesort) > query_plan.scanned_rows= scanned_limit; > } > diff --git a/sql/sql_select.cc b/sql/sql_select.cc > index 129dae9eedb..1f05156b96f 100644 > --- a/sql/sql_select.cc > +++ b/sql/sql_select.cc > @@ -23584,7 +23584,7 @@ test_if_skip_sort_order(JOIN_TAB *tab,ORDER *order,ha_rows select_limit, > If ref_key used index tree reading only ('Using index' in EXPLAIN), > and best_key doesn't, then revert the decision. > */ > - if (table->covering_keys.is_set(best_key)) > + if (table->covering_keys.is_set(best_key) && !table->no_keyread) > table->file->ha_start_keyread(best_key); > else > table->file->ha_end_keyread(); > @@ -28568,8 +28568,6 @@ test_if_cheaper_ordering(const JOIN_TAB *tab, ORDER *order, TABLE *table, > if (new_used_key_parts != NULL) > *new_used_key_parts= best_key_parts; > table->file->ha_end_keyread(); > - if (is_best_covering && !table->no_keyread) > - table->file->ha_start_keyread(best_key); I find it very confusing that test_if_cheaper_ordering has a side effect of ending keyread. It was ok, when it ended previous keyread and started a new one. But just disabling - it's looks very strange. > DBUG_RETURN(TRUE); > } Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 3

Re: [Maria-developers] bc5c062b1d1: Don't try to open temporary tables if there are no temporary tables
by Sergei Golubchik 17 Apr '20

17 Apr '20

Hi, Michael! On Apr 13, Michael Widenius wrote: > revision-id: bc5c062b1d1 (mariadb-10.5.2-124-gbc5c062b1d1) > parent(s): fb29c886701 > author: Michael Widenius <monty(a)mariadb.com> > committer: Michael Widenius <monty(a)mariadb.com> > timestamp: 2020-04-09 01:37:02 +0300 > message: > > Don't try to open temporary tables if there are no temporary tables Why? > diff --git a/sql/sql_base.cc b/sql/sql_base.cc > index 4d7a7606136..be19a2e1d82 100644 > --- a/sql/sql_base.cc > +++ b/sql/sql_base.cc > @@ -3704,9 +3704,9 @@ open_and_process_table(THD *thd, TABLE_LIST *tables, uint *counter, uint flags, > The problem is that since those attributes are not set in merge > children, another round of PREPARE will not help. > */ > - error= thd->open_temporary_table(tables); > - > - if (!error && !tables->table) > + if (!thd->has_temporary_tables() || > + (!(error= thd->open_temporary_table(tables)) && > + !tables->table)) please don't write conditions like that. keep it as before, two statements: if (thd->has_temporary_tables()) error= thd->open_temporary_table(tables); if (!error && !tables->table) > error= open_table(thd, tables, ot_ctx); > > thd->pop_internal_handler(); > @@ -3723,9 +3723,9 @@ open_and_process_table(THD *thd, TABLE_LIST *tables, uint *counter, uint flags, > Repair_mrg_table_error_handler repair_mrg_table_handler; > thd->push_internal_handler(&repair_mrg_table_handler); > > - error= thd->open_temporary_table(tables); > - > - if (!error && !tables->table) > + if (!thd->has_temporary_tables() || > + (!(error= thd->open_temporary_table(tables)) && > + !tables->table)) > error= open_table(thd, tables, ot_ctx); > > thd->pop_internal_handler(); > @@ -3740,7 +3740,8 @@ open_and_process_table(THD *thd, TABLE_LIST *tables, uint *counter, uint flags, > still might need to look for a temporary table if this table > list element corresponds to underlying table of a merge table. > */ > - error= thd->open_temporary_table(tables); > + if (thd->has_temporary_tables()) > + error= thd->open_temporary_table(tables); > } > > if (!error && !tables->table) > diff --git a/sql/temporary_tables.cc b/sql/temporary_tables.cc > index 407072a7b49..553460729a1 100644 > --- a/sql/temporary_tables.cc > +++ b/sql/temporary_tables.cc > @@ -338,9 +338,9 @@ bool THD::open_temporary_table(TABLE_LIST *tl) > have invalid db or table name. > Instead THD::open_tables() should be used. > */ > - DBUG_ASSERT(!tl->derived && !tl->schema_table); > + DBUG_ASSERT(!tl->derived && !tl->schema_table && has_temporary_tables()); please, never do asserts with &&-ed conditions, this should be DBUG_ASSERT(!tl->derived); DBUG_ASSERT(!tl->schema_table); DBUG_ASSERT(has_temporary_tables()); > - if (tl->open_type == OT_BASE_ONLY || !has_temporary_tables()) > + if (tl->open_type == OT_BASE_ONLY) > { > DBUG_PRINT("info", ("skip_temporary is set or no temporary tables")); > DBUG_RETURN(false); Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 2

Re: [Maria-developers] bf32018be96: Added support for VISIBLE attribute for indexes in CREATE TABLE
by Sergei Golubchik 17 Apr '20

17 Apr '20

Hi, Michael! On Apr 13, Michael Widenius wrote: > revision-id: bf32018be96 (mariadb-10.5.2-126-gbf32018be96) > parent(s): 62c2d0f3e1f > author: Michael Widenius <monty(a)mariadb.com> > committer: Michael Widenius <monty(a)mariadb.com> > timestamp: 2020-04-09 01:37:02 +0300 > message: > > Added support for VISIBLE attribute for indexes in CREATE TABLE > > MDEV-22199 Add VISIBLE attribute for indexes in CREATE TABLE > > This was done to make it easier to read in dumps from MySQL 8.0 This is not a "dump from MySQL 8.0". It is an SQL export file generated by MySQL Workbench. And MySQL Workbench has a configuration option whether to put this VISIBLE in the sql file or not. This is a client tool misconfiguration issue. I think the user can trivially tell his tool not to generate MariaDB-incompatible syntax and we don't have to change the server for this. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 2

Re: [Maria-developers] fb29c886701: Handle errors from external_unlock & mysql_unlock_tables
by Sergei Golubchik 16 Apr '20

16 Apr '20

Hi, Michael! On Apr 13, Michael Widenius wrote: > revision-id: fb29c886701 (mariadb-10.5.2-123-gfb29c886701) > parent(s): 22fb7f8995c > author: Michael Widenius <monty(a)mariadb.com> > committer: Michael Widenius <monty(a)mariadb.com> > timestamp: 2020-04-09 01:37:02 +0300 > message: > > Handle errors from external_unlock & mysql_unlock_tables > > Other things: > - Handler errors from ha_maria::implict_commit > - Disable DBUG in safe_mutex_lock to get trace file easier to read > > diff --git a/mysys/thr_mutex.c b/mysys/thr_mutex.c > index 4f495048f63..f32132136b8 100644 > --- a/mysys/thr_mutex.c > +++ b/mysys/thr_mutex.c > @@ -233,6 +233,7 @@ int safe_mutex_lock(safe_mutex_t *mp, myf my_flags, const char *file, > int error; > DBUG_PRINT("mutex", ("%s (0x%lx) locking", mp->name ? mp->name : "Null", > (ulong) mp)); > + DBUG_PUSH(""); > > pthread_mutex_lock(&mp->global); > if (!mp->file) > @@ -283,7 +284,7 @@ int safe_mutex_lock(safe_mutex_t *mp, myf my_flags, const char *file, > { > error= pthread_mutex_trylock(&mp->mutex); > if (error == EBUSY) > - return error; > + goto end; > } > else > error= pthread_mutex_lock(&mp->mutex); > @@ -393,6 +394,8 @@ int safe_mutex_lock(safe_mutex_t *mp, myf my_flags, const char *file, > } > } > > +end: > + DBUG_POP(); > DBUG_PRINT("mutex", ("%s (0x%lx) locked", mp->name, (ulong) mp)); this message is no longer always correct, is it? > return error; > } Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 1

Re: [Maria-developers] 3cbe15bd78c: Fixed core dump in alter table if ADD PARTITION fails
by Sergei Golubchik 16 Apr '20

16 Apr '20

Hi, Michael! On Apr 13, Michael Widenius wrote: > revision-id: 3cbe15bd78c (mariadb-10.5.2-120-g3cbe15bd78c) > parent(s): d4d332d196d > author: Michael Widenius <monty(a)mariadb.com> > committer: Michael Widenius <monty(a)mariadb.com> > timestamp: 2020-04-09 01:37:01 +0300 > message: > > Fixed core dump in alter table if ADD PARTITION fails > > I didn't add a test case as to reproduce this we need to > have a failed write in on of the engines. Bug found and bug fix > verified while debugging S3 and partitions. > > --- > sql/sql_partition.cc | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/sql/sql_partition.cc b/sql/sql_partition.cc > index ef8ef5114a8..4e984fa775d 100644 > --- a/sql/sql_partition.cc > +++ b/sql/sql_partition.cc > @@ -6817,7 +6817,8 @@ static void handle_alter_part_error(ALTER_PARTITION_PARAM_TYPE *lpt, > DBUG_ENTER("handle_alter_part_error"); > DBUG_ASSERT(table->m_needs_reopen); > > - if (close_table) > + /* The table may not be open if ha_partition::change_partitions() failed */ > + if (close_table && !table->file->is_open()) This looks *very* confusing, like you close the table only if it's *not* open. But then the whole if() is very confusing, as it closes in both branches, so the meaning of "close_table" is totally not clear. May be instead of this your fix you'd better cherry-pick 9c02b7d6670b069866 from MySQL? It removes close_table completely, so I expect it to work for S3 just as you wanted. > { > /* > All instances of this table needs to be closed. > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 1

Re: [Maria-developers] 62c2d0f3e1f: Added mariadb-config.1 to .gitignore
by Sergei Golubchik 13 Apr '20

13 Apr '20

Hi, Michael! On Apr 13, Michael Widenius wrote: > revision-id: 62c2d0f3e1f (mariadb-10.5.2-125-g62c2d0f3e1f) > parent(s): bc5c062b1d1 > author: Michael Widenius <monty(a)mariadb.com> > committer: Michael Widenius <monty(a)mariadb.com> > timestamp: 2020-04-09 01:37:02 +0300 > message: > > Added mariadb-config.1 to .gitignore Where does it come from, where is it created? I've just grepped the whole tree, I don't see it ever gets created anywhere, so why to ignore it? > diff --git a/.gitignore b/.gitignore > index 8d3b5245447..8bd46093cbe 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -581,6 +581,7 @@ man/mariadb-check.1 > man/mariadb-client-test.1 > man/mariadb-client-test-embedded.1 > man/mariadb_config.1 > +man/mariadb-config.1 > man/mariadb-convert-table-format.1 > man/mariadbd.8 > man/mariadbd-multi.1 > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 22fb7f8995c: Updated client and server to use new binary names in --debug traces
by Sergei Golubchik 13 Apr '20

13 Apr '20

Hi, Michael! On Apr 13, Michael Widenius wrote: > revision-id: 22fb7f8995c (mariadb-10.5.2-122-g22fb7f8995c) > parent(s): faf8de3aa98 > author: Michael Widenius <monty(a)mariadb.com> > committer: Michael Widenius <monty(a)mariadb.com> > timestamp: 2020-04-09 01:37:02 +0300 > message: > > Updated client and server to use new binary names in --debug traces ok to push Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 9941c6a3179: MDEV-17554 Auto-create new partition for system versioned tables with history partitioned by INTERVAL/LIMIT
by Sergei Golubchik 13 Apr '20

13 Apr '20

Hi, Aleksey! On Apr 07, Aleksey Midenkov wrote: > revision-id: 9941c6a3179 (mariadb-10.5.2-164-g9941c6a3179) > parent(s): 920c3c6b237 > author: Aleksey Midenkov <midenok(a)gmail.com> > committer: Aleksey Midenkov <midenok(a)gmail.com> > 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? > 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? > +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? > +# > +# 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? > 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? > -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 > +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. > +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 > +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 > +select * from t1; > +x > +1 I don't understand, there was an error above, why did insert succeed? > +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 > +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? 2. please add a test with insert under start transaction. what should happen in this case? > { > 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<partition_element> 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) 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; 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. > + { > + TABLE_SHARE *share; > + List_iterator_fast<TABLE_SHARE> 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? > + 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<partition_element> 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<partition_element> 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. > +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. > + 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? > 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(a)mariadb.org

2 3