* 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.
--
