- developers - lists.mariadb.org

[Maria-developers] an observation
by AugustQ 22 Feb '17

22 Feb '17

Hi, by playing with the code I think I found something interesting. My environment: MariaDB 10.0.10, MyISAM-engine I played with a table-scan, no index is defined on this table. When I execute a SQL-statement that forces the server to do a second table- scan on a table this 2nd table-scan will be slow. The reason for this behaviour is the usage of a buffer: during the 1st scan this buffer is filled, used and filled again until the whole table is processed. At the end of the 1st scan it contains the last bytes of the file. When a 2nd scan is started the reading of the table starts from the beginning of the file but the buffer and all associated variables are not reset: the buffer still contains the bytes from the end of the file, the request cannot be fulfilled by the buffer so the request has to be handled by reading the bytes directly from the file using the read()-function of the Std-library. This takes much more time then simply copying the bytes from the internal buffer. My idea is: somewhere in the code this situation must be detected and the buffer (and all associated variables) reset to initial values. rein it_io_cache() looks like the right candidate for this. I assume the engine Maria will show the same effect (I didn't check other engines). Some questions: is my observation correct? Do you think this needs to be fixed? Or is there no further development on this part of the code? Thanks Regards AugustQ PS: you will find more details on this topic here: http://augustq.blogs pot.com/2017/01/subselect-execution-3.html

3 6

[Maria-developers] engine condition pushdown
by AugustQ 22 Feb '17

22 Feb '17

Hi, by playing with a SQL-statement I encountered a problem. I'm not sure if this is a bug or if I did not use the correct method to get the information I expected. So let me please explain the situation in which I encountered this problem. But it will take some lines of text to come to the point. My environment: MariaDB 10.0.10 source-code-version on Linux. The statement: select SQL_NO_CACHE B.PZN, B.ArtikelText from TestSmall B where exists ( select 1 from TestBig A where A.Hersteller = '36367' and A.PZN = B.PZN) and B.Hersteller = '00020' Engine used is MyISAM but the effect should be independent of any engine (not checked). No indexes exist on the tables involved. You will find a lot more of details here: http://augustq.blogspot.com/2 017/02/subselect-execution-4.html Execution of this (silly) statement is done by a table-scan on the table TestSmall. When a record is found the matches the WHERE-condition the server switches over to the table TestBig and does a table-scan on this table. When a match is found it returns to the table TestSmall, otherwise the table TestBig is read to the end and then the server returns to the table TestSmall. The server continues scanning the table TestSmall, switching over to TestBig, returning to TestSmall, continues reading TestSmall and so on, until it reaches the end of the table TestSmall. Of interest for me was the condition tree which can be inspected by looking at the function cond_push() (in my case ha_myisam::cond_push()). So let's start. The function cond_push() is initally called for the tables TestSmall and TestBig, this looks good. Then the server starts reading the table TestSmall and therefore the function cond_push() is called again. This looks good too. Now a matching record is found in TestSmall: it switches over to the table TestBig and calls cond_push() with the condition-tree for this table. This tree looks like this: COND-ITEM args: 0 type=[COND_AND_FUNC] FUNC-ITEM [=] args: 2 type=[EQ_FUNC] FIELD-ITEM [TestOpt] [A] [Hersteller] name=<Hersteller> STRING-ITEM str_length=<5> str_value=<36367> name=<36367> FUNC-ITEM [=] args: 2 type=[EQ_FUNC] FIELD-ITEM [TestOpt] [A] [PZN] name=<PZN> FIELD-ITEM [TestOpt] [B] [PZN] name=<PZN> The last line is of interest (I've marked it in bold). For the table with the alias B (= TestSmall) no value is given for the column PZN but this value is available. This is the first point of interest. Let's continue with the operation. When the table TestBig is read to the end, the scanning of TestSmall continues and a matching record is found over there the server again switches over to TestBig. Before it starts reading the records from this table the function cond_push() is called again. This is the output of the condition-tree, restricted to the line of interest (the last line): FIELD-ITEM [TestOpt] [B] [PZN] str_length=<2> str_value=<37> name=<PZN> So let's continue: when it switches to TestBig for the 3rd time this output looks like: FIELD-ITEM [TestOpt] [B] [PZN] str_length=<7> str_value=<222 > name=<PZN> Enough of data. I can deliver more examples if needed or you may look into my text. Judgement: The output of the first case des not show the value of the column PZN from the current record in TestSmall although the value is available. The output in the second case shows the value 37 (because of the length eq. 2) but the correct value would be 3717968 (and length eq. 7). The output in the third case shows the value 222 followed by 4 spaces (length eq. 7), but the correct value is 222 (length eq. 3). What I see is the correct value (exclude the first example presented here) but the length-information is one row behind (one row of TestSmall). For accessing the information presented here I used COND->str_value. So the question is: - the information is available but I used the wrong function/variable? OR - is this a bug in the code? What do you think? Thanks AugustQ

2 4

Re: [Maria-developers] 2c36ba67240: MySQL Bug#12766319 - 61865: RENAME USER DOES NOT WORK CORRECTLY - REQUIRES FLUSH PRIVILEGES
by Sergei Golubchik 22 Feb '17

22 Feb '17

Hi, Oleksandr! On Jan 10, Oleksandr Byelkin wrote: > revision-id: 2c36ba67240850d909bf75fc36346261f37019fb (mariadb-10.0.28-43-g2c36ba67240) > parent(s): eed319b6fb543849046c8009c38575455e173dc2 > committer: Oleksandr Byelkin > timestamp: 2017-01-10 16:02:42 +0100 > message: > > MySQL Bug#12766319 - 61865: RENAME USER DOES NOT WORK CORRECTLY - REQUIRES FLUSH PRIVILEGES > > Port & fix of MySQL patch for the problem. Don't mix the refactoring and a bug fix. I don't see from this big patch what the bug fix was. Please split in two (or more) commits. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] 0310b44: MDEV-10418 Assertion `m_extra_cache' failed in ha_partition::late_extra_cache(uint)
by Sergei Golubchik 21 Feb '17

21 Feb '17

Hi, Alexey! On Dec 20, Alexey Botchkov wrote: > revision-id: 0310b444dba1b9cbd6921963dd3bff689ca23a24 (mariadb-5.5.53-34-g0310b44) > parent(s): f23b41b9b8a30e0e54a1ec7a8923057b0e57e0f5 > committer: Alexey Botchkov > timestamp: 2016-12-20 00:24:20 +0400 > message: > > MDEV-10418 Assertion `m_extra_cache' failed in ha_partition::late_extra_cache(uint) > > The m_extra_prepare_for_update parameter should be set to FALSE > as the query ends. This seems fairly obvious, that it should. But why MySQL doesn't have this bug (despite not resetting m_extra_prepare_for_update)? > diff --git a/sql/ha_partition.cc b/sql/ha_partition.cc > index 15fa7d1..4374354 100644 > --- a/sql/ha_partition.cc > +++ b/sql/ha_partition.cc > @@ -6667,6 +6667,7 @@ int ha_partition::reset(void) > DBUG_ENTER("ha_partition::reset"); > if (m_part_info) > bitmap_set_all(&m_part_info->used_partitions); > + m_extra_prepare_for_update= FALSE; > file= m_file; > do > { Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 2

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