November 2014 - developers - lists.mariadb.org

[Maria-developers] PLEASE REVIEWA: pre-requisite patch for the data type plugins, also fixing a number of optimizer bugs
by Alexander Barkov 12 Nov '14

12 Nov '14

Hello, While working on pluggable data types, I noticed a few problems related to optimizer: > MDEV-6950 Bad results with joins comparing DATE/DATETIME and INT/DECIMAL/DOUBLE/ENUM/VARCHAR columns > MDEV-6971 Bad results with joins comparing TIME and DOUBLE/DECIMAL columns > MDEV-6978 Bad results with joins comparing case insensitive VARCHAR/ENUM/SET expression to a _bin ENUM column > MDEV-6982 LEFT JOIN table elimination is not always used when it could > MDEV-6989 BINARY and COLLATE xxx_bin comparisions are not used for optimization in some cases > MDEV-6990 GROUP_MIN_MAX optimization is not applied in some cases when it could > MDEV-6991 GROUP_MIN_MAX optimization is erroneously applied in some cases In some cases wrong result sets are returned. So it would be nice to have this fixed in 10.0. I made these changes as a standalone patch. Please review. I also noticed some other optimizer bugs but not sure how to fix them properly: > MDEV-6986 Bad results with join comparing INT and VARCHAR columns > MDEV-6969 Bad results with joins comparing DOUBLE to BIGINT/DECIMAL columns > MDEV-6993 Bad results with join comparing DECIMAL and ENUM/SET columns Igor or SergeyP can possibly fix them on top of my changes. Also, I found some other problems (not related to optimizer): > MDEV-6973 XOR aggregates argument collations > MDEV-7005 NULLIF does not work as documented Not sure which version to fix them in. Please suggest. Thanks.

2 2

Re: [Maria-developers] MDEV-6885 - debs do not build on PPC64 in lp:maria/5.5
by Sergei Golubchik 11 Nov '14

11 Nov '14

Hi, Sergey! On Oct 17, Sergey Vojtovich wrote: > At lp:maria/5.5 > > ------------------------------------------------------------ > revno: 4327 > revision-id: svoj(a)mariadb.org-20141017112032-b0p2qvvlbn1xrfec > parent: svoj(a)mariadb.org-20141017110850-5rlt8fm3blxv4wtb > committer: Sergey Vojtovich <svoj(a)mariadb.org> > branch nick: 5.5 > timestamp: Fri 2014-10-17 15:20:32 +0400 > message: > MDEV-6885 - debs do not build on PPC64 > > Include ha_oqgraph.so into deb package only if it was able to build. Sorry, cannot help much. It looks ok, but I've had so many problems with our debian packaging - all these fixes looks right, and still need twenty iterations to actually be right. So, try it in buildbot, push in some tree that builds deb packages. If it'll work in all deb builders - please, push into 5.5. Regards, Sergei

3 3

Re: [Maria-developers] [Commits] MDEV-60 Support for Spatial Reference systems for the GIS data.
by Sergei Golubchik 11 Nov '14

11 Nov '14

Hi, Alexey! First, please, please, *do not* send patches from Thunderbird. It took me a lot of time to fix it, the patch was badly garbled. May be broken formatting everywhere in the patch was also caused by this. Then, I didn't see any tests in the patch, not even syntax tests, but also not create field, store, read tests. Please add them. Now, the review: > diff --git a/sql/field.cc b/sql/field.cc > --- a/sql/field.cc > +++ b/sql/field.cc > @@ -7808,6 +7808,35 @@ uint Field_blob::is_equal(Create_field *new_field) > > #ifdef HAVE_SPATIAL > > +uint gis_field_options_image(uchar *buff, List<Create_field> &create_fields) > +{ > + uint image_size= 0; > + uint n_field= 0; > + List_iterator<Create_field> it(create_fields); > + Create_field *field; > + while ((field= it++)) > + { > + if (field->sql_type != MYSQL_TYPE_GEOMETRY) > + continue; > + if (buff) > + int2store(buff + image_size, ((uint16) n_field)); > + image_size+= 2; > + if (buff) > + int2store(buff + image_size, ((uint16) field->length)); > + image_size+= 2; > + if (buff) > + int2store(buff + image_size, ((uint16) field->decimals)); > + image_size+= 2; > + if (buff) > + int4store(buff + image_size, ((uint32) field->srid)); > + image_size+= 4; > + n_field++; > + } > + > + return image_size; > +} Hmm... So, you basically, put an array of properties in frm, under EXTRA2_GIS Two comments: 1. Why n_field, length and decimals? I thought you only add srid, other properties are already stored elsewhere, aren't they? 2. Really think about upgrades downgrades. The "extra" section in the frm doesn't handle them well, I know how these problems happen. Think, what happens when you add more properties here? What if you remove some? How new server will parse old frms, how old server will read new frms? > + > + > void Field_geom::sql_type(String &res) const > { > CHARSET_INFO *cs= &my_charset_latin1; > @@ -9181,7 +9210,7 @@ bool Create_field::init(THD *thd, char *fld_name, enum_field_types fld_type, > Item *fld_on_update_value, LEX_STRING *fld_comment, > char *fld_change, List<String> *fld_interval_list, > CHARSET_INFO *fld_charset, uint fld_geom_type, > - Virtual_column_info *fld_vcol_info, > + char *fld_srid, Virtual_column_info *fld_vcol_info, fix the formatting, please > engine_option_value *create_opt, bool check_exists) > { > uint sign_len, allowed_type_modifier= 0; > @@ -9232,6 +9261,7 @@ bool Create_field::init(THD *thd, char *fld_name, enum_field_types fld_type, > pack_length= key_length= 0; > charset= fld_charset; > geom_type= (Field::geometry_type) fld_geom_type; > + srid= fld_srid ? (uint)atoi(fld_srid) : 0; and here > interval_list.empty(); > > comment= *fld_comment; > @@ -9635,7 +9665,7 @@ Field *make_field(TABLE_SHARE *share, uchar *ptr, uint32 field_length, > uint pack_flag, > enum_field_types field_type, > CHARSET_INFO *field_charset, > - Field::geometry_type geom_type, > + Field::geometry_type geom_type, uint srid, and here, and everywhere, I won't comment on that anymore > Field::utype unireg_check, > TYPELIB *interval, > const char *field_name) > diff --git a/sql/sql_lex.h b/sql/sql_lex.h > --- a/sql/sql_lex.h > +++ b/sql/sql_lex.h > @@ -2358,6 +2358,7 @@ struct LEX: public Query_tables_list > char *backup_dir; /* For RESTORE/BACKUP */ > char* to_log; /* For PURGE MASTER LOGS TO */ > char* x509_subject,*x509_issuer,*ssl_cipher; > + char *srid; really? it can only be used for field declarations, I don't think there's a need to put it in LEX see below. > String *wild; /* Wildcard in SHOW {something} LIKE 'wild'*/ > sql_exchange *exchange; > select_result *result; > diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy > --- a/sql/sql_yacc.yy > +++ b/sql/sql_yacc.yy > @@ -6359,6 +6360,16 @@ precision: > } > ; > > +srid_option: > + /* empty */ > + { Lex->srid= (char *)0; } > + | > + SRID_SYM EQ NUM > + { > + Lex->srid=$3.str; > + } return the value of srid here, don't store it in lex. Like $$ = $3.str; or even $$ = atoi($3.str); > + ; > + > field_options: > /* empty */ {} > | field_opt_list {} > diff --git a/sql/table.cc b/sql/table.cc > --- a/sql/table.cc > +++ b/sql/table.cc > @@ -1451,6 +1464,20 @@ int TABLE_SHARE::init_from_binary_frm_image(THD *thd, bool write, > #ifdef HAVE_SPATIAL > geom_type= (Field::geometry_type) strpos[14]; > charset= &my_charset_bin; > + if (gis_options && gis_options_len >= 10) > + { > + gis_length= uint2korr(gis_options+2); > + gis_decimals= uint2korr(gis_options+4); > + srid= uint4korr(gis_options+6); > + gis_options_len-= 10; > + gis_options+= 10; If you have the function to generate this data in field.cc, I'd put a small function to read it in field.cc too. Just to have all reading/writing code in the same place. > + } > + else > + { > + srid= 0; > + gis_length= 0; > + gis_decimals= 0; > + } > #else > goto err; > #endif > diff --git a/sql/unireg.h b/sql/unireg.h > --- a/sql/unireg.h > +++ b/sql/unireg.h > @@ -190,6 +190,7 @@ enum extra2_frm_value_type { > #define EXTRA2_ENGINE_IMPORTANT 128 > > EXTRA2_ENGINE_TABLEOPTS=128, > + EXTRA2_GIS=129, better make it 2, not 129 (below EXTRA2_ENGINE_IMPORTANT threshold), because you de facto ignore it anyway when HAVE_SPATIAL is not defined. > }; > > int rea_create_table(THD *thd, LEX_CUSTRING *frm, > diff --git a/unittest/mysys/CMakeLists.txt b/unittest/mysys/CMakeLists.txt > --- a/unittest/mysys/CMakeLists.txt > +++ b/unittest/mysys/CMakeLists.txt > @@ -17,7 +17,7 @@ MY_ADD_TESTS(bitmap base64 my_vsnprintf my_atomic my_rdtsc lf my_malloc > LINK_LIBRARIES mysys) > > MY_ADD_TESTS(ma_dyncol > - LINK_LIBRARIES mysqlclient) > + LINK_LIBRARIES mysqlclient stdc++) Why? > > IF(WIN32) > MY_ADD_TESTS(my_delete LINK_LIBRARIES mysys) > Regards, Sergei

2 2

[Maria-developers] Fwd: [debian-mysql] Bug#766996: libmariadb-client-lgpl-dev-compat: misses a mysql_config symlink
by Daniel Schepler 10 Nov '14

10 Nov '14

I recently got the bug report below for the MariaDB C client library (Debian package). The request to add a compatibility symlink from mysql_config to mariadb_config makes sense to me -- do you see any potential problems with adding it? -- Daniel Schepler ---------- Forwarded message ---------- From: Rene Engelhard <rene(a)debian.org> Date: Mon, Oct 27, 2014 at 6:48 AM Subject: [debian-mysql] Bug#766996: libmariadb-client-lgpl-dev-compat: misses a mysql_config symlink To: Debian Bug Tracking System <submit(a)bugs.debian.org> Package: libmariadb-client-lgpl-dev-compat Version: 2.0.0-1 Severity: important https://packages.debian.org/sid/libmariadb-client-lgpl-dev-compat says: "MariaDB Client Library for C, compatibility symlinks MariaDB is a fast, stable and true multi-user, multi-threaded SQL database server. SQL (Structured Query Language) is the most popular database query language in the world. The main goals of MariaDB are speed, robustness and ease of use. This package includes compatibility symlinks to allow sources expecting the MySQL client libraries to be built against the MariaDB Client Library for C." The full contents are: https://packages.debian.org/sid/amd64/libmariadb-client-lgpl-dev-compat/fil… so it won't work with packages expecting mysql_config to give them the flags -> it should also ship a mysql_config -> mariadb_config link. Regards, Rene _______________________________________________ pkg-mysql-maint mailing list pkg-mysql-maint(a)lists.alioth.debian.org http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-mysql-maint

2 1

Re: [Maria-developers] [Commits] Rev 4343: MDEV-6179: dynamic columns functions/cast()/convert() doesn't play nice with CREATE/ALTER TABLE in lp:~maria-captains/maria/5.5
by Oleksandr Byelkin 10 Nov '14

10 Nov '14

Hi, Sergei! Thank you! The patch is OK. On 08.11.14 19:54, Sergei Golubchik wrote: > At lp:~maria-captains/maria/5.5 > > ------------------------------------------------------------ > revno: 4343 > revision-id: sergii(a)pisem.net-20141108185442-37i07fh0bvm2atjr > parent: sergii(a)pisem.net-20141102163302-8ukz7pi4mwsx89n0 > committer: Sergei Golubchik <sergii(a)pisem.net> > branch nick: 5.5 > timestamp: Sat 2014-11-08 19:54:42 +0100 > message: > MDEV-6179: dynamic columns functions/cast()/convert() doesn't play nice with CREATE/ALTER TABLE > > When parsing a field declaration, grab type information from LEX before it's overwritten > by further rules. Pass type information through the parser stack to the rule that needs it. > === modified file 'mysql-test/r/create.result' > --- a/mysql-test/r/create.result 2013-05-11 17:31:50 +0000 > +++ b/mysql-test/r/create.result 2014-11-08 18:54:42 +0000 > @@ -2430,3 +2430,50 @@ a b > 1 1 > unlock tables; > drop table t1,t2; > +# > +# MDEV-6179: dynamic columns functions/cast()/convert() doesn't > +# play nice with CREATE/ALTER TABLE > +# > +create table t1 ( > +color char(32) as (COLUMN_GET(dynamic_cols, 1 as char)) persistent, > +cl char(32) as (COLUMN_GET(COLUMN_ADD(COLUMN_CREATE(1 , 'blue' as char), 2, 'ttt'), i as char)) persistent, > +item_name varchar(32) primary key, -- A common attribute for all items > +i int, > +dynamic_cols blob -- Dynamic columns will be stored here > +); > +INSERT INTO t1(item_name, dynamic_cols, i) VALUES > +('MariaDB T-shirt', COLUMN_CREATE(1, 'blue', 2, 'XL'), 1); > +INSERT INTO t1(item_name, dynamic_cols, i) VALUES > +('Thinkpad Laptop', COLUMN_CREATE(1, 'black', 3, 500), 2); > +select item_name, color, cl from t1; > +item_name color cl > +MariaDB T-shirt blue blue > +Thinkpad Laptop black ttt > +show create table t1; > +Table Create Table > +t1 CREATE TABLE `t1` ( > + `color` char(32) AS (COLUMN_GET(dynamic_cols, 1 as char)) PERSISTENT, > + `cl` char(32) AS (COLUMN_GET(COLUMN_ADD(COLUMN_CREATE(1 , 'blue' as char), 2, 'ttt'), i as char)) PERSISTENT, > + `item_name` varchar(32) NOT NULL, > + `i` int(11) DEFAULT NULL, > + `dynamic_cols` blob, > + PRIMARY KEY (`item_name`) > +) ENGINE=MyISAM DEFAULT CHARSET=latin1 > +drop table t1; > +create table t1 ( > +n int, > +c char(32) as (convert(cast(n as char), char)) persistent > +); > +insert into t1(n) values (1),(2),(3); > +select * from t1; > +n c > +1 1 > +2 2 > +3 3 > +show create table t1; > +Table Create Table > +t1 CREATE TABLE `t1` ( > + `n` int(11) DEFAULT NULL, > + `c` char(32) AS (convert(cast(n as char), char)) PERSISTENT > +) ENGINE=MyISAM DEFAULT CHARSET=latin1 > +drop table t1; > > === modified file 'mysql-test/t/create.test' > --- a/mysql-test/t/create.test 2012-12-17 20:34:56 +0000 > +++ b/mysql-test/t/create.test 2014-11-08 18:54:42 +0000 > @@ -2021,3 +2021,35 @@ connection default; > select * from t1; > unlock tables; > drop table t1,t2; > + > +--echo # > +--echo # MDEV-6179: dynamic columns functions/cast()/convert() doesn't > +--echo # play nice with CREATE/ALTER TABLE > +--echo # > +create table t1 ( > + color char(32) as (COLUMN_GET(dynamic_cols, 1 as char)) persistent, > + cl char(32) as (COLUMN_GET(COLUMN_ADD(COLUMN_CREATE(1 , 'blue' as char), 2, 'ttt'), i as char)) persistent, > + item_name varchar(32) primary key, -- A common attribute for all items > + i int, > + dynamic_cols blob -- Dynamic columns will be stored here > +); > +INSERT INTO t1(item_name, dynamic_cols, i) VALUES > + ('MariaDB T-shirt', COLUMN_CREATE(1, 'blue', 2, 'XL'), 1); > +INSERT INTO t1(item_name, dynamic_cols, i) VALUES > + ('Thinkpad Laptop', COLUMN_CREATE(1, 'black', 3, 500), 2); > + > +select item_name, color, cl from t1; > +show create table t1; > + > +drop table t1; > + > +create table t1 ( > + n int, > + c char(32) as (convert(cast(n as char), char)) persistent > +); > +insert into t1(n) values (1),(2),(3); > + > +select * from t1; > +show create table t1; > + > +drop table t1; > > === modified file 'sql/sql_lex.h' > --- a/sql/sql_lex.h 2014-08-02 19:26:16 +0000 > +++ b/sql/sql_lex.h 2014-11-08 18:54:42 +0000 > @@ -105,6 +105,15 @@ struct sys_var_with_base > LEX_STRING base_name; > }; > > +struct LEX_TYPE > +{ > + enum enum_field_types type; > + char *length, *dec; > + CHARSET_INFO *charset; > + void set(int t, char *l, char *d, CHARSET_INFO *cs) > + { type= (enum_field_types)t; length= l; dec= d; charset= cs; } > +}; > + > #ifdef MYSQL_SERVER > /* > The following hack is needed because mysql_yacc.cc does not define > > === modified file 'sql/sql_yacc.yy' > --- a/sql/sql_yacc.yy 2014-08-02 19:26:16 +0000 > +++ b/sql/sql_yacc.yy 2014-11-08 18:54:42 +0000 > @@ -737,6 +737,7 @@ static bool add_create_index (LEX *lex, > LEX_STRING lex_str; > LEX_STRING *lex_str_ptr; > LEX_SYMBOL symbol; > + LEX_TYPE lex_type; > Table_ident *table; > char *simple_string; > Item *item; > @@ -1471,13 +1472,15 @@ bool my_yyoverflow(short **a, YYSTYPE ** > %type <string> > text_string opt_gconcat_separator > > +%type <lex_type> field_def > + > %type <num> > type type_with_opt_collate int_type real_type order_dir lock_option > udf_type if_exists opt_local opt_table_options table_options > table_option opt_if_not_exists opt_no_write_to_binlog > opt_temporary all_or_any opt_distinct > opt_ignore_leaves fulltext_options spatial_type union_option > - start_transaction_opts field_def > + start_transaction_opts > union_opt select_derived_init option_type2 > opt_natural_language_mode opt_query_expansion > opt_ev_status opt_ev_on_completion ev_on_completion opt_ev_comment > @@ -5483,11 +5486,11 @@ field_spec: > field_def > { > LEX *lex=Lex; > - if (add_field_to_list(lex->thd, &$1, (enum enum_field_types) $3, > - lex->length,lex->dec,lex->type, > + if (add_field_to_list(lex->thd, &$1, $3.type, > + $3.length, $3.dec, lex->type, > lex->default_value, lex->on_update_value, > &lex->comment, > - lex->change,&lex->interval_list,lex->charset, > + lex->change, &lex->interval_list, $3.charset, > lex->uint_geom_type, > lex->vcol_info, lex->option_list)) > MYSQL_YYABORT; > @@ -5495,13 +5498,15 @@ field_spec: > ; > > field_def: > - type opt_attribute {} > - | type opt_generated_always AS '(' virtual_column_func ')' > - vcol_opt_specifier > - vcol_opt_attribute > - { > - $$= (enum enum_field_types)MYSQL_TYPE_VIRTUAL; > - Lex->vcol_info->set_field_type((enum enum_field_types) $1); > + type opt_attribute > + { $$.set($1, Lex->length, Lex->dec, Lex->charset); } > + | type opt_generated_always AS > + { $<lex_type>$.set($1, Lex->length, Lex->dec, Lex->charset); } > + '(' virtual_column_func ')' vcol_opt_specifier vcol_opt_attribute > + { > + $$= $<lex_type>4; > + Lex->vcol_info->set_field_type($$.type); > + $$.type= (enum enum_field_types)MYSQL_TYPE_VIRTUAL; > } > ; > > @@ -6892,11 +6897,11 @@ alter_list_item: > { > LEX *lex=Lex; > if (add_field_to_list(lex->thd,&$3, > - (enum enum_field_types) $5, > - lex->length,lex->dec,lex->type, > + $5.type, > + $5.length, $5.dec, lex->type, > lex->default_value, lex->on_update_value, > &lex->comment, > - $3.str, &lex->interval_list, lex->charset, > + $3.str, &lex->interval_list, $5.charset, > lex->uint_geom_type, > lex->vcol_info, lex->option_list)) > MYSQL_YYABORT; > > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits

1 0

Re: [Maria-developers] MDEV-6179: dynamic columns functions/cast()/convert() doesn't play nice with CREATE/ALTER TABLE in file:///home/bell/maria/bzr/work-maria-5.5-MDEV-6179/
by Sergei Golubchik 08 Nov '14

08 Nov '14

Hi, Sanja! On Nov 02, sanja(a)askmonty.org wrote: > At file:///home/bell/maria/bzr/work-maria-5.5-MDEV-6179/ > > ------------------------------------------------------------ > revno: 4341 > revision-id: sanja(a)askmonty.org-20141102133114-v46li6r2goyzhhp7 > parent: sanja(a)askmonty.org-20141031130729-dya4hci39x6zrv89 > committer: sanja(a)askmonty.org > branch nick: work-maria-5.5-MDEV-6179 > timestamp: Sun 2014-11-02 14:31:14 +0100 > message: > MDEV-6179: dynamic columns functions/cast()/convert() doesn't play > nice with CREATE/ALTER TABLE > > Type attributes (length, dec, charset) which parsed to global > variables now saved and restored during prsing types which can > appeared in a expression. Why did you create a stack in LEX, when bison generated parser already has a stack of states? Also, there's no need to store that in LEX, it doesn't need to live longer than the field_spec rule. By coincidence I've spent a couple of last days rewriting this very part of the parser. I've also fixed this bug without knowing it :) But in 10.1, and the patch is too big to put it in 10.0, let alone 5.5, so it's useless here. Still, I tried to create a smaller patch for this bug in 5.5. Please, review, it's on the commit list. Regards, Sergei

1 0

Re: [Maria-developers] MariaDB 10.0.14 got rid of replication safety warning without warning in release notes?
by Pavel Ivanov 08 Nov '14

08 Nov '14

And here's response from Monty to my last email. And this response together with the whole thread makes me really worried. It alludes to the fact that all MySQL and MariaDB developers before making any changes should read the whole codebase first and then make sure that their changes work the same way and that they conform to the intentions of original code developers even though there's no way they can talk to those original developers. AFAIK that's not how development on big projects with hundreds of participants works. And it really raises the question for me: what kind of quality the product that is developed with such principles in mind can have? Sergei Golubchik, I believe you are the technical leader of MariaDB project. Could you please tell me what you think about this issue? On Tue, Nov 4, 2014 at 6:32 AM, Michael Widenius <michael.widenius(a)gmail.com> wrote: > > Hi! > >>>>>> "Pavel" == Pavel Ivanov <pivanof(a)google.com> writes: > > Pavel> On Sun, Nov 2, 2014 at 3:27 PM, Michael Widenius <monty(a)mariadb.org> wrote: >>>>> This is the same order that keys are presented in SHOW CREATE TABLE >>> > Pavel> Why is it safe to assume that keys have the same order on master and on slave? >>> >>> Primary and Unique keys should always be ordered the same way on >>> master and slave. This is handled by the 'sort_keys()' function that >>> you can find at: sql/sql_table.cc, line 2770. > > Pavel> This function seem to sort only by "type" of the key. If several > Pavel> unique keys have the same "type" according to its classification then > Pavel> they will be in the "original order" as the comment in the function > Pavel> says. And the original order may be different. > > This function ensures that the table will be identical on slave and > master as long as one of the following requirements are fulfilled: > > - You are using the same create table and alter table statements on > both machines. > - You are using the same CREATE TABLE statement as 'show create' shows > on the master. > - You are adding unique keys in the same order on master and slave. > > This is always the case when you are using the replication that MySQL > and MariaDB uses. > > If you are creating tables differently on the slave than on the > master, then you are on your own. > > So in all normal and most abnormal cases, the table will be created > identically on the master and slave, even if the slave is using a > different storage engine, have different columns or have additional > keys than on the master. > > <cut> > >>> I added a test for this to: >>> mysql-test/t/insert.test >>> >>> --echo # >>> --echo # MDEV-5168: Ensure that we can disable duplicate key warnings >>> --echo # from INSERT IGNORE >>> --echo # > > Pavel> This test doesn't check which duplicate key is asserted to execute ON > Pavel> DUPLICATE KEY action. > > This test verifies that we don't get warnings for the tests. > Elinas test verifies which key is used. > >>> Elena added another test for this in >>> mysql-test/suite/storage_engine/insert_with_keys.test >>> >>> See line 142, that start with: >>> --let $create_definition = a $int_indexed_col UNIQUE KEY, b $int_indexed_col UNIQUE KEY, c $int_col > > Pavel> This test seem to check that UPDATE is executed on the correct > Pavel> duplicate key, but it's not executed during regular mtr run. How and > Pavel> when it's executed in MariaDB? > > The storage engine tests are to be used by people that implements > their own storage engine for MariaDB. > > We run the extended tests from time to time to verify storage engines. > We will also run it on some buildbot servers when running releae builds. > The storage engine tests is not something that has to be run for > normal builds. > >>> What is important is that all storage engine should work the same >>> way. If one returns something different compared to any of the >>> existing engines then it's a bug. >>> >>> In this case the implicit documentation is how MyISAM, Aria and InnoDB >>> works. All of them returns the first conflicting key. > > Pavel> I'm sorry to say, but you've explained exactly the reason why this > Pavel> behavior is not guaranteed and can be changed at any time. > > It can only be changed if someone adds new code that breaks the > storage engine interface, as defined by existing engines and one > doesn't check it with the storage engine test suite. > > Pavel> Yes, I > Pavel> understand that when you wrote the interface you had some particular > Pavel> requirements in mind. But ultimately it's up to people actively > Pavel> working on the code (who I believe are counted by hundreds now) to > Pavel> decide whether to maintain the particular behavior or change > it. > > It's the MariaDB captains that are responsible to maintain the > interface. The only way one can change something is to propose a > change and get it accepted. > > It's not free for anyone to just change old functionality and expect > that to be accepted in the standard version. > > In all cases, including this, it's an absolute requirement that > engines MUST work exactly as existing engines, so that a user can > switch engines and expect them to work identically. > > (The only exceptions are things that are impossible to fix, like > the differences between transactional and not transactional tables and > that there is no defined row order in tables). > > If engine writes don't ensure this, then you will get data corruption > on the server by just running with different engines. > > For case on insert on duplicate key, there is no acceptable reason > for engines to work differently. > > Pavel> And > Pavel> as nobody knows the original requirements. > > The original requirements was well defined. Just that non one bother > to ask doesn't mean that the requirement didn't exist. > > And the fact that all existing engines in MariaDB ensures this shows > that the requirements has been understood. > > In the case when we notice that two engines works differently, it's up > to the MariaDB captains to fix this! > The fix is NOT to extend the API and allow things to work differently. > > > Pavel> and the reasons why they > Pavel> can't be changed, and when the behavior is not tested anywhere > > We add new tests when we notice that they are needed, as we did in > this case. > For example, the case with INSERT ... ON DUPLICATE KEY is now tested. > > Pavel> and > Pavel> doesn't have any documentation even in the weakest form of comments or > Pavel> asserts, the decision is usually very easy -- change at will, nobody > Pavel> should ever depend on implementation details. > > Yes, no one should depend in implementation details, except when the > implementation detail is different from other engines and it results > in different data. > > If that is the case then the engine that does things differently than > the existing engines must be changed. > > Pavel> Even if you promise to > Pavel> fix storage engines that change this behavior, by the time you notice > Pavel> the change you'll likely won't be able to fix it (as e.g. in case of > Pavel> https://mariadb.atlassian.net/browse/MDEV-6916) or someone will > Pavel> already experience a data corruption. > > In most cases we can fix this. What was unfortunate with the views > code was that we didn't store the MariaDB version number in the view > definition. This is to be fixed in 10.1 so that this will never happen > again! > > Pavel> I'd strongly recommend to revert this change. > > Sorry, but I can't agree to revert the change because: > > a) Something that can't ever happen in MariaDB (when used correctly > with the including engines). > b) Allowing a behavior that could for the same command result in > different data sets, depending on the storage engine. > c) I have not seen a real world > > One of my main jobs is to ensure that the storage engine API is > evolved in a resonable manner and if there is a conflict between how > storage engine works ensure that things are resolved so that the user > can expect the same results independent on the storage engine. > > Reverting the change would go directly agains the above and also > against what I belive is the future of the storage engine API. > > Regards, > Monty

2 1

Re: [Maria-developers] MariaDB 10.0.14 got rid of replication safety warning without warning in release notes?
by Pavel Ivanov 08 Nov '14

08 Nov '14

Monty, can you please subscribe all your email address to maria-develoeprs@ ? People reading the list cannot see your emails. I'm forwarding here the unaltered message from Monty that I was answering to in my last email. On Sun, Nov 2, 2014 at 3:27 PM, Michael Widenius <monty(a)mariadb.org> wrote: > > Hi! > > I have again to apologize for a late reply. > After my last reply to your topic I traveled to China and noticed > that the firewall there has got much better; You can't access google, > twitter, facebook, blogspot etc :( I tried to connect over a VPN, but > that didn't work for me either. > > It did feel a bit strange to not be able to do computer work for more > than a week, but the lovely people in China who took care of me did > compensate for that. > >>>>>> "Pavel" == Pavel Ivanov <pivanof(a)google.com> writes: > > Pavel> Which key is "the first duplicate key". >>> >>> The first key that has a duplicate value in the order of keys in the KEY >>> structure that MariaDB provides for the storage engine. > > Pavel> I'm definitely not that familiar with the relevant MariaDB code, but I > Pavel> didn't find KEY structure to be passed to any insert-related function. > Pavel> Can you elaborate (preferably with code references) how this > Pavel> restriction is supposed to work? > > The following code examples are from 10.0. > > All handler functions get access to the TABLE object, which defines > the record format and the keys. > > The keys are stored in TABLE->key_info (see table.h, line 1062) > > When one reads a key, one refers to keys according to the number in > the key_info structure. For example: > > A set of reads by key is done by calling: > > handler->ha_index_init(key_number,..); > read_until_no_more_rows > handler->ha_index_read_map(). > handler->ha->index_end() > > When one calls handler->info() to ask for a which key caused a > duplicate error, it's supposed to store the first duplicate key (key > with smallest number) in handler->errkey. > >>> This is the same order that keys are presented in SHOW CREATE TABLE > > Pavel> Why is it safe to assume that keys have the same order on master and on slave? > > Primary and Unique keys should always be ordered the same way on > master and slave. This is handled by the 'sort_keys()' function that > you can find at: sql/sql_table.cc, line 2770. > > The only case when this is not the case, is if you add an unique key > on the slave that was not on the master and there is conflicts when > you insert things because of this key. > > However, if you do this, I would say that you are on your own. > >>> To help engine writes we are constantly adding new test in the storage >>> engine test suite, to make it easier for storage engine vendors to >>> test that their storage engine follows the interface. >>> We have now added a test to check for the correct behaviour for the >>> case of INSERT ... ON DUPLICATE KEY. > > Pavel> This is very interesting. Can you say which test it is and what > Pavel> exactly does it test? Is it present in 10.0.14 or it's only at the > Pavel> latest 10.0 branch? > > I added a test for this to: > mysql-test/t/insert.test > > --echo # > --echo # MDEV-5168: Ensure that we can disable duplicate key warnings > --echo # from INSERT IGNORE > --echo # > > Elena added another test for this in > mysql-test/suite/storage_engine/insert_with_keys.test > > See line 142, that start with: > --let $create_definition = a $int_indexed_col UNIQUE KEY, b $int_indexed_col UNIQUE KEY, c $int_col > >>> Do you have or use a storage engine that doesn't provide the expected value >>> for the conflicting key ? > > Pavel> I don't have a specific example of storage engine that detects > Pavel> duplicate key in different order, but I'm not convinced and don't have > Pavel> hard proof that such engine cannot exist and that InnoDB itself is not > Pavel> susceptible to this problem. I can easily imagine though that even > Pavel> given the particular order of keys in table definition the key > Pavel> reported as having duplicate can depend on the way storage engine > Pavel> stores those keys. > > As key order is defined on the upper level, above InnoDB, this should > be 100 % safe (at least for InnoDB). > > For other store engines, this is safe for any engine that test for > duplicate key in key order. > > If we ever find a case where it's not safe with any storage engine > that we deliver with MariaDB, we will fix the storage engine. > > It's ok if they do thing differently so that they can detect duplicate > key for other keys faster. > > However, it has been required from all storage engines from start that > if one calls 'handler->info()' to request which key was duplicated, > they must return the first key. > > If not, they are breaking the storage engine interface (both for MySQL > and MariaDB) as there is no way the upper level can guarantee that > things will work identically between storage engines. > > Pavel> BTW, can you point to MySQL documentation and/or comment in the code > Pavel> that indicates this requirement to storage engine implementation? > > Most of the storage engine interface is not 100 % documented. This is > one of the case. > However, I wrote the code related to errkey and this is how it was > supposed to work. > In general if something is not documented, then it's up to the people > that created the interface or are actively working on it to define how > it should work. > > What is important is that all storage engine should work the same > way. If one returns something different compared to any of the > existing engines then it's a bug. > > In this case the implicit documentation is how MyISAM, Aria and InnoDB > works. All of them returns the first conflicting key. > > Pavel> On the related note Release Notes mention the error log flood > Pavel> protection. Why it's not behind flag? How should I disable the feature > Pavel> and get back to the old behavior? > >>> We already had in MariaDB flood protection for some error >>> messages. > > Pavel> Can you say exactly which error messages are suppressed already? Is > Pavel> there a documentation link about that? > > All 'unsafe logging' errors are suppressed. > See enum enum_binlog_stmt_unsafe in sql_lex.h > (This is what I know). > >>> I am happy to look into adding a flag that is usable for any kind of >>> suppressed messages. >>> >>> A couple of ways to do this: >>> >>> a) Not suppress anything if you give --log-warnings a value > 2 >>> b) make LIMIT_UNSAFE_WARNING_ACTIVATION_TIMEOUT a configurable >>> variable, where 0 would mean no suppression. >>> >>> Which of the above, or both, would you prefer? >>> Or do you have any other suggestion of how to do this? > > Pavel> Reading documentation on --log-warnings I don't see how it can be > Pavel> related to the error suppression. So I think making > Pavel> LIMIT_UNSAFE_WARNING_ACTIVATION_TIMEOUT configurable would be much > Pavel> better, though it should apply to all possible error suppressions. > > Currently we only have suppression for a limited set of 'unsafe' > errors, but I agree that when we add a flag we should name this with > a general name to be able to reuse it for all possible future > suppressions. > > In 10.1 we are looking at adding a feature that one can suppress any > error from the error log, but that's a different topic. > > Regards, > Monty

1 0

[Maria-developers] Maybe a new information schema table for replication?
by Roberto Spadim 06 Nov '14

06 Nov '14

Hi guys, i got one question these days and i don't know how to solve... the question is: "How to see the queries executing by replication thread?" i'm thinking about a information schema plugin to see whats is 'queued' in replication thread, at "master" (write to binlog) and "slave" (executing from binlog) side any idea if this is possible? i didn't checked or read any part of binlog/replication code yet thanks guys! :) -- Roberto Spadim SPAEmpresarial Eng. Automação e Controle

1 0

[Maria-developers] pcre.h should be installed if MariaDB is built with bundled pcre ?
by Kazuhiko Shiozaki 06 Nov '14

06 Nov '14

Hi, pcre.h is not installed by 'make install' even if MariaDB is built with bundled pcre. But pcre.h is referred in one of installed header files like : include/mysql/private/item_cmpfunc.h:#include "pcre.h" /* pcre header file */ So I think that it is better to install pcre.h as well if MariaDB is built with bundled pcre. What do you think ? -- Kazuhiko Shiozaki, Nexedi SA Senior Consultant Nexedi: Consulting and Development of Free / Open Source Software http://www.nexedi.com/ ERP5: Full Featured High End Open Source ERP http://www.erp5.com/

2 1