List:Commits« Previous MessageNext Message »
From:Guilhem Bichot Date:August 2 2010 6:21pm
Subject:Re: bzr commit into mysql-next-mr-opt-backporting branch (guilhem.bichot:3219)
Bug#54481
View as plain text  
Hello Olav,

Olav Sandstaa a écrit, Le 02.08.2010 14:28:
> Hi Guilhem,
> 
> Thanks for the GREAT comments to the patch. The patch looks fine and 
> correct.
> 
> Minor comments (that you can feel free to ignore):
> 
> 1. Add a "#" to the third line in the test:
> 
> --echo #
> --echo # BUG#54481 "GROUP BY loses effect with JOIN + ORDER BY + LIMIT
> --echo and join_cache_level=5-8"
> --echo #

ok

> 2. Add a short per-file-commit message for your change to sql_select.h?

ok (may be short and then point to the comment of sql_select.cc).

> 3. I understand the motivation for your "second code change" where you 
> remove the code from reverting from JT_NEXT to JT_ALL. Ideally, I would 
> have preferred this to be a separate change/patch.

I agree; but If I don't do this change, other plans are going to change 
(see this comment:
         After this change, all tests pass except that for this query in
         group_min_max.test:
          EXPLAIN SELECT COUNT(DISTINCT t1_1.a) FROM t1 t1_1, t1 t1_2 
GROUP BY t1_1.a;
         table scan for t1_1 is now picked instead of index scan.
         This is because the "if(sort_by_tab)" block is now entered, and so
         JT_NEXT (index scan) is changed to JT_ALL (table scan)).

Let's see what the second reviewer thinks and then decide.

> Question: This bug is reported as a bug in mysql-next-mr-opt-bugfixing 
> tree and tagged as a "new optimizer feature bug". With your second test 
> query where you use force/ignore indexes you also show that this is an 
> issue with non-BKA implementation. I have run the query against 
> mysql-next-mr-bugfixing and see the same "wrong result" issue there. 
> Which tree do you plan to push this fix into?

Good catch. I just verified that it exists in 5.1/trunk/next-mr. So I 
may have to look for a fix for 5.1 (code is different). I just asked for 
a re-triage, to see whether this should be fixed in 5.1 or not.
Thread
bzr commit into mysql-next-mr-opt-backporting branch (guilhem.bichot:3219)Bug#54481Guilhem Bichot30 Jul
Re: bzr commit into mysql-next-mr-opt-backporting branch (guilhem.bichot:3219)Bug#54481Olav Sandstaa2 Aug
  • Re: bzr commit into mysql-next-mr-opt-backporting branch (guilhem.bichot:3219)Bug#54481Guilhem Bichot2 Aug
  • Re: bzr commit into mysql-next-mr-opt-backporting branch (guilhem.bichot:3219)Bug#54481Guilhem Bichot6 Aug