Re: bzr commit into mysql-next-mr-bugfixing branch (jorgen.loland:3242) WL#4800

[Guilhem Bichot]

Hello,

Jorgen Loland a écrit, Le 14.01.2011 16:44:
> On 01/14/2011 03:51 PM, Guilhem Bichot wrote:
>> Hello,
>>
>> I deleted all "ok, no comment" points.
>>
>> Jorgen Loland a écrit, Le 14.01.2011 12:49:
>>> > But wait, this is interesting. I have the following issue (in the
>>> > current wl4800 tree): make_join_select() looks like this:
>>> >
>>> > open array in trace;
>>> > for (take tables from first to last)
>>> > {
>>> > create object in trace for this table;
>>> > write table's name to trace;
>>> > determine attached condition;
>>> > write attached condition to trace;
>>> > }
>>> >
>>> > But "determine attached condition" contains pushdown_on_conditions()
>>> > which may attach condition to tables located *before* the one which 
>>> the
>>> > iteration is positioned on. And as I have already printed the 
>>> condition
>>> > for this earlier table, the earlier printed condition is now
>>> incomplete.
>>> > The example is "t1 LEFT JOIN (t2 LEFT JOIN t3 ON ...) ON ...": when
>>> > positioned on t3, bits of the ON conditions get attached to t1 and t2.
>>> > As a solution, I considered switching to this "summarizing" 
>>> pseudocode:
>>> >
>>> > for (take tables from first to last)
>>> > {
>>> > determine attached condition;
>>> > }
>>> > open array in trace;
>>> > for (take tables from first to last)
>>> > {
>>> > create object in trace for this table;
>>> > write table's name to trace;
>>> > write attached condition to trace;
>>> > }
>>> > But "determine attached condition" calls the range opt (right below in
>>> > your patch), so this blows up (range opt tracing without the proper
>>> > context => crash).
>>> > So maybe we need:
>>> >
>>> > open array in trace, for key "attached_conditions_computation"
>>> > for (take tables from first to last)
>>> > {
>>> > determine attached condition; as part of it, if using the range opt
>>> > then create proper enclosing context with table's name as you do in 
>>> the
>>> > patch
>>> > }
>>> > close array
>>> > open array in trace, for key "attached_conditions_summary"
>>> > for (take tables from first to last)
>>> > {
>>> > create object in trace for this table;
>>> > write table's name to trace;
>>> > write attached condition to trace;
>>> > }
>>> >
>>> > If you find this is ok, I can handle implementation.
>>>
>>> If I understand correctly, you want to execute range optimization
>>> all over again for all tables?
>>
>> No. Current code does:
>> open array in trace;
>> for (take tables from first to last)
>> {
>> create object in trace for this table;
>> write table's name to trace;
>> determine attached condition;
>> write attached condition to trace;
>> }
>> and I would change to
>> open array in trace, for key "attached_conditions_computation"
>> for (take tables from first to last)
>> {
>> determine attached condition; as part of it, if using the range opt
>> then create proper enclosing context with table's name as you do in the
>> patch
>> }
>> close array
>> open array in trace, for key "attached_conditions_summary"
>> for (take tables from first to last)
>> {
>> create object in trace for this table;
>> write table's name to trace;
>> write attached condition to trace;
>> }
>> so the second for() loop only does tracing, nothing else.
>> The first loop does no tracing except if range opt is used, in which
>> case tracing is done by your:
>> Opt_trace_object trace_table(join->thd->opt_trace,
>> "rechecking_index_usage");
>> trace_table.add_utf8_table(tab->table);
> 
> Ok, sounds like a good idea

So I pushed it.