September 2015 - developers - lists.mariadb.org

[Maria-developers] MDEV-8372 Use helper methods introduced in MDEV-7824 all around the code
by Alexander Barkov 11 Sep '15

11 Sep '15

Hi Sergei, Please review my patch for mdev-8372. I found only a few places to reuse the new methods. Also, had to add some more helper methods to make the code clearer. Thanks.

2 3

Re: [Maria-developers] [Commits] 32b1497: MDEV-7990: ERROR 1526 when procedure executed for second time ALTER TABLE partition ... pMAX values less than MAXVALUE
by Sergei Golubchik 11 Sep '15

11 Sep '15

Hi, Sanja! On Aug 06, sanja(a)mariadb.com wrote: > revision-id: 32b14970a1b88571aa96f604e5c59ab6bf1b298c > parent(s): aa5095627e2619bdad7916d33d1016802a84a9e1 > committer: Oleksandr Byelkin > branch nick: server > timestamp: 2015-08-06 14:00:48 +0200 > message: > > MDEV-7990: ERROR 1526 when procedure executed for second time ALTER > TABLE partition ... pMAX values less than MAXVALUE > > Added cleaning partition data which are saved in LEX structure to make > possible correct re-execution of a SP or PS using the code. This seems to be wrong now. It's an old comment, but now your fix works differently, doesn't it? The patch is ok to push, thanks! Regards, Sergei

1 0

Re: [Maria-developers] [Commits] 0a1b60a: MDEV-8450: PATCH] Wrong macro expansion in Query_cache::send_result_to_client()
by Sergey Vojtovich 06 Sep '15

06 Sep '15

Hi Sanja, ok to push, a few minor notes JFYI inline. I found only one real bug, but generally I agree that we should fix these fragile macros. On Tue, Sep 01, 2015 at 11:47:09AM +0200, sanja(a)mariadb.com wrote: > revision-id: 0a1b60a8aee4bce3235fd5a1d8cb2231b51ff5cc (mariadb-10.0.21-7-g0a1b60a) > parent(s): 4b41e3c7f33714186c97a6cc2e6d3bb93b050c61 > committer: Oleksandr Byelkin > timestamp: 2015-09-01 11:47:06 +0200 > message: > > MDEV-8450: PATCH] Wrong macro expansion in Query_cache::send_result_to_client() > > Expression in macro protected by () > > diff --git a/include/maria.h b/include/maria.h > index 16a9bea..cdaeade 100644 > --- a/include/maria.h > +++ b/include/maria.h > @@ -43,7 +43,7 @@ extern "C" { > #define MARIA_NAME_IEXT ".MAI" > #define MARIA_NAME_DEXT ".MAD" > /* Max extra space to use when sorting keys */ > -#define MARIA_MAX_TEMP_LENGTH 2*1024L*1024L*1024L > +#define MARIA_MAX_TEMP_LENGTH (2*1024L*1024L*1024L) > /* Possible values for maria_block_size (must be power of 2) */ > #define MARIA_KEY_BLOCK_LENGTH 8192 /* default key block length */ > #define MARIA_MIN_KEY_BLOCK_LENGTH 1024 /* Min key block length */ Was this used at all? May be remove it? > diff --git a/sql/log_slow.h b/sql/log_slow.h > index 2ae07da..3ae2060 100644 > --- a/sql/log_slow.h > +++ b/sql/log_slow.h > @@ -16,23 +16,23 @@ > /* Defining what to log to slow log */ > > #define LOG_SLOW_VERBOSITY_INIT 0 > -#define LOG_SLOW_VERBOSITY_INNODB 1 << 0 > -#define LOG_SLOW_VERBOSITY_QUERY_PLAN 1 << 1 > -#define LOG_SLOW_VERBOSITY_EXPLAIN 1 << 2 > +#define LOG_SLOW_VERBOSITY_INNODB (1 << 0) Was LOG_SLOW_VERBOSITY_INNODB used at all? May be remove it? > +#define LOG_SLOW_VERBOSITY_QUERY_PLAN (1 << 1) > +#define LOG_SLOW_VERBOSITY_EXPLAIN (1 << 2) > > #define QPLAN_INIT QPLAN_QC_NO > > -#define QPLAN_ADMIN 1 << 0 > -#define QPLAN_FILESORT 1 << 1 > -#define QPLAN_FILESORT_DISK 1 << 2 > -#define QPLAN_FULL_JOIN 1 << 3 > -#define QPLAN_FULL_SCAN 1 << 4 > -#define QPLAN_QC 1 << 5 > -#define QPLAN_QC_NO 1 << 6 > -#define QPLAN_TMP_DISK 1 << 7 > -#define QPLAN_TMP_TABLE 1 << 8 > -#define QPLAN_FILESORT_PRIORITY_QUEUE 1 << 9 > +#define QPLAN_ADMIN (1 << 0) > +#define QPLAN_FILESORT (1 << 1) > +#define QPLAN_FILESORT_DISK (1 << 2) > +#define QPLAN_FULL_JOIN (1 << 3) > +#define QPLAN_FULL_SCAN (1 << 4) > +#define QPLAN_QC (1 << 5) > +#define QPLAN_QC_NO (1 << 6) > +#define QPLAN_TMP_DISK (1 << 7) > +#define QPLAN_TMP_TABLE (1 << 8) > +#define QPLAN_FILESORT_PRIORITY_QUEUE (1 << 9) > > /* ... */ > -#define QPLAN_MAX ((ulong) 1) << 31 /* reserved as placeholder */ > +#define QPLAN_MAX (((ulong) 1) << 31) /* reserved as placeholder */ > log_slow.h was fixed in rev.203f4d419 in my 10.0. Did you use outdated tree? I may be wrong, but there seem to be no real bug with QPLAN_QC_NO currently. Just because nobody sets it. So in fact it clears QPLAN_ADMIN, which FWICS is never set at this point. But it is a good to have it fixed nevertheless. Btw, QPLAN_QC_NO doesn't seem to be used in fact. May be remove it? > diff --git a/storage/archive/archive_test.c b/storage/archive/archive_test.c > index d01c1e0..bb052b8 100644 > --- a/storage/archive/archive_test.c > +++ b/storage/archive/archive_test.c > @@ -33,7 +33,7 @@ > > #define ARCHIVE_ROW_HEADER_SIZE 4 > > -#define BUFFER_LEN 1024 + ARCHIVE_ROW_HEADER_SIZE > +#define BUFFER_LEN (1024 + ARCHIVE_ROW_HEADER_SIZE) > > char test_string[BUFFER_LEN]; Alright, there was a bug in this line: assert(write_length != count * BUFFER_LEN); > diff --git a/strings/ctype.c b/strings/ctype.c > index 048fbe3..d8a1dd7 100644 > --- a/strings/ctype.c > +++ b/strings/ctype.c > @@ -264,7 +264,7 @@ static const struct my_cs_file_section_st > } > > #define MY_CS_CSDESCR_SIZE 64 > -#define MY_CS_TAILORING_SIZE 32*1024 > +#define MY_CS_TAILORING_SIZE (32*1024) > #define MY_CS_UCA_VERSION_SIZE 64 > #define MY_CS_CONTEXT_SIZE 64 Was this used at all? May be remove it? Regards, Sergey

2 1

Re: [Maria-developers] [Commits] 62fe87c: MDEV-8663: IF Statement returns multiple values erroneously (or Assertion `!null_value' failed in Item::send(Protocol*, String*))
by Alexander Barkov 04 Sep '15

04 Sep '15

Hi Sanja, From my understanding, the patch fixes consequences of the problem, but not the problem itself. It very likely will still have crashes with slightly modified SQL scripts. The source of the problem is that Item_func_hybrid_result_type::val_decimal() does not return NULL when int_op() or real_op() set null_value to "true". By the way, compare Item_func_hybrid_result_type::val_decimal() with a very similar method Item_func_hybrid_result_type::val_str(). The latter already handle the INT_RESULT and REAL_RESULT cases well and return NULL properly in case of null_value. get_date(), val_int(), val_real() also look fine for me. So the are problems only in val_decimal(). I propose a small patch that fixes the source of the problem. This is the small patch: > @@ -1038,17 +1026,21 @@ my_decimal *Item_func_hybrid_result_type::val_decimal(my_decimal *decimal_value) > case INT_RESULT: > { > longlong result= int_op(); > + if (null_value) > + return NULL; > int2my_decimal(E_DEC_FATAL_ERROR, result, unsigned_flag, decimal_value); > break; > } > case REAL_RESULT: > { > double result= (double)real_op(); > + if (null_value) > + return NULL; > double2my_decimal(E_DEC_FATAL_ERROR, result, decimal_value); > break; > } I also propose an extended patch version, to cover the code with more DBUG_ASSERTs, so in case of the null_value related bugs it crashes much earlier than in Item::send(). It will be easier to debug problems this way. To avoid much code duplication, I added three private helper methods. The extended version of the patch is attached. The scripts from the bug report start to return expected results, with no crashes in DEBUG build, and "mtr --suite=main" works fine. I have not run the entire mtr though, to save time. Greetings. On 09/03/2015 08:00 PM, sanja(a)mariadb.com wrote: > revision-id: 62fe87c5a239c7304b245ce5954c87aecb6382ad (mariadb-5.5.45-1-g62fe87c) > parent(s): fa51f70dc68fe2f3afe943e2c81fcbdb34f16cad > committer: Oleksandr Byelkin > timestamp: 2015-09-03 18:00:43 +0200 > message: > > MDEV-8663: IF Statement returns multiple values erroneously (or Assertion `!null_value' failed in Item::send(Protocol*, String*)) > > keeping contract: NULL value mean NULL pointer in val_str and val_deciman. > > --- > mysql-test/r/func_if.result | 17 +++++++++++++++++ > mysql-test/t/func_if.test | 14 ++++++++++++++ > sql/item_cmpfunc.cc | 6 ++++-- > sql/item_func.cc | 5 +++++ > 4 files changed, 40 insertions(+), 2 deletions(-) > > diff --git a/mysql-test/r/func_if.result b/mysql-test/r/func_if.result > index c7f548a..61f63cc 100644 > --- a/mysql-test/r/func_if.result > +++ b/mysql-test/r/func_if.result > @@ -234,3 +234,20 @@ SELECT if(1, NULL, (SELECT min('hello'))); > if(1, NULL, (SELECT min('hello'))) > NULL > End of 5.2 tests > +# > +# MDEV-8663: IF Statement returns multiple values erroneously > +# (or Assertion `!null_value' failed in Item::send(Protocol*, String*) > +# > +CREATE TABLE `t1` ( > +`datas` VARCHAR(25) NOT NULL > +) ENGINE=InnoDB DEFAULT CHARSET=utf8; > +Warnings: > +Warning 1286 Unknown storage engine 'InnoDB' > +Warning 1266 Using storage engine MyISAM for table 't1' > +INSERT INTO `t1` VALUES ('1,2'), ('2,3'), ('3,4'); > +SELECT IF(FIND_IN_SET('1', `datas`), 1.5, IF(FIND_IN_SET('2', `datas`), 2, NULL)) AS `First`, '1' AS `Second`, '2' AS `Third` FROM `t1`; > +First Second Third > +1.5 1 2 > +2.0 1 2 > +NULL 1 2 > +drop table t1; > diff --git a/mysql-test/t/func_if.test b/mysql-test/t/func_if.test > index 2b89a61..8fdba77 100644 > --- a/mysql-test/t/func_if.test > +++ b/mysql-test/t/func_if.test > @@ -206,6 +206,20 @@ SELECT if(1, NULL, (SELECT min('hello'))); > > --echo End of 5.2 tests > > +--echo # > +--echo # MDEV-8663: IF Statement returns multiple values erroneously > +--echo # (or Assertion `!null_value' failed in Item::send(Protocol*, String*) > +--echo # > +CREATE TABLE `t1` ( > +`datas` VARCHAR(25) NOT NULL > +) ENGINE=InnoDB DEFAULT CHARSET=utf8; > + > +INSERT INTO `t1` VALUES ('1,2'), ('2,3'), ('3,4'); > + > +SELECT IF(FIND_IN_SET('1', `datas`), 1.5, IF(FIND_IN_SET('2', `datas`), 2, NULL)) AS `First`, '1' AS `Second`, '2' AS `Third` FROM `t1`; > + > +drop table t1; > + > --disable_query_log > # Restore timezone to default > set time_zone= @@global.time_zone; > diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc > index 998cb1c..4f07318 100644 > --- a/sql/item_cmpfunc.cc > +++ b/sql/item_cmpfunc.cc > @@ -2692,7 +2692,8 @@ Item_func_if::str_op(String *str) > String *res=arg->val_str(str); > if (res) > res->set_charset(collation.collation); > - null_value=arg->null_value; > + if (null_value=arg->null_value) > + res= NULL; > return res; > } > > @@ -2703,7 +2704,8 @@ Item_func_if::decimal_op(my_decimal *decimal_value) > DBUG_ASSERT(fixed == 1); > Item *arg= args[0]->val_bool() ? args[1] : args[2]; > my_decimal *value= arg->val_decimal(decimal_value); > - null_value= arg->null_value; > + if (null_value= arg->null_value) > + value= NULL; > return value; > } > > diff --git a/sql/item_func.cc b/sql/item_func.cc > index 2f51308..cbb8aa0 100644 > --- a/sql/item_func.cc > +++ b/sql/item_func.cc > @@ -924,6 +924,7 @@ String *Item_func_hybrid_result_type::val_str(String *str) > ltime.time_type= mysql_type_to_time_type(field_type()); > str->length(my_TIME_to_str(&ltime, const_cast<char*>(str->ptr()), decimals)); > str->set_charset(&my_charset_bin); > + DBUG_ASSERT(!null_value); > return str; > } > return str_op(&str_value); > @@ -932,6 +933,7 @@ String *Item_func_hybrid_result_type::val_str(String *str) > case IMPOSSIBLE_RESULT: > DBUG_ASSERT(0); > } > + DBUG_ASSERT(!null_value || (str == NULL)); > return str; > } > > @@ -1071,7 +1073,10 @@ my_decimal *Item_func_hybrid_result_type::val_decimal(my_decimal *decimal_value) > } > String *res; > if (!(res= str_op(&str_value))) > + { > + null_value= 1; > return NULL; > + } > > str2my_decimal(E_DEC_FATAL_ERROR, (char*) res->ptr(), > res->length(), res->charset(), decimal_value); > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits >

1 0

[Maria-developers] [GSoC] MDEV-4674 Port InnoDB memcached interface to MariaDB - Merging the code
by Piotr Jurkiewicz 02 Sep '15

02 Sep '15

Hi again, there are some issues which I think needs to be resolved before merging the code. 1. Bundle libevent or not? Memcached daemon is built with the help of libevent library. MySQL decided to bundle this library inside its sources, in 'libevent' top level directory. This allows to compile and run the plugin on machines without installed system-wide libevent library. The bundled libevent is used by default during the compilation of plugin. Optionally, it can be specified in CMake variable to use system-wide libevent instead. It needs to be decided whether MariaDB should bundle libevent as well inside its sources. Possible outcomes: a) Leave bundled libevent as is, in the 'libevent' top level directory. b) Move bundled libevent into the 'daemon_memcached' plugin directory, in order to not clutter the top level directory. c) Remove bundled libevent. After that, system libevent headers will become compile-time dependency and system libevent shared library runtime dependency of the plugin. 2. How to transfer InnoDB API callback array pointer to the plugin? InnoDB Memcached interface engine needs to obtain the pointer to InnoDB API callback array in some way. MySQL originally implemented that in 'sql/sql_plugin.cc:plugin_initialize()' in the following way: https://github.com/piotrjurkiewicz/mariadb-server/commit/4eb8fd9c211f396694… As you can see, this is not a clean solution. Possible outcomes: a) Leave the MySQL's solution as is. b) Figure out a better way to transfer the pointer. 3. Where should I post the documentation? 4. How should I prepare the pull request? Should I rebase my branch on the top of current branch before that? I looking forward for your suggestions and decisions. Piotr

2 1