Hi, Igor! On Jun 09, Igor Babaev wrote:

revno: 4234 revision-id: igor@askmonty.org-20140610050716-0x8xaquf3itvofjc parent: bar@mariadb.org-20140606062952-x2jwt4mgrgr6tpw6 committer: Igor Babaev <igor@askmonty.org> branch nick: maria-10.0-bugs timestamp: Mon 2014-06-09 22:07:16 -0700 message: Fixed bug mdev-6071. The method JOIN_CACHE::init may fail (return 1) if some conditions on the used join buffer is not satisfied. For example it fails if join_buffer_size is greater than join_buffer_space_limit. The conditions should be checked when running the EXPLAIN command for the query. That's why the method JOIN_CACHE::init has to be called for EXPLAIN commands as well.

It's an improvement over the old behavior - as EXPLAIN shows correctly whether BKA is used. But I think there's still not enough diagnostics. In the bug report Sergey wrote that " I think it's a big gotcha that @join_buffer_size must be lower than @@join_buffer_space_limit when optimizer_space_limit optimization is off. " So, I'd believe users will be very confused that in some cases BKA is suddenly turned off, when they expect it to be used. Of course, without this bugfix they would've not even noticed that, which is even worse. So, this bugfix is good. But it would be nice to have some kind of an explanation of why BKA is not used. At the very least in the manual. Better - here as a warning, like if (for_explain) push_warning("BKA was not used because ..."; goto fail;

=== modified file 'sql/sql_select.cc' --- a/sql/sql_select.cc 2014-06-05 22:07:27 +0000 +++ b/sql/sql_select.cc 2014-06-10 05:07:16 +0000 @@ -10541,7 +10541,7 @@ if (cache_level == 1) prev_cache= 0; if ((tab->cache= new JOIN_CACHE_BNL(join, tab, prev_cache)) && - ((options & SELECT_DESCRIBE) || !tab->cache->init())) + !tab->cache->init(MY_TEST(options & SELECT_DESCRIBE)))

MY_TEST here (and in other similar places) is redundant, you could've simply written !tab->cache->init(options & SELECT_DESCRIBE))

{ tab->icp_other_tables_ok= FALSE; return (2 - MY_TEST(!prev_cache));

and while it wasn't part of your bug fix, I cannot help but noticing that MY_TEST() is redundant here too. !prev_cache is boolean, it's always 0 or 1. Regards, Sergei