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
> 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 #
> 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
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.