- developers - lists.mariadb.org

Re: [Maria-developers] [Commits] Rev 4346: MDEV-7026 - Occasional hang during startup on Power8 in lp:maria/5.5
by Kristian Nielsen 17 Nov '14

17 Nov '14

Sergey Vojtovich <svoj(a)mariadb.org> writes: > revno: 4346 > revision-id: svoj(a)mariadb.org-20141112114907-thnro6e3kg2ofdsw > committer: Sergey Vojtovich <svoj(a)mariadb.org> > timestamp: Wed 2014-11-12 15:49:07 +0400 > message: > MDEV-7026 - Occasional hang during startup on Power8 > === modified file 'storage/xtradb/sync/sync0sync.c' > --- a/storage/xtradb/sync/sync0sync.c 2014-11-03 13:43:44 +0000 > +++ b/storage/xtradb/sync/sync0sync.c 2014-11-12 11:49:07 +0000 > @@ -467,26 +467,15 @@ mutex_set_waiters( > mutex_t* mutex, /*!< in: mutex */ > ulint n) /*!< in: value to set */ > { > -#ifdef INNODB_RW_LOCKS_USE_ATOMICS > - ut_ad(mutex); > - > - if (n) { > - os_compare_and_swap_ulint(&mutex->waiters, 0, 1); > - } else { > - os_compare_and_swap_ulint(&mutex->waiters, 1, 0); > - } > -#else > volatile ulint* ptr; /* declared volatile to ensure that > the value is stored to memory */ > ut_ad(mutex); > > ptr = &(mutex->waiters); > > - os_wmb; > - > *ptr = n; /* Here we assume that the write of a single > word in memory is atomic */ > -#endif > + os_sync; > } This code is still substantially different from what it was in 5.5.39 and 10.0.12, right? I think we need to instead revert the code to what it was before introducing the problem. The InnoDB mutex code clearly does not work by careful design - mutex_reset_lock_word() needs a full barrier but uses what is documented by GCC as only an acquire barrier. Rather, it works by having been tweaked over the years to just happen to work (eg. GCC in fact uses a full barrier on x86). So changing this to be "correct" is very dangerous in a stable release. It is not possible to ensure correctness by testing, either. But it is not necessary to change it. Instead, leave the code as it was before, and use #ifdef __powerpc__ to ensure that it will work also on PowerPC. For example, something like this (just a sketch, I didn't check exactly how the code should look): #ifdef __powerpc__ /* InnoDB mutex implementation needs full memory barrier for this operation */ # define os_atomic_test_and_set_byte(ptr, new_val) \ do { sync(); __sync_lock_test_and_set(ptr, (byte) new_val); } while (0) #else # define os_atomic_test_and_set_byte(ptr, new_val) \ __sync_lock_test_and_set(ptr, (byte) new_val) #endif Then we leave stable releases 5.5 and 10.0 unchanged, reducing the risk of introducing another serious breakage. And we can still fix PowerPC to work correctly. And then perhaps in 10.1 we can do refactoring of the use of memory barriers in InnoDB, if we really want. (Just in case it's not obvious, yes, I _really_ have the InnoDB mutex code). Remember, the 5.5 and 10.0 minor releases appear as security updates in distros, so it is very important to minimise the risk of introducing serious regressions. > Since isync is not a memory barrier and lwsync doesn't guarantee StoreLoad > order, the only option we have is full memory barrier. It's not a problem to have a full barrier here, I think. Because at this point, the waiter has already gone through a lot of spin loops, and now it is going to take a global mutex on the sync wait array, scan the array, and go to sleep with a context switch. One less full barrier is not going to help a lot. To improve this contended case of the InnoDB mutexes, more radical changes are needed. > InnoDB/XtraDB was getting stuck during server startup. In particular (but not > limited to) while creating doublewrite buffer. The stuck during startup is kind of the minor part of the bug. The major part is stalling the server during operation. (The hang during startup was just how the problem was detected in Buildbot). Better clarify in the commit message that this fixes a bug that could cause the entire server to stall (or even hang completely if error monitor thread itself got stuck). This will help people who experienced the problem and look at the changelog. > It happens because mutex_exit() didn't wake up waiters of affected mutex. This > is kind of acceptable because error monitor thread is supposed to wake up such > waiters. But since error monitor thread hasn't been started that early, nobody > did inform waiting threads that mutex was released. I do not think the error monitor is "supposed to wake up such waiters". In fact, it seems clear that the error monitor wakeup is _not_ used, at least not in commonly used binaries. Because it just never worked - it itself needs to take mutexes, which will prevent it from doing the wakeup. So if it was needed, someone would have noticed this problem. The symptom is _very_ hard to miss, as the whole server gets stuck hard, unable to do anything. It really is not acceptable to have a mutex implementation that can miss wakeups. This can stall the entire server. Even if the error monitor resolves the stall after 1 second, that is still unacceptable for many applications. Hope this helps, - Kristian.

2 3

[Maria-developers] Review: [MDEV-7062] parser refactoring: don't store field properties in LEX
by Alexander Barkov 16 Nov '14

16 Nov '14

Hi Sergei, > Sergei Golubchik edited comment on MDEV-7062 at 11/10/14 11:40 AM: > ------------------------------------------------------------------- > > http://lists.askmonty.org/pipermail/commits/2014-November/006908.html > > {noformat} > From: serg(a)mariadb.org > To: commits(a)mariadb.org > Subject: [Commits] 2c904b9: parser cleanup: don't store field properties > in LEX, use Create_field directly > Date: Mon, 10 Nov 2014 10:26:48 +0100 > {noformat} > Please find my comments below: > > [Commits] 2c904b9: parser cleanup: don't store field properties in LEX, use Create_field directly > serg at mariadb.org serg at mariadb.org > Mon Nov 10 11:26:48 EET 2014 > > Previous message: [Commits] Rev 4344: MDEV-6179: dynamic columns functions/cast()/convert() doesn't play nice with CREATE/ALTER TABLE in lp:~maria-captains/maria/5.5 > Next message: [Commits] Rev 4345: MDEV-6862 "#error <my_config.h>" and third-party libraries in lp:~maria-captains/maria/5.5 > Messages sorted by: [ date ] [ thread ] [ subject ] [ author ] > > revision-id: 2c904b937b5ef7fbec60ebb9cb514a501c5bd8e4 > parent(s): f59588b9112965849986c45fcd150d9abed74063 > committer: Sergei Golubchik > branch nick: maria > timestamp: 2014-11-09 22:33:17 +0100 > message: > > parser cleanup: don't store field properties in LEX, use Create_field directly > > length/dec/charset are still in LEX, because they're also used > for CAST and dynamic columns. > > also > 1. fix "MDEV-7041 COLLATION(CAST('a' AS CHAR BINARY)) returns a wrong result" > 2. allow BINARY modifier in stored function RETURN clause > 3. allow "COLLATION without CHARSET" in SP/SF (parameters, RETURN, DECLARE) > 4. print correct variable name in error messages for stored routine parameters A very nice clean up. I only found it a little bit not easy to track that all Lex and Create_field attributes get initialized and copied to each other properly. I suggest to go a little bit further and get rid from Lex->dec, Lex->length and Lex->charset at all, and write the attributes directly to the destination as soon as they get parsed. See a proposed addon patch attached. > diff --git a/mysql-test/r/cast.result b/mysql-test/r/cast.result > index c81af13..8ad4568 100644 > --- a/mysql-test/r/cast.result > +++ b/mysql-test/r/cast.result > @@ -796,3 +796,32 @@ DATE("foo") > NULL > Warnings: > Warning 1292 Incorrect datetime value: 'foo' > +create table t1 (a int, b char(5) as (cast("a" as char(10) binary) + a) ); > +show create table t1; > +Table Create Table > +t1 CREATE TABLE `t1` ( > + `a` int(11) DEFAULT NULL, > + `b` char(5) AS (cast("a" as char(10) binary) + a) VIRTUAL > +) ENGINE=MyISAM DEFAULT CHARSET=latin1 > +drop table t1; > +select collation(cast("a" as char(10) binary)); > +collation(cast("a" as char(10) binary)) > +latin1_bin > +select collation(cast("a" as char(10) charset utf8 binary)); > +collation(cast("a" as char(10) charset utf8 binary)) > +utf8_bin > +select collation(cast("a" as char(10) ascii binary)); > +collation(cast("a" as char(10) ascii binary)) > +latin1_bin > +select collation(cast("a" as char(10) unicode binary)); > +collation(cast("a" as char(10) unicode binary)) > +ucs2_bin UNICODE is an alias for UCS2, which is not always compiled. Please move ucs2 related tests to ctype_ucs.test. <skip> > diff --git a/mysql-test/t/cast.test b/mysql-test/t/cast.test > index b6c37ca..aa43ee6 100644 > --- a/mysql-test/t/cast.test > +++ b/mysql-test/t/cast.test > @@ -456,3 +456,21 @@ SELECT CAST(TIME('10:20:30') AS DATE) + INTERVAL 1 DAY; > SET SQL_MODE=ALLOW_INVALID_DATES; > SELECT DATE("foo"); > > +# > +# CAST and field definition using same fields in LEX > +# > +create table t1 (a int, b char(5) as (cast("a" as char(10) binary) + a) ); > +show create table t1; > +drop table t1; > + > +# > +# CAST (... BINARY) > +# > +select collation(cast("a" as char(10) binary)); > +select collation(cast("a" as char(10) charset utf8 binary)); > +select collation(cast("a" as char(10) ascii binary)); > +select collation(cast("a" as char(10) unicode binary)); > +select collation(cast("a" as char(10) binary charset utf8)); > +select collation(cast("a" as char(10) binary ascii)); > +select collation(cast("a" as char(10) binary unicode)); More UCS2. You removed this code in field.cc: > - if (fld_length != NULL) > - { > - errno= 0; > - length= strtoul(fld_length, NULL, 10); > - if ((errno != 0) || (length > MAX_FIELD_BLOBLENGTH)) > - { > - my_error(ER_TOO_BIG_DISPLAYWIDTH, MYF(0), fld_name, MAX_FIELD_BLOBLENGTH); > - DBUG_RETURN(TRUE); > - } > - > - if (length == 0) > - fld_length= NULL; /* purecov: inspected */ > - } and added this in sql_yacc.yy: > + if (length) > + { > + int err; > + last_field->length= (ulong)my_strtoll10(length, NULL, &err); > + if (err) > + last_field->length= ~0UL; // safety > + } > + else > + last_field->length= 0; I have a feeling that on a 32bit platforms it will stop sending errors on attempt to create a blob larger that 4Gb. last_field->length is ulong, which is 32bit is a 32 bit platform, and it gets assigned to a 64 bit value.

2 2

[Maria-developers] Please review: sql_yacc.yy and HA_CREATE_INFO refactoring (pre-requisite for MDEV-5359 CREATE OR REPLACE, CREATE IF NOT EXISTS, DROP IF EXISTS)
by Alexander Barkov 14 Nov '14

14 Nov '14

Hi Sergei, The patch is attached. Thanks.

1 1

Re: [Maria-developers] [JIRA] (MDEV-7076) Build fails around libgroonga on some platforms in buildbot
by Alexander Barkov 12 Nov '14

12 Nov '14

Hello Kentoku, Mroonga fails to compile on some platforms. Can you please have a look? Thanks. On 11/12/2014 04:39 PM, Alexander Barkov (JIRA) wrote: > > [ https://mariadb.atlassian.net/browse/MDEV-7076?page=com.atlassian.jira.plug… ] > > Alexander Barkov updated MDEV-7076: > ----------------------------------- > Assignee: Kentoku (was: Alexander Barkov) > >> Build fails around libgroonga on some platforms in buildbot >> ----------------------------------------------------------- >> >> Key: MDEV-7076 >> URL: https://mariadb.atlassian.net/browse/MDEV-7076 >> Project: MariaDB Server >> Issue Type: Bug >> Components: OTHER >> Affects Versions: 10.0 >> Reporter: Elena Stepanova >> Assignee: Kentoku >> Priority: Critical >> Labels: buildbot, compile >> Fix For: 10.0.15 >> >> >> http://buildbot.askmonty.org/buildbot/builders/kvm-bintar-centos5-amd64/bui… >> http://buildbot.askmonty.org/buildbot/builders/kvm-deb-debian6-amd64/builds… >> http://buildbot.askmonty.org/buildbot/builders/kvm-deb-lucid-amd64/builds/4… >> It looks approximately like this, with some variations >> {noformat} >> standard input}: Assembler messages: >> {standard input}:28252: Error: open CFI at the end of file; missing .cfi_endproc directive >> gcc-4.4.real: Internal error: Killed (program cc1) >> Please submit a full bug report. >> See <file:///usr/share/doc/gcc-4.4/README.Bugs> for instructions. >> make[3]: *** [storage/mroonga/vendor/groonga/lib/CMakeFiles/libgroonga.dir/expr.c.o] Error 1 >> make[3]: *** Waiting for unfinished jobs.... >> {noformat} >> Given the list of platforms, I guess they all feature pretty old compilers, maybe that's the distinction. > > > > -- > This message was sent by Atlassian JIRA > (v6.4-OD-09-008#64005) >

1 0

[Maria-developers] Mroonga: wrapper.version_56_or_later_performance_schema is failing on BuildBot
by Alexander Barkov 12 Nov '14

12 Nov '14

Hello Kentoku, This test is failing in the latest builds on BB: mroonga/wrapper.version_56_or_later_performance_schema http://buildbot.askmonty.org/buildbot/builders/bld-dan-release/builds/1793/… Can you please check it? Thanks.

2 1

[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