- developers - lists.mariadb.org

Re: [Maria-developers] 34098adf32b: Get rid of rea_create_table()
by Sergei Golubchik 14 Mar '19

14 Mar '19

Hi, Sergey! On Mar 13, Sergey Vojtovich wrote: > revision-id: 34098adf32b (mariadb-10.4.1-138-g34098adf32b) > parent(s): d953dc02d9a > author: Sergey Vojtovich <svoj(a)mariadb.org> > committer: Sergey Vojtovich <svoj(a)mariadb.org> > timestamp: 2019-02-07 20:04:51 +0400 > message: > > Get rid of rea_create_table() > > Moved rea_create_table() to the sole caller. > > Visible difference: regular ALTER TABLE doesn't create .frm for temporary > and discoverable tables anymore. > > Also ha_create_partitioning_metadata(CHF_CREATE_FLAG) does cleanup on > error now. > > Part of MDEV-17805 - Remove InnoDB cache for temporary tables. ok to push Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] d953dc02d9a: Fix inplace ALTER TABLE to not register tmp table
by Sergei Golubchik 14 Mar '19

14 Mar '19

Hi, Sergey! On Mar 13, Sergey Vojtovich wrote: > revision-id: d953dc02d9a (mariadb-10.4.1-137-gd953dc02d9a) > parent(s): 9496394002f > author: Sergey Vojtovich <svoj(a)mariadb.org> > committer: Sergey Vojtovich <svoj(a)mariadb.org> > timestamp: 2019-02-07 20:04:38 +0400 > message: > > Fix inplace ALTER TABLE to not register tmp table > > Do not register intermediate tables created by inplace ALTER TABLE in > THD::temporary_tables. > > For inplace ALTER TABLE moved .frm creation to > create_table_for_inplace_alter(). For regular ALTER TABLE it is (and was > redundantly) created by ha_create_table(). > > Removed open_in_engine argument of create_and_open_tmp_table() and > open_temporary_table(): it became unused after this patch. > > Part of MDEV-17805 - Remove InnoDB cache for temporary tables. Looks ok code wise. But I don't see what are you gaining by this transformation. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

[Maria-developers] Self Introduction (gsoc)
by shreya pohekar 14 Mar '19

14 Mar '19

Hey! I am Shreya Pohekar. I am currently in my 3rd year of BE in Computer Science and Engineering from University Institute of Technology, RGPV Bhopal. I have a conceptual proficiency on Java and C++. On scrolling through the projects of mariadb, i found most of the projects interesting. But i would like to contribute to the project, "Implement multi-table UPDATE/DELETE returning a result set".@Igor Babaev, If you can guide me on where to get started, that would be great. Looking forward to hearing from you. Thank you.

1 0

[Maria-developers] Self Introduction (gsoc)
by shreya pohekar 14 Mar '19

14 Mar '19

Hey! I am Shreya Pohekar. I am currently in my 3rd year of BE in Computer Science and Engineering from University Institute of Technology, RGPV Bhopal. I have a conceptual proficiency on Java and C++. On scrolling through the projects of mariadb, i found most of the projects interesting. But i would like to contribute to the project, "Implement multi-table UPDATE/DELETE returning a result set". If you can guide me on where to get started, that would be great. Looking forward to hearing from you. Thank you.

1 0

Re: [Maria-developers] 9496394002f: Removed redundant partitioning check
by Sergei Golubchik 14 Mar '19

14 Mar '19

Hi, Sergey! On Mar 13, Sergey Vojtovich wrote: > revision-id: 9496394002f (mariadb-10.4.1-136-g9496394002f) > parent(s): 311beaffa63 > author: Sergey Vojtovich <svoj(a)mariadb.org> > committer: Sergey Vojtovich <svoj(a)mariadb.org> > timestamp: 2019-02-06 13:56:42 +0400 > message: > > Removed redundant partitioning check > > This check was introduced in 602a222 and then became redundant in ad1553e, > where we attempt to open a table even for non-copy algorithms. > > Added missing test case from 602a222. > > Part of MDEV-17805 - Remove InnoDB cache for temporary tables. ok to push Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] 311beaffa63: Simplified dd_recreate_table()
by Sergei Golubchik 14 Mar '19

14 Mar '19

Hi, Sergey! On Mar 13, Sergey Vojtovich wrote: > revision-id: 311beaffa63 (mariadb-10.4.1-135-g311beaffa63) > parent(s): 3323b013c13 > author: Sergey Vojtovich <svoj(a)mariadb.org> > committer: Sergey Vojtovich <svoj(a)mariadb.org> > timestamp: 2019-02-06 13:56:42 +0400 > message: > > Simplified dd_recreate_table() > > It is used only with persistent tables, so remove path argument, which > was introduced in 9594107f and became useless after ce6a63e. > > Part of MDEV-17805 - Remove InnoDB cache for temporary tables. ok to push Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] 3323b013c13: Removed redundant SE lock for tmp tables
by Sergei Golubchik 14 Mar '19

14 Mar '19

Hi, Sergey! On Mar 13, Sergey Vojtovich wrote: > revision-id: 3323b013c13 (mariadb-10.4.1-134-g3323b013c13) > parent(s): 4b3656a44d6 > author: Sergey Vojtovich <svoj(a)mariadb.org> > committer: Sergey Vojtovich <svoj(a)mariadb.org> > timestamp: 2019-02-06 13:56:28 +0400 > message: > > Removed redundant SE lock for tmp tables > > CREATE TEMPORARY TABLE locks SE plugin 6 times. 5 of these locks are > released by the end of the statement. And only 1 acquired by > init_from_binary_frm_image() / plugin_lock() remains. > > The lock removed in this patch was clearly redundant. > > Part of MDEV-17805 - Remove InnoDB cache for temporary tables. > > diff --git a/sql/temporary_tables.cc b/sql/temporary_tables.cc > index 917a85e6c3b..438b670b5f5 100644 > --- a/sql/temporary_tables.cc > +++ b/sql/temporary_tables.cc > @@ -951,8 +947,6 @@ TMP_TABLE_SHARE *THD::create_temporary_table(handlerton *hton, > init_tmp_table_share(this, share, saved_key_cache, key_length, > strend(saved_key_cache) + 1, tmp_path); > > - share->db_plugin= ha_lock_engine(this, hton); > - I suppose, share->db_plugin will be set in the following share->init_from_binary_frm_image ? Ok to push. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] 14c2f90ad08: Idempotent INSERT events for system versioning
by Aleksey Midenkov 13 Mar '19

13 Mar '19

Hi, Sergei, Thank you for observations! This task is in progress. While doing it I found out that RBR doesn't replicate timestamp fractions. That is: it gets seconds from event timestamp marker and adds fractions from system time. This causes bugs in System Versioning. More on it in Bug 6 here: https://github.com/tempesta-tech/mariadb/issues/578#issuecomment-470533050 I propose to fix it by adding timestamp fractions field to Log_event header after FLAGS_OFFSET. To support backwards compatibility it is needed to increment fdle->binlog_version and probably add new flag LOG_EVENT_HAS_SEC_PART_F (for support sending Log_event without fractions). Do you agree? On Thu, Jan 31, 2019 at 1:10 PM Sergei Golubchik <serg(a)mariadb.com> wrote: > Hi, Aleksey! > > On Jan 31, Aleksey Midenkov wrote: > > revision-id: 14c2f90ad08 (versioning-1.0.7-1-g14c2f90ad08) > > parent(s): a8efe7ab1f2 > > author: Aleksey Midenkov <midenok(a)gmail.com> > > committer: Aleksey Midenkov <midenok(a)gmail.com> > > timestamp: 2018-11-17 16:30:10 +0300 > > message: > > > > Idempotent INSERT events for system versioning > > > > Case 1: Rows_log_event::write_row() always overwrite historical row. > > > > Related to MDEV-16370. > > I realized that the whole RBR logging of system versioned tables is > wrong. > > For an update you log update_row event and write_row event separately > and execute them separately. It allows trivially to manipulate the history. > See a test case attached. > > To fix this, the slave should ignore all events that modify the history. > And generate them locally. Say, an update on the master generates > update_row and write_row events. The slave executes update_row event > which updates a current row and this also should generate the historical > row. Then the slave ignores write_row event. > > Of course, as an optimization, the master should not generate historical > write_row events, but as my test case shows, they can be forged, so the > slave should ignore them anyway, if they happen to come. > > I've created a new https://jira.mariadb.org/browse/MDEV-18432 for that, > let's not have it tied to MDEV-16370. > > Regards, > Sergei > Chief Architect MariaDB > and security(a)mariadb.org > -- All the best, Aleksey Midenkov @midenok

3 7

Re: [Maria-developers] 1aa6d5bdbec: MDEV-18888 Server crashes in Item_field::register_field_in_read_map upon...
by Sergei Golubchik 12 Mar '19

12 Mar '19

Hi, Sachin! On Mar 12, Sachin Setiya wrote: > revision-id: 1aa6d5bdbec (mariadb-10.4.3-63-g1aa6d5bdbec) > parent(s): b5d2e8577ef > author: Sachin Setiya <sachin.setiya(a)mariadb.com> > committer: sachin <sachin.setiya(a)mariadb.com> > timestamp: 2019-03-12 19:29:58 +0530 > message: > > MDEV-18888 Server crashes in Item_field::register_field_in_read_map upon... > MODIFY COLUMN > > Do Not create prefix field for long unique key > > diff --git a/sql/table.cc b/sql/table.cc > index 4da873b2e8f..5ca284fd8dd 100644 > --- a/sql/table.cc > +++ b/sql/table.cc > @@ -3709,7 +3709,33 @@ enum open_frm_error open_table_from_share(THD *thd, TABLE_SHARE *share, > Field *field= key_part->field= outparam->field[key_part->fieldnr - 1]; > > if (field->key_length() != key_part->length && > - !(field->flags & BLOB_FLAG)) > + !(field->flags & BLOB_FLAG) && > + /* > + MDEV-18888 > + We will not make copy of the field because if we do this this this will > + make this test case to fail > + CREATE TABLE t1 ( > + a CHAR(128), > + b CHAR(128) AS (a), > + c varchar(5000), > + UNIQUE(c,b(64)) > + ) ENGINE=InnoDB; > + And the reason for this is because Item_func_hash will Item_field based on > + this new field. But if this field is virtual field then since it is created > + before calling parse_vcol_defs so it does not have field->vcol_info->expr item > + initiated. And this will make segfault when field->vcol_info->expr is accessed. > + So the question may be why similar unique with just virtual column is not crashing ? > + CREATE TABLE t2 ( > + a CHAR(128), > + b CHAR(128) AS (a), > + c char(128) as (a+b), > + UNIQUE(a, c(64)) > + ) ENGINE=InnoDB; > + In this case also new prefix field will be created whose vcol_info->expr will be NULL. > + But Since this key is not a long unique in innodb_base_col_setup field will be > + table->field[i] which will have not null vcol_info->expr > + */ > + key_info->algorithm != HA_KEY_ALG_LONG_HASH) don't put a comment (particularly a long one) inside the if(), move it to be before the if(), please. and I'd used a simpler comment, like "there's no need to create a prefix Field for HA_KEY_ALG_LONG_HASH indexes, as they implement prefixing via Iten_func_left anyway (see parse_vcol_defs()" > { > /* > We are using only a prefix of the column as a key: Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] b5d2e8577ef: MDEV-18889 Long unique on virtual fields crashes server
by Sergei Golubchik 12 Mar '19

12 Mar '19

Hi, Sachin! On Mar 12, Sachin Setiya wrote: > revision-id: b5d2e8577ef (mariadb-10.4.3-62-gb5d2e8577ef) > parent(s): 202d1b0c897 > author: Sachin Setiya <sachin.setiya(a)mariadb.com> > committer: sachin <sachin.setiya(a)mariadb.com> > timestamp: 2019-03-12 19:29:58 +0530 > message: > > MDEV-18889 Long unique on virtual fields crashes server > > When we receive record from ha_index_read_map it does not have vfield > values. So we need to update it. > > diff --git a/sql/handler.cc b/sql/handler.cc > index 0746f8a81fc..c4f81c6432e 100644 > --- a/sql/handler.cc > +++ b/sql/handler.cc > @@ -6522,6 +6522,26 @@ static int wsrep_after_row(THD *thd) > } > #endif /* WITH_WSREP */ > > +/* > + Long Unique can have virtual fields. But when we receive record from > + ha_index_read_map it does not have vfield values. So we need to update it. > + IN table Table Object > + IN handler Handler Object > + OUT updated Set to true on successful update Doxygen style for new comments, please. And feel free to rewrite old comments to use Doxygen style whenever you need to edit them anyway. > +*/ > +static void update_table_vcol(TABLE *table, handler *h, bool& updated) > +{ > + uint length= table->s->reclength; > + uchar temp_record[length]; Not a good idea: 1. you'll get compiler warnings for using VLA (-Wvla: Warn if a variable-length array is used") 2. length can be rather large, stack can be rather small. 3. two big memcpy's per row read. Two other options: 1. use field->move(check_unique_buf - record[0]) to move field pointers to check_unique_buf. You can do it once, at the beginning and restore them at the end, no need to do it per read. See TABLE::move_fields() 2. Copy record[0] to check_unique_buf(). And read potential duplicates into record[0]. Then you won't need to do anything special at all, ha_index_read_map() will update vcols automatically. Just don't forget to restore record[0] at the end. See store_record(), restore_record(). I think the second approach might be easier. > + memcpy(temp_record, table->record[0], length); > + restore_record(table, check_unique_buf); > + if (!table->update_virtual_fields(h, VCOL_UPDATE_FOR_READ)) > + updated= true; > + store_record(table, check_unique_buf); > + memcpy(table->record[0], temp_record, length); > + > +} > + Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0