- developers - lists.mariadb.org

Re: [Maria-developers] [Commits] e0a1e63: MDEV-9619: Assertion `null_ref_table' failed in virtual table_map Item_direct_view_ref::used_tables() const on 2nd execution of PS
by Sergey Petrunia 21 Feb '17

21 Feb '17

Hi Sanja, Two questions: 1. Why does the patch define Item_ptr type twice? 2. (The big one) In class Item_in_subselect, I see your comment made in 2015: ... public: Item *left_expr; /* Important for PS/SP: left_expr_orig is the item that left_expr originally pointed at. That item is allocated on the statement arena, while left_expr could later be changed to something on the execution arena. */ Item *left_expr_orig; Considering this comment, is it really safe to keep pointers to left_expr? Should left_expr_orig be used instead? On Sat, Feb 18, 2017 at 05:47:32PM +0100, Oleksandr Byelkin wrote: > revision-id: e0a1e632e4e6084cd989f217943618f4e9ac9179 (mariadb-5.5.54-25-ge0a1e63) > parent(s): 29d78dbb44ee9890b6bc28873344f20fc9157928 > committer: Oleksandr Byelkin > timestamp: 2017-02-18 17:47:31 +0100 > message: > > MDEV-9619: Assertion `null_ref_table' failed in virtual table_map Item_direct_view_ref::used_tables() const on 2nd execution of PS > > Refer left expression indirectly in case it changes from execution to execution. > > --- > mysql-test/r/ps.result | 24 ++++++++++++++++++++++++ > mysql-test/t/ps.test | 22 ++++++++++++++++++++++ > sql/item.h | 2 ++ > sql/opt_subselect.cc | 10 +++++----- > sql/table.h | 3 ++- > 5 files changed, 55 insertions(+), 6 deletions(-) > > diff --git a/mysql-test/r/ps.result b/mysql-test/r/ps.result > index f954583..7233fe8 100644 > --- a/mysql-test/r/ps.result > +++ b/mysql-test/r/ps.result > @@ -4173,4 +4173,28 @@ Warnings: > Note 1003 select `test`.`t1`.`id` AS `id`,`test`.`t1`.`c` AS `c` from `test`.`t1` where 0 > deallocate prepare stmt2; > drop table t1; > +# > +# MDEV-9619: Assertion `null_ref_table' failed in virtual > +# table_map Item_direct_view_ref::used_tables() const on 2nd > +# execution of PS > +# > +CREATE TABLE t1 (f1 VARCHAR(10)) ENGINE=MyISAM; > +CREATE ALGORITHM=MERGE VIEW v1 AS SELECT * FROM t1; > +INSERT INTO t1 VALUES ('a'),('b'); > +CREATE TABLE t2 (f2 VARCHAR(10)) ENGINE=MyISAM; > +INSERT INTO t2 VALUES ('c'),('d'); > +PREPARE stmt FROM "SELECT * FROM v1 WHERE f1 = SOME ( SELECT f2 FROM t2 )"; > +EXECUTE stmt; > +f1 > +EXECUTE stmt; > +f1 > +insert into t1 values ('c'); > +EXECUTE stmt; > +f1 > +c > +EXECUTE stmt; > +f1 > +c > +drop view v1; > +drop table t1,t2; > # End of 5.5 tests > diff --git a/mysql-test/t/ps.test b/mysql-test/t/ps.test > index cfd810f..627a9bf 100644 > --- a/mysql-test/t/ps.test > +++ b/mysql-test/t/ps.test > @@ -3714,4 +3714,26 @@ deallocate prepare stmt2; > > drop table t1; > > +--echo # > +--echo # MDEV-9619: Assertion `null_ref_table' failed in virtual > +--echo # table_map Item_direct_view_ref::used_tables() const on 2nd > +--echo # execution of PS > +--echo # > + > +CREATE TABLE t1 (f1 VARCHAR(10)) ENGINE=MyISAM; > +CREATE ALGORITHM=MERGE VIEW v1 AS SELECT * FROM t1; > +INSERT INTO t1 VALUES ('a'),('b'); > + > +CREATE TABLE t2 (f2 VARCHAR(10)) ENGINE=MyISAM; > +INSERT INTO t2 VALUES ('c'),('d'); > + > +PREPARE stmt FROM "SELECT * FROM v1 WHERE f1 = SOME ( SELECT f2 FROM t2 )"; > +EXECUTE stmt; > +EXECUTE stmt; > +insert into t1 values ('c'); > +EXECUTE stmt; > +EXECUTE stmt; > + > +drop view v1; > +drop table t1,t2; > --echo # End of 5.5 tests > diff --git a/sql/item.h b/sql/item.h > index 89155ac..dff7bc4 100644 > --- a/sql/item.h > +++ b/sql/item.h > @@ -1543,6 +1543,8 @@ class Item { > virtual void mark_as_condition_AND_part(TABLE_LIST *embedding) {}; > }; > > +typedef Item (*Item_ptr); > + > > /** > Compare two Items for List<Item>::add_unique() > diff --git a/sql/opt_subselect.cc b/sql/opt_subselect.cc > index 5137d8a..7f26d01 100644 > --- a/sql/opt_subselect.cc > +++ b/sql/opt_subselect.cc > @@ -1602,7 +1602,7 @@ static bool convert_subq_to_sj(JOIN *parent_join, Item_in_subselect *subq_pred) > > if (subq_pred->left_expr->cols() == 1) > { > - nested_join->sj_outer_expr_list.push_back(subq_pred->left_expr); > + nested_join->sj_outer_expr_list.push_back(&subq_pred->left_expr); > /* > Create Item_func_eq. Note that > 1. this is done on the statement, not execution, arena > @@ -1624,7 +1624,7 @@ static bool convert_subq_to_sj(JOIN *parent_join, Item_in_subselect *subq_pred) > for (uint i= 0; i < subq_pred->left_expr->cols(); i++) > { > nested_join->sj_outer_expr_list.push_back(subq_pred->left_expr-> > - element_index(i)); > + addr(i)); > Item_func_eq *item_eq= > new Item_func_eq(subq_pred->left_expr->element_index(i), > subq_lex->ref_pointer_array[i]); > @@ -3181,8 +3181,8 @@ void restore_prev_sj_state(const table_map remaining_tables, > ulonglong get_bound_sj_equalities(TABLE_LIST *sj_nest, > table_map remaining_tables) > { > - List_iterator<Item> li(sj_nest->nested_join->sj_outer_expr_list); > - Item *item; > + List_iterator<Item_ptr> li(sj_nest->nested_join->sj_outer_expr_list); > + Item **item; > uint i= 0; > ulonglong res= 0; > while ((item= li++)) > @@ -3193,7 +3193,7 @@ ulonglong get_bound_sj_equalities(TABLE_LIST *sj_nest, > class and see if there is an element that is bound? > (this is an optional feature) > */ > - if (!(item->used_tables() & remaining_tables)) > + if (!(item[0]->used_tables() & remaining_tables)) > { > res |= 1ULL << i; > } > diff --git a/sql/table.h b/sql/table.h > index dde01a8..c981243 100644 > --- a/sql/table.h > +++ b/sql/table.h > @@ -33,6 +33,7 @@ > /* Structs that defines the TABLE */ > > class Item; /* Needed by ORDER */ > +typedef Item (*Item_ptr); > class Item_subselect; > class Item_field; > class GRANT_TABLE; > @@ -2348,7 +2349,7 @@ typedef struct st_nested_join > table_map sj_depends_on; > /* Outer non-trivially correlated tables */ > table_map sj_corr_tables; > - List<Item> sj_outer_expr_list; > + List<Item_ptr> sj_outer_expr_list; > /** > True if this join nest node is completely covered by the query execution > plan. This means two things. > _______________________________________________ > 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

[Maria-developers] MDEV-10697 - bb-10.2-compatibility
by jerome brauge 20 Feb '17

20 Feb '17

Hello, I've wrote a patch for MDEV-10697 (goto statement for sql_mode=oracle). If someone can review it. Thanks. Best regards, Jérôme.

2 8

Re: [Maria-developers] [Commits] 13ebc9c: MDEV-11645: archive.archive fails in buildbot with valgrind (Use of uninitialised value)
by Sergey Petrunia 20 Feb '17

20 Feb '17

Hi Varun, On Mon, Feb 13, 2017 at 03:47:57AM +0530, Varun wrote: > revision-id: 13ebc9c826de080e416071b7881585b45922bef9 (mariadb-10.2.3-185-g13ebc9c) > parent(s): 3ae038b732ce503fb839e9095355e05f5c6866f9 > author: Varun Gupta > committer: Varun Gupta > timestamp: 2017-02-13 03:45:59 +0530 > message: > > MDEV-11645: archive.archive fails in buildbot with valgrind (Use of uninitialised value) > > Set null values to record_buffer->buffer for the VARCHAR fields > > --- > storage/archive/ha_archive.cc | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/storage/archive/ha_archive.cc b/storage/archive/ha_archive.cc > index 9d6d100..329e3d5 100644 > --- a/storage/archive/ha_archive.cc > +++ b/storage/archive/ha_archive.cc > @@ -372,10 +372,31 @@ int Archive_share::write_v1_metafile() > > unsigned int ha_archive::pack_row_v1(uchar *record) > { > + uint offset= 0,length; > uint *blob, *end; > uchar *pos; > DBUG_ENTER("pack_row_v1"); > memcpy(record_buffer->buffer, record, table->s->reclength); > + for(Field** field= table->field; (*field) ; field++) > + { > + > + Field *fld= *field; > + > + if(fld->type() == MYSQL_TYPE_VARCHAR) > + { > + uint start= (fld->ptr - table->record[0]); > + uint index_end= start+fld->field_length; There is no reason to compute the above variables when the field value is an SQL NULL. Please move them into the if. Please also use ptrdiff_t datatype instead of uint. A different question: did you check what happens when one attempts to write an SQL NULL value? > + > + if(!(fld->is_real_null(record - table->record[0]))) > + { > + Field_varstring *const field_var = (Field_varstring *)fld; > + offset = field_var->data_length() + field_var->length_size(); Coding style says there should be no space before the assignment. Please fix the above two lines. > + > + for(uint index= start + offset; index <= index_end ; index++) > + record_buffer->buffer[index]=0; Is there any reason you're not using bzero or memset call here instead? (please check with Serg which one is guaranteed to be available) > + } > + } > + } > pos= record_buffer->buffer + table->s->reclength; > for (blob= table->s->blob_field, end= blob + table->s->blob_fields; > blob != end; blob++) BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

Re: [Maria-developers] [Commits] 01d5d6db4c3: Fix GCC 6.3.0 warnings.
by Jan Lindström 16 Feb '17

16 Feb '17

ok to push. On Thu, Feb 16, 2017 at 11:17 AM, <marko.makela(a)mariadb.com> wrote: > revision-id: 01d5d6db4c337daae0aa593341e081cc6bfebb8a (mariadb-10.0.29-25- > g01d5d6db4c3) > parent(s): 6011fb6daa5ac9e6cb97a23b877e4d2568c85d94 > author: Marko Mäkelä > committer: Marko Mäkelä > timestamp: 2017-02-16 11:16:27 +0200 > message: > > Fix GCC 6.3.0 warnings. > > --- > storage/innobase/fsp/fsp0fsp.cc | 1 + > storage/innobase/row/row0ftsort.cc | 4 ---- > storage/xtradb/fsp/fsp0fsp.cc | 1 + > 3 files changed, 2 insertions(+), 4 deletions(-) > > diff --git a/storage/innobase/fsp/fsp0fsp.cc b/storage/innobase/fsp/ > fsp0fsp.cc > index afdd6667711..836ed2de27a 100644 > --- a/storage/innobase/fsp/fsp0fsp.cc > +++ b/storage/innobase/fsp/fsp0fsp.cc > @@ -2801,6 +2801,7 @@ fsp_reserve_free_extents( > } > } else { > ut_a(alloc_type == FSP_CLEANING); > + reserve = 0; > } > > success = fil_space_reserve_free_extents(space, n_free, n_ext); > diff --git a/storage/innobase/row/row0ftsort.cc b/storage/innobase/row/ > row0ftsort.cc > index f662ef6b461..4865c8e850a 100644 > --- a/storage/innobase/row/row0ftsort.cc > +++ b/storage/innobase/row/row0ftsort.cc > @@ -620,7 +620,6 @@ fts_parallel_tokenization( > mem_heap_t* blob_heap = NULL; > fts_doc_t doc; > dict_table_t* table = psort_info->psort_common->new_ > table; > - dict_field_t* idx_field; > fts_tokenize_ctx_t t_ctx; > ulint retried = 0; > dberr_t error = DB_SUCCESS; > @@ -642,9 +641,6 @@ fts_parallel_tokenization( > doc.charset = fts_index_get_charset( > psort_info->psort_common->dup->index); > > - idx_field = dict_index_get_nth_field( > - psort_info->psort_common->dup->index, 0); > - > block = psort_info->merge_block; > zip_size = dict_table_zip_size(table); > > diff --git a/storage/xtradb/fsp/fsp0fsp.cc b/storage/xtradb/fsp/fsp0fsp.cc > index 0bdb2ea0d4c..64576c93545 100644 > --- a/storage/xtradb/fsp/fsp0fsp.cc > +++ b/storage/xtradb/fsp/fsp0fsp.cc > @@ -2810,6 +2810,7 @@ fsp_reserve_free_extents( > } > } else { > ut_a(alloc_type == FSP_CLEANING); > + reserve = 0; > } > > success = fil_space_reserve_free_extents(space, n_free, n_ext); > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits

1 0

Re: [Maria-developers] [Commits] ab0943d5b11: Fix gcc 6.3.x compiler warnings.
by Marko Mäkelä 16 Feb '17

16 Feb '17

On Thu, Feb 16, 2017 at 10:26 AM, jan <jan.lindstrom(a)mariadb.com> wrote: > revision-id: ab0943d5b11f01fb3f4638134bd1b8fb6a14a5e0 (mariadb-5.5.54-23- > gab0943d5b11) > parent(s): 2e8fa1c2b2867b99b5cc7f987ad64b666e0af2c4 > author: Jan Lindström > committer: Jan Lindström > timestamp: 2017-02-16 10:25:27 +0200 > message: > > Fix gcc 6.3.x compiler warnings. > > These are caused by fact that functions are declared with > __attribute__((nonnull)) or left shit like ~0 << macro > when ~0U << macro should be used. > OK to push, except for the following unrelated changes (which are OK in a separate commit): diff --git a/storage/innobase/include/trx0trx.h b/storage/innobase/include/ > trx0trx.h > index 4ade245f03e..94707ecf7cb 100644 > --- a/storage/innobase/include/trx0trx.h > +++ b/storage/innobase/include/trx0trx.h > @@ -481,6 +481,7 @@ struct trx_struct{ > of view of concurrency control: > TRX_ACTIVE, > TRX_COMMITTED_IN_MEMORY, > ... */ > + os_thread_id_t thread_id; > /*------------------------------*/ > /* MySQL has a transaction coordinator to coordinate two phase > commit between multiple storage engines and the binary > log. When > diff --git a/storage/innobase/trx/trx0trx.c b/storage/innobase/trx/ > trx0trx.c > index 2d585b7507f..9a80478f6f1 100644 > --- a/storage/innobase/trx/trx0trx.c > +++ b/storage/innobase/trx/trx0trx.c > @@ -40,6 +40,7 @@ Created 3/26/1996 Heikki Tuuri > #include "srv0srv.h" > #include "btr0sea.h" > #include "os0proc.h" > +#include "os0thread.h" > #include "trx0xa.h" > #include "trx0purge.h" > #include "ha_prototypes.h" > @@ -97,7 +98,6 @@ trx_create( > trx_t* trx; > > ut_ad(mutex_own(&kernel_mutex)); > - ut_ad(sess); > > trx = mem_alloc(sizeof(trx_t)); > > @@ -1770,7 +1770,7 @@ trx_print( > { > ibool newline; > > - fprintf(f, "TRANSACTION " TRX_ID_FMT, (ullint) trx->id); > + fprintf(f, "TRANSACTION " TRX_ID_FMT " OS thread %lu", (ullint) > trx->id, trx->thread_id); > > switch (trx->conc_state) { > case TRX_NOT_STARTED: > Furthermore, I think that in that separate commit for adding these, a macro could be useful instead of "%lu", because pthread_t may be incompatible with "%lu". Marko -- 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 0

Re: [Maria-developers] [Commits] f380e7b5535: MDEV-12072 Do not unnecessarily construct rec_printer objects in debug builds
by Jan Lindström 15 Feb '17

15 Feb '17

ok to push. On Wed, Feb 15, 2017 at 4:02 PM, <marko.makela(a)mariadb.com> wrote: > revision-id: f380e7b5535358bd3d7caf81d22579ef97f6ca24 (mariadb-10.2.3-271- > gf380e7b5535) > parent(s): 24911cee4e3be988848f6eab4d768849709f5256 > author: Marko Mäkelä > committer: Marko Mäkelä > timestamp: 2017-02-15 16:02:00 +0200 > message: > > MDEV-12072 Do not unnecessarily construct rec_printer objects in debug > builds > > I introduced the rec_printer object in MySQL to pretty-print raw InnoDB > records and index tuples in diagnostic messages. These objects are being > constructed unconditionally, even though the DBUG_PRINT is not enabled. > > The unnecessary work would be avoided by simply passing > rec_printer(…).str() to the DBUG_LOG that was introduced in MDEV-11713. > > --- > storage/innobase/btr/btr0cur.cc | 54 ++++++++---------------- > storage/innobase/row/row0log.cc | 58 ++++++++------------------ > storage/innobase/row/row0merge.cc | 88 +++++++++++++++--------------- > --------- > 3 files changed, 68 insertions(+), 132 deletions(-) > > diff --git a/storage/innobase/btr/btr0cur.cc b/storage/innobase/btr/ > btr0cur.cc > index 9033c399ce2..bdf3bd2eca7 100644 > --- a/storage/innobase/btr/btr0cur.cc > +++ b/storage/innobase/btr/btr0cur.cc > @@ -3012,18 +3012,10 @@ btr_cur_optimistic_insert( > > page_cursor = btr_cur_get_page_cur(cursor); > > -#ifdef UNIV_DEBUG > - { > - rec_printer p(entry); > - DBUG_PRINT("ib_cur", ("insert %s (" IB_ID_FMT ") by " > IB_ID_FMT " %s", > - index->name(), index->id, > - thr != NULL > - ? trx_get_id_for_print(thr_get_trx(thr)) > - : 0, > - p.str().c_str())); > - } > -#endif > - > + DBUG_LOG("ib_cur", > + "insert " << index->name << " (" << index->id << ") by " > + << ib::hex(thr ? trx_get_id_for_print(thr_get_trx(thr)) > : 0) > + << ' ' << rec_printer(entry).str()); > DBUG_EXECUTE_IF("do_page_reorganize", > btr_page_reorganize(page_cursor, index, mtr);); > > @@ -3635,14 +3627,10 @@ btr_cur_update_in_place( > ut_ad(fil_page_index_page_check(btr_cur_get_page(cursor))); > ut_ad(btr_page_get_index_id(btr_cur_get_page(cursor)) == > index->id); > > -#ifdef UNIV_DEBUG > - { > - rec_printer p(rec, offsets); > - DBUG_PRINT("ib_cur", ("update-in-place %s (" IB_ID_FMT ") > by " IB_ID_FMT ": %s", > - index->name(), index->id, trx_id, > - p.str().c_str())); > - } > -#endif > + DBUG_LOG("ib_cur", > + "update-in-place " << index->name << " (" << index->id > + << ") by " << ib::hex(trx_id) << ": " > + << rec_printer(rec, offsets).str()); > > block = btr_cur_get_block(cursor); > page_zip = buf_block_get_page_zip(block); > @@ -3842,14 +3830,10 @@ btr_cur_optimistic_update( > } > } > > -#ifdef UNIV_DEBUG > - { > - rec_printer p(rec, *offsets); > - DBUG_PRINT("ib_cur", ("update %s (" IB_ID_FMT ") by " > IB_ID_FMT ": %s", > - index->name(), index->id, trx_id, > - p.str().c_str())); > - } > -#endif > + DBUG_LOG("ib_cur", > + "update " << index->name << " (" << index->id << ") by " > + << ib::hex(trx_id) << ": " > + << rec_printer(rec, *offsets).str()); > > page_cursor = btr_cur_get_page_cur(cursor); > > @@ -4669,15 +4653,11 @@ btr_cur_del_mark_set_clust_rec( > ut_ad(trx_state_eq(trx, TRX_STATE_ACTIVE)); > ut_ad(!trx->in_rollback); > > -#ifdef UNIV_DEBUG > - { > - rec_printer p(rec, offsets); > - DBUG_PRINT("ib_cur", ("delete-mark clust %s (" IB_ID_FMT > ") by " IB_ID_FMT ": %s", > - index->table_name, index->id, > - trx_get_id_for_print(trx), > - p.str().c_str())); > - } > -#endif > + DBUG_LOG("ib_cur", > + "delete-mark clust " << index->table->name > + << " (" << index->id << ") by " > + << ib::hex(trx_get_id_for_print(trx)) << ": " > + << rec_printer(rec, offsets).str()); > > if (dict_index_is_online_ddl(index)) { > row_log_table_delete(rec, entry, index, offsets, NULL); > diff --git a/storage/innobase/row/row0log.cc b/storage/innobase/row/ > row0log.cc > index d43fb333cdc..0fc1dd097cc 100644 > --- a/storage/innobase/row/row0log.cc > +++ b/storage/innobase/row/row0log.cc > @@ -1,6 +1,7 @@ > /*********************************************************** > ****************** > > Copyright (c) 2011, 2016, Oracle and/or its affiliates. All Rights > Reserved. > +Copyright (c) 2017, MariaDB Corporation. > > This program is free software; you can redistribute it and/or modify it > under > the terms of the GNU General Public License as published by the Free > Software > @@ -1641,15 +1642,9 @@ row_log_table_apply_insert_low( > ut_ad(dtuple_validate(row)); > ut_ad(trx_id); > > -#ifdef UNIV_DEBUG > - { > - rec_printer p(row); > - DBUG_PRINT("ib_alter_table", > - ("insert table " IB_ID_FMT " (index " IB_ID_FMT > "): %s", > - index->table->id, index->id, > - p.str().c_str())); > - } > -#endif > + DBUG_LOG("ib_alter_table", > + "insert table " << index->table->id << " (index " > + << index->id << "): " << rec_printer(row).str()); > > static const ulint flags > = (BTR_CREATE_FLAG > @@ -1777,15 +1772,10 @@ row_log_table_apply_delete_low( > > ut_ad(dict_index_is_clust(index)); > > -#ifdef UNIV_DEBUG > - { > - rec_printer p(btr_pcur_get_rec(pcur), offsets); > - DBUG_PRINT("ib_alter_table", > - ("delete table " IB_ID_FMT " (index " IB_ID_FMT > "): %s", > - index->table->id, index->id, > - p.str().c_str())); > - } > -#endif > + DBUG_LOG("ib_alter_table", > + "delete table " << index->table->id << " (index " > + << index->id << "): " > + << rec_printer(btr_pcur_get_rec(pcur), offsets).str()); > > if (dict_table_get_next_index(index)) { > /* Build a row template for purging secondary index > entries. */ > @@ -2266,17 +2256,11 @@ row_log_table_apply_update( > cur_offsets, NULL, NULL, NULL, &old_ext, heap); > ut_ad(old_row); > > -#ifdef UNIV_DEBUG > - { > - rec_printer old(old_row); > - rec_printer new_row(row); > - DBUG_PRINT("ib_alter_table", > - ("update table " IB_ID_FMT " (index " > IB_ID_FMT "): %s to %s", > - index->table->id, index->id, > - old.str().c_str(), > - new_row.str().c_str())); > - } > -#endif > + DBUG_LOG("ib_alter_table", > + "update table " << index->table->id > + << " (index " << index->id > + << ": " << rec_printer(old_row).str() > + << " to " << rec_printer(row).str()); > } else { > old_row = NULL; > old_ext = NULL; > @@ -3298,17 +3282,11 @@ row_log_apply_op_low( > ut_ad(!dict_index_is_corrupted(index)); > ut_ad(trx_id != 0 || op == ROW_OP_DELETE); > > -#ifdef UNIV_DEBUG > - { > - rec_printer p(entry); > - DBUG_PRINT("ib_create_index", > - ("%s %s index " IB_ID_FMT "," IB_ID_FMT ": > %s", > - op == ROW_OP_INSERT ? "insert" : "delete", > - has_index_lock ? "locked" : "unlocked", > - index->id, trx_id, > - p.str().c_str())); > - } > -#endif > + DBUG_LOG("ib_create_index", > + (op == ROW_OP_INSERT ? "insert " : "delete ") > + << (has_index_lock ? "locked index " : "unlocked index ") > + << index->id << ',' << ib::hex(trx_id) << ": " > + << rec_printer(entry).str()); > > mtr_start(&mtr); > mtr.set_named_space(index->space); > diff --git a/storage/innobase/row/row0merge.cc b/storage/innobase/row/ > row0merge.cc > index 57dd8f75e8e..85f1e0b6ecc 100644 > --- a/storage/innobase/row/row0merge.cc > +++ b/storage/innobase/row/row0merge.cc > @@ -1,6 +1,7 @@ > /*********************************************************** > ****************** > > Copyright (c) 2005, 2016, Oracle and/or its affiliates. All Rights > Reserved. > +Copyright (c) 2017, MariaDB Corporation. > > This program is free software; you can redistribute it and/or modify it > under > the terms of the GNU General Public License as published by the Free > Software > @@ -1100,16 +1101,11 @@ row_merge_buf_write( > row_merge_buf_encode(&b, index, entry, n_fields); > ut_ad(b < &block[srv_sort_buf_size]); > > -#ifdef UNIV_DEBUG > - { > - rec_printer p(entry->fields, n_fields); > - DBUG_PRINT("ib_merge_sort", > - ("%p,fd=%d,%lu %lu: %s", > - reinterpret_cast<const void*>(b), > of->fd, > - (ulint)of->offset, (ulint)i, > - p.str().c_str())); > - } > -#endif > + DBUG_LOG("ib_merge_sort", > + reinterpret_cast<const void*>(b) << ',' > + << of->fd << ',' << of->offset << ' ' << > + i << ": " << > + rec_printer(entry->fields, n_fields).str()); > } > > /* Write an "end-of-chunk" marker. */ > @@ -1121,10 +1117,9 @@ row_merge_buf_write( > to avoid bogus warnings. */ > memset(b, 0xff, &block[srv_sort_buf_size] - b); > #endif /* UNIV_DEBUG_VALGRIND */ > - DBUG_PRINT("ib_merge_sort", > - ("write %p,%d,%lu EOF", > - reinterpret_cast<const void*>(b), of->fd, > - (ulint)of->offset)); > + DBUG_LOG("ib_merge_sort", > + "write " << b << ',' << of->fd << ',' << of->offset << > + " EOF"); > DBUG_VOID_RETURN; > } > > @@ -1177,7 +1172,7 @@ row_merge_read( > os_offset_t ofs = ((os_offset_t) offset) * srv_sort_buf_size; > > DBUG_ENTER("row_merge_read"); > - DBUG_PRINT("ib_merge_sort", ("fd=%d ofs=" UINT64PF, fd, ofs)); > + DBUG_LOG("ib_merge_sort", "fd=" << fd << " ofs=" << ofs); > DBUG_EXECUTE_IF("row_merge_read_failure", DBUG_RETURN(FALSE);); > > IORequest request; > @@ -1224,7 +1219,7 @@ row_merge_write( > void* out_buf = (void *)buf; > > DBUG_ENTER("row_merge_write"); > - DBUG_PRINT("ib_merge_sort", ("fd=%d ofs=" UINT64PF, fd, ofs)); > + DBUG_LOG("ib_merge_sort", "fd=" << fd << " ofs=" << ofs); > DBUG_EXECUTE_IF("row_merge_write_failure", DBUG_RETURN(FALSE);); > > IORequest request(IORequest::WRITE); > @@ -1296,11 +1291,10 @@ row_merge_read_rec( > if (UNIV_UNLIKELY(!extra_size)) { > /* End of list */ > *mrec = NULL; > - DBUG_PRINT("ib_merge_sort", > - ("read %p,%p,%d,%lu EOF\n", > - reinterpret_cast<const void*>(b), > - reinterpret_cast<const void*>(block), > - fd, ulint(*foffs))); > + DBUG_LOG("ib_merge_sort", > + "read " << reinterpret_cast<const void*>(b) << > ',' << > + reinterpret_cast<const void*>(block) << ',' << > + fd << ',' << *foffs << " EOF"); > DBUG_RETURN(NULL); > } > > @@ -1413,18 +1407,11 @@ row_merge_read_rec( > b += extra_size + data_size - avail_size; > > func_exit: > - > -#ifdef UNIV_DEBUG > - { > - rec_printer p(*mrec, 0, offsets); > - DBUG_PRINT("ib_merge_sort", > - ("%p,%p,fd=%d,%lu: %s", > - reinterpret_cast<const void*>(b), > - reinterpret_cast<const void*>(block), > - fd, ulint(*foffs), > - p.str().c_str())); > - } > -#endif > + DBUG_LOG("ib_merge_sort", > + reinterpret_cast<const void*>(b) << ',' << > + reinterpret_cast<const void*>(block) > + << ",fd=" << fd << ',' << *foffs << ": " > + << rec_printer(*mrec, 0, offsets).str()); > DBUG_RETURN(b); > } > > @@ -1455,15 +1442,9 @@ row_merge_write_rec_low( > #endif /* DBUG_OFF */ > DBUG_ASSERT(e == rec_offs_extra_size(offsets) + 1); > > -#ifdef UNIV_DEBUG > - { > - rec_printer p(mrec, 0, offsets); > - DBUG_PRINT("ib_merge_sort", > - ("%p,fd=%d,%lu: %s", > - reinterpret_cast<const void*>(b), fd, > ulint(foffs), > - p.str().c_str())); > - } > -#endif > + DBUG_LOG("ib_merge_sort", > + reinterpret_cast<const void*>(b) << ",fd=" << fd << ',' > + << foffs << ": " << rec_printer(mrec, 0, offsets).str()); > > if (e < 0x80) { > *b++ = (byte) e; > @@ -1573,11 +1554,10 @@ row_merge_write_eof( > ut_ad(foffs); > > DBUG_ENTER("row_merge_write_eof"); > - DBUG_PRINT("ib_merge_sort", > - ("%p,%p,fd=%d,%lu", > - reinterpret_cast<const void*>(b), > - reinterpret_cast<const void*>(block), > - fd, ulint(*foffs))); > + DBUG_LOG("ib_merge_sort", > + reinterpret_cast<const void*>(b) << ',' << > + reinterpret_cast<const void*>(block) << > + ",fd=" << fd << ',' << *foffs); > > if (b == &block[0]) { > b+= ROW_MERGE_RESERVE_SIZE; > @@ -2937,10 +2917,9 @@ row_merge_blocks( > ulint* offsets1;/* offsets of mrec1 */ > > DBUG_ENTER("row_merge_blocks"); > - DBUG_PRINT("ib_merge_sort", > - ("fd=%d,%lu+%lu to fd=%d,%lu", > - file->fd, ulint(*foffs0), ulint(*foffs1), > - of->fd, ulint(of->offset))); > + DBUG_LOG("ib_merge_sort", > + "fd=" << file->fd << ',' << *foffs0 << '+' << *foffs1 > + << " to fd=" << of->fd << ',' << of->offset); > > heap = row_merge_heap_create(dup->index, &buf, &offsets0, > &offsets1); > > @@ -3050,10 +3029,9 @@ row_merge_blocks_copy( > ulint* offsets1;/* dummy offsets */ > > DBUG_ENTER("row_merge_blocks_copy"); > - DBUG_PRINT("ib_merge_sort", > - ("fd=%d,%lu to fd=%d,%lu", > - file->fd, ulint(foffs0), > - of->fd, ulint(of->offset))); > + DBUG_LOG("ib_merge_sort", > + "fd=" << file->fd << ',' << foffs0 > + << " to fd=" << of->fd << ',' << of->offset); > > heap = row_merge_heap_create(index, &buf, &offsets0, &offsets1); > > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits

1 0

[Maria-developers] Blackhole, Archive and others only as loadable plugins in 10.1 - intentional change?
by Hartmut Holzgraefe 15 Feb '17

15 Feb '17

In MariaDB 10.1 binary packages several storage engines, e.g BLACKHOLE and ARCHIVE, are no longer compiled in by default but only come as loadable plugins that need to be loaded into the server explicitly. This is mentioned on the knowledge base pages for these engines, e.g. https://mariadb.com/kb/en/mariadb/blackhole/ and https://mariadb.com/kb/en/mariadb/archive/ but is not mentioned in the release notes and the detailed changelog pages for 10.1.x releases at all. I also searched my "maria-developers" and "maria-discuss" mail archives but could not find any discussion or announcement on this topic either. Same for Jira, can't find any ticket that would describe that sort of change there either. Problems with the current situation: * comes as a surprise for installation using these storage engines when upgraded to 10.1 * mysql_upgrade should handle this, but doesn't: https://jira.mariadb.org/browse/MDEV-11942 * it is not even easily possible to identify affected tables after an upgrade to 10.1. For BLACKHOLE tables I_S.TABLES only shows these under specific conditions, and for ARCHIVE it seems to be impossible to identify affected tables at all if the plugin is not loaded https://jira.mariadb.org/browse/MDEV-11943 I have submitted a a fix for MDEV-11942, but without having a fix for MDEV-11943 it only works for BLACKHOLE so far, not for ARCHIVE (haven't tested FEDERATED or other potentially affected engines yet). https://github.com/MariaDB/server/pull/304 -- Hartmut Holzgraefe, Principal Support Engineer (EMEA) MariaDB Corporation | http://www.mariadb.com/

2 1

[Maria-developers] Fwd: 41f885f64fe: MDEV-10859: Wrong result of aggregate window function in query with HAVING and no ORDER BY
by Vicențiu Ciorbaru 14 Feb '17

14 Feb '17

Hi Igor, Sergey! I need a review on this patch that fixes a problem with Window Functions being computed over too many rows. The details are in the commit message. Thank you! Vicențiu ---------- Forwarded message --------- From: vicentiu <vicentiu(a)mariadb.org> Date: Tue, 14 Feb 2017 at 14:08 Subject: 41f885f64fe: MDEV-10859: Wrong result of aggregate window function in query with HAVING and no ORDER BY To: <commits(a)mariadb.org> revision-id: 41f885f64fe4c02e03ec655f225b1dc01e9d5376 (mariadb-10.2.3-252-g41f885f64fe) parent(s): d731ce21a7bc25a49958789d24b3f0eec5845aea author: Vicențiu Ciorbaru committer: Vicențiu Ciorbaru timestamp: 2017-02-14 14:02:29 +0200 message: MDEV-10859: Wrong result of aggregate window function in query with HAVING and no ORDER BY Window functions need to be computed after applying the HAVING clause. An optimization that we have for regular, non-window function, cases is to apply having only during sending of the rows to the client. This allows rows that should be filtered from the temporary table used to store aggregation results to be stored there. This behaviour is undesireable for window functions, as we have to compute window functions on the result-set after HAVING is applied. Storing extra rows in the table leads to wrong values as the frame bounds might capture those -to be filtered afterwards- rows. --- mysql-test/r/win.result | 33 +++++++++++++++++++++++++++++++++ mysql-test/t/win.test | 23 +++++++++++++++++++++++ sql/sql_select.cc | 11 ++++++++--- sql/sql_window.cc | 6 ++++++ 4 files changed, 70 insertions(+), 3 deletions(-) diff --git a/mysql-test/r/win.result b/mysql-test/r/win.result index fd3aea80083..b3adf358be6 100644 --- a/mysql-test/r/win.result +++ b/mysql-test/r/win.result @@ -2925,3 +2925,36 @@ WHERE EXISTS ( SELECT * FROM t2 WHERE c IN ( SELECT MAX(d) FROM t3 ) ); a MAX(a) AVG(a) OVER (PARTITION BY b) NULL NULL NULL DROP TABLE t1,t2,t3; +# +# MDEV-MDEV-10859: Wrong result of aggregate window function in query +# with HAVING and no ORDER BY +# +create table empsalary (depname varchar(32), empno smallint primary key, salary int); +insert into empsalary values +('develop', 1, 5000), ('develop', 2, 4000),('sales', 3, '6000'),('sales', 4, 5000); +SELECT depname, empno, salary, avg(salary) OVER (PARTITION BY depname) FROM empsalary; +depname empno salary avg(salary) OVER (PARTITION BY depname) +develop 1 5000 4500.0000 +develop 2 4000 4500.0000 +sales 3 6000 5500.0000 +sales 4 5000 5500.0000 +SELECT depname, empno, salary, avg(salary) OVER (PARTITION BY depname) FROM empsalary ORDER BY depname; +depname empno salary avg(salary) OVER (PARTITION BY depname) +develop 1 5000 4500.0000 +develop 2 4000 4500.0000 +sales 3 6000 5500.0000 +sales 4 5000 5500.0000 +# +# These last 2 should have the same row results, ignoring order. +# +SELECT depname, empno, salary, avg(salary) OVER (PARTITION BY depname) FROM empsalary HAVING empno > 1; +depname empno salary avg(salary) OVER (PARTITION BY depname) +develop 2 4000 4000.0000 +sales 3 6000 5500.0000 +sales 4 5000 5500.0000 +SELECT depname, empno, salary, avg(salary) OVER (PARTITION BY depname) FROM empsalary HAVING empno > 1 ORDER BY depname; +depname empno salary avg(salary) OVER (PARTITION BY depname) +develop 2 4000 4000.0000 +sales 3 6000 5500.0000 +sales 4 5000 5500.0000 +drop table empsalary; diff --git a/mysql-test/t/win.test b/mysql-test/t/win.test index aa636f7a294..90bbf41688f 100644 --- a/mysql-test/t/win.test +++ b/mysql-test/t/win.test @@ -1711,3 +1711,26 @@ SELECT a, MAX(a), AVG(a) OVER (PARTITION BY b) FROM t1 WHERE EXISTS ( SELECT * FROM t2 WHERE c IN ( SELECT MAX(d) FROM t3 ) ); DROP TABLE t1,t2,t3; + +--echo # +--echo # MDEV-MDEV-10859: Wrong result of aggregate window function in query +--echo # with HAVING and no ORDER BY +--echo # + +create table empsalary (depname varchar(32), empno smallint primary key, salary int); +insert into empsalary values + ('develop', 1, 5000), ('develop', 2, 4000),('sales', 3, '6000'),('sales', 4, 5000); + +--sorted_result +SELECT depname, empno, salary, avg(salary) OVER (PARTITION BY depname) FROM empsalary; +--sorted_result +SELECT depname, empno, salary, avg(salary) OVER (PARTITION BY depname) FROM empsalary ORDER BY depname; +--echo # +--echo # These last 2 should have the same row results, ignoring order. +--echo # +--sorted_result +SELECT depname, empno, salary, avg(salary) OVER (PARTITION BY depname) FROM empsalary HAVING empno > 1; +--sorted_result +SELECT depname, empno, salary, avg(salary) OVER (PARTITION BY depname) FROM empsalary HAVING empno > 1 ORDER BY depname; + +drop table empsalary; diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 5c7ae1e88c1..473270f8a69 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -2411,13 +2411,18 @@ bool JOIN::make_aggr_tables_info() If having is not handled here, it will be checked before the row is sent to the client. + + In the case of window functions however, we *must* make sure to not + store any rows which don't match HAVING within the temp table, + as rows will end up being used during their computation. */ if (having && - (sort_and_group || (exec_tmp_table->distinct && !group_list))) + (sort_and_group || (exec_tmp_table->distinct && !group_list) || + select_lex->have_window_funcs())) { - // Attach HAVING to tmp table's condition + /* Attach HAVING to tmp table's condition */ curr_tab->having= having; - having= NULL; // Already done + having= NULL; /* Already done */ } /* Change sum_fields reference to calculated fields in tmp_table */ diff --git a/sql/sql_window.cc b/sql/sql_window.cc index 37095ad78ca..2bdac89f293 100644 --- a/sql/sql_window.cc +++ b/sql/sql_window.cc @@ -2840,6 +2840,12 @@ bool Window_funcs_computation::setup(THD *thd, order_window_funcs_by_window_specs(window_funcs); SQL_SELECT *sel= NULL; + /* + If the tmp table is filtered during sorting + (ex: SELECT with HAVING && ORDER BY), we must make sure to keep the + filtering conditions when we perform sorting for window function + computation. + */ if (tab->filesort && tab->filesort->select) { sel= tab->filesort->select;

1 0

[Maria-developers] bb-10.2-compatibility - MDEV 10585 or 10866
by jerome brauge 14 Feb '17

14 Feb '17

Hello Alexander, Attached script fail on my local bb-10.2-compatibility branch on the following assert : /* If called from a stored procedure, ensure that we won't rollback external changes when cleaning up after validation. */ DBUG_ASSERT(thd->change_list.is_empty()); Stack trace : mysqld.exe!my_sigabrt_handler(int sig) Line 477 C mysqld.exe!raise(int signum) Line 516 C++ mysqld.exe!abort() Line 64 C++ mysqld.exe!common_assert_to_stderr_direct(const wchar_t * const expression, const wchar_t * const file_name, const unsigned int line_number) Line 124 C++ mysqld.exe!common_assert_to_stderr<wchar_t>(const wchar_t * const expression, const wchar_t * const file_name, const unsigned int line_number) Line 138 C++ mysqld.exe!common_assert<wchar_t>(const wchar_t * const expression, const wchar_t * const file_name, const unsigned int line_number, void * const return_address) Line 383 C++ mysqld.exe!_wassert(const wchar_t * expression, const wchar_t * file_name, unsigned int line_number) Line 404 C++ > mysqld.exe!Prepared_statement::prepare(const char * packet, unsigned int packet_len) Line 3848 C++ mysqld.exe!Prepared_statement::execute_immediate(const char * query, unsigned int query_len) Line 4740 C++ mysqld.exe!mysql_sql_stmt_execute_immediate(THD * thd) Line 2863 C++ mysqld.exe!mysql_execute_command(THD * thd) Line 3722 C++ mysqld.exe!sp_instr_stmt::exec_core(THD * thd, unsigned int * nextp) Line 3335 C++ mysqld.exe!sp_lex_keeper::reset_lex_and_exec_core(THD * thd, unsigned int * nextp, bool open_tables, sp_instr * instr) Line 3098 C++ mysqld.exe!sp_instr_stmt::execute(THD * thd, unsigned int * nextp) Line 3251 C++ mysqld.exe!sp_head::execute(THD * thd, bool merge_da_on_success) Line 1265 C++ mysqld.exe!sp_head::execute_procedure(THD * thd, List<Item> * args) Line 2090 C++ mysqld.exe!do_execute_sp(THD * thd, sp_head * sp) Line 2871 C++ mysqld.exe!mysql_execute_command(THD * thd) Line 5900 C++ mysqld.exe!mysql_parse(THD * thd, char * rawbuf, unsigned int length, Parser_state * parser_state, bool is_com_multi, bool is_next_command) Line 7990 C++ mysqld.exe!dispatch_command(enum_server_command command, THD * thd, char * packet, unsigned int packet_length, bool is_com_multi, bool is_next_command) Line 1809 C++ mysqld.exe!do_command(THD * thd) Line 1367 C++ mysqld.exe!threadpool_process_request(THD * thd) Line 346 C++ mysqld.exe!tp_callback(TP_connection * c) Line 192 C++ mysqld.exe!tp_callback(_TP_CALLBACK_INSTANCE * instance, void * context) Line 377 C++ mysqld.exe!work_callback(_TP_CALLBACK_INSTANCE * instance, void * context, _TP_WORK * work) Line 451 C++ Can you reproduce this error ? Regards, Jérôme.

2 1

Re: [Maria-developers] [Commits] 822aa21672b: MDEV-11170: MariaDB 10.2 cannot start on MySQL 5.7 datadir
by Sergei Golubchik 13 Feb '17

13 Feb '17

Hi, Vicentiu! Looks fine, thanks. Few comments below. On Feb 13, vicentiu wrote: > revision-id: 822aa21672ba6650a31a3ec05ab5c1bce7a80c1e (mariadb-10.2.3-198-g822aa21672b) > parent(s): b745ea489f0698c328383d9b8ec20d2f9777b1b8 > author: Vicențiu Ciorbaru > committer: Vicențiu Ciorbaru > timestamp: 2017-02-13 14:44:00 +0200 > message: > > MDEV-11170: MariaDB 10.2 cannot start on MySQL 5.7 datadir > > PART 2 of the fix adds the logic of not using password column, unless it > exists. If password column is missing we attempt to use plugin && > authentication_string columns. > > diff --git a/mysql-test/r/no_password_column-mdev-11170.result b/mysql-test/r/no_password_column-mdev-11170.result > new file mode 100644 > index 00000000000..c3920a1b530 > --- /dev/null > +++ b/mysql-test/r/no_password_column-mdev-11170.result > @@ -0,0 +1,170 @@ ... > +# Reset to final original state. > +# > +drop table mysql.user; > +create table mysql.user like backup_user; > +insert into mysql.user select * from backup_user; you could RENAME TABLE backup_user to mysql.user. > +drop table backup_user; > +flush privileges; > diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc > index 568aff9ac92..7e03253ca98 100644 > --- a/sql/sql_acl.cc > +++ b/sql/sql_acl.cc > @@ -2071,7 +2103,7 @@ static bool acl_load(THD *thd, const Grant_tables& tables) > 8, 8, MYF(0)); > > /* check default role, if any */ > - if (!is_role && user_table.num_fields() > DEFAULT_ROLE_COLUMN_IDX) > + if (!is_role && user_table.default_role()) all these changes could've went into the first patch... > { > user.default_rolename.str= > get_field(&acl_memroot, user_table.default_role()); > @@ -4030,11 +4092,16 @@ static int replace_user_table(THD *thd, const User_table &user_table, > > rights= user_table.get_access(); > > - DBUG_PRINT("info",("table fields: %d",user_table.num_fields())); > - if (combo.pwhash.str[0]) > + DBUG_PRINT("info",("table fields: %d", user_table.num_fields())); > + /* If we don't have a password column, we'll use the authentication_string > + column later. */ > + if (combo.pwhash.str[0] && user_table.password()) > user_table.password()->store(combo.pwhash.str, combo.pwhash.length, > system_charset_info); > - if (user_table.num_fields() >= 31) /* From 4.0.0 we have more fields */ > + /* We either have the password column, the plugin column, or both. Otherwise > + we have a corrupt user table. */ > + DBUG_ASSERT(user_table.password() || user_table.plugin()); you cannot assert the table structure. ALTER TABLE shouldn't crash the server. > + if (user_table.ssl_type()) /* From 4.0.0 we have more fields */ > { > /* We write down SSL related ACL stuff */ > switch (lex->ssl_type) { Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 1