maria-developers team mailing list archive

[Sergey Petrunia <sergey@xxxxxxxxxxx>]

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