MySQL Bugs: #28158: table->read_set is set incorrectly, wrong error message in Falcon and NDB

Bug #28158	table->read_set is set incorrectly, wrong error message in Falcon and NDB
Submitted:	30 Apr 2007 6:11	Modified:	18 Jul 2007 19:27
Reporter:	Hakan Küçükyılmaz	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Server	Severity:	S3 (Non-critical)
Version:	5.1	OS:	Any
Assigned to:	Ingo Strüwing	CPU Architecture:	Any
Tags:	falcon

Description:
Trying to insert a duplicate entry gives wrong error message.

How to repeat:
CREATE TABLE t2 (a int, b int, UNIQUE KEY (a, b)) Engine Falcon;
INSERT INTO t2 VALUES (1,2);
INSERT INTO t2 VALUES (2,2);

UPDATE t2 SET a = 1;

-- Output is:
ERROR 23000: Duplicate entry '1-NULL' for key 'a'

-- It should be:
ERROR 23000: Duplicate entry '1-2' for key 'a'

Note: I get correct error message with ORDER BY
UPDATE t2 SET a = 1 ORDER BY a;
ERROR 23000: Duplicate entry '1-2' for key 'a'

Thank you for the bug report. Verified as described.

Commenting out decodeRecord() in NfsStorageTable::rnd_next() around line 485 gives correct error message but duplicate entries in table:

CREATE TABLE t1 (a int, b int, UNIQUE KEY (a, b));
INSERT INTO t1 VALUES (1, 2);
INSERT INTO t1 VALUES (2, 2);
UPDATE t1 SET a = 1;
ERROR 23000: Duplicate entry '1-2' for key 'a'
SELECT count(*) FROM t1;
count(*)
2
SELECT * FROM t1 ORDER by a;
a       b
1       2
1       2

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/25885

ChangeSet@1.2578, 2007-05-02 09:22:04+02:00, hakank@lu0011.wdf.sap.corp +5 -0
  Fix for Bug#28158.
  
  Problem was that even if the table->read_set bitmap is not set
  the field has to be read to get right error messages in case
  of duplicate key errors.
  
  This patch ignores the table->read_set bitmap and reads all fields
  which could be negative for performance.
  
  A better solution could be to not read a field if table->read_set bitmap
  is not set and the field in question is not an unique key. Pseudo code
  would be:
  
    if (data_type == edsTypeNull ||
        (!bitmap_is_set(table->read_set, field_index) && !is_unique_key(field->field_index))

After a code review and discussion on IRC, it seems most appropriate to use the original code that relies on bitmap_is_set(table->read_set, field->field_index) to determine whether to materialize a field for the server.  Serg will work on getting the table->read_set bitmap to indicate the correct fields to decode.

It's a bug in the server. read_set doesn't contain all columns in UNIQUE indexes, and falcon (rightfully) does not read them. Thus their values aren't available for a error message. On the other hand, we don't want a storage engine to read all columns of UNIQUE indexes just in case a constraint violation happens. On the yet another hand, we cannot just set a bit in a read_set to read more columns when an error happened - a storage engine may not expect read_set to increase during the scan.

This is a correct fix:

1. We set all columns of UNIQUE indexes in read_set before opening the cursor (rnd_init or index_init).
2. We restore read_set after the cursor is opened
3. we document the contract - read_set NEVER grows after the cursor is opened. It may shrink, though
4. on a duplicate key error we check if the conflicting columns were in the read_set, if no - we add them and re-read the row.
5. for MyISAM, heap, and other storage engines that always return the full row it means that they need to set all bits in read_set to indicate that all columns are read (which also removes the need to use dbug_tmp_use_all_columns).

One note on this, when fixed it should be fixed so that the Field class has read and write inline methods to determine access.

I think NDB would be adversely affected by this suggestion: "We set all columns of UNIQUE indexes in read_set before opening the cursor (rnd_init or index_init)."

The reason is that this would cause the reading of extra columns on every row. In NDB, the scan is performed on the data nodes and streamed to the mysqld node, and it is not possible to change the read set during the scan.

Can we instead do this?

 - Open the cursor without extra unique indexed columns in read_set.
 - On duplicate error, save the cursor position with position(), then re-read the row with rnd_pos(), now setting the extra unique indexed columns in read_set().
 - The storage engine should be able to handle the bigger read set in rnd_pos(), as there is the column_bitmaps_signal() to handle this.

But we'll need to wrap rnd_pos in rnd_init/rnd_end. Which basically means opening another cursor (as we may need to continue the scan on the first one). Which is handler::clone at the moment :(

See Bug#26827 (table->read_set is set incorrectly, causing update of a different column) for the patch.

Patch rejected

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/29901

ChangeSet@1.2567, 2007-06-28 22:24:01+02:00, istruewing@chilla.local +2 -0
  Bug#28158 - table->read_set is set incorrectly,
              causing wrong error message in Falcon
  
  An error message about a duplicate key could show a wrong key
  value when not all columns of the key were used to select the
  rows for update.
  
  Some storage engines return a record with only the selected
  columns filled.
  
  This is fixed by re-reading the record with a read_set which
  includes all columns of the duplicate key after a duplicate key
  error happens and before the error message is printed.

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

ChangeSet@1.2567, 2007-07-04 15:32:31+02:00, istruewing@chilla.local +4 -0
  Bug#28158 - table->read_set is set incorrectly,
              causing wrong error message in Falcon
  
  An error message about a duplicate key could show a wrong key
  value when not all columns of the key were used to select the
  rows for update.
  
  Some storage engines return a record with only the selected
  columns filled.
  
  This is fixed by re-reading the record with a read_set which
  includes all columns of the duplicate key after a duplicate key
  error happens and before the error message is printed.

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/30724

ChangeSet@1.2584, 2007-07-11 18:55:48+02:00, istruewing@chilla.local +6 -0
  Bug#28158 - table->read_set is set incorrectly,
            causing wrong error message in Falcon
  
  An error message about a duplicate key could show a wrong key
  value when not all columns of the key were used to select the
  rows for update.
  
  Some storage engines return a record with only the selected
  columns filled.
  
  This is fixed by re-reading the record with a read_set which
  includes all columns of the duplicate key after a duplicate key
  error happens and before the error message is printed.

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/30823

ChangeSet@1.2542, 2007-07-12 20:21:17+02:00, istruewing@chilla.local +3 -0
  Bug#28158 - table->read_set is set incorrectly,
          causing wrong error message in Falcon
  
  An error message about a duplicate key could show a wrong key
  value when not all columns of the key were used to select the
  rows for update.
  
  Some storage engines return a record with only the selected
  columns filled.
  
  This is fixed by re-reading the record with a read_set which
  includes all columns of the duplicate key after a duplicate key
  error happens and before the error message is printed.

Queued to 5.1-engines.

Pushed into 5.1.21-beta

Noted in 5.1.21 changelog.

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/33950

ChangeSet@1.2557, 2007-09-07 19:07:45+10:00, stewart@flamingspork.com +2 -0
  [PATCH] Add test for BUG#28158 (unique key violation gives wrong error message) using NDB engine
  
  Index: ndb-work/mysql-test/r/ndb_bug28158_unique_key_err.result
  ===================================================================