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@askmonty.org-20120103040636-nc6o55vsxqadd1n0 parent: psergey@askmonty.org-20111230211905-he458ysn3sse6wlm committer: Igor Babaev<igor@askmonty.org> 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@mariadb.org https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits