- developers - lists.mariadb.org

Re: [Maria-developers] [Commits] 41a12f9: MDEV-8320 Allow index usage for DATE(datetime_column) = const.
by Sergey Petrunia 21 Nov '16

21 Nov '16

In-Reply-To: <20160928105123.6D948140DDC(a)nebo.localdomain> Hi Alexey, Thanks for your patience in waiting for the review. Please find it below. On Wed, Sep 28, 2016 at 02:50:19PM +0400, Alexey Botchkov wrote: > revision-id: 41a12f990519fb68eaa66ecc6860985471e6ba5a (mariadb-10.1.8-264-g41a12f9) > parent(s): 28f441e36aaaec15ce7d447ef709fad7fbc7cf7d > committer: Alexey Botchkov > timestamp: 2016-09-28 14:48:54 +0400 > message: > > MDEV-8320 Allow index usage for DATE(datetime_column) = const. > > Test for 'sargable functions' added. > First, t/range.test crashes after I apply the patch. MTR output is here: https://gist.github.com/spetrunia/d10165820664e0d18d4a667d44d226ee but I've got the crash on two different machines so should be easy to repeat > diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h > index 6d432bd..516bb07 100644 > --- a/sql/item_cmpfunc.h > +++ b/sql/item_cmpfunc.h > @@ -136,6 +136,14 @@ class Item_bool_func :public Item_int_func > { > protected: > /* > + Some functions modify it's arguments for the optimizer. > + So for example the condition 'Func(fieldX) = constY' turned into > + 'fieldX = cnuR(constY)' so that optimizer can use an index on fieldX. > + */ What's cnuR? Ok, I eventually got it, but the comments should not have such puzzles. > + Item *opt_args[3]; > + uint opt_arg_count; > + > + /* > +static Item_field *get_local_field (Item *field) > +{ > + Item *ri= field->real_item(); > + return (ri->type() == Item::FIELD_ITEM > + && !(field->used_tables() & OUTER_REF_TABLE_BIT) > + && !((Item_field *)ri)->get_depended_from()) ? (Item_field *) ri : 0; > +} Please fix indentation and add comments. Does this function do what is_local_field does, or there is some difference? > + > + > +static Item_field *field_in_sargable_func(Item *fn) > +{ > + fn= fn->real_item(); > + > + if (fn->type() == Item::FUNC_ITEM && > + strcmp(((Item_func *)fn)->func_name(), "cast_as_date") == 0) > + > + { > + Item_date_typecast *dt= (Item_date_typecast *) fn; > + return get_local_field(dt->arguments()[0]); > + } > + return 0; Please use NULL instead of 0, and !strcmp() instead of strcmp()=0. > @@ -5036,6 +5060,25 @@ Item_func_like::add_key_fields(JOIN *join, KEY_FIELD **key_fields, > } > > > +bool Item_bool_rowready_func2::add_extra_key_fields(THD *thd, > + JOIN *join, KEY_FIELD **key_fields, > + uint *and_level, > + table_map usable_tables, > + SARGABLE_PARAM **sargables) > +{ > + Item_field *f; > + if ((f= field_in_sargable_func(args[0])) && args[1]->const_item()) What is the difference between add_key_fields and add_extra_key_fields? Any cases where one should call one but not the other? Please also do indentation as coding style specifies. > diff --git a/sql/item_timefunc.cc b/sql/item_timefunc.cc > index 41dc967..3124444 100644 > --- a/sql/item_timefunc.cc > +++ b/sql/item_timefunc.cc > @@ -2569,6 +2569,39 @@ bool Item_date_typecast::get_date(MYSQL_TIME *ltime, ulonglong fuzzy_date) > } > > > +bool Item_date_typecast::create_reverse_func(enum Functype cmp_type, > + THD *thd, Item *r_arg, uint *a_cnt, Item** a) > +{ We need a specification of what exactly this function does, and a usage scenario in the comment. This function actually creates multiple (up to 3?) functions. If one has a condition DATE(t1.d) < '2000-01-04' then we get (gdb) p ((Item*)cond)->opt_arg_count $37 = 3 (gdb) p dbug_print_item(((Item*)cond)->opt_args[0]) $38 = 0x555557083e20 <dbug_item_print_buf> "t1.d" (gdb) p dbug_print_item(((Item*)cond)->opt_args[1]) $39 = 0x555557083e20 <dbug_item_print_buf> "day_begin('2000-01-19')" (gdb) p dbug_print_item(((Item*)cond)->opt_args[2]) $40 = 0x555557083e20 <dbug_item_print_buf> "day_end('2000-01-19')" which makes sense, but the description is lacking. Probably the name "create_reverse_func" is not good, because 1. multiple functions are created and 2. neither of them is the reverse. I can't suggest a better name at the moment, though. Let's both think about how to make this code cleared for an uninformed reader. BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

2 1

Re: [Maria-developers] MDEV-11245 Move prepare_create_field and sp_prepare_create_field() as methods to Column_definition
by Alexander Barkov 20 Nov '16

20 Nov '16

Hi Sanja, On 11/20/2016 12:29 AM, Oleksandr Byelkin wrote: > Hi! > > Patch is OK to push. Thanks for reviewing! > Actually it is good sign that 'refefence->field' > turned to 'field' now! Yeah, this makes the code easier to read. Greetings! > > On Fri, Nov 18, 2016 at 12:43 PM, Alexander Barkov <bar(a)mariadb.org > <mailto:bar@mariadb.org>> wrote: > > Hello Sanja, > > can you please review a patch for MDEV-11245 for 10.3? > > It's a small refactoring intended to make patches for these tasks > look better: > > - MDEV-10577 sql_mode=ORACLE: %TYPE in variable declarations > - MDEV-10914 ROW data type for stored routine variables > > Thanks! > >

1 0

[Maria-developers] MDEV-11245 Move prepare_create_field and sp_prepare_create_field() as methods to Column_definition
by Alexander Barkov 18 Nov '16

18 Nov '16

Hello Sanja, can you please review a patch for MDEV-11245 for 10.3? It's a small refactoring intended to make patches for these tasks look better: - MDEV-10577 sql_mode=ORACLE: %TYPE in variable declarations - MDEV-10914 ROW data type for stored routine variables Thanks!

1 0

[Maria-developers] SHOW CREATE TABLE does not display REF_SYSTEM_ID
by Alexander Barkov 17 Nov '16

17 Nov '16

Hello Alexey, I have a question about REF_SYSTEM_ID. DROP TABLE IF EXISTS t1; CREATE TABLE t1 (a POINT REF_SYSTEM_ID=10); SELECT G_TABLE_NAME,G_GEOMETRY_COLUMN,SRID FROM INFORMATION_SCHEMA.GEOMETRY_COLUMNS WHERE G_TABLE_NAME='t1'; The above script returns: +--------------+-------------------+------+ | G_TABLE_NAME | G_GEOMETRY_COLUMN | SRID | +--------------+-------------------+------+ | t1 | a | 10 | +--------------+-------------------+------+ This looks correct. The REF_SYSTEM_ID (which is 10) is correctly displayed in the SRID column. But now if I do: SHOW CREATE TABLE t1; It prints: +-------+-------------------------------------------------------------------------------------+ | Table | Create Table | +-------+-------------------------------------------------------------------------------------+ | t1 | CREATE TABLE `t1` ( `a` point DEFAULT NULL ) ENGINE=InnoDB DEFAULT CHARSET=latin1 | +-------+-------------------------------------------------------------------------------------+ It does not display the REF_SYSTEM_ID part of the column definition. Looks like a bug from a glance. Can you comment please?

1 0

[Maria-developers] Please review MDEV-11298 Split Item_func_hex::val_str_ascii() into virtual methods in Type_handler
by Alexander Barkov 17 Nov '16

17 Nov '16

Hello Alexey, Please review a patch for MDEV-11298 (for 10.3). Thanks.

1 0

[Maria-developers] Please review MDEV-11294 Move definitions of Derivation, DTCollation, Type_std_attributes from field.h and item.h to sql_type.h
by Alexander Barkov 16 Nov '16

16 Nov '16

Hello Alexey, Please review a patch for MDEV-11294. Thanks!

1 0

Re: [Maria-developers] [Commits] 9ed264a391b160ef84121c8699bb3ff61c0059ce Fixed bug mdev-11072. In a general case the conditions with outer fields cannot be pushed into materialized views / derived tables. However if the outer field in the condition refers to a single row table then the condition may be pushable. In this case a special care should be taken for outer fields when pushing the condition into a materialized view / derived table.
by Oleksandr Byelkin 15 Nov '16

15 Nov '16

Hi, Igor! On 11/15/2016 08:23 AM, Igor Babaev wrote: > commit 9ed264a391b160ef84121c8699bb3ff61c0059ce > Author: Igor Babaev <igor(a)askmonty.org> > Commit: Igor Babaev <igor(a)askmonty.org> > > Fixed bug mdev-11072. > In a general case the conditions with outer fields cannot > be pushed into materialized views / derived tables. > However if the outer field in the condition refers to a > single row table then the condition may be pushable. > In this case a special care should be taken for outer > fields when pushing the condition into a materialized view / > derived table. Thank you a lot, OK to push! [skip]

1 0

[Maria-developers] Mdev-10715 -- Galera: Replicate MariaDB GTID to other nodes in the cluster
by Sachin Setiya 15 Nov '16

15 Nov '16

Hi Nirbyay, Serg, Kristian, In this mail I am answering some questions raised by Kristian and blue print for this task. 1. Questions of Kristian <knielsen> sachin_setiya_7: so maybe the problem is - that a node broadcasts its write set before the commit order has been determined? I do not think , this is the problem. Galera enforces the commit order. Yes, it broadcast write set in prepare phase. but it also guarantees that t1->t2 order will be maintained in all participating N nodes. <knielsen> sachin_setiya_7: how is the galera internal transaction id allocated and broadcast? I am here assuming that we are talking about gtid-sequence no. Suppose our initial seqno is S. So basically at this time all N have same sequence no. Some transaction T is executed at node Ni .It broadcast the writeset with its current sequence no S. At all Node Nj (including Ni).It receives this message. It checks some conditions Like it “totally ordered action”. If yes then Nj updates its sequence no to + 1. Here is the relevant code. if (gu_likely(GCS_ACT_TORDERED == rcvd->act.type && GCS_GROUP_PRIMARY == group->state && group->nodes[sender_idx].status >= GCS_NODE_STATE_DONOR && !(group->frag_reset && local) && commonly_supported_version)) { /* Common situation - * increment and assign act_id only for totally ordered actions * and only in PRIM (skip messages while in state exchange) */ rcvd->id = ++group->act_id_; } In function static inline ssize_t gcs_group_handle_act_msg (gcs_group_t* const group, const gcs_act_frag_t* const frg, const gcs_recv_msg_t* const msg, struct gcs_act_rcvd* const rcvd, bool commonly_supported_version) So basically It is like certification(generated at each node ) but done in quite early phase. Blueprint of task:- We can do something like galera GTID, we will take initial sequence no from server. We will add one more variable in gcs_group_t Named s_sequence_no and will increment it at each node. We also have to Create a gtid event and append it to message received at Nj , so that on late stages wsrep_apply_cb() can take care of gtid. Please let me know what you think. -- Regards Sachin Setiya Software Engineer at MariaDB

2 1

[Maria-developers] Changing the error message for ER_LOCK_WAIT_TIMEOUT
by Sergey Petrunia 12 Nov '16

12 Nov '16

Hi Sergei and everyone, MariaDB defines ER_LOCK_WAIT_TIMEOUT as share/errmsg-utf8.txt: eng "Lock wait timeout exceeded; try restarting transaction" facebook/mysql-5.6 has an enhancement: it also shows what kind of lock is held: share/errmsg-utf8.txt: eng "Lock wait timeout exceeded; try restarting transaction: %-.256s" the new error messages have more info and look like this: https://gist.github.com/spetrunia/266272c384a1b43081572e1ba2baf3f3 note that MyRocks also provides extra information. So, the questions are: - Is it (generally) possible to change error message texts in 10.2 still? - Can/should we change the ER_LOCK_WAIT_TIMEOUT error text? - (non-question) I assume that adding another error code with the new error text is not a good solution: two error codes for the same error will be very confusing. The number of times ER_LOCK_WAIT_TIMEOUT is used in the source is actually quite small: https://gist.github.com/spetrunia/2bc2ed7040a930d75b39162becbc7963 (25 occurences, and most of them actually dont care about the error message). There are lots of .result files to update, though. Any opinions? BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

2 1

[Maria-developers] No MTR tests for --encrypt-binlog feature?
by Laurynas Biveinis 11 Nov '16

11 Nov '16

MariaDB - On 10.1 branch, grep on mysql-test for encrypt.binlog returns ./r/mysqld--help.result:180: --encrypt-binlog Encrypt binary logs (including relay logs) ./r/mysqld--help.result:1179:encrypt-binlog FALSE ./suite/sys_vars/r/sysvars_server_embedded.result:698:VARIABLE_NAME ENCRYPT_BINLOG ./suite/sys_vars/r/sysvars_server_notembedded.result:712:VARIABLE_NAME ENCRYPT_BINLOG Thus it does not look that the feature has actual tests. How do you know it works? Thanks, -- Laurynas

2 3