MySQL Bugs: #32194: Falcon: incorrect count of changed rows

Bug #32194	Falcon: incorrect count of changed rows
Submitted:	8 Nov 2007 18:17	Modified:	3 Dec 2007 13:12
Reporter:	Peter Gulutzan	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Server: Falcon storage engine	Severity:	S3 (Non-critical)
Version:	6.0.4-alpha-debug	OS:	Linux (SUSE 10 64-bit)
Assigned to:	Kevin Lewis	CPU Architecture:	Any

Description:
I create a table with engine=falcon with one column.
I insert three rows, with column values NULL, NULL, and 9.
I update the table to increment the column value.
The update succeeds but the client display is:
"Rows matched: 3  Changed: 2  Warnings: 0"
That's wrong. If engine=myisam the display would be:
"Rows matched: 3  Changed: 1  Warnings: 0"

How to repeat:
mysql> create table t8 (s1 int) engine=falcon;
Query OK, 0 rows affected (0.01 sec)

mysql> insert into t8 values (null),(null),(9);
Query OK, 3 rows affected (0.00 sec)
Records: 3  Duplicates: 0  Warnings: 0

mysql> update t8 set s1 = s1 + 1;
Query OK, 2 rows affected (0.00 sec)
Rows matched: 3  Changed: 2  Warnings: 0

Thank you for the bug report. Verified as described.

create table t8 (s1 int) engine=falcon;
insert into t8 values (null),(null),(9);
update t8 set s1 = s1 + 1;

Falcon rnd_next is called three times but update_row only twice, hence the value two.  The server should only be calling update_row once.

I debugged the way the server handles this with InnoDB and also with Falcon.  The server is calling handler::ha_update_row() for the first NULL record using Falcon but it does not do this with InnoDB or MYISAM.  The reason is that the record value does not seem to be set correctly in Falcon.  I am debugging this to see what the problem is specifically.

In sql_base.cc, mysql_update(), there is a call to store_record(), followed by a call to fill_record_n_invoke_before_triggers().  In this example, the field length is 5 and the first byte is 0xA7, which has the 0x02 flag meaning it is a null field.  Falcon did not zero out the other four bytes when returned the field, since all that is needed is the null bit.  The other bytes should matter.  

After Falcon calls StorageInterface::encodeRecord(), the value of the null field is;
   table->record[0] = 0x A7 A5 A5 A5 A5. 
This is a debug compile resulting in the 0xA5 bytes of 'unititialized' data.  The second time I try the update, this field contains the contents of the last time it was used, which is the record with a real autoincrement value. So at the start of the second update, it looks like this;
   table->record[0] = 0x A7 0A 00 00 00. 
Which means that in a non-debug build, where initially allocated buffers may be padded with zeros, this problem would exist for the second update.

So in mysql_update(), store_record() puts table->record[0] into table->record[1].  Then fill_record_n_invoke_before_triggers() is called, which pads table->record[0] with zeros.  
   table->record[0] = 0x A7 00 00 00 00. 

Then compare_record(table) is called and finds that 
   table->record[1] = 0x A7 0A 00 00 00. 
is different from 
   table->record[0] = 0x A7 00 00 00 00. 
and decides to update the row by calling handler::update_row(), even though both of these fields have the null flag on them.

There are multiple ways to fix this;
1) Make a more complex version of compare_record(table) so that it recognizes null fields and sees them as equal, not needing an update
2) Improve fill_record_n_invoke_before_triggers() so that it does not pad a null field.  It should leave null fields the way the storage engine reported them.
3) Both of the above.
4) Add a special tweak into the Falcon engine so that StorageInterface::encodeRecord() will pad null fields with zeros.  This is ultimately the wrong solution, but it will probably get things going.

In the previous description, I was actually refering to the Falcon function StorageInterface::decodeRecord(), not StorageInterface::encodeRecord().

decodeRecord() puts Falcon data into the MySQL form.

In order to get this working I decided to fix it in Falcon by using field->reset() just after field->set_null().  This is also done in some other places in the server code.  I added falcon_bug_32194.test to the falcon test suite.  I will open another bug against mysql_update().

falcon_bug_32194 passes now.

Pushed into 6.0.4-alpha

A note has been added to the 6.0.4 changelog: 

Altering a Falcon table to support an auto increment column on a column with existing data and null values would incorrectly update the table and return an incorrect count of the altered rows.