June 2020 - developers - lists.mariadb.org

Re: [Maria-developers] e14dcfe0bc7: MDEV-22715: SIGSEGV in radixsort_for_str_ptr and in native_compare/my_qsort2 (optimized builds)
by Sergey Petrunia 03 Jun '20

03 Jun '20

Hi Varun, Please add a comment above the declaration of Field::sort_string() that some implementations actually may write up to 8 bytes regardless of what size was requested. Ok to push after that. > revision-id: e14dcfe0bc743aa6b4ed2b3c4f7e9314aa01d6eb (mariadb-10.1.43-158-ge14dcfe0bc7) > parent(s): db1f73b26edaace41345f19bca507a7848aa39a8 > author: Varun Gupta > committer: Varun Gupta > timestamp: 2020-05-29 21:13:38 +0530 > message: > > MDEV-22715: SIGSEGV in radixsort_for_str_ptr and in native_compare/my_qsort2 (optimized builds) > > For DECIMAL[(M[,D])] datatype max_sort_length was not being honoured which was leading to buffer > overflow while making the sort key. The fix to this problem would be to create sort keys for decimals > with atmost max_sort_key bytes > > Important: > The minimum value of max_sort_length has been raised to 8 (previously was 4), > so fixed size datatypes like DOUBLE and BIGINIT are not truncated for > lower values of max_sort_length. > ... BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

Re: [Maria-developers] 7df57cc8206: MDEV-22719 Long unique keys are not created when individual key_part->length < max_key_length but SUM(key_parts->length) > max_key_length
by Sergei Golubchik 03 Jun '20

03 Jun '20

Hi, Sachin! ok to push On Jun 03, Sachin wrote: > revision-id: 7df57cc8206 (mariadb-10.4.11-226-g7df57cc8206) > parent(s): 0f91c62d332 > author: Sachin <sachin.setiya(a)mariadb.com> > committer: Sachin <sachin.setiya(a)mariadb.com> > timestamp: 2020-06-03 13:47:39 +0530 > message: > > MDEV-22719 Long unique keys are not created when individual key_part->length < max_key_length but SUM(key_parts->length) > max_key_length > > Make UNIQUE HASH key in case when key_info->key_length > max_key_length > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 1e9ea675a18: MDEV-21282 Assertion 'mariadb_table' failed in gcol.innodb_virtual_debug_purge
by Sergei Golubchik 03 Jun '20

03 Jun '20

Hi, Thirunarayanan! Sure, it's purely InnoDB functionality. If you say that purge thread already hold MDL for the table, then open_purge_table() doesn't need to. Ok to push as you see fit. On Jun 03, Thirunarayanan Balathandayuthapani wrote: > revision-id: 1e9ea675a18 (mariadb-10.5.2-304-g1e9ea675a18) > parent(s): f22093ad392 > author: Thirunarayanan Balathandayuthapani <thiru(a)mariadb.com> > committer: Thirunarayanan Balathandayuthapani <thiru(a)mariadb.com> > timestamp: 2020-06-01 18:02:17 +0530 > message: > > MDEV-21282 Assertion 'mariadb_table' failed in gcol.innodb_virtual_debug_purge > > - commit ea37b144094a0c2ebfc6774047fd473c1b2a8658 (MDEV-16678) caused > a regression. when purge thread tries to open the table for virtual > column computation, there is no need to acquire MDL for the table. > Because purge thread already hold MDL for the table Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] c01b3d91690: MDEV-22551 Server crashes in skip_trailing_space upon ANALYZE on table with long unique, or Assertion `marked_for_read()' fails
by Sergei Golubchik 03 Jun '20

03 Jun '20

Hi, Sachin! On Jun 03, Sachin wrote: > revision-id: c01b3d91690 (mariadb-10.4.11-224-gc01b3d91690) > parent(s): 0552ed86782 > author: Sachin <sachin.setiya(a)mariadb.com> > committer: Sachin <sachin.setiya(a)mariadb.com> > timestamp: 2020-06-02 08:24:13 +0530 > message: > > MDEV-22551 Server crashes in skip_trailing_space upon ANALYZE on table with long unique, or Assertion `marked_for_read()' fails > > If Blob(or parent) field read_set is not set then Do not calc hash in > that case. No, I think this is wrong. The problem, again, is that read_set bits are set incorrectly, and the fix is, again, to set them correctly, not to perform more read_set checks at run time. Think about what's happening here. The server tries to calculate a generated column value (because gcol is present in read_set) when not all argument columns were read (because they are not in read_set). This is an impossible situation, a gcol can _only_ be in read_set if all its dependencies are also there, you've seen in TABLE::mark_virtual_columns_for_write() that for every marked gcol all dependencies are also recursively marked. So in this case ANALYZE breaks this rule and should be fixed not to do it. There are two possible fixes - either ANALYZE should mark dependencies for all gcols (marking blobs in this bug) or ANALYZE should skip and not mark gcols. Better ask somebody who's more famiiar with EITS about what fix is preferred. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] e2017fc3a5f: MDEV-22441 implement a generic way to change a value of a variable in scope
by Sergei Golubchik 03 Jun '20

03 Jun '20

Hi, Eugene! On Jun 03, Eugene Kosov wrote: > revision-id: e2017fc3a5f (mariadb-10.2.31-219-ge2017fc3a5f) This should go into 10.5, not into old-old GA 10.2. Besides, 10.2 cannot use C++11 features, 10.5 can. > parent(s): 1b3adaab25c > author: Eugene Kosov <claprix(a)yandex.ru> > committer: Eugene Kosov <eugene.kosov(a)mariadb.com> > timestamp: 2020-05-27 12:45:41 +0300 > message: > > MDEV-22441 implement a generic way to change a value of a variable in scope > > Scope_value<T> class changes a variable value to some temporary value while > it exits and restores an old value in it's destructor. > > make_scope_value() is a function targeted for C++11 which allows to omit > a template parameter which is required by Scope_value<T> I think I'd name it backup_and_change_scoped() or something like that. But it's just a suggestion, feel free to keep make_scope_value() name if you like it better. > Sql_mode_save removed and the only usage is replaced with Scope_value<T> > > Sql_mode_save removal should not be merged in newer branches! > > --- > include/scope.h | 46 ++++++++++++++++++++++++++++++++++++++++++++++ > sql/sql_acl.cc | 7 ++++--- > sql/sql_class.h | 16 ---------------- > 3 files changed, 50 insertions(+), 19 deletions(-) > > diff --git a/include/scope.h b/include/scope.h > new file mode 100644 > index 00000000000..81061ceed37 > --- /dev/null > +++ b/include/scope.h > @@ -0,0 +1,46 @@ > +/* > + Copyright (c) 2020, MariaDB Corporation. > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; version 2 of the License. > + > + This program is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program; if not, write to the Free Software > + Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335 USA */ > + > +#pragma once use include guards, please, not a pragma > + > +namespace maria and, as you remember, we didn't quite decide on a global server-wide namespace, so it's not the MDEV that should introduce it. > +{ > + > +template <typename T> class Scope_value > +{ > +public: > + Scope_value(T &variable, const T &scope_value) > + : variable_(variable), saved_value_(variable) > + { > + variable= scope_value; > + } > + > + ~Scope_value() { variable_= saved_value_; } > + > +private: > + T &variable_; > + T saved_value_; > +}; > + > +// Starting from C++11 this allows to omit template argument like this: > +// auto _ = make_scope_value(var, tmp_value); > +template <typename T> > +Scope_value<T> make_scope_value(T &variable, const T &scope_value) > +{ > + return Scope_value<T>(variable, scope_value); > +} as you've seen from Sql_mode_save usage, sometimes the value is assigned later, not when it's saved. Please, add another helper that doesn't require to provide a value. Like template <typename T> Scope_value<T> backup_scoped(T &variable) { return Scope_value<T>(variable, variable); } > + > +} // namespace maria Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 0552ed86782: MDEV-21804 Assertion `marked_for_read()' failed upon INSERT into table with long unique blob under binlog_row_image=NOBLOB
by Sergei Golubchik 03 Jun '20

03 Jun '20

Hi, Sachin! ok to push with one change, see below. On Jun 03, Sachin wrote: > revision-id: 0552ed86782 (mariadb-10.4.11-223-g0552ed86782) > parent(s): 61b2cd38d48 > author: Sachin <sachin.setiya(a)mariadb.com> > committer: Sachin <sachin.setiya(a)mariadb.com> > timestamp: 2020-06-02 08:24:02 +0530 > message: > > MDEV-21804 Assertion `marked_for_read()' failed upon INSERT into table with long unique blob under binlog_row_image=NOBLOB > > Problem:- Calling mark_columns_per_binlog_row_image() earlier may change the > result of mark_virtual_columns_for_write() , Since it can set the bitmap on > for virtual column, and henceforth mark_virtual_column_deps(field) will > never be called in mark_virtual_column_with_deps. > > This bug is not specific for long unique, It also fails for this case > create table t2(id int primary key, a blob, b varchar(20) as (LEFT(a,2))); > > diff --git a/sql/table.cc b/sql/table.cc > index bc8ddbc0f9d..8edbbe8d599 100644 > --- a/sql/table.cc > +++ b/sql/table.cc > @@ -7040,7 +7040,6 @@ void TABLE::mark_columns_needed_for_update() > DBUG_ENTER("TABLE::mark_columns_needed_for_update"); > bool need_signal= false; > > - mark_columns_per_binlog_row_image(); > > if (triggers) > triggers->mark_fields_used(TRG_EVENT_UPDATE); > @@ -7048,6 +7047,7 @@ void TABLE::mark_columns_needed_for_update() > mark_default_fields_for_write(FALSE); > if (vfield) > need_signal|= mark_virtual_columns_for_write(FALSE); > + mark_columns_per_binlog_row_image(); I think that's too early. In that if() below you can see > if (file->ha_table_flags() & HA_REQUIRES_KEY_COLUMNS_FOR_DELETE) > { > KEY *end= key_info + s->keys; ... if (bitmap_fast_test_and_set(read_set, idx)) continue; if (field[idx]->vcol_info) mark_virtual_col(field[idx]); and mark_columns_per_binlog_row_image() might break that. Better to put mark_columns_per_binlog_row_image() at the very end, just before signalling. > @@ -7132,7 +7132,6 @@ void TABLE::mark_columns_needed_for_update() > void TABLE::mark_columns_needed_for_insert() > { > DBUG_ENTER("mark_columns_needed_for_insert"); > - mark_columns_per_binlog_row_image(); > > if (triggers) > { > @@ -7152,6 +7151,7 @@ void TABLE::mark_columns_needed_for_insert() > /* Mark virtual columns for insert */ > if (vfield) > mark_virtual_columns_for_write(TRUE); > + mark_columns_per_binlog_row_image(); this looks ok. > if (check_constraints) > mark_check_constraint_columns_for_read(); > DBUG_VOID_RETURN; Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

[Maria-developers] Progress report - Week 4
by Mohammed Hammaad Mateen 02 Jun '20

02 Jun '20

Greetings, Hope you are safe and doing great, This post describes the things I've done in my fourth week [25-31 May] of Community Bonding Period under the mentor-ship of Sergei Golubchik and Oleksandr Byelkin for GSoC-20. The tasks taken up for this week was to study and analyze *INSERT SELECT RETURNING* and developing a test suite for *MULTIPLE TABLE DELETE RETURNING.* The following attachment contains the test cases for evaluating Multiple Delete Returning, as they are developed before actual implementation of code, please do notify me of any improvement needed, I'd be glad to resolve. Regards, Mohammed Hammaad Mateen

2 1

Re: [Maria-developers] fab8349af86: MDEV-21804 Assertion `marked_for_read()' failed upon INSERT into table with long unique blob under binlog_row_image=NOBLOB
by Sergei Golubchik 01 Jun '20

01 Jun '20

Hi, Sachin! On May 31, Sachin wrote: > revision-id: fab8349af86 (mariadb-10.4.11-223-gfab8349af86) > parent(s): 61b2cd38d48 > author: Sachin <sachin.setiya(a)mariadb.com> > committer: Sachin <sachin.setiya(a)mariadb.com> > timestamp: 2020-05-29 11:56:45 +0530 > message: > > MDEV-21804 Assertion `marked_for_read()' failed upon INSERT into table with long unique blob under binlog_row_image=NOBLOB > > Problem:- > In the case of BINLOG_ROW_IMAGE_NOBLOB table->readset is PKE + non blob fields > But while updating virtual fields we need to read the blob field, which will > generate assert failure. > Solution:- > If binlog_row_image == NOBLOB, then set read_bit for the blob field. > This bug is not specific for long unique, It also fails for this case > create table t2(id int primary key, a blob, b varchar(20) as (LEFT(a,2))); > > diff --git a/sql/field.cc b/sql/field.cc > index 0a8fdc3d3f5..bcad056e76a 100644 > --- a/sql/field.cc > +++ b/sql/field.cc > @@ -8467,6 +8467,10 @@ longlong Field_blob::val_int(void) > String *Field_blob::val_str(String *val_buffer __attribute__((unused)), > String *val_ptr) > { > + ulong binlog_row_image= table->in_use->variables.binlog_row_image; > + if (table->read_set && !bitmap_is_set(table->read_set, field_index) && > + binlog_row_image == BINLOG_ROW_IMAGE_NOBLOB) > + bitmap_set_bit(table->read_set, field_index); > DBUG_ASSERT(marked_for_read()); > char *blob; > memcpy(&blob, ptr+packlength, sizeof(char*)); A couple of red flags here. First, you don't fix BINLOG_ROW_IMAGE_MINIMAL. And generally you should not modify read/write sets from val_str, if should be done earlier before the execution phase. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0