List:Commits« Previous MessageNext Message »
From:Konstantin Osipov Date:September 24 2010 5:48am
Subject:Re: bzr commit into mysql-5.1-bugteam branch (Sergey.Glukhov:3467)
Bug#54494
View as plain text  
* Davi Arnaut <Davi@stripped> [10/09/24 01:00]:
> But this is not such case. The explain code discards the references
> in sel->where/having with its own. That is, it kind of swaps the
> where clause condition. If whatever was originally in
> sel->where/having gets allocated in permanent memory, we've got
> ourselves a nice memory leak.

The substituted item is not in permanent memory:

  if (!conds && outer_join)
  {
    /* Handle the case where we have an OUTER JOIN without a WHERE */
    conds=new Item_int((longlong) 1,1); // Always true
  }


Any allocation from permanent memory must also affect
sel->prep_where/sel->prep_having, otherwise there would be a memory
leak with or without EXPLAIN.

> >Gluh's patch is fixing this restoration mechanism,
> >which only worked when the original WHERE clause
> >was not NULL. I.e. it is a bug in the
> >reinit_stmt_before_use() that it doesn't set
> >where/having to their original value
> >if this value was NULL.
> 
> I disagree. In my opinion, it is a superficial change that is a
> futile attempt to disguise the true problem, which is that we do not
> properly restore the original context.

If you believe that the original problem is disguised, how do you
think it can resurface?

> In reinit_stmt_before_use() we know have a logic that, without a
> single comment, that erases any reference in SELECT_LEX::where if
> there is no reference in SELECT_LEX::prep_where.

Right. I would put it this way: In reinit_stmt_before_use() we now
have a logic that, without a single comment that is specific to
the case when prep_where is NULL, restores the WHERE clause to the
state after parsing.

Do we need a special comment for the case when the original
WHERE was NULL? I do not disagree.

> >The bug wasn't hit before because there was
> >an assumption that these parse subtrees are never
> >transformed from empty to non-empty values.
> >EXPLAIN EXTENDED is a case which breaks
> >this assumption.
> 
> There is no such transformation.

It's perhaps not obvious, since the root of the tree is not 
changed, but the tree itself is changed in-place on numerous occasions:

    conds= simplify_joins(this, join_list, conds, TRUE);
  conds= optimize_cond(this, conds, join_list, &cond_value);


     if (conds && !(thd->lex->describe & DESCRIBE_EXTENDED))
      {
        COND *table_independent_conds=
          make_cond_for_table(conds, PSEUDO_TABLE_BITS, 0);
        DBUG_EXECUTE("where",
                     print_where(table_independent_conds,
                                 "where after opt_sum_query()",
                                 QT_ORDINARY););
        conds= table_independent_conds;
      }

> I don't know how you guys can came
> up with this explanation. This is a coding bug, it fails to properly
> track references.

There is no issue with an approach that tracks references.
One could use thd->change_item_tree() instead of a plain assignment
of SELECT_LEX->where.

I don't see, however, how it's particularly advantageous.

-- 
Thread
bzr commit into mysql-5.1-bugteam branch (Sergey.Glukhov:3467)Bug#54494Sergey Glukhov7 Jul
  • Re: bzr commit into mysql-5.1-bugteam branch (Sergey.Glukhov:3467)Bug#54494Davi Arnaut9 Jul
    • Re: bzr commit into mysql-5.1-bugteam branch (Sergey.Glukhov:3467)Bug#54494Sergey Glukhov19 Aug
      • Re: bzr commit into mysql-5.1-bugteam branch (Sergey.Glukhov:3467)Bug#54494Davi Arnaut6 Sep
        • Re: bzr commit into mysql-5.1-bugteam branch (Sergey.Glukhov:3467)Bug#54494Sergey Glukhov17 Sep
  • Re: bzr commit into mysql-5.1-bugteam branch (Sergey.Glukhov:3467)Bug#54494Konstantin Osipov22 Sep
    • Re: bzr commit into mysql-5.1-bugteam branch (Sergey.Glukhov:3467)Bug#54494Davi Arnaut23 Sep
      • Re: bzr commit into mysql-5.1-bugteam branch (Sergey.Glukhov:3467)Bug#54494Konstantin Osipov24 Sep