From: Jon Olav Hauglid
Date: January 31 2011 12:57pm
Subject: Re: bzr commit into mysql-5.5 branch (tor.didriksen:3290) Bug#36022
List-Archive: http://lists.mysql.com/commits/130041
Message-Id: <4D46B1C8.30901@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

Hello,

On 01/31/2011 11:39 AM, Tor Didriksen wrote:
>
>   3290 Tor Didriksen	2011-01-31
>        Bug #36022 please log more information about "Sort aborted" queries

Patch approved, see comments below.

Please update the copyright header for my_sys.h, protocol.cc, unireg.h

Could you verify that all tests pass on Windows now that the
suppression rule was removed?

> === added file 'mysql-test/t/filesort_debug.test'
> --- a/mysql-test/t/filesort_debug.test	1970-01-01 00:00:00 +0000
> +++ b/mysql-test/t/filesort_debug.test	2011-01-31 10:39:55 +0000
> @@ -0,0 +1,23 @@
> +--source include/have_debug.inc
> +
> +SET @old_debug = @@session.debug;
> +
> +--echo #
> +--echo # Bug#36022 please log more information about "Sort aborted" queries
> +--echo #
> +
> +CREATE TABLE t1(f0 int auto_increment primary key, f1 int);
> +INSERT INTO t1(f1) VALUES (0),(1),(2),(3),(4),(5);
> +
> +set session debug= '+d,make_char_array_fail';
> +CALL mtr.add_suppression("Out of sort memory");
> +--error ER_OUT_OF_SORTMEMORY
> +SELECT * FROM t1 ORDER BY f1 ASC, f0;
> +set session debug=@old_debug;
> +
> +CREATE FUNCTION f1() RETURNS INT RETURN 1;
> +--error ER_SP_WRONG_NO_OF_ARGS
> +DELETE FROM t1 ORDER BY (f1(10)) LIMIT 1;
> +
> +DROP TABLE t1;
> +DROP FUNCTION f1;

The test case is a bit inconsistent wrt. "coding style".
E.g. SET/set, spaces and =.

> === modified file 'sql/filesort.cc'
> --- a/sql/filesort.cc	2011-01-11 09:07:37 +0000
> +++ b/sql/filesort.cc	2011-01-31 10:39:55 +0000
> @@ -324,8 +324,24 @@ ha_rows filesort(THD *thd, TABLE *table,
>       }
>     }
>     if (error)
> -    my_message(ER_FILSORT_ABORT, ER(ER_FILSORT_ABORT),
> -               MYF(ME_ERROR+ME_WAITTANG));
> +  {
> +    DBUG_ASSERT(thd->stmt_da->status() == Diagnostics_area::DA_ERROR);

This check can be simplified to just thd->is_error().

Otherwise it looks great - I quite liked the cleanups done in the patch.

Thanks,

--- Jon Olav