Comment 4 for bug 598972

Revision history for this message
Sergey Petrunia (sergefp) wrote : Re: Valgrind failure in having.test in Item_in_subselect::init_left_expr_cache()

I think that the above-quoted part of code uses an approach that's in general, incorrect.

As far as I understand it, the code tries to solve the following issue:

if a subquery in HAVING/select_list/{other part of query that is evaluated after grouping is done} has references to a table column TC, it t needs to look not at TC but at its image in the grouping table (aka "result field").

This wasn't coded correctly.

First,
- The above "if (end_select == ..." ignores the location of the subquery predicate. What if the subquery predicate is in the WHERE, and so is evaluated before the grouping? In that case we won't need to access the "result field", while with this code we'll still do.

Second, I don't see a reason why would Item_in_subselect::init_left_expr_cache() and co. bother at all with the question of whether they should use the "result field"? That question is abstracted away by wrapping needed items inside Item_ref(...) which does val_int() -> val_result() conversions when necessary.
I suspect that if we just removed real_item() calls from new_Cached_item(), then it would use Item_ref objects that would automatically do the right thing and we won't have any need for the above quoted "if (end_select ==..." statement.

Anybody who has an idea about this code please give some feedback.