- developers - lists.mariadb.org

Re: [Maria-developers] 1581f65be57: MDEV-13301 Optimize DROP INDEX, ADD INDEX into RENAME INDEX
by Sergei Golubchik 02 Apr '19

02 Apr '19

Hi, Eugene! See the review below. This looked pretty much ok, a couple of style comments. And why did you not implement the new ALTER TABLE .. .RENAME INDEX syntax? I agree that a pair of DROP/ADD should act as RENAME. But a dedicated RENAME would be also good to have, at least for compatibility reasons. On Apr 01, Eugene Kosov wrote: > revision-id: 1581f65be57 (mariadb-10.4.3-69-g1581f65be57) > parent(s): e012d266807 > author: Eugene Kosov <claprix(a)yandex.ru> > committer: Eugene Kosov <claprix(a)yandex.ru> > timestamp: 2019-03-15 18:44:05 +0300 > message: > > MDEV-13301 Optimize DROP INDEX, ADD INDEX into RENAME INDEX > > Just rename index in data dictionary and in InnoDB cache when it's possible. > Introduce ALTER_INDEX_RENAME for that purpose so that engines can optimize > such operation. > > Unused code between macro MYSQL_RENAME_INDEX was removed. > > compare_keys_but_name(): compare index definitions except for index names > > Alter_inplace_info::rename_keys: > ha_innobase_inplace_ctx::rename_keys: vector of rename indexes > > fill_alter_inplace_info():: fills Alter_inplace_info::rename_keys > > diff --git a/sql/sql_table.cc b/sql/sql_table.cc > index e1337a0f710..e1131422117 100644 > --- a/sql/sql_table.cc > +++ b/sql/sql_table.cc > @@ -7062,6 +7061,43 @@ static bool fill_alter_inplace_info(THD *thd, TABLE *table, bool varchar, > new_key->option_struct; > } > > + uint &add_count= ha_alter_info->index_add_count; Coding style: no modifiable references please. write ha_alter_info->index_add_count++ same below > + for (uint i= 0; i < add_count; i++) > + { > + uint *&add_buffer= ha_alter_info->index_add_buffer; > + const KEY *new_key= ha_alter_info->key_info_buffer + add_buffer[i]; > + > + uint &drop_count= ha_alter_info->index_drop_count; > + for (uint j= 0; j < drop_count; j++) > + { > + KEY **&drop_buffer= ha_alter_info->index_drop_buffer; > + const KEY *old_key= drop_buffer[j]; > + > + if (!lex_string_cmp(system_charset_info, &old_key->name, &new_key->name)) > + continue; Why? I'd think and if it's Compare_keys::Equal and the name is the same too, you can simply remove add/drop elements without adding a rename element. > + > + if (Compare_keys::Equal != compare_keys_but_name(old_key, new_key, > + alter_info, table, > + new_pk, old_pk)) Coding style: don't invert comparisons > + { > + continue; > + } > + > + ha_alter_info->handler_flags|= ALTER_RENAME_INDEX; > + ha_alter_info->rename_keys.push_back( > + Alter_inplace_info::Rename_key_pair(old_key, new_key)); > + > + memcpy(add_buffer + i, add_buffer + i + 1, > + sizeof(add_buffer[0]) * (add_count - i - 1)); > + memcpy(drop_buffer + j, drop_buffer + j + 1, > + sizeof(drop_buffer[0]) * (drop_count - j - 1)); > + --add_count; > + --drop_count; > + --i; // this index once again > + break; > + } > + } > + Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 3

Re: [Maria-developers] 126a9b6db25: MDEV-18875 Assertion `thd->transaction.stmt.ha_list == __null || trans == &thd->transaction.stmt' failed or bogus ER_DUP_ENTRY upon ALTER TABLE with versioning
by Sergei Golubchik 02 Apr '19

02 Apr '19

Hi, Nikita! On Apr 02, Nikita Malyavin wrote: > revision-id: 126a9b6db25 (mariadb-10.3.12-101-g126a9b6db25) > parent(s): 1c60f40868f > author: Nikita Malyavin <nikitamalyavin(a)gmail.com> > committer: Nikita Malyavin <nikitamalyavin(a)gmail.com> > timestamp: 2019-03-23 03:37:10 +1000 > message: > > MDEV-18875 Assertion `thd->transaction.stmt.ha_list == __null || trans == &thd->transaction.stmt' failed or bogus ER_DUP_ENTRY upon ALTER TABLE with versioning > > diff --git a/sql/handler.cc b/sql/handler.cc > index e01392d236c..9d295afb631 100644 > --- a/sql/handler.cc > +++ b/sql/handler.cc > @@ -1420,7 +1420,8 @@ int ha_commit_trans(THD *thd, bool all) > > #if 1 // FIXME: This should be done in ha_prepare(). > if (rw_trans || (thd->lex->sql_command == SQLCOM_ALTER_TABLE && > - thd->lex->alter_info.flags & ALTER_ADD_SYSTEM_VERSIONING)) > + thd->lex->alter_info.flags & ALTER_ADD_SYSTEM_VERSIONING && > + is_real_trans)) Why there's a special check for SQLCOM_ALTER_TABLE + ALTER_ADD_SYSTEM_VERSIONING in the first place? Does it mean that ALTER TABLE ... ADD SYSTEM VERSIONING does not marks a transaction as read/write? That's be weird. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 2

Re: [Maria-developers] 5469d88e7b4: MDEV-19049 Server crashes in check_duplicate_long_entry_key, ASAN stack-buffer-overflow in Field_blob::get_key_image
by Sergei Golubchik 02 Apr '19

02 Apr '19

Hi, Sachin! On Apr 02, Sachin Setiya wrote: > revision-id: 5469d88e7b4 (mariadb-10.4.3-104-g5469d88e7b4) > parent(s): 0bc42602266 > author: Sachin <sachin.setiya(a)mariadb.com> > committer: Sachin <sachin.setiya(a)mariadb.com> > timestamp: 2019-03-28 11:29:25 +0530 > message: > > MDEV-19049 Server crashes in check_duplicate_long_entry_key, ASAN stack-buffer-overflow in Field_blob::get_key_image > > Long Unique keys should always be last unique key. Why? > diff --git a/sql/sql_table.cc b/sql/sql_table.cc > index ad62ecc1103..c755a74e174 100644 > --- a/sql/sql_table.cc > +++ b/sql/sql_table.cc > @@ -2796,6 +2797,14 @@ static int sort_keys(KEY *a, KEY *b) > { > if (!(b_flags & HA_NOSAME)) > return -1; > + /* > + Long Unique keys should always be last unique key. > + Before this patch they used to change order wrt to partial keys (MDEV-19049) don't write the code history "before this patch it worked like that" in the code itself. When you look at the code there is no "patch", just a comment. The concepts of "patch" and "before it used to be" belongs to the history, and logically should be in the commit comment, not in a code comment. > + */ > + if (a->algorithm == HA_KEY_ALG_LONG_HASH) > + return 1; > + if (b->algorithm == HA_KEY_ALG_LONG_HASH) > + return -1; > if ((a_flags ^ b_flags) & HA_NULL_PART_KEY) > { > /* Sort NOT NULL keys before other keys */ > Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

[Maria-developers] GSOC 2019 proposal draft
by Алексей Могилёвкин 02 Apr '19

02 Apr '19

Hello. My name is Alexey Mogilyovkin. I would like to contribute to mariadb during GSOC 2019. More information can be found in my draft of proposal on the attached google doc. Any feedback would be appreciated. MariaDB proposal <https://docs.google.com/document/d/12utv0vUoV2FfPMSH2fOR3lrYXT_mS5er_S1XVuM…>

3 2

[Maria-developers] Buildbot image update request
by Otto Kekäläinen 01 Apr '19

01 Apr '19

Hello! Can you please update the image used by test kvm-deb-jessie-ppc64le so that the 'apt-get update' is fresh and package lists are recent? It would fix the error in http://buildbot.askmonty.org/buildbot/builders/kvm-deb-jessie-ppc64le/build… E: Failed to fetch http://ftp.us.debian.org/debian/pool/main/m/mpfr4/libmpfr4_3.1.2-2_ppc64el.… 404 Not Found [IP: 64.50.233.100 80] E: Failed to fetch http://ftp.us.debian.org/debian/pool/main/g/gcc-4.9/libatomic1_4.9.2-10_ppc… 404 Not Found [IP: 64.50.233.100 80] E: Failed to fetch http://ftp.us.debian.org/debian/pool/main/r/rsync/rsync_3.1.1-3_ppc64el.deb 404 Not Found [IP: 64.50.233.100 80] E: Failed to fetch http://ftp.us.debian.org/debian/pool/main/s/socat/socat_1.7.2.4-2_ppc64el.d… 404 Not Found [IP: 64.50.233.100 80] E: Failed to fetch http://ftp.us.debian.org/debian/pool/main/libd/libdbd-mysql-perl/libdbd-mys… 404 Not Found [IP: 64.50.233.100 80] Also, can you please run 'apt-get install libzstd-dev' in the image used in arm64-xenial-deb? It would fix http://buildbot.askmonty.org/buildbot/builders/arm64-xenial-deb/builds/2973… dpkg-checkbuilddeps: error: Unmet build dependencies: libzstd-dev dpkg-buildpackage: warning: build dependencies/conflicts unsatisfied; aborting Thanks!

2 1

Re: [Maria-developers] required mysql_upgrade; MDEV-14637
by Michal Schorm 01 Apr '19

01 Apr '19

On Thu, Mar 28, 2019 at 11:16 AM Michal Schorm <mschorm(a)redhat.com> wrote: > > Hello, > I have a questions regarding the warnings showing up at 10.2 and 10.3 releases: > > When upgrading from MariaDB 10.2.16 or earlier to MariaDB 10.2.17 or higher, > running mysql_upgrade is required due to changes introduced in MDEV-14637. > > When upgrading from MariaDB 10.3.8 or earlier to MariaDB 10.3.9 or higher, > running mysql_upgrade is required due to changes introduced in MDEV-14637. > > -- > > 1) The same applies for upgrading <=10.2.16 to >= 10.3.9, right? > > 2) Does it mean, it is impossible to use "dump and restore" technique, > or it only means I need to run the mysql_upgrade afterwards? > > 3) The problem is not triggered when upgrading from 5.5, 10.0 and 10.1? > 4) The problem is not triggered when upgrading to 10.4 ? I found a reply in the mailing list archive which haven't reached my inbox: From: Reindl Harald Date: Thu, 28 Mar 2019 11:59:50 +0100 > just run "mysql_upgrade" after each update and you are done > our production datadirs date back to 2002, originally on windows then > moved to MacOS and in 2008 to Fedora and i did not dump/restore a single > time in my whole life, this is not postgresql My question is focused on the clarification in our downstream documentation. So while usually I'd be fine with personal experience, in this case, I'd prefer more in-depth answer covering. I'm forwarding this mail to devel mailing list. -- Michal Schorm Software Engineer Core Services - Databases Team Red Hat --

1 0

Re: [Maria-developers] 9976699033c: Test fixes (versioning.replace, versioning.foreign)
by Sergei Golubchik 31 Mar '19

31 Mar '19

Hi, Aleksey! On Mar 27, Aleksey Midenkov wrote: > revision-id: 9976699033c (mariadb-10.3.7-30-g9976699033c) > parent(s): c0f97710582 > author: Aleksey Midenkov <midenok(a)gmail.com> > committer: Aleksey Midenkov <midenok(a)gmail.com> > timestamp: 2018-05-30 13:12:47 +0300 > message: > > Test fixes (versioning.replace, versioning.foreign) > > * replace.test: unwrapped create_table procedure; > * foreign.test: use key_type combinations. > > diff --git a/mysql-test/suite/versioning/key_type.combinations b/mysql-test/suite/versioning/key_type.combinations > index 1929aee9a84..fb3888d3d0f 100644 > --- a/mysql-test/suite/versioning/key_type.combinations > +++ b/mysql-test/suite/versioning/key_type.combinations > @@ -1,2 +1,3 @@ > [unique] > -[pk] > +[primary] > +[secondary] generally prefer shorter combination names, otherwise mtr output becomes garbled and difficult to read. > diff --git a/mysql-test/suite/versioning/primary_or_unique.inc b/mysql-test/suite/versioning/primary_or_unique.inc > new file mode 100644 > index 00000000000..199d8bab942 > --- /dev/null > +++ b/mysql-test/suite/versioning/primary_or_unique.inc > @@ -0,0 +1,5 @@ > +--source suite/versioning/key_type.inc > +--require suite/versioning/primary_or_unique.require don't do .require, please, it's a technique from 1998, before mysqltest had the "skip" command. Use if (...) { skip; } > +disable_query_log; > +eval select "$KEY_TYPE" != "key" as pk_or_unique; > +enable_query_log; > diff --git a/mysql-test/suite/versioning/t/engines.combinations b/mysql-test/suite/versioning/t/engines.combinations > deleted file mode 100644 > index 561c5656929..00000000000 > --- a/mysql-test/suite/versioning/t/engines.combinations > +++ /dev/null > @@ -1,8 +0,0 @@ > -[timestamp] > -default-storage-engine=innodb > - > -[trx_id] > -default-storage-engine=innodb > - > -[myisam] > -default-storage-engine=myisam Is that intentional? I thought many tests use it to run in different versioning modes. > diff --git a/mysql-test/suite/versioning/t/foreign.test b/mysql-test/suite/versioning/t/foreign.test > index 566d481c2a8..72e5b047cbb 100644 > --- a/mysql-test/suite/versioning/t/foreign.test > +++ b/mysql-test/suite/versioning/t/foreign.test > @@ -1,11 +1,14 @@ > +--source suite/versioning/primary_only.inc > --source suite/versioning/common.inc what's the point of this? if you're only allowing primary key here, you can just as well write manually "primary key" in all tests, without $KEY_TYPE and replace_result. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 1

[Maria-developers] EITS member variables in both TABLE and TABLE_SHARE?
by Sergey Petrunia 29 Mar '19

29 Mar '19

Hello Igor, TABLE_SHARE has TABLE_STATISTICS_CB, which has these members: struct TABLE_STATISTICS_CB { ... bool stats_is_read; /* Statistical data for table has been read from statistical tables */ bool histograms_are_read; } At the same time, TABLE has: struct TABLE { ... bool stats_is_read; /* Persistent statistics is read for the table */ bool histograms_are_read; I would like to understand why both structures need these variables? Is there any difference in meaning between them? BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

Re: [Maria-developers] 74b2eba1ca6: MDEV-15458 Segfault in heap_scan() upon UPDATE after ADD SYSTEM VERSIONING
by Sergei Golubchik 29 Mar '19

29 Mar '19

Hi, Aleksey! On Mar 26, Aleksey Midenkov wrote: > revision-id: 74b2eba1ca6 (mariadb-10.3.12-115-g74b2eba1ca6) > parent(s): 2c0901d808b > author: Aleksey Midenkov <midenok(a)gmail.com> > committer: Sergei Golubchik <serg(a)mariadb.com> > timestamp: 2019-03-26 17:43:38 +0100 > message: > > MDEV-15458 Segfault in heap_scan() upon UPDATE after ADD SYSTEM VERSIONING > > * HA_EXTRA_REMEMBER_POS, HA_EXTRA_RESTORE_POS for HEAP storage engine > * Versioning tests support > > diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc > index abe4254db3e..1d51947cc86 100644 > --- a/sql/sql_insert.cc > +++ b/sql/sql_insert.cc > @@ -1643,7 +1643,12 @@ int vers_insert_history_row(TABLE *table) > if (row_start->cmp(row_start->ptr, row_end->ptr) >= 0) > return 0; > > - return table->file->ha_write_row(table->record[0]); > + int res; > + if ((res= table->file->extra(HA_EXTRA_REMEMBER_POS))) > + return res; > + if ((res= table->file->ha_write_row(table->record[0]))) > + return res; > + return table->file->extra(HA_EXTRA_RESTORE_POS); > } Frankly, I don't like it. I mean, this particular fix is fine, but here's the problem: We now have (at least) three features where the server might need to do something (insert/update/search) during the index/rnd scan. And three different solutions: you use HA_EXTRA_REMEMBER_POS in system versioning, Nikita simply sets delete_while_scanning=false in application time period tables, and Sachin uses handler::clone() to create a second handler that can be used to check for hash collisions without disrupting the scan on the primary handler. I think it's getting somewhat out of hands. Can we please reduce the number of approaches to fix the same issue? Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

4 4

Re: [Maria-developers] ab6ef429a84: MDEV-16370 row-based binlog events (updates by primary key) can not be applied multiple times to system versioned tables
by Sergei Golubchik 29 Mar '19

29 Mar '19

Hi, Aleksey! On Jan 31, Aleksey Midenkov wrote: > revision-id: ab6ef429a84 (versioning-1.0.7-2-gab6ef429a84) > parent(s): 14c2f90ad08 > author: Aleksey Midenkov <midenok(a)gmail.com> > committer: Aleksey Midenkov <midenok(a)gmail.com> > timestamp: 2018-11-26 12:46:37 +0300 > message: > > MDEV-16370 row-based binlog events (updates by primary key) can not be applied multiple times to system versioned tables > > Case 2: > > * Idempotent BINLOG updates: duplicate key on history row is skipped; > * Fix row start and warn when applying BINLOG from the past. As far as I can see now you cannot just arbitrarily adjust row_start. Think of it in term or application time period tables, a primary key is (kind of) defined WITHOUT OVERLAPS. There is no explicit constraint, it happens automatically when the history is valid. Now, here's your test case: timestamp=100; insert (1,1) -> the table has (1,1,100,MAX); timestamp=300; update to (1,3) -> (1,1,100,300),(1,3,300,MAX); timestamp=200; update to (1,2) -> here the hisory row should've been (1,3,300,200) but you adjust it to get 1,1,100,300 1,3,199,200 1,2,200,MAX and if you select AS OF 199, you'll get two rows for the pk=1 So, let's * ignore HA_ERR_FOUND_DUPP_KEY (please check if it's needed for insert or delete too) * add TIMESTAMP=xxx to mysqldumps. can vers_insert_history_row see row_start>row_end in a normal use case? no replication or any other way of adjusting the timestamp. equality can surely happen, but greater than ? Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 1