From: Jorgen Loland
Date: January 5 2011 3:21pm
Subject: Re: bzr commit into mysql-5.1 branch (ole.john.aske:3534) Bug#59308
List-Archive: http://lists.mysql.com/commits/127985
Message-Id: <4D248C82.8080500@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 8bit

Hi Ole John,

I've run into some problems with the patch:

First, compilation:
-------------------
cc1plus: warnings being treated as errors
sql_select.cc: In function ‘bool test_if_skip_sort_order(JOIN_TAB*, ORDER*, 
ha_rows, bool, key_map*)’:
sql_select.cc:13674: error: value computed is not used
sql_select.cc:13703: error: unused variable ‘tmp’
sql_select.cc:13821: error: label ‘skiped_sort_order’ defined but not used
make[3]: *** [sql_select.o] Error 1

When I deal with these, I get a failed assertion from another test order_by:
--------------------
CURRENT_TEST: main.order_by
mysqltest: At line 239: query 'explain select * from t1 where a >= 1 and a < 3 
order by a desc' failed: 2013: Lost connection to MySQL server during query
(...)
#6  0x00007ff305eaf941 in *__GI___assert_fail (
     assertion=0xba6d4b "select->quick->sorted", file=<value optimized out>,
     line=13795,
     function=0xba7780 "bool test_if_skip_sort_order(JOIN_TAB*, ORDER*, ha_rows, 
bool, key_map*)") at assert.c:81
#7  0x000000000071e6d4 in test_if_skip_sort_order (tab=0x1607520,
     order=0x160a9b0, select_limit=18446744073709551615, no_changes=false,
     map=0x162df98) at sql_select.cc:13795
#8  0x00000000006feea8 in JOIN::exec (this=0x1605c18) at sql_select.cc:1879
#9  0x00000000007013d1 in mysql_select (thd=0x15aaa88,
     rref_pointer_array=0x15acb70, tables=0x1609e78, wild_num=1, fields=...,
     conds=0x160a7a0, og_num=1, order=0x160a9b0, group=0x0, having=0x0,
     proc_param=0x0, select_options=2147764740, result=0x160aa78,
     unit=0x15ac578, select_lex=0x15ac9a0) at sql_select.cc:2545
#10 0x00000000007279b4 in mysql_explain_union (thd=0x15aaa88, unit=0x15ac578,
     result=0x160aa78) at sql_select.cc:16971
#11 0x000000000069463d in execute_sqlcom_select (thd=0x15aaa88,
     all_tables=0x1609e78) at sql_parse.cc:5136
#12 0x000000000068b62f in mysql_execute_command (thd=0x15aaa88)
     at sql_parse.cc:2293
#13 0x0000000000696cec in mysql_parse (thd=0x15aaa88,
     rawbuf=0x1609c58 "explain select * from t1 where a >= 1 and a < 3 order by 
a desc", length=63, found_semicolon=0x7ff306db4960) at sql_parse.cc:6075

I can provide more details if you're unable to reproduce this assert.

$ bzr parent
bzr+ssh://bk-internal.mysql.com/bzrroot/server/mysql-5.1/
$ bzr revision-info
3534 georgi.kodinov@stripped

compiled with
BUILD/compile-amd64-debug-max-no-ndb

On 01/05/2011 02:48 PM, Ole John Aske wrote:
> #At file:///net/fimafeng09/export/home/tmp/oleja/mysql/mysql-5.1/ based on revid:nirbhay.choubey@stripped
>
>   3534 Ole John Aske	2011-01-05
>        Fix for bug#59308: Incorrect result for SELECT DISTINCT<col>... ORDER BY<col>  DESC.
>
>        Also fix bug#59110: Memory leak of QUICK_SELECT_I allocated memory.
>
>        Root cause of these bugs are that test_if_skip_sort_order() decided to
>        revert the 'skip_sort_order' descision (and use filesort) after the
>        query plan has been updated to reflect a 'skip' of the sort order.
>
>        This might happen in 'check_reverse_order:' if we have a
>        select->quick which could not be made descending by appending
>        a QUICK_SELECT_DESC. ().
>
>        The original 'save_quick' was then restored after the QEP has been modified, which caused:
>
>         - An incorrect 'precomputed_group_by= TRUE' may have been set, and not reverted, as
>           part of the already modifified QEP (Bug#59398)
>         - A 'select->quick' might have been created which we fail to delete (bug#59110).
>
>        This fix is a refactorication of test_if_skip_sort_order() where all logic
>        related to modification of QEP (controlled by argument 'bool no_changes'), is
>        moved to the end of test_if_skip_sort_order(), and done after *all* 'test_if_skip'
>        checks has been performed - including the 'check_reverse_order:' checks.
>
>        The refactorication above contains now intentional changes to the logic which has been
>        moved to the end of the function.
>
>        Furthermore, a smaller part of the fix address the handling of the select->quick objects
>        which may already exists when we call 'test_if_skip_sort_order()' (save_quick) - and
>        new select->quick's created during test_if_skip_sort_order():
>
>          - Before new select->quick may be created by calling ::test_quick_select(), we
>            set 'select->quick= 0' to avoid that ::test_quick_select() prematurely
>            delete the save_quick's. (After this call we may have both a 'save_quick'
>            and 'select->quick')
>
>          - All returns from ::test_if_skip_sort_order() where we may have both a
>            'save_quick' and a 'select->quick' has been changed to goto's to the
>            exit points 'skiped_sort_order:' or 'need_filesort:' where we
>            decide which of the QUICK_SELECT's to keep, and delete the other.
>
>      modified:
>        mysql-test/r/order_by.result
>        mysql-test/t/order_by.test
>        sql/sql_select.cc

-- 
Jørgen Løland | Senior Software Engineer | +47 73842138
Oracle MySQL
Trondheim, Norway