From: Jorgen Loland
Date: January 7 2011 9:31am
Subject: Re: bzr commit into mysql-trunk branch (olav.sandstaa:3461) Bug#58816
List-Archive: http://lists.mysql.com/commits/128140
Message-Id: <4D26DD8A.3040906@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 8bit

Olav,

The patch looks good, but I have a few suggestions for you to consider inline.

On 01/06/2011 04:36 PM, Olav Sandstaa wrote:
> #At file:///export/home/tmp/mysql2/opt-bug58816/ based on revid:nirbhay.choubey@stripped
>
>   3461 Olav Sandstaa	2011-01-06
>        Fix for Bug#58816 Extra temporary duplicate rows in result set when
>                          switching ICP off.
>
> === modified file 'mysql-test/include/icp_tests.inc'
> --- a/mysql-test/include/icp_tests.inc	2010-12-13 15:22:45 +0000
> +++ b/mysql-test/include/icp_tests.inc	2011-01-06 15:36:46 +0000
> @@ -653,3 +653,28 @@
>   insert into t1 values ('',1);
>   select 1 from t1 where b<= 1 and a<>  '';
>   drop table t1;
> +
> +--echo #
> +--echo # Bug #58816 "Extra temporary duplicate rows in result set when
> +--echo #             switching ICP off"
> +--echo #
> +
> +CREATE TABLE t1 (
> +  pk INT NOT NULL,
> +  c1 INT NOT NULL,
> +  PRIMARY KEY (pk)
> +);
> +
> +INSERT INTO t1 VALUES (1,9);
> +INSERT INTO t1 VALUES (2,7);
> +INSERT INTO t1 VALUES (3,6);
> +INSERT INTO t1 VALUES (4,3);
> +INSERT INTO t1 VALUES (5,1);

You could do INSERT INTO t1 VALUES (1,9),(2,7),...

> === modified file 'sql/handler.cc'
> --- a/sql/handler.cc	2010-12-16 19:36:57 +0000
> +++ b/sql/handler.cc	2011-01-06 15:36:46 +0000
> @@ -5820,7 +5820,7 @@
>   */
>   int handler::ha_reset()
>   {
> -  DBUG_ENTER("ha_reset");
> +  DBUG_ENTER("handler::ha_reset");
>     /* Check that we have called all proper deallocation functions */
>     DBUG_ASSERT((uchar*) table->def_read_set.bitmap +
>                 table->s->column_bitmap_size ==
> @@ -5833,6 +5833,11 @@
>     free_io_cache(table);
>     /* reset the bitmaps to point to defaults */
>     table->default_column_bitmaps();
> +  /* Reset information about pushed index conditions */
> +  pushed_idx_cond= NULL;
> +  pushed_idx_cond_keyno= MAX_KEY;
> +  in_range_check_pushed_down= false;
> +
>     const int retval= reset();
>     DBUG_RETURN(retval);
>   }

Looks good, but I wonder if other variables should be reset as well for 
bonus-points. How about pushed_cond for instance?

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