From: Ole John Aske Date: February 2 2011 2:50pm Subject: bzr commit into mysql-5.5 branch (ole.john.aske:3295) Bug#59308 List-Archive: http://lists.mysql.com/commits/130246 X-Bug: 59308 Message-Id: <20110202145018.ED4F6223@fimafeng09.norway.sun.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6341756641798441949==" --===============6341756641798441949== MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline #At file:///net/fimafeng09/export/home/tmp/oleja/mysql/mysql-5.5/ based on revid:ole.john.aske@stripped 3295 Ole John Aske 2011-02-02 Updated Fix for bug#59308: Incorrect result for SELECT DISTINCT ... ORDER BY DESC. Rebased to mysql-5.5 which has changed considerably in the areas affected by the original bugfix. (based on mysql-5.1) See http://lists.mysql.com/commits/130245 for original commit comments. modified: mysql-test/r/order_by.result mysql-test/t/order_by.test sql/sql_select.cc === modified file 'mysql-test/r/order_by.result' --- a/mysql-test/r/order_by.result 2010-09-13 12:46:55 +0000 +++ b/mysql-test/r/order_by.result 2011-02-02 14:50:12 +0000 @@ -1638,6 +1638,31 @@ id select_type table type possible_keys 1 SIMPLE t1 index NULL a 8 NULL 10 Using index; Using temporary; Using filesort 1 SIMPLE t2 eq_ref PRIMARY PRIMARY 4 test.t1.b 1 Using where DROP TABLE t1, t2; +# +# Bug #59110: Memory leak of QUICK_SELECT_I allocated memory +# and +# Bug #59308: Incorrect result for +SELECT DISTINCT ... ORDER BY DESC + +# Use Valgrind to detect #59110! +# +CREATE TABLE t1 (a INT,KEY (a)); +INSERT INTO t1 VALUES (1),(2),(3),(4),(5),(6),(7),(8),(9),(10); +EXPLAIN SELECT DISTINCT a,1 FROM t1 WHERE a <> 1 ORDER BY a DESC; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 index a a 5 NULL 10 Using where; Using index; Using filesort +SELECT DISTINCT a,1 FROM t1 WHERE a <> 1 ORDER BY a DESC; +a 1 +10 1 +9 1 +8 1 +7 1 +6 1 +5 1 +4 1 +3 1 +2 1 +DROP TABLE t1; End of 5.1 tests # # Bug #38745: MySQL 5.1 optimizer uses filesort for ORDER BY === modified file 'mysql-test/t/order_by.test' --- a/mysql-test/t/order_by.test 2010-09-13 12:46:55 +0000 +++ b/mysql-test/t/order_by.test 2011-02-02 14:50:12 +0000 @@ -1492,6 +1492,23 @@ LIMIT 2; DROP TABLE t1, t2; +--echo # +--echo # Bug #59110: Memory leak of QUICK_SELECT_I allocated memory +--echo # and +--echo # Bug #59308: Incorrect result for +--echo SELECT DISTINCT ... ORDER BY DESC +--echo +--echo # Use Valgrind to detect #59110! +--echo # + +CREATE TABLE t1 (a INT,KEY (a)); +INSERT INTO t1 VALUES (1),(2),(3),(4),(5),(6),(7),(8),(9),(10); + +EXPLAIN SELECT DISTINCT a,1 FROM t1 WHERE a <> 1 ORDER BY a DESC; +SELECT DISTINCT a,1 FROM t1 WHERE a <> 1 ORDER BY a DESC; + +DROP TABLE t1; + --echo End of 5.1 tests === modified file 'sql/sql_select.cc' --- a/sql/sql_select.cc 2011-02-01 14:19:34 +0000 +++ b/sql/sql_select.cc 2011-02-02 14:50:12 +0000 @@ -13619,12 +13619,14 @@ test_if_skip_sort_order(JOIN_TAB *tab,OR { int ref_key; uint ref_key_parts; - int order_direction; + int order_direction= 0; uint used_key_parts; TABLE *table=tab->table; SQL_SELECT *select=tab->select; key_map usable_keys; QUICK_SELECT_I *save_quick= 0; + int best_key= -1; + DBUG_ENTER("test_if_skip_sort_order"); LINT_INIT(ref_key_parts); @@ -13728,13 +13730,14 @@ test_if_skip_sort_order(JOIN_TAB *tab,OR new_ref_key_map.clear_all(); // Force the creation of quick select new_ref_key_map.set_bit(new_ref_key); // only for new_ref_key. + select->quick= 0; if (select->test_quick_select(tab->join->thd, new_ref_key_map, 0, (tab->join->select_options & OPTION_FOUND_ROWS) ? HA_POS_ERROR : tab->join->unit->select_limit_cnt,0) <= 0) - DBUG_RETURN(0); + goto need_filesort; } ref_key= new_ref_key; } @@ -13749,7 +13752,6 @@ test_if_skip_sort_order(JOIN_TAB *tab,OR uint best_key_parts= 0; uint saved_best_key_parts= 0; int best_key_direction= 0; - int best_key= -1; JOIN *join= tab->join; ha_rows table_records= table->file->stats.records; @@ -13769,72 +13771,21 @@ test_if_skip_sort_order(JOIN_TAB *tab,OR tab->join->tables > tab->join->const_tables + 1) && ((unsigned) best_key != table->s->primary_key || !table->file->primary_key_is_clustered())) - DBUG_RETURN(0); + goto need_filesort; if (best_key >= 0) { - bool quick_created= FALSE; if (table->quick_keys.is_set(best_key) && best_key != ref_key) { key_map map; map.clear_all(); // Force the creation of quick select map.set_bit(best_key); // only best_key. - quick_created= - select->test_quick_select(join->thd, map, 0, - join->select_options & OPTION_FOUND_ROWS ? - HA_POS_ERROR : - join->unit->select_limit_cnt, - 0) > 0; - } - if (!no_changes) - { - /* - If ref_key used index tree reading only ('Using index' in EXPLAIN), - and best_key doesn't, then revert the decision. - */ - if (!table->covering_keys.is_set(best_key)) - table->set_keyread(FALSE); - if (!quick_created) - { - tab->index= best_key; - tab->read_first_record= best_key_direction > 0 ? - join_read_first:join_read_last; - tab->type=JT_NEXT; // Read with index_first(), index_next() - if (select && select->quick) - { - delete select->quick; - select->quick= 0; - } - if (table->covering_keys.is_set(best_key)) - table->set_keyread(TRUE); - table->file->ha_index_or_rnd_end(); - if (join->select_options & SELECT_DESCRIBE) - { - tab->ref.key= -1; - tab->ref.key_parts= 0; - if (select_limit < table_records) - tab->limit= select_limit; - } - } - else if (tab->type != JT_ALL) - { - /* - We're about to use a quick access to the table. - We need to change the access method so as the quick access - method is actually used. - */ - DBUG_ASSERT(tab->select->quick); - tab->type=JT_ALL; - tab->use_quick=1; - tab->ref.key= -1; - tab->ref.key_parts=0; // Don't use ref key. - tab->read_first_record= join_init_read_record; - if (tab->is_using_loose_index_scan()) - join->tmp_table_param.precomputed_group_by= TRUE; - /* - TODO: update the number of records in join->best_positions[tablenr] - */ - } + select->quick= 0; + select->test_quick_select(join->thd, map, 0, + join->select_options & OPTION_FOUND_ROWS ? + HA_POS_ERROR : + join->unit->select_limit_cnt, + 0); } order_direction= best_key_direction; /* @@ -13847,10 +13798,12 @@ test_if_skip_sort_order(JOIN_TAB *tab,OR saved_best_key_parts : best_key_parts; } else - DBUG_RETURN(0); + goto need_filesort; } check_reverse_order: + DBUG_ASSERT(order_direction != 0); + if (order_direction == -1) // If ORDER BY ... DESC { if (select && select->quick) @@ -13859,9 +13812,10 @@ check_reverse_order: Don't reverse the sort order, if it's already done. (In some cases test_if_order_by_key() can be called multiple times */ - if (!select->quick->reverse_sorted()) + if (select->quick->reverse_sorted()) + goto skiped_sort_order; + else { - QUICK_SELECT_I *tmp; int quick_type= select->quick->get_type(); if (quick_type == QUICK_SELECT_I::QS_TYPE_INDEX_MERGE || quick_type == QUICK_SELECT_I::QS_TYPE_ROR_INTERSECT || @@ -13869,37 +13823,128 @@ check_reverse_order: quick_type == QUICK_SELECT_I::QS_TYPE_GROUP_MIN_MAX) { tab->limit= 0; - select->quick= save_quick; - DBUG_RETURN(0); // Use filesort + goto need_filesort; // Use filesort } - - /* ORDER BY range_key DESC */ - tmp= select->quick->make_reverse(used_key_parts); - if (!tmp) - { - select->quick= save_quick; - tab->limit= 0; - DBUG_RETURN(0); // Reverse sort not supported - } - select->set_quick(tmp); } } - else if (tab->type != JT_NEXT && tab->type != JT_REF_OR_NULL && - tab->ref.key >= 0 && tab->ref.key_parts <= used_key_parts) + } + + /* + Update query plan with access pattern for doing + ordered access according to what we have decided + above. + */ + if (!no_changes) // We are allowed to update QEP + { + if (best_key >= 0) { - /* - SELECT * FROM t1 WHERE a=1 ORDER BY a DESC,b DESC + bool quick_created= + (select && select->quick && select->quick!=save_quick); - Use a traversal function that starts by reading the last row - with key part (A) and then traverse the index backwards. + /* + If ref_key used index tree reading only ('Using index' in EXPLAIN), + and best_key doesn't, then revert the decision. */ - tab->read_first_record= join_read_last_key; - tab->read_record.read_record= join_read_prev_same; + if (!table->covering_keys.is_set(best_key)) + table->set_keyread(FALSE); + if (!quick_created) + { + if (select) // Throw any existing quick select + select->quick= 0; // Cleanup either reset to save_quick, + // or 'delete save_quick' + tab->index= best_key; + tab->read_first_record= order_direction > 0 ? + join_read_first:join_read_last; + tab->type=JT_NEXT; // Read with index_first(), index_next() + + if (table->covering_keys.is_set(best_key)) + table->set_keyread(TRUE); + table->file->ha_index_or_rnd_end(); + if (tab->join->select_options & SELECT_DESCRIBE) + { + tab->ref.key= -1; + tab->ref.key_parts= 0; + if (select_limit < table->file->stats.records) + tab->limit= select_limit; + } + } + else if (tab->type != JT_ALL) + { + /* + We're about to use a quick access to the table. + We need to change the access method so as the quick access + method is actually used. + */ + DBUG_ASSERT(tab->select->quick); + tab->type=JT_ALL; + tab->use_quick=1; + tab->ref.key= -1; + tab->ref.key_parts=0; // Don't use ref key. + tab->read_first_record= join_init_read_record; + if (tab->is_using_loose_index_scan()) + tab->join->tmp_table_param.precomputed_group_by= TRUE; + /* + TODO: update the number of records in join->best_positions[tablenr] + */ + } + } // best_key >= 0 + + if (order_direction == -1) // If ORDER BY ... DESC + { + if (select && select->quick) + { + /* ORDER BY range_key DESC */ + QUICK_SELECT_I *tmp= select->quick->make_reverse(used_key_parts); + if (!tmp) + { + tab->limit= 0; + goto need_filesort; // Reverse sort failed -> filesort + } + if (select->quick == save_quick) + save_quick= 0; // make_reverse() consumed it + select->set_quick(tmp); + } + else if (tab->type != JT_NEXT && tab->type != JT_REF_OR_NULL && + tab->ref.key >= 0 && tab->ref.key_parts <= used_key_parts) + { + /* + SELECT * FROM t1 WHERE a=1 ORDER BY a DESC,b DESC + + Use a traversal function that starts by reading the last row + with key part (A) and then traverse the index backwards. + */ + tab->read_first_record= join_read_last_key; + tab->read_record.read_record= join_read_prev_same; + } } + else if (select && select->quick) + select->quick->sorted= 1; + + } // QEP has been modified + + /* + Cleanup: + We may have both a 'select->quick' and 'save_quick' (original) + at this point. Delete the one that we wan't use. + */ + +skiped_sort_order: + // Keep current (ordered) select->quick + if (select && save_quick != select->quick) + { + delete save_quick; + save_quick= NULL; } - else if (select && select->quick) - select->quick->sorted= 1; DBUG_RETURN(1); + +need_filesort: + // Restore original save_quick + if (select && select->quick != save_quick) + { + delete select->quick; + select->quick= save_quick; + } + DBUG_RETURN(0); } --===============6341756641798441949== MIME-Version: 1.0 Content-Type: text/bzr-bundle; charset="us-ascii"; name="bzr/ole.john.aske@stripped" Content-Transfer-Encoding: 7bit Content-Disposition: inline # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: ole.john.aske@stripped\ # uktypgjodknlpb8r # target_branch: file:///net/fimafeng09/export/home/tmp/oleja/mysql\ # /mysql-5.5/ # testament_sha1: dff592ca1ca9ce5001c1366c619ff03f3191b07f # timestamp: 2011-02-02 15:50:18 +0100 # source_branch: file:///net/fimafeng09/export/home/tmp/oleja/mysql\ # /mysql-5.1/ # base_revision_id: ole.john.aske@stripped\ # ppx2z4tn6vv0u8q9 # # Begin bundle IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWabOQcMAByb/gFf9AEV59/// ////6r////pgDkbfb3lt29Xee127qOpUqNxZQ2scdb208vbHbtXa3bnTRXpu9uoSSEIyGSp+R6mj J6CZAnqY0jaaUyPKbRGj2lMRoDQ2o9QSSTCNAARDU9ET0gPUaaANPUBoAAaAAaeoGgTTSTQ1I8mQ J6j8pMgbUNMg0aAAABoNA0ASIkAp6T0Keiemk9CQ9TZJshNDTCANBo00ZAAAONDJppk0ADBAGgya AAZNAAADJkA0EkQgBNDRMMpoE2o0ZSenqR+kaJowBGj1HqMAJpicEC36ZT24seTey5t/Lv6pf5r8 McPm+Lw937/D4dUDZUzbOqPxhe4nHNJTe2eiPeZDWvEePg5j5fE/w/S/hq6eg1U3JylqKbZFtRlk ebsMwSttehBnkaVcFQhjeIZoQVWcS9eqpQBQpOazwZE6XT56emeNbr4O7IW+BvdvZtzXmBQdGmdP 8vrtPrlfPH3auvKUCPW8pdPmtQqdxdvspaZrU1b7oPvcDi3BgnYjMAsuMStR6550spDrY8wuJL9I oJHhIiIFdw9+7H8Nbps5odyBCV4kgzNAMY9kRdCVBgcA0h3JnUxWFfKAvnM9RMXrc9VXKFz8ituH 7GON7QzuQprprvhZlgszbbvUozQriZmQDMMy53me+4XyQZMubinVhvYBV2KHO7Gm4bd0xz055SwY WX1SqphG5OMEHIwyQtJwizZaMzNU3Bgu5IGUtQbGK47Sv2reIDmAeWo8+W+TswWknx17wZ8khIR8 q3Vce8fo37CmRJJAZeT1k/gd5VNgN5ysrGRD8KAxRIvCD46XmhkdpHcd2fqsNvsRhr4+vPL+ZvRH Zr3FAmNijJEgLd1SwXZ8cYNbIt4bJljtnB77iXfpHluqh26ajdVRSV1G3pqDkNibpUXzuxs5nQr9 Zmw5dMSysQ4oWGoBJ2zrFiJZpyIKGOWgsdo/M/NDik+uNornDUuyYy0t2ukwM9zpa1cKfLaJMysP IV8FebcFnGSp1ho0cGk5LQ6JtEuFNp810IMAWupmNsY0tO2BgNBnWvRO+zUmjPOqkx3KjrCByMVB vohHxsCGsLurg+XVUcfh3PXOjlrkZ37Mn58bm6mbrNUOUznuHQek6zoOsvNehlhq03SjLUdwtx8W ScjejrTQ2onCCkMB9W8BLwDKQPmAYQfkxo4kTyCBxDs8SI8XeBFDN6gOYjiSfOd+RyneDB1SoO4I 6tZw6JYJ3mddzJaeuseLF1iSkmJiW5nFY5JUCdtTunF6RYxKcxLH93sIHtZtfc81/bqtQVFm5RcW BcpZ0nb5JBQw4WB22UaoyIhVqYI8/ciTiC44wskeLjZOTEhxBaIgELJBNASmEWuvKNXNRArGVqRk Fomu/iLBV065af6HRy2o40DBsqagMUNQ/gFcQkld29msssrPdiWBwiJVwHvGDsOz04bScS9fnIEA mnqgI16Rw2l1JDKrKEOiJUkBk3tI0BeFwiuR/K9wsBlmGvrG23nRLmDEmlmIZRbjQZKnUxWK5IOF xjMYr05cr9H8CKReCHiTi8m0REOVSCu+UCeJmK+cpgMqrpBChYMLGRafo+PpBncsGhDkazLQi+Jb wVQ4u0jGZINONlb4G+tFFWtCQ4ZBFhGsh5aWWR2s59/tyKtdat+LXOlLYkKEG0FG1gNcXNOYcdki JzwOa3XpnNpaOXlQay++b9jsLuzuH/Bv7uETDIsdtydPKaHBZwSMzaprDWNBv10k+BmNc4QDKq03 T6T+yW4v5/gMjMObI9xE2dhy5ZaMz3kKJNkQTzFm+KwbOM1oVkuQ320DRhGrkPHPslQVjuSSLEVG KlRu88MjRp41OytYvjAbQlm8zseR31lS3lhpmjWOzjkBtB7hphdMWOOsDhS6InUWTFuwx7RkbRsx nkalFcQaGY4sIOmWODZpjZNB7s2KljLKknJcTthWIyyqwxwyMjEYKMRykDzIbDCJQ8qmb3lJLEiO oa8y0Zx091lGw83PfxrWJ8UbqXl7v0QptdVzNMhW0Vyh2OoLRW2OlXvNvxxJeQ/sdFdJ37rdbVNX 8xWHyyudFCWCmUOuMdw2NlI+OIHWueddi1cNBnYkgnR2IBWiia/P62nV1WFXeMJrGaWcgTeTSoWA yrQyenLqZAk4rJKycd4L68T+PAcYT+pZcAzNNvzfQH7KpBYw9AipVkeWpjSYMKvyflfpQMItZLv1 FNBjkSQ4PWs8Y8xgCc8F4zFBmi54Uj0iNpZaeM8O+cEaNNfN/qIKt6t3rRK4qB16Ufuw7LbPKd1R vI/ANYaAWYxbDgR1rRmdgVaBwtnQjiYtSiTEGZBGznf9FsxuxflS+o6GHRQMgZGZAbCtF5ZCKLym rcw85GSNfE+H3M5mKlznA3bzelbTnswjEYGKiisKSaW+Bdxjj2FYMqKzI5l3FNYbdPaqLmP0bZkE mjAvSrQd/rrOwbW5yMeNL493Mq4tPd0+cgGlLUyOWJci6We9exkRDEPeMJ54qKlsDZVREVSENAyw HIxuTev+v7LsfdVZaKuV3UNDixFjCI4BhhgtAevtO47O8m1muYjvE03voEjIg4RKPmTPoKBaWRSV r5iD6TV8BWQPCcCDHwH3zX21RiqzHh+26VS8LbkXjrS/p0nAcIeNYwSpIaH3pK5Uw3sO6AwKiiuv mkaAsiPo2eFshI+4ZXD3f73+jO5w4dhA5MBFZ0Hq3ddfipLJammZhj/N1xSbTADgyxPspgYOHwpY GVHEQmfazGstOrXYOSNpP7BR0qcDjoKEVG3lLuLaTTnEQArZJJgCH0itEFZ5t62RixZethcdwoJI o8Czckommw0sG7sMSZ3HN2Cfr28mWrtLOgsRAnQ9FIwTJQgKwGTCY7QfZr4jIp97qKqwT2IFYjqK w1kR3BQELNkbIqDleeg52RC21uYCeofmTdBuUulpITBwZThJSgmhh5YRKKIFKBqnQBl5N2wrOFkc snvGjOdmzT3py2NklyzZbOyWjWCbQFRIbDDruBb2MSoHLhBxceoD+BBBNeMh0ySUnDKUZRa/x5sI j6WygTwciJnNR78qTj15JXlRxybXw9usCYxfkKurujNW13lcuIQXl1aONjbNZgrYjjfaRlUjkR7M ljNU5Cexz0SkTJYYVYzJZ8lCCCKJVOAydlkFZFIFoiM5FGwiIjHKlLgIVGrMkH0XkGfIUAOUbzHM +Ctwc6jzFhyhqGswR3jZPeXrHMsRddI24G+WmusDtD0E8YMA0bldL463gYTtgaSYPoEYtjwzHad/ HlKdGUqYbUNxAydeBuJvOzmodIxZRG4gE4DnIRNX9aedIJly5YF/A8sbECvZCpn1JZqYb7Ldxt7i ChAbe5AbnRnLwwKkjnZBWZBSGwUBkdIUGQvfmDixqbU6rCth0xoyI+DyeecwpTMDK0rDBE+HQX3d xkdexJ5Spk1wxrFbiK56p4KX07gihJWfdbvM5/+3nE/UQbRoDSMSzAEmjb1szLmm1BLgY0cx3mHC pOYR7FR5Yh9nUWq5jZGASJC3VzIBSKSxz5iw9X58ygGxvdSVySkUlh9RghBS6qEEhgPN7e/R3WWb XmRFu2IxJhIWRDu88r38+YBaxnnbE4p7e8nB4G2bX2RciyYzVlAwexNpDQSYqCcxtttKuLoL69xn NFzpRuI3JfE6CkrSWRJOYGk6CxTeCmdESlhVMIn7yjJkQxkhULYZq4+YRP4uWJdsXxDpLB0JnkOc wmYV8DUenWz1AhqkucjNuq6SD/1EW8Vpd8BCfknMhduiqDeOwGTuaAg3DI2K5v5slsGh2M6kWatJ zGs3lDBD9UliKkUDRujZ0lp7cVqyim5UWQlv7CxjN+TPG3kFooXEGl8fqYHI8UE4zDy6myE0OEO8 RzlfobsDyJbDTpoEeccCuQ21vo2cwR6oYwtIUh5XWGluoiRzSvs7wzEJwNsbY2022mN9mYvLBcD4 IkKNQsRjYirHskyhFm2olJE+RxV1S/FNN6sSziMTKsUpuCxCsu3TWCbRPaggFKrrJ47e1wsJZw1k B1lFPlyjCy3XnvWy8WlErO5YhwBHINEs7+/w15zKbqS+1QpMUpdEOwa1jLXatvRdwatK4Welr2eb I12Rq5TfZCjAHvuHvDWHMi5wm8zXE58UbvieUFqrqsWPPnmIrjEILVuwC/alhMvpwSXTm45I5s9p K4gnYVSz9pd1tt6euEQQViyvN2eFjiBNhCZhfo1Cohgp9EM0xcZkRyoyGuYlkWg8jpkzBRD75uIF JuKiggvZ1KyFsySPHmtadsDxdqoBuFyKyZZU1iJ1bdpx+/IelzmcVKYW700mjqjP/2CxOM0GSbyY 97GdhtfMdy37DMJsbVAiwJ2rPhrmrxGL7keMqju3fzd1heEUMzBNLtLOzWCfcDRwuenodR3qeHLc j6REhe7Qn4OjTrDpLY8SDiEbp1bAthk6h5tUkVOuCmZb9c+XhUeQLC2wnYFJOPBmDo9KcW039ITm 7LIa8IzsTYbWPIMN6iS+56ykBlOVIKPgIlkGgJ3Q7CuOVAOj2xekJFFoUzVYGk9HbgvQQUzMolTL f/i7kinChIU2cg4Y --===============6341756641798441949==--