Hello Igor, On Sat, Jun 01, 2019 at 01:16:04PM -0700, Igor Babaev wrote: > Please review this patch for the bug that we discussed on slack a couple of > days ago. > As you can see the extra calls of make_cond_for_table() with > used_table=RAND_TABLE_BIT were not enough (it can be seen in debugger why). > It took me quite a lot of time to figure out that I could not trust > Item::marker==2 to check whether a conjunct was previously attached > or not. >
First, I've found a testcase that stopped working after the patch. (As far as I understand, we should pass RAND_TABLE_BIT when constructing JOIN::outer_ref_cond): create table ten(a int); insert into ten values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9); create table t1 (a int, b int, c int); insert into t1 select a,a,a from ten; create table t2 as select a,b,c from t1; mysql> explain format=json select (select max(c) from t1 where t1.a=t2.a and rand()=t2.b) from t2\G *************************** 1. row *************************** EXPLAIN: { "query_block": { "select_id": 1, "table": { "table_name": "t2", "access_type": "ALL", "rows": 10, "filtered": 100 }, "subqueries": [ { "query_block": { "select_id": 2, "table": { "table_name": "t1", "access_type": "ALL", "rows": 10, "filtered": 100, "attached_condition": "((t1.a = t2.a) and (rand() = t2.b))" } } } ] } } 1 row in set (0.00 sec) If I run the select, it hits a breakpoint in Item_func_rand::val_real. Now, after this patch (I applied it to 10.1): mysql> explain format=json select (select max(c) from t1 where t1.a=t2.a and rand()=t2.b) from t2\G *************************** 1. row *************************** EXPLAIN: { "query_block": { "select_id": 1, "table": { "table_name": "t2", "access_type": "ALL", "rows": 10, "filtered": 100 }, "subqueries": [ { "query_block": { "select_id": 2, "table": { "table_name": "t1", "access_type": "ALL", "rows": 10, "filtered": 100, "attached_condition": "(t1.a = t2.a)" } } } ] } } and running the SELECT doesn't hit the breakpoint in Item_func_rand::val_real A bit more of cosmetic feedback below, marked with 'psergey:' > > @@ -8968,6 +8963,20 @@ make_join_select(JOIN *join,SQL_SELECT *select,COND > *cond) > { > tmp= make_cond_for_table(thd, cond, used_tables, current_map, i, > FALSE, FALSE); > + if (tab == join->join_tab + last_top_base_tab_idx) > + { > + /* > + This pushes conjunctive conditions of WHERE condition such > that: > + - their used_tables() contain RAND_TABLE_BIT > + - the conditions does not refer to any fields > + (such like rand() > 0.5) > + */ > + table_map rand_table_bit= (table_map) RAND_TABLE_BIT; psergey: what is the purpose of rand_table_bit? If we can't use RAND_TABLE_BIT where a table bitmap is expected, we should fix that. The way it's done now it looks redundant and is confusing. (this also applies to other declarations if rand_table_bit) > + COND *rand_cond= make_cond_for_table(thd, cond, used_tables, > + rand_table_bit, -1, > + FALSE, FALSE); > + add_cond_and_fix(thd, &tmp, rand_cond); > + } > } > /* Add conditions added by add_not_null_conds(). */ > if (tab->select_cond) ... > @@ -18852,11 +18880,27 @@ make_cond_for_table_from_pred(THD *thd, Item > *root_cond, Item *cond, > Item *item; > while ((item=li++)) > { > + /* > + Special handling of top level conjuncts with RAND_TABLE_BIT: > + if such a conjunct contains a reference to a field then it is > pushed > + to the corresponding table by the same rule as all other > conjuncts. > + Otherwise, if the conjunct is used in WHERE is is pushed to the > last > + joined table, if is it is used in ON condition of an outer join > it > + is pushed into the last inner table of the outer join. Such > conjuncts > + are pushed in a call of make_cond_for_table_from_pred() with the > + parameter 'used_table' equal to RAND_TABLE_BIT. > + */ > + if (is_top_and_level && used_table == rand_table_bit && > + item->used_tables() != rand_table_bit) > + { > + /* The conjunct with RAND_TABLE_BIT has been allready pushed */ psergey: typo: allready > + continue; > + } > Item *fix=make_cond_for_table_from_pred(thd, root_cond, item, > tables, used_table, > - join_tab_idx_arg, > + join_tab_idx_arg, > exclude_expensive_cond, > - retain_ref_cond); > + retain_ref_cond, false); > if (fix) > new_cond->argument_list()->push_back(fix); BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp