Bug #42196 Issue with key look up for bit field in Falcon table after insert + stored func
Submitted: 19 Jan 2009 11:34 Modified: 15 May 2009 13:04
Reporter: Dmitry Lenev Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: Falcon storage engine Severity:S3 (Non-critical)
Version:6.0.10-bzr OS:Any
Assigned to: Lars-Erik Bjørk CPU Architecture:Any
Tags: F_INDEX
Triage: Triaged: D2 (Serious)

[19 Jan 2009 11:34] Dmitry Lenev
Description:
For Falcon table look ups on key on BIT() field fail to find value which was 
inserted by statement involving stored function (Actually I suspect that all
INSERTs which require table pre-locking are affected).
This behavior is repeatable for Falcon and not repeatable for InnoDB/MyISAM.
Also I have an impression that sporadically (very seldom) this behavior is
repeatable even for ordinary inserts.

How to repeat:
# Script for mysqltest tool which reproduces the problem
--source include/have_falcon.inc
set storage_engine= Falcon;

--disable_warnings
drop tables if exists t2;
drop function if exists f1;
--enable_warnings
create function f1() returns int return 1;
create table t2 (s1 bit(3), key(s1));
insert into t2 values (2*f1());
select s1+0 from t2 where s1 = 2;
# Statement above fails to return any rows. Which is a bug.
# s1+0
select s1+0 from t2;
# Statement above returns row with value '2'
# s1+0
# 2
drop tables t2;
[19 Jan 2009 11:47] Valeriy Kravchuk
Verified just as described:

openxs@suse:/home2/openxs/dbs/6.0/mysql-test> ./mysql-test-run.pl 42196
Logging: ./mysql-test-run.pl 42196
MySQL Version 6.0.10
Using dynamic switching of binlog format
Using ndbcluster when necessary, mysqld supports it
Setting mysqld to support SSL connections
Binaries are debug compiled
Using MTR_BUILD_THREAD      = 0
Using MASTER_MYPORT         = 9306
Using MASTER_MYPORT1        = 9307
Using SLAVE_MYPORT          = 9308
Using SLAVE_MYPORT1         = 9309
Using SLAVE_MYPORT2         = 9310
Using NDBCLUSTER_PORT       = 9311
Using NDBCLUSTER_PORT_SLAVE = 9312
Killing Possible Leftover Processes
Removing Stale Files
Creating Directories
Installing Master Database
=======================================================

TEST                           RESULT         TIME (ms)
-------------------------------------------------------

main.42196                     [ fail ]

mysqltest: The specified result file does not exist: '/home2/openxs/dbs/6.0/mysql-test/r/42196.result'

The result from queries just before the failure was:
set storage_engine= Falcon;
drop tables if exists t2;
drop function if exists f1;
create function f1() returns int return 1;
create table t2 (s1 bit(3), key(s1));
insert into t2 values (2*f1());
select s1+0 from t2 where s1 = 2;
s1+0
select s1+0 from t2;
s1+0
2
drop tables t2;

More results from queries before failure can be found in /home2/openxs/dbs/6.0/mysql-test/var/log/42196.log

Aborting: main.42196 failed in default mode.
To continue, re-run with '--force'.
Stopping All Servers
[20 Jan 2009 11:15] Lars-Erik Bjørk
If I run the test without attaching a debugger, the test will fail, if I attach a debugger, however, the test will pass.

It seems that segment->isUnsigned is false without the debugger, and true with the debugger. This will cause two different paths through StorageInterface::getSegmentValue.

This will be *masked* by the patch for bug#40607 and bug#41582, since the same code path will be chosen regardless of the value of segment->isUnsigned.

However, we should find out why the value differs depending on timing.
[20 Jan 2009 11:40] Lars-Erik Bjørk
Actually, I can reproduce this sporadically without functions as well.
[20 Jan 2009 14:18] Lars-Erik Bjørk
In StorageInterface::getKeyDesc we do the following:

segment->isUnsigned = (part->field->flags & ENUM_FLAG) ?
	true : ((Field_num*) part->field)->unsigned_flag;

We do this even if the 'field' is not a subclass of Field_num, and therefore do not have the 'unsigned_flag' member. Normally, we won't use this value unless it is correct, but we do for the BIT field. This caused two different code paths to be exercised depending on the 'garbage'. This was done in the old version of the now rewritten StorageDatabase::getSegmentValue (patch in review). In the new version it is no longer checked on, and the key is computed correctly.

Because this does not seem to be a timing issue, but simply mistakingly checking the isUnsigned flag when there is just 'garbage' there, I believe that this is covered by the previously mentioned patch.

However, reading the non-existing flag in the first place, does not look good although it won't be used for anything.

To me it is also strange that the Field_bit and Field_bit_as_char classes do not inherit Field_num when BIT is covered as a numeric type in the reference manual. But then again, this is new to me :)
[21 Jan 2009 9:43] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/63668

2967 lars-erik.bjork@sun.com	2009-01-21
      This is a fix for Bug#42196 
      Issue with key look up for bit field in Falcon table after insert + stored func
      
      
      This bug was already partly fixed by the patch for #40607 and #41582,
      but some additional cleaning and testing is necessary. The problem was
      that the old implementation of StorageDatabase::getSegmentValue would
      choose code paths depending on 'segment->isUnsigned' when extracting
      the value from keys of type HA_KEYTYPE_BINARY. 'segment->isUnsigned'
      was set in StorageInterface::getKeyDesc in the following way, when
      creating the index:
      
          segment->isUnsigned = (part->field->flags & ENUM_FLAG) ?
                  true : ((Field_num*) part->field)->unsigned_flag;
      
      The BIT type (class Field_bit_as_char) is not a subclass of Field_num,
      which this is casted to, and has no 'unsigned_flag'. For BITs,
      segment->isUnsigned would therefore contain more or less random
      data. In the cases where this random data was '0', the bug would
      occur.
      
      The previously mentioned patch removed the need to check on
      segment->isUnsiged. This patch removes the flag from the
      StorageSegment altogether, as well as adding a test for the bug.
      
      
      
      Modified file 'storage/falcon/StorageTableShare.h'
      --------------------------------------------------
      Removed 'isUnsigned' from the StorageSegment
      
      
      Modified file 'storage/falcon/ha_falcon.cpp'
      --------------------------------------------------
      Removed the usage of 'segment->isUnsigned'
      
      
      Added file 'mysql-test/suite/falcon/t/falcon_bug_42196.test'
      --------------------------------------------------------------
      Added a test for the bug, based on the report
      
      
      Added file 'mysql-test/suite/falcon/r/falcon_bug_42196.result'
      --------------------------------------------------------------
      The expected output of the test
[21 Jan 2009 16:48] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/63744

2967 lars-erik.bjork@sun.com	2009-01-21
      This is a fix for Bug#42196 
      Issue with key look up for bit field in Falcon table after insert + stored func
      
      
      This bug was already partly fixed by the patch for #40607 and #41582,
      but some additional cleaning and testing is necessary. The problem was
      that the old implementation of StorageDatabase::getSegmentValue would
      choose code paths depending on 'segment->isUnsigned' when extracting
      the value from keys of type HA_KEYTYPE_BINARY. 'segment->isUnsigned'
      was set in StorageInterface::getKeyDesc in the following way, when
      creating the index:
      
          segment->isUnsigned = (part->field->flags & ENUM_FLAG) ?
                  true : ((Field_num*) part->field)->unsigned_flag;
      
      The BIT type (class Field_bit_as_char) is not a subclass of Field_num,
      which this is casted to, and has no 'unsigned_flag'. For BITs,
      segment->isUnsigned would therefore contain more or less random
      data. In the cases where this random data was '0', the bug would
      occur.
      
      The previously mentioned patch removed the need to check on
      segment->isUnsiged. This patch removes the flag from the
      StorageSegment altogether, as well as adding a test for the bug.
      
      
      
      Modified file 'storage/falcon/StorageTableShare.h'
      --------------------------------------------------
      Removed 'isUnsigned' from the StorageSegment
      
      
      Modified file 'storage/falcon/ha_falcon.cpp'
      --------------------------------------------------
      Removed the usage of 'segment->isUnsigned'
      
      
      Added file 'mysql-test/suite/falcon/t/falcon_bug_42196.test'
      --------------------------------------------------------------
      Added a test for the bug, based on the report
      
      
      Added file 'mysql-test/suite/falcon/r/falcon_bug_42196.result'
      --------------------------------------------------------------
      The expected output of the test
[13 Feb 2009 7:25] Bugs System
Pushed into 6.0.10-alpha (revid:alik@sun.com-20090211182317-uagkyj01fk30p1f8) (version source revid:lars-erik.bjork@sun.com-20090121164825-gfv73hu3y45vuu91) (merge vers: 6.0.10-alpha) (pib:6)
[15 May 2009 13:04] MC Brown
An entry has been added to the 6.0.10 changelog: 

INSERT operations to a Falcon table involving BIT columns with an index would fail to find the correct rows to update.