List:Commits« Previous MessageNext Message »
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
View as plain text  
 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).
Thread
bzr push into mysql-trunk branch (ole.john.aske:3934 to 3935) WL#5558Ole John Aske22 Feb