July 2012 - developers - lists.mariadb.org

Re: [Maria-developers] MDEV-318 IF (NOT) EXIST clauses for ALTER TABLE
by Sergei Golubchik 13 Jan '13

13 Jan '13

Hi, Holyfoot! On Jul 08, holyfoot(a)askmonty.org wrote: > At file:///home/hf/wmar/mdev-318/ > > ------------------------------------------------------------ > revno: 3552 > revision-id: holyfoot(a)askmonty.org-20120708152354-d7iu7yoqo3cohidz > parent: sanja(a)montyprogram.com-20120626184334-ptpg39ptq3t79dg5 > committer: Alexey Botchkov <holyfoot(a)askmonty.org> > branch nick: mdev-318 > timestamp: Sun 2012-07-08 20:23:54 +0500 > message: > MDEV-318 IF (NOT) EXIST clauses for ALTER TABLE (MWL #252). > ALTER TABLE syntax modified and related implementations added to allow: > ALTER TABLE ADD/DROP COLUMN > ALTER TABLE ADD/DROP INDEX > ALTER TABLE ADD/DROP FOREIGN KEY > ALTER TABLE ADD/DROP PARTITION > ALTER TABLE CHANGE COLUMN > ALTER TABLE MODIFY COLUMN Please, find my comments below: > === modified file 'mysql-test/r/alter_table.result' > --- mysql-test/r/alter_table.result 2011-11-21 17:13:14 +0000 > +++ mysql-test/r/alter_table.result 2012-07-11 14:24:32 +0000 > @@ -1282,3 +1282,30 @@ > # Clean-up. > drop table t1; > End of 5.1 tests > +CREATE TABLE t1 ( > +id INT(11) NOT NULL, > +x_param INT(11) DEFAULT NULL, > +PRIMARY KEY (id), > +KEY x_param (x_param) > +); > +ALTER TABLE t1 ADD COLUMN IF NOT EXISTS id INT, > +ADD COLUMN IF NOT EXISTS lol INT AFTER id; > +Warnings: > +Note 1060 Duplicate column name 'id' > +ALTER TABLE t1 ADD COLUMN IF NOT EXISTS lol INT AFTER id; > +Warnings: > +Note 1060 Duplicate column name 'lol' > +ALTER TABLE t1 DROP COLUMN IF EXISTS lol; > +ALTER TABLE t1 DROP COLUMN IF EXISTS lol; > +Warnings: > +Note 1091 Can't DROP 'lol'; check that column/key exists > +ALTER TABLE t1 ADD KEY IF NOT EXISTS x_param(x_param); > +Warnings: > +Note 1061 Duplicate key name 'x_param' > +ALTER TABLE t1 ADD KEY IF NOT EXISTS x_param(x_param); typo? you test ADD KEY IF NOT EXISTS twice. > +Warnings: > +Note 1061 Duplicate key name 'x_param' > +ALTER TABLE t1 MODIFY IF EXISTS lol INT; > +Warnings: > +Note 1054 Unknown column 'lol' in 't1' > +DROP TABLE t1; > === modified file 'sql/field.cc' > --- sql/field.cc 2012-03-13 14:38:43 +0000 > +++ sql/field.cc 2012-07-11 14:28:40 +0000 > @@ -9182,6 +9182,7 @@ void Create_field::init_for_tmp_table(en First: please, see the last section in http://kb.askmonty.org/en/how-to-get-more-out-of-bzr-when-working-on-mariad… and do as it suggests I had to apply your patch locally and re-run bzr diff to get the function names in the diff :( > (maybe_null ? FIELDFLAG_MAYBE_NULL : 0) | > (is_unsigned ? 0 : FIELDFLAG_DECIMAL)); > vcol_info= 0; > + create_if_not_exists= FALSE; > stored_in_db= TRUE; > } > > === modified file 'sql/sql_lex.h' > --- sql/sql_lex.h 2011-12-11 16:39:33 +0000 > +++ sql/sql_lex.h 2012-07-11 14:28:40 +0000 > @@ -1816,6 +1816,7 @@ typedef struct st_lex : public Query_tab > uint16 create_view_algorithm; > uint8 create_view_check; > bool drop_if_exists, drop_temporary, local_file, one_shot_set; > + bool check_exists; /* Enabled if the IF [NOT] EXISTS specified for ALTER */ I think, you can remove drop_if_exists and use check_exists instead > bool autocommit; > bool verbose, no_write_to_binlog; > > === modified file 'sql/sql_yacc.yy' > --- sql/sql_yacc.yy 2012-06-10 09:53:06 +0000 > +++ sql/sql_yacc.yy 2012-07-11 14:28:40 +0000 > @@ -4656,8 +4659,16 @@ table_option: > ; > > opt_if_not_exists: > - /* empty */ { $$= 0; } > - | IF not EXISTS { $$=HA_LEX_CREATE_IF_NOT_EXISTS; } > + /* empty */ > + { > + Lex->check_exists= FALSE; Let's initialize check_exists either before the grammar is parsed (like in the add_create_index_prepare and in the create rule) of in the grammar (here, in the opt_if_not_exists). But don't do it twice. > + $$= 0; > + } > + | IF not EXISTS > + { > + Lex->check_exists= TRUE; > + $$=HA_LEX_CREATE_IF_NOT_EXISTS; > + } > ; > > opt_create_table_options: > @@ -5879,6 +5891,18 @@ opt_ident: > | field_ident { $$=$1.str; } > ; > > +opt_if_not_exists_ident: > + opt_if_not_exists opt_ident > + { > + LEX *lex= Lex; > + if (lex->check_exists && lex->sql_command != SQLCOM_ALTER_TABLE) > + { > + my_parse_error(ER(ER_SYNTAX_ERROR)); > + MYSQL_YYABORT; > + } > + $$= $2; > + }; > + why did you create opt_if_not_exists_ident rule, instead of adding opt_if_not_exists before ident in the alter table grammar? > opt_component: > /* empty */ { $$= null_lex_str; } > | '.' ident { $$= $2; } > @@ -6455,6 +6482,11 @@ opt_column: > | COLUMN_SYM {} > ; > > +opt_column_if_exists: > + opt_column if_exists > + {} > + ; > + same as above - why a special rule? > opt_ignore: > /* empty */ { Lex->ignore= 0;} > | IGNORE_SYM { Lex->ignore= 1;} > @@ -10096,8 +10128,16 @@ table_alias_ref: > ; > > if_exists: please, rename this old rule to opt_if_exists > - /* empty */ { $$= 0; } > - | IF EXISTS { $$= 1; } > + /* empty */ > + { > + Lex->check_exists= FALSE; > + $$= 0; > + } > + | IF EXISTS > + { > + Lex->check_exists= TRUE; > + $$= 1; > + } > ; > > opt_temporary: you didn't change CREATE INDEX and DROP INDEX, did you? and ALTER TABLE DROP PRIMARY KEY isn't covered either > === modified file 'sql/sql_table.cc' > --- sql/sql_table.cc 2012-04-05 21:07:18 +0000 > +++ sql/sql_table.cc 2012-07-11 14:28:40 +0000 > @@ -5788,6 +5788,226 @@ is_index_maintenance_unique (TABLE *tabl > > > /* > + Preparation for table creation > + > + SYNOPSIS > + handle_if_exists_option() Why did you do that in a separate function? You have basically duplicated all checks. normal alter table code checks for duplicate columns/key names and for non-existant columns/keys for DROP. this code is still in place after your patch. so, basically, you'll check for duplicates and non-existant names twice. what's the point of that? I'd expect you to take check_exists flag into account in the existing checking code and convert errors into warnings as appropriate. > + thd Thread object. > + table The altered table. > + alter_info List of columns and indexes to create > + > + DESCRIPTION > + Looks for the IF [NOT] EXISTS options, checks the states and remove items > + from the list if existing found. > + > + RETURN VALUES > + NONE > +*/ > + > +static void > +handle_if_exists_options(THD *thd, TABLE *table, Alter_info *alter_info) > +{ > + Field **f_ptr; Regards, Sergei

2 9

Re: [Maria-developers] [Commits] Rev 3353: Performed re-factoring and re-structuring of the code for mwl#248: in file:///home/igor/maria/maria-5.5-mwl248-changes/
by Sergei Golubchik 28 Aug '12

28 Aug '12

Hi, Davi! On Jul 31, Davi Arnaut wrote: > Also, I would like to point out that my main argument a few e-mails > ago is that you cannot memset/bzero a non-POD type in C++ without > occurring in undefined/implementation-defined behavior, which was > ignored. This significantly lowers my confidence in MariaDB being a > high quality code base (save for some developers who tend to get it). :) After your first email I've checked that code. Perhaps I've missed something, but it did look like all bzero-ed structures in the igor's patch were of POD types. Regards, Sergei

2 3

[Maria-developers] Review request: SHOW EXPLAIN
by Sergei Petrunia 08 Aug '12

08 Aug '12

Hi Sergei, Please find attached the combined SHOW EXPLAIN patch. It is ready for review of the concurrency part. BR Sergei -- Sergei Petrunia, Software Developer Monty Program AB, http://askmonty.org Blog: http://s.petrunia.net/blog

3 12

Re: [Maria-developers] [Commits] Rev 3353: Performed re-factoring and re-structuring of the code for mwl#248: in file:///home/igor/maria/maria-5.5-mwl248-changes/
by Michael Widenius 31 Jul '12

31 Jul '12

Hi! >>>>> "Davi" == Davi Arnaut <davi(a)twitter.com> writes: Davi> On Mon, Jul 30, 2012 at 7:34 AM, Oleksandr Byelkin Davi> <sanja(a)montyprogram.com> wrote: >> 27.07.2012 20:57, Davi Arnaut пишет: >>> >>> Also, bzero is deprecated. >> AFAIK bzero is macro in our sources. Davi> It is a macro if not available. Which then becomes a problem because Davi> the bzero interface is not well defined (first argument begin char Davi> pointer versus void pointer). It should be pretty simple, let's not Davi> use deprecated and broken interfaces. Actually, in mariadb code (and original mysql code) we use bzero a lot and it's a stable, well tested, well known and well defined function. Better to always use the same code to reset memory than doing it many different ways. In MySQL code bzero() has been the offical and recommended way to reset memory. Regards, Monty

1 0

Re: [Maria-developers] [Commits] Rev 3453: MWL#182: Explain running statements
by Sergei Golubchik 28 Jul '12

28 Jul '12

Hi, Sergey! On Jul 17, Sergey Petrunya wrote: > ------------------------------------------------------------ > revno: 3453 > revision-id: psergey(a)askmonty.org-20120717192703-bgt3xgkbs8qvz3tc > parent: psergey(a)askmonty.org-20120717175208-ruovz91shg0ph875 > committer: Sergey Petrunya <psergey(a)askmonty.org> > branch nick: 5.5-show-explain-r21 > timestamp: Tue 2012-07-17 23:27:03 +0400 > message: > MWL#182: Explain running statements > Address feedback from the second code review: > - Make SHOW EXPLAIN code convert the query string before pushing it as a warning text. > @@ -2135,8 +2135,35 @@ int fill_show_explain(THD *thd, TABLE_LI > } > else > { > + /* > + Push the query string as a warning. The query may be in a different > + charset than the charset that's used for error messages, so, convert it > + if needed. > + */ there's no test case for this. was it too difficult to create? Regards, Sergei

1 0

[Maria-developers] question about MTR
by Rich Prohaska 26 Jul '12

26 Jul '12

Hello, It appears that MTR looks at a test and figures out the appropriate mysqld configuration needed to run the test. For example, if I have lines like "source include/have_XYZ.inc" in my test, MTR will run the test with the right storage engines loaded (and only those storage engines). Is this accurate? Thanks Rich Prohaska

2 1

[Maria-developers] Please test: BUG#992942 & MDEV-325 patch
by Sergei Petrunia 19 Jul '12

19 Jul '12

Hi Elena, I've made a patch that fixes both BUG#992942 and MDEV-325. It is rather risky, as it touches the part of the code whose interaction with other parts of the code is not very well defined. This is why I request RQG testing. Could you please run some test that would use ORDER BY ... LIMIT, GROUP BY, subqueries, tables that may or may not be read with range/index_merge access, index condition pushdown, etc? (Yes, you may really need all of the above to trigger the problem. Check out analysis comments in the bug). The code is at: bzr+ssh://bazaar.launchpad.net/~maria-captains/maria/5.5-show-explain-test1 it is based on 5.5-show-explain, so SHOW EXPLAIN part can be tested, too. BR Sergei -- Sergei Petrunia, Software Developer Monty Program AB, http://askmonty.org Blog: http://s.petrunia.net/blog

1 1

[Maria-developers] Statistics tables show up in userstat - intentional?
by Sergei Petrunia 12 Jul '12

12 Jul '12

Hello Igor, I've noticed that engine-independent statistics show up in userstats: +--------------+-------------+-----------+--------------+-------------------------+ | Table_schema | Table_name | Rows_read | Rows_changed | Rows_changed_x_#indexes | +--------------+-------------+-----------+--------------+-------------------------+ | bug929732 | c2 | 249 | 0 | 0 | | mysql | index_stat | 84 | 0 | 0 | | bug929732 | v | 276 | 0 | 0 | | mysql | column_stat | 44 | 0 | 0 | | bug929732 | c | 249 | 0 | 0 | | bug929732 | v3 | 837280 | 0 | 0 | | mysql | table_stat | 12 | 0 | 0 | | bug929732 | v4 | 3994 | 0 | 0 | | bug929732 | v2 | 53506 | 0 | 0 | +--------------+-------------+-----------+--------------+-------------------------+ Is this intentional? I've also started collecting some minimal docs at http://kb.askmonty.org/en/engine-independent-table-statistics/ BR Sergei -- Sergei Petrunia, Software Developer Monty Program AB, http://askmonty.org Blog: http://s.petrunia.net/blog

1 0

[Maria-developers] 10.0-base tree created and is in buildbot
by Sergei Petrunia 11 Jul '12

11 Jul '12

Hello, As discussed before, I have created a 10.0-base tree at lp:~maria-captains/maria/10.0-base I've pushed there contents of 10.0-monty tree and have added the new tree to buildbot. lp:~maria-captains/maria/10.0 tree was deleted (it was created in error) BR Sergei -- Sergei Petrunia, Software Developer Monty Program AB, http://askmonty.org Blog: http://s.petrunia.net/blog

1 0

Re: [Maria-developers] problem with savepoints in mariadb 5.5.25
by Rich Prohaska 11 Jul '12

11 Jul '12

Hello, Thanks. Per Elena's request, i created bug #1023404. Rich Prohaska On Wed, Jul 11, 2012 at 7:27 AM, Michael Widenius <michael.widenius(a)gmail.com> wrote: > > Hi! > >>>>>> "Rich" == Rich Prohaska <prohaska(a)tokutek.com> writes: > > Rich> Hello, > Rich> I posted our fix at lp:~prohaska7/maria/5.5-savepoint-offset-fix. > Rich> Does it make sense? > Rich> Thanks > Rich> Rich Prohaska > > Sergei, who wrote the handlerton interface, is just now on his way > home from his conference trip to China. He should be able to help you > with this issue as soon as he is back. > > Regards, > Monty

1 0