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;

+ 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: 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) 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