From: Ole John Aske Date: February 21 2012 11:38am Subject: bzr push into mysql-trunk branch (ole.john.aske:3934 to 3935) WL#5558 List-Archive: http://lists.mysql.com/commits/143005 Message-Id: <20120221113839.8C9BF23B@fimafeng09.norway.sun.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 3935 Ole John Aske 2012-02-21 Followup fix for WL#5558 in order to remove compiler warning on some platforms. modified: sql/sql_optimizer.h 3934 Ole John Aske 2012-02-21 Fix for WL#5558: Resolve ORDER BY execution method at the optimization stage Recommit, including review comments, and some new tests after trunk-backporting was merged into trunk. The decision whether to sort result dataset or use ordered index scan to get sorted result was partly made in JOIN::optimize, and partly in JOIN::execute(). Furthermore the logic for EXPLAIN'ing the sort was different/incorrect compared to the logic used for determine filesort or not during execution. This sometimes caused EXPLAIN to be incorrect. This refactoring WL moves the decision on how to handle ORDER BY to the optimization stage. It is a pure refactoring in the sense that it does not change the current filesort cost model. However, several test results has changed as the EXPLAIN of those used to be incorrect. During the refactoring task several other bugs has also been reported and fixed as standalone bugs, these are: - bug 13514959: query plan fails to 'using index' where it should - bug 13528826: test_if_cheaper_ordering(): calculates incorrect 'select_limit' - bug 13529048: test_if_skip_sort_order() don't have to filesort a single row. - bug 13531865: test_if_skip_sort_order() incorrectly skip filesort if 'type' is ref_or_null As of today these fixes has already been reviewed and pushed into mysql-trrunk. As an extension of the refactoring in this commit, one should considder to move test_if_skip_sort_order() w/ family from sql_select.cc into sql_optimize.cc as this is new the only place it is used. (make it 'static' also) There is likely also some code in JOIN::execute() related to maintaining 'pre_idx_push_cond' which now is obsolete and should be considdered for removal. (later) @ mysql-test/r/group_by.result Fixed 2 bugs: 1. An incorrect explain where 'keys_in_use_for_query' was incorrectly used as argument to test_if_skip.. as part of explain. The 'IGNORE INDEX FOR...' was therefore ignored for EXPLAIN (Did check with gdb that filesort was (and is) used during execute) 2. SQL_BIG_RESULT incorrectly explained as we should avoid filesort with this option. @ mysql-test/r/join_cache_bka.result 'LIMIT 2' was incorrectly used to limit #rows being input to 'GROUP BY' leading to incorrect access type being choosen - or at least explained. @ mysql-test/r/join_cache_bka_nixbnl.result 'LIMIT 2' was incorrectly used to limit #rows being input to 'GROUP BY' leading to incorrect access type being choosen - or at least explained. @ mysql-test/r/join_cache_bkaunique.result 'LIMIT 2' was incorrectly used to limit #rows being input to 'GROUP BY' leading to incorrect access type being choosen - or at least explained. @ mysql-test/r/join_cache_bnl.result 'LIMIT 2' was incorrectly used to limit #rows being input to 'GROUP BY' leading to incorrect access type being choosen - or at least explained. @ mysql-test/r/join_cache_nojb.result 'LIMIT 2' was incorrectly used to limit #rows being input to 'GROUP BY' leading to incorrect access type being choosen - or at least explained. @ mysql-test/r/select_found.result When SQL_CALC_FOUND_ROWS is specified *all* rows should be read from table, so 'LIMIT 10' should be ignored. As there actually are 200 rows in the table, this should now be used to calculate access 'type' which cause that to change. This also leads to another ordering of the rows such that the final 'LIMIT 10' clause will cause another part of the resultset to be returned within the limit-window. @ mysql-test/r/subquery_mat_none.result These result changes are all other materializations of bug 13528826: test_if_cheaper_ordering(): calculates incorrect 'select_limit', which now surface on other places as there now is a single call to test_if_skip_... where there used to be two calls to this function. @ mysql-test/suite/opt_trace/r/bugs_no_prot_all.result Trace output changed as test_if_skip_sort_order is now called as part of ::optimize() instead of ::execute() @ mysql-test/suite/opt_trace/r/bugs_no_prot_none.result Trace output changed as test_if_skip_sort_order is now called as part of ::optimize() instead of ::execute() @ mysql-test/suite/opt_trace/r/bugs_ps_prot_all.result Trace output changed as test_if_skip_sort_order is now called as part of ::optimize() instead of ::execute() @ mysql-test/suite/opt_trace/r/bugs_ps_prot_none.result Trace output changed as test_if_skip_sort_order is now called as part of ::optimize() instead of ::execute() @ mysql-test/suite/opt_trace/r/range_no_prot.result Trace output changed as test_if_skip_sort_order is now called as part of ::optimize() instead of ::execute() @ mysql-test/suite/opt_trace/r/range_ps_prot.result Trace output changed as test_if_skip_sort_order is now called as part of ::optimize() instead of ::execute() @ sql/sql_executor.cc Moved all usage of test_if_skip_sort_order() out of the execute path and replace it with 'ordered_index_usage'which contans the skip / no skip decision as made by optimizer. @ sql/sql_optimizer.cc Move all usage of test_if_skip_sort_order() into JOIN::optimize() and set 'ordered_index_usage' to reflect the desicion made by test_if_skip.... Also fix a bug in how the LIMIT-clause was incorrectly used to limit #rows when there was a GROUP BY followed by an ORDER BY. @ sql/sql_optimizer.h Introduce 'enum ordered_index_usage' which remember optimizers decision to possibly use an ordered index to skip filesort for either ORDER BY or GROUP BY @ sql/sql_select.cc Fix up EXPLAIN to use the 'ordered_index_usage' as calculated by ::optimize() instead of calling test_if_skip_sort_order() modified: mysql-test/r/group_by.result mysql-test/r/join_cache_bka.result mysql-test/r/join_cache_bka_nixbnl.result mysql-test/r/join_cache_bkaunique.result mysql-test/r/join_cache_bnl.result mysql-test/r/join_cache_nojb.result mysql-test/r/select_found.result mysql-test/r/subquery_mat_none.result mysql-test/suite/opt_trace/r/bugs_no_prot_all.result mysql-test/suite/opt_trace/r/bugs_no_prot_none.result mysql-test/suite/opt_trace/r/bugs_ps_prot_all.result mysql-test/suite/opt_trace/r/bugs_ps_prot_none.result mysql-test/suite/opt_trace/r/range_no_prot.result mysql-test/suite/opt_trace/r/range_ps_prot.result sql/sql_executor.cc sql/sql_optimizer.cc sql/sql_optimizer.h sql/sql_select.cc === modified file 'sql/sql_optimizer.h' --- a/sql/sql_optimizer.h 2012-02-21 10:31:44 +0000 +++ b/sql/sql_optimizer.h 2012-02-21 11:38:19 +0000 @@ -197,7 +197,7 @@ public: { ordered_index_void, // No ordered index avail. ordered_index_group_by, // Use index for GROUP BY - ordered_index_order_by, // Use index for ORDER BY + ordered_index_order_by // Use index for ORDER BY } ordered_index_usage; /** No bundle (reason: useless for push emails).