- developers - lists.mariadb.org

Re: [Maria-developers] [Commits] 98b2a9c: MDEV-4774 Strangeness with max_binlog_stmt_cache_size Settings
by Nirbhay Choubey 20 Jan '17

20 Jan '17

Hi Sachin, The overall patch looks ok. I, however, have a few minor comments inline. On Thu, Jan 19, 2017 at 1:21 AM, SachinSetiya <sachin.setiya(a)mariadb.com> wrote: > revision-id: 98b2a9c967a5eaa1f99bb3ef229ff2af62018ffe > (mariadb-10.0.28-34-g98b2a9c) > parent(s): 9bf92706d19761722b46d66a671734466cb6e98e > author: Sachin Setiya > committer: Sachin Setiya > timestamp: 2017-01-19 11:50:59 +0530 > message: > > MDEV-4774 Strangeness with max_binlog_stmt_cache_size Settings > > Problem:- When setting max_binlog_stmt_cache_size=18446744073709547520 > from either command line or .cnf file, server fails to start. > > Solution:- Added one more function eval_num_suffix_ull , which uses > strtoull to get unsigned ulonglong from string. And getopt_ull calles this > Typo s/calles/calls/ > function instead of eval_num_suffix. Also changed previous eval_num_suffix > to > eval_num_suffix_ll to remain consistent. > > --- > .../r/binlog_max_binlog_stmt_cache_size.result | 14 +++++ > .../binlog/t/binlog_max_binlog_stmt_cache_size.opt | 1 + > .../t/binlog_max_binlog_stmt_cache_size.test | 11 ++++ > mysys/my_getopt.c | 66 > ++++++++++++++++++---- > 4 files changed, 81 insertions(+), 11 deletions(-) > > diff --git a/mysql-test/suite/binlog/r/binlog_max_binlog_stmt_cache_size.result > b/mysql-test/suite/binlog/r/binlog_max_binlog_stmt_cache_size.result > new file mode 100644 > index 0000000..6fbec90 > --- /dev/null > +++ b/mysql-test/suite/binlog/r/binlog_max_binlog_stmt_cache_size.result > @@ -0,0 +1,14 @@ > +select @@max_binlog_stmt_cache_size; > +@@max_binlog_stmt_cache_size > +18446744073709547520 > +set global max_binlog_stmt_cache_size= 18446744073709547520; > +select @@max_binlog_stmt_cache_size; > +@@max_binlog_stmt_cache_size > +18446744073709547520 > +set global max_binlog_stmt_cache_size= 18446744073709547519; > +Warnings: > +Warning 1292 Truncated incorrect max_binlog_stmt_cache_size > value: '18446744073709547519' > +select @@max_binlog_stmt_cache_size; > +@@max_binlog_stmt_cache_size > +18446744073709543424 > +set global max_binlog_stmt_cache_size= 18446744073709547520; > diff --git a/mysql-test/suite/binlog/t/binlog_max_binlog_stmt_cache_size.opt > b/mysql-test/suite/binlog/t/binlog_max_binlog_stmt_cache_size.opt > new file mode 100644 > index 0000000..c52ef14 > --- /dev/null > +++ b/mysql-test/suite/binlog/t/binlog_max_binlog_stmt_cache_size.opt > @@ -0,0 +1 @@ > +--max_binlog_stmt_cache_size=18446744073709547520 > diff --git a/mysql-test/suite/binlog/t/binlog_max_binlog_stmt_cache_size.test > b/mysql-test/suite/binlog/t/binlog_max_binlog_stmt_cache_size.test > new file mode 100644 > index 0000000..bc30b48 > --- /dev/null > +++ b/mysql-test/suite/binlog/t/binlog_max_binlog_stmt_cache_size.test > @@ -0,0 +1,11 @@ > +source include/have_log_bin.inc; > +select @@max_binlog_stmt_cache_size; > + > +--let $cache_size=`select @@max_binlog_stmt_cache_size;` > + > +set global max_binlog_stmt_cache_size= 18446744073709547520; > +select @@max_binlog_stmt_cache_size; > + > +set global max_binlog_stmt_cache_size= 18446744073709547519; > +select @@max_binlog_stmt_cache_size; > I would also add tests for ULLONG_MAX and ULLONG_MAX +/- 1. +--eval set global max_binlog_stmt_cache_size= $cache_size > diff --git a/mysys/my_getopt.c b/mysys/my_getopt.c > index 2a45571..0934a50 100644 > --- a/mysys/my_getopt.c > +++ b/mysys/my_getopt.c > @@ -895,11 +895,32 @@ my_bool getopt_compare_strings(register const char > *s, register const char *t, > /* > function: eval_num_suffix > > + Transforms suffix like k/m/g to there real value. > Typo : s/there/their/ > +*/ > +static inline long eval_num_suffix(char *suffix, int *error) > +{ > + long num= 1; > + if (*suffix == 'k' || *suffix == 'K') > + num*= 1024L; > + else if (*suffix == 'm' || *suffix == 'M') > + num*= 1024L * 1024L; > + else if (*suffix == 'g' || *suffix == 'G') > + num*= 1024L * 1024L * 1024L; > + else if (*suffix) > + { > + *error= 1; > + return 0; > + } > + return num; > +} > Please add a new line here (as a separator). > +/* > + function: eval_num_suffix_ll > + > Transforms a number with a suffix to real number. Suffix can > be k|K for kilo, m|M for mega or g|G for giga. > */ > > -static longlong eval_num_suffix(char *argument, int *error, char > *option_name) > +static longlong eval_num_suffix_ll(char *argument, int *error, char > *option_name) > { > char *endchar; > longlong num; > DBUG_ENTER("eval_num_suffix"); You should also update the function name. > @@ -916,23 +937,46 @@ static longlong eval_num_suffix(char *argument, int > *error, char *option_name) > *error= 1; > DBUG_RETURN(0); > } > - if (*endchar == 'k' || *endchar == 'K') > - num*= 1024L; > - else if (*endchar == 'm' || *endchar == 'M') > - num*= 1024L * 1024L; > - else if (*endchar == 'g' || *endchar == 'G') > - num*= 1024L * 1024L * 1024L; > - else if (*endchar) > - { > + num*= eval_num_suffix(endchar, error); > + if (*error) > fprintf(stderr, > "Unknown suffix '%c' used for variable '%s' (value '%s')\n", > *endchar, option_name, argument); > + DBUG_RETURN(num); > +} > + > +/* > + function: eval_num_suffix_ull > + > + Transforms a number with a suffix to positive Integer. Suffix can > + be k|K for kilo, m|M for mega or g|G for giga. > +*/ > + > +static ulonglong eval_num_suffix_ull(char *argument, int *error, char > *option_name) > The above line >80 chars, please break it into 2 lines. > +{ > + char *endchar; > + ulonglong num; > + DBUG_ENTER("eval_num_suffix"); > Update the function name. > + > + *error= 0; > + errno= 0; > + num= strtoull(argument, &endchar, 10); > + if (errno == ERANGE) > + { > + my_getopt_error_reporter(ERROR_LEVEL, > + "Incorrect integer value: '%s'", argument); > *error= 1; > DBUG_RETURN(0); > } > + num*= eval_num_suffix(endchar, error); > + if (*error) > + fprintf(stderr, > + "Unknown suffix '%c' used for variable '%s' (value '%s')\n", > + *endchar, option_name, argument); > DBUG_RETURN(num); > } > > + > /* > function: getopt_ll > > @@ -946,7 +990,7 @@ static longlong eval_num_suffix(char *argument, int > *error, char *option_name) > > static longlong getopt_ll(char *arg, const struct my_option *optp, int > *err) > { > - longlong num=eval_num_suffix(arg, err, (char*) optp->name); > + longlong num=eval_num_suffix_ll(arg, err, (char*) optp->name); > return getopt_ll_limit_value(num, optp, NULL); > } > > @@ -1023,7 +1067,7 @@ longlong getopt_ll_limit_value(longlong num, const > struct my_option *optp, > > static ulonglong getopt_ull(char *arg, const struct my_option *optp, int > *err) > { > - ulonglong num= eval_num_suffix(arg, err, (char*) optp->name); > + ulonglong num= eval_num_suffix_ull(arg, err, (char*) optp->name); > return getopt_ull_limit_value(num, optp, NULL); > } > > Best, Nirbhay

2 3

[Maria-developers] Contribution in mariadb
by Dulton Ghosh 19 Jan '17

19 Jan '17

Respected Sir/Ma'am, I am Dulton Ghosh, a final year student of IIT Guwahati. I want to contribute in MariaDB in upcoming Google Summer of Code. Please give me some instuctions to do so. Regards, Dulton Ghosh IIT Guwahati

2 1

Re: [Maria-developers] Creating test cases for Spider
by Michael Widenius 18 Jan '17

18 Jan '17

Hi! > I will today update the 10.2 spider tree with the latest 10.2 tree > that includes all gcov patches and then try this myself. I have now merged 10.2-spider tree with the latest 10.2 tree, which includes the new HASH and better support for gcov. I am now reviewing the last part of multi_range_read (all files except ha_partition.cc are reviewed). Hope to have this last part done soon. Regards, Monty

2 1

Re: [Maria-developers] [Commits] 14747e4: MDEV-10731: Wrong NULL match results in "Subquery returns more than 1 row" (error code 1242)
by Sergey Petrunia 17 Jan '17

17 Jan '17

Hi Varun, On Tue, Jan 17, 2017 at 03:24:39PM +0530, Varun wrote: > revision-id: 14747e4b84f68ee7e10a6e5a24f4b7ec6f3240f9 (mariadb-10.1.20-38-g14747e4) > parent(s): b56f726e42e8bc0427a0c33cfffb7e1c5399ea16 > author: Varun Gupta > committer: Varun Gupta > timestamp: 2017-01-17 15:22:22 +0530 > message: > > MDEV-10731: Wrong NULL match results in "Subquery returns more than 1 row" (error code 1242) > > NOT NULL predicate was not added to tables in case of an update query having a subquery. > > --- > mysql-test/r/update_innodb.result | 30 +++++++++++++++++ > mysql-test/t/update_innodb.test | 31 +++++++++++++++++ > sql/sql_select.cc | 70 +++++++++++++++++++++++++-------------- > 3 files changed, 106 insertions(+), 25 deletions(-) > > diff --git a/mysql-test/r/update_innodb.result b/mysql-test/r/update_innodb.result > index 88c86c5..6a02dfd 100644 > --- a/mysql-test/r/update_innodb.result > +++ b/mysql-test/r/update_innodb.result > @@ -29,3 +29,33 @@ CREATE ALGORITHM=UNDEFINED VIEW `v1` AS select `t4`.`c1` AS `c1`,`t4`.`c2` AS `c > UPDATE t1 a JOIN t2 b ON a.c1 = b.c1 JOIN v1 vw ON b.c2 = vw.c1 JOIN t3 del ON vw.c2 = del.c2 SET a.c2 = ( SELECT max(t.c1) FROM t3 t, v1 i WHERE del.c2 = t.c2 AND vw.c3 = i.c3 AND t.c3 = 4 ) WHERE a.c2 IS NULL OR a.c2 < '2011-05-01'; > drop view v1; > drop table t1,t2,t3,t4; > +# > +# MDEV-10232 Scalar result of subquery changes after adding an outer select stmt > +# The MDEV number and title are wrong! Please fix. > +CREATE TABLE a ( Please make tables names to follow the t1,t2,t3,... naming convention used in the testcase. > +a_id INT(20) UNSIGNED NOT NULL AUTO_INCREMENT, > +b_id INT(20) UNSIGNED NULL DEFAULT NULL, > +c_id VARCHAR(255) NULL DEFAULT NULL, > +PRIMARY KEY (a_id) > +) > +COLLATE = 'utf8_general_ci' > +ENGINE = InnoDB; > +CREATE TABLE b ( > +b_id INT(20) UNSIGNED NOT NULL AUTO_INCREMENT, > +c_id VARCHAR(255) NULL DEFAULT NULL, > +PRIMARY KEY (b_id), > +INDEX idx_c_id (c_id) > +) > +COLLATE = 'utf8_general_ci' > +ENGINE = InnoDB; > +INSERT INTO a (b_id, c_id) VALUES (1, NULL); > +INSERT INTO b (c_id) VALUES (NULL); > +INSERT INTO b (c_id) VALUES (NULL); > +select b_id from a; > +b_id > +1 > +UPDATE a SET b_id = (SELECT b.b_id FROM b b WHERE b.c_id = a.c_id); > +select b_id from a; > +b_id > +NULL > +drop table a,b; > diff --git a/mysql-test/t/update_innodb.test b/mysql-test/t/update_innodb.test > index 67c356c..f2b7ade 100644 > --- a/mysql-test/t/update_innodb.test > +++ b/mysql-test/t/update_innodb.test > @@ -37,3 +37,34 @@ UPDATE t1 a JOIN t2 b ON a.c1 = b.c1 JOIN v1 vw ON b.c2 = vw.c1 JOIN t3 del ON v > > drop view v1; > drop table t1,t2,t3,t4; > + > +--echo # > +--echo # MDEV-10232 Scalar result of subquery changes after adding an outer select stmt > +--echo # > + > +CREATE TABLE a ( > + a_id INT(20) UNSIGNED NOT NULL AUTO_INCREMENT, > + b_id INT(20) UNSIGNED NULL DEFAULT NULL, > + c_id VARCHAR(255) NULL DEFAULT NULL, > + PRIMARY KEY (a_id) > + ) > +COLLATE = 'utf8_general_ci' > +ENGINE = InnoDB; > + > +CREATE TABLE b ( > + b_id INT(20) UNSIGNED NOT NULL AUTO_INCREMENT, > + c_id VARCHAR(255) NULL DEFAULT NULL, > + PRIMARY KEY (b_id), > + INDEX idx_c_id (c_id) > + ) > +COLLATE = 'utf8_general_ci' > +ENGINE = InnoDB; > + > +INSERT INTO a (b_id, c_id) VALUES (1, NULL); > +INSERT INTO b (c_id) VALUES (NULL); > +INSERT INTO b (c_id) VALUES (NULL); > + > +select b_id from a; > +UPDATE a SET b_id = (SELECT b.b_id FROM b b WHERE b.c_id = a.c_id); > +select b_id from a; > +drop table a,b; > diff --git a/sql/sql_select.cc b/sql/sql_select.cc > index 85bfb6b..3543cc2 100644 > --- a/sql/sql_select.cc > +++ b/sql/sql_select.cc > @@ -9363,34 +9363,54 @@ static void add_not_null_conds(JOIN *join) > not_null_item is the t1.f1, but it's referred_tab is 0. > */ > if (!referred_tab) > - continue; > - if (!(notnull= new (join->thd->mem_root) > + { > + if (!(notnull= new (join->thd->mem_root) > Item_func_isnotnull(join->thd, item))) > - DBUG_VOID_RETURN; > - /* > - We need to do full fix_fields() call here in order to have correct > - notnull->const_item(). This is needed e.g. by test_quick_select > - when it is called from make_join_select after this function is > - called. > - */ Note this as (LOC1) > - if (notnull->fix_fields(join->thd, &notnull)) > - DBUG_VOID_RETURN; > - DBUG_EXECUTE("where",print_where(notnull, > - referred_tab->table->alias.c_ptr(), > - QT_ORDINARY);); > - if (!tab->first_inner) > - { > - COND *new_cond= referred_tab->join == join ? > - referred_tab->select_cond : > - join->outer_ref_cond; > - add_cond_and_fix(join->thd, &new_cond, notnull); > - if (referred_tab->join == join) > - referred_tab->set_select_cond(new_cond, __LINE__); > - else > - join->outer_ref_cond= new_cond; > + DBUG_VOID_RETURN; > + /* > + We need to do full fix_fields() call here in order to have correct > + notnull->const_item(). This is needed e.g. by test_quick_select > + when it is called from make_join_select after this function is > + called. > + */ > + if (notnull->fix_fields(join->thd, &notnull)) > + DBUG_VOID_RETURN; > + > + if (!tab->first_inner) > + add_cond_and_fix(join->thd, &join->outer_ref_cond, notnull); > + else > + add_cond_and_fix(join->thd, tab->first_inner->on_expr_ref, notnull); > } > else > - add_cond_and_fix(join->thd, tab->first_inner->on_expr_ref, notnull); > + { > + if (!(notnull= new (join->thd->mem_root) > + Item_func_isnotnull(join->thd, item))) > + DBUG_VOID_RETURN; > + /* > + We need to do full fix_fields() call here in order to have correct > + notnull->const_item(). This is needed e.g. by test_quick_select > + when it is called from make_join_select after this function is > + called. > + */ This is LOC2. After the patch, there is a lot of code duplication between LOC1 and LOC2: - creation of Item_func_isnotnull - fix_fields call and its comment - etc Please factor out the common code. The "print_where" call can print "outer_ref_table_cond" when referred_tab=NULL. > + if (notnull->fix_fields(join->thd, &notnull)) > + DBUG_VOID_RETURN; > + DBUG_EXECUTE("where",print_where(notnull, > + referred_tab->table->alias.c_ptr(), > + QT_ORDINARY);); > + if (!tab->first_inner) > + { > + COND *new_cond= referred_tab->join == join ? > + referred_tab->select_cond : > + join->outer_ref_cond; > + add_cond_and_fix(join->thd, &new_cond, notnull); > + if (referred_tab->join == join) > + referred_tab->set_select_cond(new_cond, __LINE__); > + else > + join->outer_ref_cond= new_cond; > + } > + else > + add_cond_and_fix(join->thd, tab->first_inner->on_expr_ref, notnull); > + } > } > } > } BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

Re: [Maria-developers] [Commits] 43e2a78f2a0: Remove MYSQL_COMPRESSION.
by Jan Lindström 16 Jan '17

16 Jan '17

Hi, On Mon, Jan 16, 2017 at 2:10 PM, <marko.makela(a)mariadb.com> wrote: > > diff --git a/storage/innobase/btr/btr0btr.cc b/storage/innobase/btr/ > btr0btr.cc > index d1d9dfe64fe..1fb5cb06949 100644 > --- a/storage/innobase/btr/btr0btr.cc > +++ b/storage/innobase/btr/btr0btr.cc > @@ -215,10 +215,6 @@ btr_root_get( > buf_block_t* root = btr_root_block_get(index, RW_SX_LATCH, > mtr); > > - if (root && root->page.encrypted == true) { > - root = NULL; > - } > - > return(root ? buf_block_get_frame(root) : NULL); > This is unrelated to MySQL encryption, it is related to MariaDB encryption, not sure if btr_root_block_get would always return NULL when page is encrypted, upper code is peppered with assertions, thus this might not be safe. > > - if (ptype == FIL_PAGE_PAGE_COMPRESSED_ENCRYPTED) { > - header_len += FIL_PAGE_COMPRESSION_METHOD_SIZE; > - } > - > - /* Do not try to uncompressed pages that are not compressed */ > - if (ptype != FIL_PAGE_PAGE_COMPRESSED && > - ptype != FIL_PAGE_PAGE_COMPRESSED_ENCRYPTED && > - ptype != FIL_PAGE_COMPRESSED) { > + switch (ptype) { > + case FIL_PAGE_PAGE_COMPRESSED_ENCRYPTED: > + header_len = FIL_PAGE_DATA + FIL_PAGE_COMPRESSED_SIZE > + + FIL_PAGE_COMPRESSION_METHOD_SIZE; > + break; > + case FIL_PAGE_PAGE_COMPRESSED: > + header_len = FIL_PAGE_DATA + FIL_PAGE_COMPRESSED_SIZE; > + break; > + default: > + /* The page is not in our format. */ > return; > This is also unrelated change but more like cosmetic, so ok. > } > > > diff --git a/storage/innobase/include/buf0buf.ic > b/storage/innobase/include/buf0buf.ic > index f2b1b151598..5e75c446bbd 100644 > --- a/storage/innobase/include/buf0buf.ic > +++ b/storage/innobase/include/buf0buf.ic > @@ -734,9 +734,6 @@ buf_block_get_frame( > case BUF_BLOCK_ZIP_PAGE: > case BUF_BLOCK_ZIP_DIRTY: > case BUF_BLOCK_NOT_USED: > - if (block->page.encrypted) { > - goto ok; > - } > Hmm, this is unrelated but could be actually correct. > ut_error; > break; > case BUF_BLOCK_FILE_PAGE: > > > @@ -145,52 +137,22 @@ fil_page_type_validate( > page_type == FIL_PAGE_TYPE_BLOB || > page_type == FIL_PAGE_TYPE_ZBLOB || > page_type == FIL_PAGE_TYPE_ZBLOB2 || > - page_type == FIL_PAGE_COMPRESSED || > - page_type == FIL_PAGE_TYPE_UNKNOWN || > - page_type == FIL_PAGE_ENCRYPTED || > - page_type == FIL_PAGE_COMPRESSED_AND_ENCRYPTED || > - page_type == FIL_PAGE_ENCRYPTED_RTREE))) { > - > - uint key_version = mach_read_from_4(page + > FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION); > - bool page_compressed = (page_type == > FIL_PAGE_PAGE_COMPRESSED); > - bool page_compressed_encrypted = (page_type == > FIL_PAGE_PAGE_COMPRESSED_ENCRYPTED); > + page_type == FIL_PAGE_TYPE_UNKNOWN))) { > Why you change this code, it is not MySQL compression code ? > + > ulint space = mach_read_from_4(page + > FIL_PAGE_ARCH_LOG_NO_OR_SPACE_ID); > ulint offset = mach_read_from_4(page + FIL_PAGE_OFFSET); > - ib_uint64_t lsn = mach_read_from_8(page + FIL_PAGE_LSN); > - ulint compressed_len = mach_read_from_2(page + > FIL_PAGE_DATA); > fil_system_enter(); > fil_space_t* rspace = fil_space_get_by_id(space); > fil_system_exit(); > > /* Dump out the page info */ > - fprintf(stderr, "InnoDB: Space %lu offset %lu name %s > page_type %lu page_type_name %s\n" > - "InnoDB: key_version %u page_compressed %d > page_compressed_encrypted %d lsn %llu compressed_len %lu\n", > - space, offset, rspace->name, page_type, > fil_get_page_type_name(page_type), > - key_version, page_compressed, > page_compressed_encrypted, (ulonglong)lsn, compressed_len); > - fflush(stderr); > - > - ut_ad(page_type == FIL_PAGE_PAGE_COMPRESSED || > - page_type == FIL_PAGE_PAGE_COMPRESSED_ENCRYPTED || > - page_type == FIL_PAGE_INDEX || > - page_type == FIL_PAGE_RTREE || > - page_type == FIL_PAGE_UNDO_LOG || > - page_type == FIL_PAGE_INODE || > - page_type == FIL_PAGE_IBUF_FREE_LIST || > - page_type == FIL_PAGE_TYPE_ALLOCATED || > - page_type == FIL_PAGE_IBUF_BITMAP || > - page_type == FIL_PAGE_TYPE_SYS || > - page_type == FIL_PAGE_TYPE_TRX_SYS || > - page_type == FIL_PAGE_TYPE_FSP_HDR || > - page_type == FIL_PAGE_TYPE_XDES || > - page_type == FIL_PAGE_TYPE_BLOB || > - page_type == FIL_PAGE_TYPE_ZBLOB || > - page_type == FIL_PAGE_TYPE_ZBLOB2 || > - page_type == FIL_PAGE_COMPRESSED || > - page_type == FIL_PAGE_TYPE_UNKNOWN || > - page_type == FIL_PAGE_ENCRYPTED || > - page_type == FIL_PAGE_COMPRESSED_AND_ENCRYPTED || > - page_type == FIL_PAGE_ENCRYPTED_RTREE); > - > + ib::fatal() << "Page " << space << ":" << offset > + << " name " << (rspace ? rspace->name : "???") > + << " page_type " << page_type > + << " key_version " > + << mach_read_from_4(page + > FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION) > + << " lsn " << mach_read_from_8(page + FIL_PAGE_LSN) > + << " compressed_len " << mach_read_from_2(page + > FIL_PAGE_DATA); > return false; > } > > > ok to push, rest of the changes. R: Jan

2 1

Re: [Maria-developers] de9ef13: MDEV-10956 Strict Password Validation Breaks Replication.
by Sergei Golubchik 16 Jan '17

16 Jan '17

Hi, Alexey! On Jan 16, Alexey Botchkov wrote: > revision-id: de9ef1390655abfb2b46a158148a9e27410298a2 (mariadb-10.1.20-44-gde9ef13) > parent(s): ab1e6fefd869242d962cb91a006f37bb9ad534a7 > committer: Alexey Botchkov > timestamp: 2017-01-16 02:20:15 +0400 > message: > > MDEV-10956 Strict Password Validation Breaks Replication. > > Password validation switched off completely for slave threads. I'd rather disable only strict_password_validation check in replication threads. Because it, indeed, makes no sense there. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] [Commits] eb8d31d: MDEV-10232: Scalar result of subquery changes after adding an outer select stmt
by Sergey Petrunia 16 Jan '17

16 Jan '17

Hi Varun, Please make the fix for MDEV-10232 to be one changeset. Ok to push that changeset. On Sun, Jan 15, 2017 at 07:44:35PM +0530, Varun wrote: > revision-id: eb8d31d96f32da53d34d4c992fba061fc8732078 (mariadb-10.1.20-38-geb8d31d) > parent(s): 905aac9c78068224839db551a84ed080167d1657 > author: Varun Gupta > committer: Varun Gupta > timestamp: 2017-01-15 19:40:46 +0530 > message: > > MDEV-10232: Scalar result of subquery changes after adding an outer select stmt > > postreview > > --- > sql/sql_select.cc | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/sql/sql_select.cc b/sql/sql_select.cc > index 13eda46..85bfb6b 100644 > --- a/sql/sql_select.cc > +++ b/sql/sql_select.cc > @@ -1614,7 +1614,8 @@ JOIN::optimize_inner() > <fields> to ORDER BY <fields>. There are three exceptions: > - if skip_sort_order is set (see above), then we can simply skip > GROUP BY; > - - if we are in a subquery, we don't have to maintain order > + - if we are in a subquery, we don't have to maintain order unless there > + is a limit clause in the subquery. > - we can only rewrite ORDER BY if the ORDER BY fields are 'compatible' > with the GROUP BY ones, i.e. either one is a prefix of another. > We only check if the ORDER BY is a prefix of GROUP BY. In this case > @@ -1626,7 +1627,7 @@ JOIN::optimize_inner() > if (!order || test_if_subpart(group_list, order)) > { > if (skip_sort_order || > - select_lex->master_unit()->item && !select_limit) // This is a subquery > + (select_lex->master_unit()->item && select_limit == HA_POS_ERROR)) // This is a subquery > order= NULL; > else > order= group_list; > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits -- BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

Re: [Maria-developers] [Commits] 905aac9: MDEV-10232: Scalar result of subquery changes after adding an outer select stmt
by Sergey Petrunia 14 Jan '17

14 Jan '17

Hi Varun, On Fri, Jan 13, 2017 at 08:22:49PM +0530, Varun wrote: > revision-id: 905aac9c78068224839db551a84ed080167d1657 (mariadb-10.1.20-37-g905aac9) > parent(s): ebb8c9fb26f86cff8c0d81bd2415f415cef952bb > author: Varun Gupta > committer: Varun Gupta > timestamp: 2017-01-13 20:17:33 +0530 > message: > > MDEV-10232: Scalar result of subquery changes after adding an outer select stmt > > In a subquery, we don't have to maintain order > Added a fix such that order is considered when there is a limit clause. > > --- > mysql-test/r/subselect4.result | 12 ++++++++++++ > mysql-test/t/subselect4.test | 10 ++++++++++ > sql/sql_select.cc | 2 +- > 3 files changed, 23 insertions(+), 1 deletion(-) > > diff --git a/mysql-test/r/subselect4.result b/mysql-test/r/subselect4.result > index 87645d1..7764783 100644 > --- a/mysql-test/r/subselect4.result > +++ b/mysql-test/r/subselect4.result > @@ -2401,5 +2401,17 @@ SELECT x FROM t1 WHERE id > (SELECT MAX(id) - 1000 FROM t1) ORDER BY x LIMIT 1; > x > 0 > drop table t1; > +# > +# MDEV-10232 Scalar result of subquery changes after adding an outer select stmt > +# > +create table t1(c1 int, c2 int, primary key(c2)); > +insert into t1 values(2,1),(1,2); > +select (select c1 from t1 group by c1,c2 order by c1 limit 1) as x; > +x > +1 > +(select c1 from t1 group by c1,c2 order by c1 limit 1); > +c1 > +1 > +drop table t1; > SET optimizer_switch= @@global.optimizer_switch; > set @@tmp_table_size= @@global.tmp_table_size; > diff --git a/mysql-test/t/subselect4.test b/mysql-test/t/subselect4.test > index 4eb9701..787c579 100644 > --- a/mysql-test/t/subselect4.test > +++ b/mysql-test/t/subselect4.test > @@ -1956,5 +1956,15 @@ SELECT x FROM t1 WHERE id > (SELECT MAX(id) - 1000 FROM t1) ORDER BY x LIMIT 1; > > drop table t1; > > +--echo # > +--echo # MDEV-10232 Scalar result of subquery changes after adding an outer select stmt > +--echo # > + > +create table t1(c1 int, c2 int, primary key(c2)); > +insert into t1 values(2,1),(1,2); > +select (select c1 from t1 group by c1,c2 order by c1 limit 1) as x; > +(select c1 from t1 group by c1,c2 order by c1 limit 1); > +drop table t1; > + > SET optimizer_switch= @@global.optimizer_switch; > set @@tmp_table_size= @@global.tmp_table_size; > diff --git a/sql/sql_select.cc b/sql/sql_select.cc > index 3f06ec8..13eda46 100644 > --- a/sql/sql_select.cc > +++ b/sql/sql_select.cc > @@ -1626,7 +1626,7 @@ JOIN::optimize_inner() Above this change we still have a comment with this statement: >> - if we are in a subquery, we don't have to maintain order Please adjust it accordingly. > if (!order || test_if_subpart(group_list, order)) > { > if (skip_sort_order || > - select_lex->master_unit()->item) // This is a subquery > + select_lex->master_unit()->item && !select_limit) // This is a subquery > order= NULL; > else > order= group_list; So I'm trying with the patch: MariaDB [test]> explain select (select c1 from t1 group by c1,c2 order by c1 limit 1) as x; +------+-------------+-------+------+---------------+------+---------+------+------+----------------+ | id | select_type | table | type | possible_keys | key | key_len | ref | rows | Extra | +------+-------------+-------+------+---------------+------+---------+------+------+----------------+ | 1 | PRIMARY | NULL | NULL | NULL | NULL | NULL | NULL | NULL | No tables used | | 2 | SUBQUERY | t1 | ALL | NULL | NULL | NULL | NULL | 2 | Using filesort | +------+-------------+-------+------+---------------+------+---------+------+------+----------------+ 2 rows in set (20.08 sec) Good. What if there is no LIMIT clause? MariaDB [test]> explain select (select c1 from t1 group by c1,c2 order by c1 ) as x; +------+-------------+-------+------+---------------+------+---------+------+------+----------------+ | id | select_type | table | type | possible_keys | key | key_len | ref | rows | Extra | +------+-------------+-------+------+---------------+------+---------+------+------+----------------+ | 1 | PRIMARY | NULL | NULL | NULL | NULL | NULL | NULL | NULL | No tables used | | 2 | SUBQUERY | t1 | ALL | NULL | NULL | NULL | NULL | 2 | Using filesort | +------+-------------+-------+------+---------------+------+---------+------+------+----------------+ 2 rows in set (6.06 sec) Bad. "Using filesort" should have been removed. The reason it fails is that "!select_limit" is not the right way to check the presense of LIMIT clause. Without limit, I have: (gdb) p/x select_limit $15 = 0xffffffffffffffff (the name of the constant to use is "HA_POS_ERROR") Another issue: I get this warning with the new code: || /home/psergey/dev-git/10.1/sql/sql_select.cc: In member function 'int JOIN::optimize_inner()': sql_select.cc|1629 col 45| warning: suggest parentheses around '&&' within '||' [-Wparentheses] || select_lex->master_unit()->item && !select_limit) // This is a subquery || ^ BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

Re: [Maria-developers] [Commits] a2a42a9: MDEV-11623: MariaDB 10.1 fails to start datadir created with MariaDB 10.0/MySQL 5.6 using innodb-page-size!=16K
by Marko Mäkelä 13 Jan '17

13 Jan '17

Sorry for the delay. This patch looks great to me, except for one thing: I think that we should write the corrected flags back to the data file. Given that 10.1 is already in GA status, we should consider our options carefully. My preference would be (1) below: (1) If we adjust 10.1.21 so that it writes back corrected flags for both old and new data files, then older 10.1.x would be unable to open the files (if compression, DATA_DIRECTORY or non-default innodb_page_size was used). (2) If we adjust 10.1.21 so that it writes correct flags for new data files (like the current patch does), then older 10.1.x would not be able to read those created-with-newer-version data files. (3) If we adjust 10.2 only, so that it writes the correct flags for new files and also adjusts the flags in old files, then we would seem to have a clear upgrade path. Whatever we choose, I would like to remove the flag-adjustment code from 10.3 or 10.4. This would mean that an upgrade (or import) directly from 10.1 is only possible if the problematic features (DATA_DIRECTORY, compression, non-standard innodb_page_size) were not used. Otherwise the upgrade would have to be done through 10.2. On Thu, Dec 29, 2016 at 10:41 PM, Jan Lindström <jan.lindstrom(a)mariadb.com> wrote: > revision-id: a2a42a9181729b661ba308f2d0a75d8b547ed09d (mariadb-10.1.20-15-ga2a42a9) > parent(s): 23cc1be270c7304963643947d8e5ab88f4e312ee > author: Jan Lindström > committer: Jan Lindström > timestamp: 2016-12-29 22:40:30 +0200 > message: > > MDEV-11623: MariaDB 10.1 fails to start datadir created with MariaDB 10.0/MySQL 5.6 using innodb-page-size!=16K > > Problem was that in MariaDB 10.1 page size flag is on different > position (13) on tablespace flags compared to MySQL 5.6 and > MariaDB 10.0 or older position (6). DATA_DIR flag position difference > was already handled. > > /** Tablespace flags position and name in MySQL 5.6/MariaDB 10.0 or older > and MariaDB 10.1.20 or older MariaDB 10.1 and in MariaDB 10.1.21 > or newer. > MySQL 5.6 MariaDB 10.1.x MariaDB 10.1.21 > ==================================================================== > Below flags in same offset > ==================================================================== > 0: POST_ANTELOPE 0:POST_ANTELOPE 0: POST_ANTELOPE > 1: ZIP_SSIZE 1:ZIP_SSIZE 1: ZIP_SSIZE > 5: ATOMIC_BLOBS 5:ATOMIC_BLOBS 5: ATOMIC_BLOBS It could be clearer to explicitly identify multi-bit fields, e.g., 1..4 for ZIP_SSIZE. > ===================================================================== > Below note the difference in order > ===================================================================== > 6: PAGE_SSIZE 6:PAGE_COMPRESSION 6: PAGE_SSIZE > 10: DATA_DIR 7:PAGE_COMPRESSION_LEVEL 10: DATA_DIR > 11: UNUSED 11:ATOMIC_WRITES > ===================================================================== > Note that below flags have been moved > ===================================================================== > 13:PAGE_SSIZE 11: RESERVED (5.7 SHARED) > 17:DATA_DIR 12: RESERVED (5.7 TEMPORARY) > 18:UNUSED 13: RESERVED (5.7 ENCRYPTION) > 14: PAGE_COMPRESSION > 15: PAGE_COMPRESSION_LEVEL > 19: ATOMIC_WRITES > 21: UNUSED s/have been moved/were in incorrect position in MariaDB 10.1.x/ It could be good to explain the impact of this. That is, what would happen if we upgrade from MariaDB 10.0 to the buggy MariaDB 10.1, or if we upgrade from the buggy MariaDB 10.1 to 10.1.21 where this has been fixed. As far as I can tell, with respect to the DATA_DIR flag we should be mostly OK, because that flag should only play a role in the table flags when determining where the data file is located, not so much in the tablespace flags inside the data file. (Moving files from MariaDB 10.1 to MySQL 5.7 could be hurt, because the DATA_DIR flag would be interpreted as the 5.7 ENCRYPTION flag.) Next, let us consider PAGE_SSIZE (bits 13..16 in the buggy MariaDB 10.1.x, 6..9 elsewhere). Upgrade from 10.0 or 5.6 to the buggy 10.1.x: We read the bits 13..16. These would always be 0 in the old data files, so we will interpret the data file as if it had innodb_page_size=16k. That is, upgrade or import from 5.6 or 10.0 to the buggy 10.1.x will be refused if innodb_page_size differs from 16k. Is this correct? (This is something that we cannot fix, because we cannot retroactively change old MariaDB 10.1.x versions.) Upgrade from the buggy 10.1.x to the fixed 10.1.x or 10.2: We read the bits 6..9 which were incorrectly used as PAGE_COMPRESSION, PAGE_COMPRESSION_LEVEL. That is, upgrade should work if innodb_page_size=16k (the flags are expected to be 0) and compression is not used (the bits 6..9 were written as 0 by the buggy 10.1.x). To help users who use the buggy 10.1.x with innodb_page_size!=16k or with compression, we can adjust the flags: > fsp0fsp.ic: fsp_flags_get_page_size() : By default read > used page size from original position i.e. after ATOMIC_BLOBS. > But if we detect that buggy MariaDB 10.1 flags are used, then > we read it from FSP_FLAGS_PAGE_SSIZE_MARIADB101. [snip] > Tested also with MySQL 5.7 datadir (after deleting redo logs > and copying mysql/* files from 10.1) but that failed because > SYS_INDEXES got an extra column in 5.7 SYS_INDEXES.MERGE_THRESHOLD. You could theoretically have initialized the database in MySQL 5.6 and then upgraded it to 5.7. All tables that were created in 5.6 should be accessible. But would not have added much value. > After shutdown, started MariaDB 10.1.21 with fix and > verified that all tables created are accessible. Were the flags corrected in the data file afterwards? I think that we should correct the flags. Please add a mysql-test case that demonstrates that the flags are corrected. Something like this: --source include/have_innodb.inc --source include/not_embedded.inc # restart will restore the old values SET GLOBAL innodb_file_per_table=1; SET GLOBAL innodb_file_format=Barracuda; let INNODB_PAGE_SIZE=`select @@innodb_page_size`; let MYSQLD_DATADIR=`select @@datadir`; CREATE TABLE tr(a INT)ENGINE=InnoDB ROW_FORMAT=REDUNDANT; CREATE TABLE tc(a INT)ENGINE=InnoDB ROW_FORMAT=COMPACT; CREATE TABLE td(a INT)ENGINE=InnoDB ROW_FORMAT=DYNAMIC; CREATE TABLE tz(a INT)ENGINE=InnoDB ROW_FORMAT=COMPRESSED KEY_BLOCK_SIZE=1; # TODO: repeat the same with DATA_DIRECTORY and COMPRESSION --source include/shutdown_mysqld.inc perl; # corrupt the FSP_FLAGS in each file; write the page checksum as 0xdeadbeef # (see log_data_file_size.test or some recent test from MySQL) EOF; --let $restart_parameters = --innodb-read-only; --source include/start_mysqld.inc CHECK TABLE tr; CHECK TABLE tc; CHECK TABLE td; CHECK TABLE tz; --source include/shutdown_mysqld.inc perl; # check that the FSP_FLAGS are still corrupted # (they must not be adjusted in read-only mode) EOF; --let $restart_parameters=; --source include/start_mysqld.inc CHECK TABLE tr; CHECK TABLE tc; CHECK TABLE td; CHECK TABLE tz; --source include/shutdown_mysqld.inc perl; # check that the FSP_FLAGS now correspond to $ENV{INNODB_PAGE_SIZE} EOF; --source include/start_mysqld.inc DROP TABLE tr,tc,td,tz; > @@ -3859,21 +3837,11 @@ fil_open_single_table_tablespace( > + /* Validate this single-table-tablespace with SYS_TABLES. */ > + bool flags_correct = dict_compare_flags(def.flags, flags); I think that the function name should be more descriptive, making clear that the first argument contains the table flags and the second the FIL_SPACE_FLAGS: dict_tf_match_space(def.flags, flags) The comment does not really add any value. I would omit it. > diff --git a/storage/innobase/fsp/fsp0fsp.cc b/storage/innobase/fsp/fsp0fsp.cc > index 87aa5f7..7ff5244 100644 > --- a/storage/innobase/fsp/fsp0fsp.cc > +++ b/storage/innobase/fsp/fsp0fsp.cc > @@ -4182,3 +4182,141 @@ fsp_page_is_free_func( > return xdes_mtr_get_bit( > descr, XDES_FREE_BIT, page_no % FSP_EXTENT_SIZE, mtr); > } > + > +/********************************************************************//** > +Verify that dictionary flags modified to tablespace flags > +and actual tablespace flags stored to FSP header match. > +@param[in] dict_flags dict_tf_to_fsp_flags(dict_table_t::flags) > +@param[in] fsp_flags Actual flags stored to FSP header in page 0 > +@return true if flags match, false if not */ > +bool > +fsp_verify_flags( > + ulint dict_flags, > + ulint fsp_flags) > +{ > + ulint dict_unused = FSP_FLAGS_GET_UNUSED(dict_flags); > + ulint dict_antelope = FSP_FLAGS_GET_POST_ANTELOPE(dict_flags); > + ulint dict_zssize = FSP_FLAGS_GET_ZIP_SSIZE(dict_flags); > + ulint dict_ablobs = FSP_FLAGS_HAS_ATOMIC_BLOBS(dict_flags); Add an early return before all these computations if dict_flags==fsp_flags. I think that the name dict_flags is misleading. Both flags are supposed to be tablespace flags, not data dictionary (or table) flags. How about this: s/dict_flags/expected/ s/fsp_flags/actual/ And then define the local variables with expected_ and actual_ prefixes, instead of dict_ and fsp_. That would make the code easier to follow. > + DBUG_EXECUTE_IF("fsp_verify_flags_failure", > + return(false);); Where is the test to exercise this? Do we even need this? I think it is better to inject faults into the data files using Perl code in mysql-test (see my previous suggestion). Other than this, the function looks OK to me. > diff --git a/storage/innobase/include/dict0dict.h b/storage/innobase/include/dict0dict.h > index 42f93b5..479954d 100644 > --- a/storage/innobase/include/dict0dict.h > +++ b/storage/innobase/include/dict0dict.h > @@ -1904,6 +1904,17 @@ dict_table_get_index_on_first_col( > #endif /* !UNIV_HOTBACKUP */ > > > +/** Compare tablespace flags. > +@param[in] flags SYS_TABLES.flags > +@param[in] mod_flags Tablespace flags. > +@return true if flags match, false if not */ > +UNIV_INLINE > +bool > +dict_compare_flags( > + ulint flags, > + ulint mod_flags) > + MY_ATTRIBUTE((warn_unused_result)); > + Can we declare the inline function directly in the .h file, instead of splitting it into the .ic file? I would suggest different names: bool dict_tf_match_space(ulint table_flags, ulint fsp_flags); > @@ -980,12 +982,11 @@ dict_sys_tables_type_to_tf( > PAGE_COMPRESSION_LEVEL, ATOMIC_WRITES are the same. */ > flags |= type & (DICT_TF_MASK_ZIP_SSIZE > | DICT_TF_MASK_ATOMIC_BLOBS > - | DICT_TF_MASK_DATA_DIR > - | DICT_TF_MASK_PAGE_COMPRESSION > - | DICT_TF_MASK_PAGE_COMPRESSION_LEVEL > - | DICT_TF_MASK_ATOMIC_WRITES > + | DICT_TF_MASK_DATA_DIR > + | DICT_TF_MASK_PAGE_COMPRESSION > + | DICT_TF_MASK_PAGE_COMPRESSION_LEVEL > + | DICT_TF_MASK_ATOMIC_WRITES); > > - ); > > return(flags); > } This appears to be a white-space-only change. We could avoid it, unless this is actually fixing the indentation. The spaces and TABs got lost in transit, so I cannot tell.. > @@ -1018,8 +1019,8 @@ dict_tf_to_sys_tables_type( > type |= flags & (DICT_TF_MASK_ZIP_SSIZE > | DICT_TF_MASK_ATOMIC_BLOBS > | DICT_TF_MASK_DATA_DIR > - | DICT_TF_MASK_PAGE_COMPRESSION > - | DICT_TF_MASK_PAGE_COMPRESSION_LEVEL > + | DICT_TF_MASK_PAGE_COMPRESSION > + | DICT_TF_MASK_PAGE_COMPRESSION_LEVEL > | DICT_TF_MASK_ATOMIC_WRITES); > > return(type); Same here. > DBUG_EXECUTE_IF("dict_tf_verify_flags_failure", > - return(ULINT_UNDEFINED);); > + return(false);); Nice catch. But is there any test to exercise this? If not, I would remove the DBUG_EXECUTE_IF altogether. > ut_a(page_ssize == 0 || page_ssize != 0); /* silence compiler */ > ut_a(compact == 0 || compact == 1); /* silence compiler */ > ut_a(data_dir == 0 || data_dir == 1); /* silence compiler */ > - ut_a(post_antelope == 0 || post_antelope == 1); /* silence compiler */ > + ut_a(post_antelope == 0 || post_antelope == 1); /* silence compiler > + */ I think it would be better to declare these variables as __attribute__((unused)) or to remove the variables altogether if they are unused. > + > + if (table_unused || fsp_unused) { > + ib_logf(IB_LOG_LEVEL_ERROR, > + "Table flags has unused %ld" > + " in the data dictionary" > + " but the flags in file has unused %ld\n", > + table_unused, fsp_unused); > + > + return (false); > + } 0x%lx would be more user-friendly here. > + ib_logf(IB_LOG_LEVEL_ERROR, > + "Table flags has page_compression %ld" > + " in the data dictionary" > + " but the flags in file ahas page_compression %ld\n", > page_compression, fsp_page_compression); s/ahas/has/ > +/********************************************************************//** > +Verify that dictionary flags modified to tablespace flags > +and actual tablespace flags stored to FSP header match. > +@param[in] dict_flags dict_tf_to_fsp_flags(dict_table_t::flags) > +@param[in] fsp_flags Actual flags stored to FSP header in page 0 > +@return true if flags match, false if not */ > +bool > +fsp_verify_flags( > + ulint dict_flags, > + ulint fsp_flags) > + MY_ATTRIBUTE((warn_unused_result)); > + This seems to be missing UNIV_INTERN? I think we need it in 10.1 at least for innodb_plugin. > + /** In MariaDB 10.1.20 or older MariaDB 10.1 PAGE_SSIZE > + was stored after ATOMIC_WRITES not after ATOMIC_BLOBS > + as in MySQL 5.6, MariaDB 10.0 or older. New tables > + in MariDB 10.1.21 or newer store PAGE_SSIZE after ATOMIC_BLOBS. > + */ > + ssize = FSP_FLAGS_GET_PAGE_SSIZE(flags); Do not use /** comments before local variables. I think that one of the ‘after’ in the comment should be ‘before’. Also fix the typo MariDB. -- DON’T MISS M|17 April 11 - 12, 2017 The Conrad Hotel New York City https://m17.mariadb.com/ Marko Mäkelä, Lead Developer InnoDB MariaDB Corporation

1 1

[Maria-developers] Please review MDEV-9522 Split sql_select.cc:can_change_cond_ref_to_const into virtual methods in Type_handler
by Alexander Barkov 13 Jan '17

13 Jan '17

Hello Vicențiu, can you please review a patch for MDEV-9522 ? Thanks!

2 3