From: Ingo Strüwing
Date: November 6 2008 1:56pm
Subject: Re: bzr commit into mysql-5.1 branch (kgeorge:2690) Bug#37936
List-Archive: http://lists.mysql.com/commits/58014
Message-Id: <4912F7AA.6070509@sun.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 8BIT

Hi Joro,

This may not be the right solution, IMHO. Please see below.

Georgi Kodinov, 05.11.2008 15:55:
> #At file:///home/kgeorge/mysql/bzr/B37936-5.1-bugteam/
> 
>  2690 Georgi Kodinov	2008-11-05
>       Bug #37936: ASSERT_COLUMN_MARKED_FOR_WRITE in Field_datetime::store , Field_varstring::store
>       
>       The code that temporary saved the bitmaps of the read set and the write set so that it 
>       can set it to all columns for debug purposes was not expecting that the table->read_set and
>       table->write_set can be the same. And was always saving both in sequence.
>       As a result the original value was never restored.
>       
>       Fixed by saving & restoring the original value only once if the two sets are the same.

Thanks for the good comment. Can you please in future keep the lines a
bit shorter so that they don't break when quoting them in a reply?

IMHO, your conclusion "As a result the original value was never
restored" is not true. We always save them as old_read_set and
old_write_set respectively. And we always restore them from their
distinct variables. So it cannot matter if the sets are the same or
different, I think. The original values are always restored.

On my machine, with a branch of 5.1-bugteam, with the test from this
patch, but without the code fix, the server crashes in
handler::ha_reset() on DBUG_ASSERT(bitmap_is_set_all(&table->s->all_set));

This means that the "all_set" in the table SHARE has been modified. This
 must not happen.

All_set is used to quickly set all bits in dbug_tmp_use_all_columns().
That is, read set and write set are temporarily modified to point to
all_set. Hence they must not modify the sets in this code area.

Your patch fixes it by not pointing read_set to all_set in the special
case that it is equal to write_set. That seems to be the case in your
test by chance. read_set thus keeps its value, which invalidates the
effort to make it better debuggable.

Since this avoids the crash, it means that read_set is modified in the
section between dbug_tmp_use_all_columns() and
dbug_tmp_restore_column_map().

I suggest to find out, where it happens and if it is necessary. You may
need to sql_alloc a new bitmap, set all bits, and assign to read_set for
modification. At least if !defined(NO_DBUG).

If this does not help, for example because read_set must preserve its
change beyond dbug_tmp_restore_column_map(), we may need to suppress the
calls like you did. But then please with detailed comments that explain
why the modification of read_set must be preserved. And if all possible
try to narrow the condition below (table->write_set != table->read_set).

...
> === modified file 'mysql-test/t/select.test'
> --- a/mysql-test/t/select.test	2008-02-18 15:20:14 +0000
> +++ b/mysql-test/t/select.test	2008-11-05 14:54:33 +0000
> @@ -3701,3 +3701,36 @@ SELECT a FROM t1 ORDER BY a LIMIT 2;
>  SELECT a FROM t1 ORDER BY a LIMIT 2,4294967296;
>  SELECT a FROM t1 ORDER BY a LIMIT 2,4294967297;
>  DROP TABLE t1;
> +
> +#
> +# Bug #37936: ASSERT_COLUMN_MARKED_FOR_WRITE in Field_datetime::store ,
> +# Field_varstring::store

Your test looks slightly different than the original one, and in my
environment the stack trace looks different from the reported one. Are
you confident that we are defeating the original bug, not only a similar
one?

Regards
Ingo
-- 
Ingo Strüwing, Database Group
Sun Microsystems GmbH, Sonnenallee 1, D-85551 Kirchheim-Heimstetten
Geschäftsführer: Thomas Schröder, Wolfgang Engels, Dr. Roland Bömer
Vorsitzender des Aufsichtsrates: Martin Häring   HRB München 161028