- developers - lists.mariadb.org

[Maria-developers] review: [Commits] Rev 2819: Fixed LP BUG#615378 Incorrect processing of NULL result in Item_cache fixed. in file:///home/bell/maria/bzr/work-maria-5.3-lb615378/
by Michael Widenius 02 Nov '10

02 Nov '10

>>>>> "sanja" == sanja <sanja(a)askmonty.org> writes: sanja> At file:///home/bell/maria/bzr/work-maria-5.3-lb615378/ sanja> ------------------------------------------------------------ sanja> revno: 2819 sanja> revision-id: sanja(a)askmonty.org-20101004100050-q1e95qec7288qkdo sanja> parent: sanja(a)askmonty.org-20100914134341-voquimk50t20zuiy sanja> committer: sanja(a)askmonty.org sanja> branch nick: work-maria-5.3-lb615378 sanja> timestamp: Mon 2010-10-04 13:00:50 +0300 sanja> message: sanja> Fixed LP BUG#615378 Incorrect processing of NULL result in Item_cache fixed. <cut> > === modified file 'sql/item.cc' > --- a/sql/item.cc 2010-09-06 12:34:24 +0000 > +++ b/sql/item.cc 2010-10-04 10:00:50 +0000 > @@ -7750,8 +7750,11 @@ void Item_cache_int::store_longlong(Item > String *Item_cache_int::val_str(String *str) > { > DBUG_ASSERT(fixed == 1); > - if (!value_cached && !cache_value()) > + if ((!value_cached && !cache_value()) || null_value) > + { > + null_value= TRUE; > return NULL; > + } > str->set(value, default_charset()); > return str; > } <cut> (All other fixes are the same). Ok to push. Just a note about the item_cache code. If you would inherit Item_cache from Item_func_numhybrid, you could get away with having just 4 val methods, not 16 as you have now. For example: class Item_cache_int : public Item_func_numhybrid { longlong int_op() { if ((!value_cached && !cache_value()) || null_value) return 0; return value; } longlong val_int() { return int_op(); } /* Optimize default case */ } And that's about it. (Except that item_cache would need to provide dummy functions that just aborts for all xxx_op() functions). As a separate note, you can remove the following code from class Item_cache as 'Item_basic_constant' already provides it: table_map used_table_map; ... void set_used_tables(table_map map) { used_table_map= map; } ... table_map used_tables() const { return used_table_map; } Regards, Monty

1 0

[Maria-developers] RQG
by Sergey Petrunya 02 Nov '10

02 Nov '10

Hi Philip, On Tue, Oct 26, 2010 at 03:34:41PM +0300, Philip Stoev wrote: >> - Our chances of making Random Query Generator a little bit friendlier for >> bug analyzers/fixers. > > Hi, > > Can you list specific issues that you are having or specific bug numbers > that were not filed properly? I can't say that any bugs were filed improperly. All ones I've dealt with so far were 'proper', i.e. contained sufficient amount of info for me to reproduce and work on the bug. However, it seems there are some possible improvements that could be made (and it makes sense to do them because currently bugfixing is the bottleneck, not testing) > I already have a list of things from Igor, so if you give me your list, I > will try to dedicate specific time to make things more user-friendly. Here it goes: - Backtick quotes are redundant and must be gone. I've never seen a single RQG-reported bug where the quotes were really necessary. Yet, they are there and make the queries longer than necessary. We have to manually delete them. - Redundant spaces are also annoying, a typical excerpt looks like (all spaces kept): table1 . `col_varchar_key` ) which, again, also forces developer to delete all of the spaces manually. - It would be nice if tables were called t0,t1,t2,... as mysql-test-run guidelines specify - It would be nice if testcase simplification removed irrelevant columns from the tables. If it is not possible to remove them (I guess the reason is that the table size changes and the optimizer choses different QEP), then they should be replaced with one CHAR(N) column. - I understand the following might be difficult to achieve: it would be extremely nice if the SELECT statement that caused the crash is was pretty-printed (i.e. with identation that would make the structure of FROM/WHERE clauses apparent). If this is too difficult, no need to spend time on this. BR Sergey -- Sergey Petrunia, Software Developer Monty Program AB, http://askmonty.org Blog: http://s.petrunia.net/blog

2 1

[Maria-developers] 5.3 compilation warnings
by Sergey Petrunya 01 Nov '10

01 Nov '10

Hi Timour, When one compiles 5.3, it produces the following warnings which I believe fall into your turf. Could you please fix them: item_subselect.cc: In member function ‘bool subselect_rowid_merge_engine::init(MY_BITMAP*, MY_BITMAP*)’: item_subselect.cc:4826: warning: ignoring return value of ‘int handler::ha_rnd_init(bool)’, declared with attribute warn_unused_result item_subselect.cc: In member function ‘virtual bool subselect_rowid_merge_engine::partial_match()’: item_subselect.cc:5007: warning: ignoring return value of ‘int handler::ha_rnd_init(bool)’, declared with attribute warn_unused_result item_subselect.cc: In member function ‘virtual bool subselect_table_scan_engine::partial_match()’: item_subselect.cc:5174: warning: ignoring return value of ‘int handler::ha_rnd_init(bool)’, declared with attribute warn_unused_result sql_class.cc: In member function ‘virtual bool select_materialize_with_stats::create_result_table(THD*, List<Item>*, bool, ulonglong, const char*, bool)’: sql_class.cc:3020: warning: the address of ‘int stat(const char*, stat*)’ will always evaluate as ‘true’ BR Sergey -- Sergey Petrunia, Software Developer Monty Program AB, http://askmonty.org Blog: http://s.petrunia.net/blog

3 2

[Maria-developers] MWL#132: TC plugin, patch for (possible) code review
by Kristian Nielsen 01 Nov '10

01 Nov '10

Hi Serg, And here is the patch for MWL#132, TC plugin. The patch is fairly short, as most of the real code changes are included in MWL#116. This patch is mainly about introducing a new plugin interface for transaction coordinators, and making the three existing TCs in log.cc into built-in plugins. My main motivation for this work was Galera, but when I discussed with them at the Istanbul meeting, I learned that they are not so interested in internal 2-phase commit (instead they download a complete database snapshot at node recovery after a node crash). So now I do not have an example of an external TC plugin implementation. My idea is to implement the MWL#120: stacked event generators. I could then use this to implement a simple TC example that does (very) basic synchroneous replication (eg. maybe insert-only with little error handling or something). This could be a nice example of how to use the facilities that TC plugins provide for controlling the commit process etc. So I would like your opinion as to whether this should be pushed to 5.3 now or wait for MWL#120 (assuming you think it can be made suitable for pushing at all, of course :-). - Kristian.

1 0

[Maria-developers] gcc -Wframe-larger-than
by Sergei Golubchik 01 Nov '10

01 Nov '10

Hi, Just noticed that gcc has a warning option -Wframe-larger-than=LEN that makes gcc to warn about functions with stack frames larger than LEN. Considering a recent issue when we had a 130K structure on the stack (which did overflow the stack in some cases), it may be a good idea to enable this warning with a reasonably conservative threshold, say, 16K or 32K. Regards, Sergei

2 1

[Maria-developers] WL#165 New (by Knielsen): Extend group commit to also handle non-transactional events
by worklog-noreply＠askmonty.org 01 Nov '10

01 Nov '10

----------------------------------------------------------------------- WORKLOG TASK -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- TASK...........: Extend group commit to also handle non-transactional events CREATION DATE..: Mon, 01 Nov 2010, 12:09 SUPERVISOR.....: IMPLEMENTOR....: COPIES TO......: CATEGORY.......: Client-RawIdeaBin TASK ID........: 165 (http://askmonty.org/worklog/?tid=165) VERSION........: Server-9.x STATUS.........: Un-Assigned PRIORITY.......: 60 WORKED HOURS...: 0 ESTIMATE.......: 0 (hours remain) ORIG. ESTIMATE.: 0 PROGRESS NOTES: DESCRIPTION: The MWL#116 implements group commit for the binary log. The work in MWL#116 only facilitates group commit for transactional commit events. Non-transactional events like DDL or updates to MyISAM tables are not improved. This is mainly an issue when running with sync_binlog=1 This worklog is for extending the optimisation to also do group commit for these non-transactional event writes. The basic way to implement this is described in the low-level design of MWL#116, section 2.3. ESTIMATED WORK TIME ESTIMATED COMPLETION DATE ----------------------------------------------------------------------- WorkLog (v4.0.0)

1 0

Re: [Maria-developers] [Commits] Rev 2951: WL#12 - MariaDB User Feedback (a.k.a. Phone Home) plugin
by Sergei Golubchik 31 Oct '10

31 Oct '10

Hi, Michael! On Oct 27, Michael Widenius wrote: > >>>>> "Sergei" == Sergei Golubchik <serg(a)askmonty.org> writes: > > >> +++ b/plugin/feedback/sender_thread.cc 2010-09-30 14:24:31 +0000 > > ... > >> + if (thd) // for nicer SHOW PROCESSLIST > >> + thd->set_query(const_cast<char*>(url->url()), url->url_length()); > >> > >> Wouldn't it be better if url->url() would return const char * ? > > > It does return const char*, but thd->set_query() wants char* (for no > > good reason), so I need to cast const away. > > Should we change set_query() to take const char * ? I can do that, yes, but this plugin will need a cast anyway to work with MySQL sources. > >> +Url* http_create(const char *url, size_t url_length) > >> +{ > >> + const char *s; > >> + LEX_STRING full_url= {const_cast<char*>(url), url_length}; > >> + LEX_STRING host, port, path; > >> > >> Would it not be better to introduce LEX_CONS_STRING and use these to > >> avoid const away casts? > > > Perhaps, if I'd have more casts because of that. But just because of one > > or two - I'd rather keep the code uniform instead and use LEX_STRING > > everywhere. > > I tink there is many cases in the code where we have to cast const > away just becasue we don't have LEX_CONST_STRING. Don't you think it's > time to introduce it ? Not in the plugin. Btw, MariaDB already has LEX_CUSTRING (const unsigned) variant. Regards, Sergei

2 1

Re: [Maria-developers] [Commits] Rev 2840: Fixed LP bug #668290. in file:///home/igor/maria/maria-5.3-mwl128-bug668290/
by Sergey Petrunya 30 Oct '10

30 Oct '10

Hello Igor, Plese find the comments below. On Sat, Oct 30, 2010 at 12:56:10PM -0700, Igor Babaev wrote: > At file:///home/igor/maria/maria-5.3-mwl128-bug668290/ > > ------------------------------------------------------------ > revno: 2840 > revision-id: igor(a)askmonty.org-20101030195609-h53be2cdzjlz38bz > parent: igor(a)askmonty.org-20101030130745-563ypxtkcwv4vbfg > committer: Igor Babaev <igor(a)askmonty.org> > branch nick: maria-5.3-mwl128-bug668290 > timestamp: Sat 2010-10-30 12:56:09 -0700 > message: > Fixed LP bug #668290. > Prohibited to use hash join algorithm BNLH if join attributes > need non-binary collations. It has to be done because BNLH does > not support join for such attributes yet. > Later this limitations will be lifted. > > Changed default collations for the schemes of some test cases > to preserve the old execution plans. ... > === modified file 'sql/field.h' > --- a/sql/field.h 2010-09-23 22:00:32 +0000 > +++ b/sql/field.h 2010-10-30 19:56:09 +0000 > @@ -585,6 +585,12 @@ > } > /* Hash value */ > virtual void hash(ulong *nr, ulong *nr2); > + > + virtual bool hash_join_is_possible(bool globally_allowed) > + { > + return globally_allowed; > + } Is there any partiular reason why have this globally_allowed parameter? In my understanding, a field either allows hash or not, and that is regardless of whether it was "globally allowed", so the check of whether hash join is globally allowed should be done outside of the function. Also, it would be nice to have here a comment specifying the meaning of this function. (I know all this is supposed to be temporary, but expirience shows that often supposedly temporary things stay for a long time, that's why it's nice to put a comment). > + > friend bool reopen_table(THD *,struct st_table *,bool); > friend int cre_myisam(char * name, register TABLE *form, uint options, > ulonglong auto_increment_value); > @@ -760,6 +766,12 @@ > my_decimal *val_decimal(my_decimal *); > virtual bool str_needs_quotes() { return TRUE; } > uint is_equal(Create_field *new_field); > + > + bool hash_join_is_possible(bool globally_allowed) > + { > + /* TODO: support hash joins for non-binary collations */ > + return globally_allowed && (flags & BINARY_FLAG); > + } > }; > > > @@ -1904,6 +1916,7 @@ > uint size_of() const { return sizeof(*this); } > int reset(void) { return !maybe_null() || Field_blob::reset(); } > geometry_type get_geometry_type() { return geom_type; }; > + bool hash_join_is_possible(bool globally_allowed) { return FALSE; } > }; > #endif /*HAVE_SPATIAL*/ > > > === modified file 'sql/sql_select.cc' > --- a/sql/sql_select.cc 2010-10-30 13:07:45 +0000 > +++ b/sql/sql_select.cc 2010-10-30 19:56:09 +0000 > @@ -3547,7 +3547,7 @@ > /* > Additional optimization: if we're processing > "t.key BETWEEN c1 AND c1" then proceed as if we were processing > - "t.key = c1". > + "t.key = c1".sis > TODO: This is a very limited fix. A more generic fix is possible. Is the above a meaningful change? > There are 2 options: > A) Make equality propagation code be able to handle BETWEEN > @@ -5946,6 +5946,44 @@ > } > > > +/** > + @brief > + Check whether hash join algorithm can be used to join this table > + > + @param is_allowed usage of hash join is allowed > + > + @details > + This function finds out whether the ref items that have been chosen > + by the planner to access this table can be used for hash join algorithms. > + The answer depends on a certain property of the the fields of the > + joined tables on which the hash join key is built. If hash join is > + allowed for all these fields the answer is positive unless hash join > + algorithms are not allowed for the query at all. > + > + @note > + The function is supposed to be called now only after the function > + get_best_combination has been called. > + > + @retval TRUE it's possible to use hash join to join this table > + @retval FALSE otherwise > +*/ > + > +bool JOIN_TAB::hash_join_is_possible(bool is_allowed) > +{ > + if (!is_allowed) > + return FALSE; Same as with globally_allowed. I think the parameter is redundant and rather confusing. > + if (type != JT_REF && type != JT_EQ_REF) > + return FALSE; > + KEY *keyinfo= &table->key_info[ref.key]; > + for (uint i= 0; i < ref.key_parts; i++) > + { > + if (!(keyinfo->key_part[i].field->hash_join_is_possible(TRUE))) > + return FALSE; > + } > + return TRUE; > +} > + > + > static uint > cache_record_length(JOIN *join,uint idx) > { > @@ -7649,8 +7687,10 @@ > &bufsz, &flags, &cost); > > if ((cache_level <=4 && !no_hashed_cache) || no_bka_cache || > - (flags & HA_MRR_NO_ASSOCIATION) && cache_level <=6) > + ((flags & HA_MRR_NO_ASSOCIATION) && cache_level <=6)) > { > + if (!tab->hash_join_is_possible(TRUE)) > + goto no_join_cache; > if (cache_level == 3) > prev_cache= 0; > if ((tab->cache= new JOIN_CACHE_BNLH(join, tab, prev_cache)) && > > === modified file 'sql/sql_select.h' > --- a/sql/sql_select.h 2010-10-30 13:07:45 +0000 > +++ b/sql/sql_select.h 2010-10-30 19:56:09 +0000 > @@ -391,6 +391,7 @@ > return max_used_fieldlength; > } > double get_partial_join_cardinality() { return partial_join_cardinality; } > + bool hash_join_is_possible(bool is_allowed); > } JOIN_TAB; > > > B8R Sergey -- Sergey Petrunia, Software Developer Monty Program AB, http://askmonty.org Blog: http://s.petrunia.net/blog

1 0

[Maria-developers] support-files/my-xxx.cnf templates cause warning when used
by Oli Sennhauser 30 Oct '10

30 Oct '10

Hello all, The support-files/my-xxx.cnf templates have an old value in it and thus give a nasty Warning message when used: 101029 21:06:04 [Warning] '--skip-locking' is deprecated and will be removed in a future release. Please use '--skip-external-locking' instead. So I have changed it. The patch should work for MariaDB 5.1 AND 5.2... Can somebody please put it into the trees when acceptable? Thanks, Oli --- support-files/my-huge.cnf.sh.orig 2010-10-29 21:09:24.000000000 +0200 +++ support-files/my-huge.cnf.sh 2010-10-29 21:10:47.000000000 +0200 @@ -25,7 +25,7 @@ socket = @MYSQL_UNIX_ADDR@ [mysqld] port = @MYSQL_TCP_PORT@ socket = @MYSQL_UNIX_ADDR@ -skip-locking +skip-external-locking key_buffer_size = 384M max_allowed_packet = 1M table_open_cache = 512 --- support-files/my-large.cnf.sh.orig 2010-10-29 21:09:22.000000000 +0200 +++ support-files/my-large.cnf.sh 2010-10-29 21:10:33.000000000 +0200 @@ -25,7 +25,7 @@ socket = @MYSQL_UNIX_ADDR@ [mysqld] port = @MYSQL_TCP_PORT@ socket = @MYSQL_UNIX_ADDR@ -skip-locking +skip-external-locking key_buffer_size = 256M max_allowed_packet = 1M table_open_cache = 256 --- support-files/my-medium.cnf.sh.orig 2010-10-29 21:09:20.000000000 +0200 +++ support-files/my-medium.cnf.sh 2010-10-29 21:10:05.000000000 +0200 @@ -26,7 +26,7 @@ socket = @MYSQL_UNIX_ADDR@ [mysqld] port = @MYSQL_TCP_PORT@ socket = @MYSQL_UNIX_ADDR@ -skip-locking +skip-external-locking key_buffer_size = 16M max_allowed_packet = 1M table_open_cache = 64 --- support-files/my-small.cnf.sh.orig 2010-10-29 21:09:17.000000000 +0200 +++ support-files/my-small.cnf.sh 2010-10-29 21:09:45.000000000 +0200 @@ -26,7 +26,7 @@ socket = @MYSQL_UNIX_ADDR@ [mysqld] port = @MYSQL_TCP_PORT@ socket = @MYSQL_UNIX_ADDR@ -skip-locking +skip-external-locking key_buffer_size = 16K max_allowed_packet = 1M table_open_cache = 4 -- FromDual - Vendor independent and neutral MySQL consulting. Oli Sennhauser Rebenweg 6 CH - 8610 Uster / Switzerland Phone +41 44 940 24 82 Mobile +41 79 830 09 33 e-Mail oli.sennhauser(a)fromdual.com Website http://www.fromdual.com/ Skype fromdual Jabber fromdual(a)swissjabber.ch Yahoo IM fromdual Xing https://www.xing.com/profile/Oliver_Sennhauser LinkedIn http://ch.linkedin.com/in/shinguz Twitter http://twitter.com/shinguz

2 1

[Maria-developers] On possible bug in get_partial_join_cost()
by Sergey Petrunya 29 Oct '10

29 Oct '10

Hi Timour, Answers to irc questions: <timour> spetrunia, I found a problem in get_partial_join_cost(), could you please look at this: <timour> for (uint i= join->const_tables; i < n_tables + join->const_tables ; i++) <timour> spetrunia, why "i < n_tables + join->const_tables" ? isn't the number of tables always the same? <timour> That is, shouldn't we just skip the constant tables assuming they are in the beginning of join->best_positions ? Yes we should skip the constant tables, and they are at the beginning of best_positions. The number of tables passed as n_tables varies across different invocations of get_partial_join_cost(): uint n_tables= my_count_bits(sj_nest->sj_inner_tables & ~join->const_table_map); SJ_MATERIALIZATION_INFO* sjm; if (!(sjm= new SJ_MATERIALIZATION_INFO) || !(sjm->positions= (POSITION*)join->thd->alloc(sizeof(POSITION)* n_tables))) DBUG_RETURN(TRUE); /* purecov: inspected */ sjm->tables= n_tables; sjm->is_used= FALSE; double subjoin_out_rows, subjoin_read_time; get_partial_join_cost(join, n_tables, &subjoin_read_time, &subjoin_out_rows); Here one can see that n_tables is "number of non-constant tables within an SJ-Materialization nest". Different nests will contain different numbers of tables. <timour> spetrunia, I changed the loop upper boundary to n_tables, then I get few improved EXPLAINS, and got a crash, then I had to leave. I think the change is wrong: if you stop at n_tables, that means that partial_join_cost() has done optimization for the first (n_tables - join->const_tables) non-constant tables, which is not right. If you need further help, could you give a concrete example that causes the problem? BR Sergey -- Sergey Petrunia, Software Developer Monty Program AB, http://askmonty.org Blog: http://s.petrunia.net/blog

1 0