← Back to team overview

maria-developers team mailing list archive

Re: [Commits] Rev 3376: Fixed LP bug #910083. in file:///home/igor/maria/maria-5.3-bug910083/

 

On 01/03/2012 06:44 AM, Timour Katchaounov wrote:
> Igor,
> 
>> On 01/03/2012 12:49 AM, Timour Katchaounov wrote:
>>> Hi Igor,
>>>
>>> The reason for removing this specific call to
>>> engine->set_thd() was because Monty claimed this is
>>> the correct way, and because with the new assert we
>>> couldn't find any case where the new assert would fail.
>>>
>>> Apparently the above assert was incorrect, however, the
>>> current commit message doesn't have any explanation what
>>> was wrong, why the assert you removed is wrong, and why
>>> it is needed to call engine->set_thd().
>>
>> Igor,
>>
>> I would rather say that you patch did not have any explanation
>> why you had removed the call. Should it be a reference to Monty's
>> opinion?I have to admit it would be pretty pretty lame.
> 
> There is a comment in the patch for bug lp:685411, this is the
> relevant diff:
> 
> @@ -183,7 +185,8 @@ bool Item_subselect::fix_fields(THD *thd
>    bool res;
> 
>    DBUG_ASSERT(fixed == 0);
> -  engine->set_thd((thd= thd_param));
> +  /* There is no reason to get a different THD. */
> +  DBUG_ASSERT(thd == thd_param);
>    if (!done_first_fix_fields)
>    {
>      done_first_fix_fields= TRUE;
> 
> The reasoning is simple - there is no reason for a subquery to
> get a different THD object compared to that of the containing
> query.
> 
> So my question to your patch is whether you figured out why the
> above assumption is not true. If you did, it would be nice to have
> an explanation. I am pretty sure the assumption is true for SELECTs.
> If that is the case, then I would refine your fix by testing whether
> a statement is a SELECT, or UPDATE (or INSERT?) and updating the THD
> only when needed.

It's relevant for the scenario when the fixed item is created by a
different client: for example, when the item belongs to SP and this SP
was created by a different client.
I returned the code that you removed. I don't think I have to explain
why you were wrong.

> 
> Timour
> 
>>
>> Regards,
>> Igor.
>>
>>>
>>> Timour
>>>
>>> On  3.01.2012 06:06, Igor Babaev wrote:
>>>> At file:///home/igor/maria/maria-5.3-bug910083/
>>>>
>>>> ------------------------------------------------------------
>>>> revno: 3376
>>>> revision-id: igor@xxxxxxxxxxxx-20120103040636-nc6o55vsxqadd1n0
>>>> parent: psergey@xxxxxxxxxxxx-20111230211905-he458ysn3sse6wlm
>>>> committer: Igor Babaev<igor@xxxxxxxxxxxx>
>>>> branch nick: maria-5.3-bug910083
>>>> timestamp: Mon 2012-01-02 20:06:36 -0800
>>>> message:
>>>>     Fixed LP bug #910083.
>>>>     The patch for bug 685411 erroneously removed a call of
>>>> engine->set_thd()
>>>>     from Item_subselect::fix_fields().
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> commits mailing list
>>>> commits@xxxxxxxxxxx
>>>> https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits
>>



References