Transaction log does not appear to be collecting all information for multi-row REPLACE statements

Bug #599582 reported by Patrick Crews
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Drizzle
Fix Released
Medium
David Shrewsbury
Dexter
Fix Released
Medium
David Shrewsbury

Bug Description

The transaction log does not appear to be capturing all relevant data for multi-row REPLACE statements:

What we see below is a multi-row REPLACE (it touches 3 rows). However, the transaction log is missing any data for the row with pk=2 (column a). The log output below has us DELETE'ing row pk=1, UPDATE'ing row pk=4, but no mention of row pk=2.

For this sequence of queries:

CREATE TABLE t1(a INT NOT NULL AUTO_INCREMENT, b INT NOT NULL, c CHAR(100), d CHAR(20), PRIMARY KEY(a), UNIQUE KEY(b), UNIQUE KEY(c));

INSERT INTO t1 (b,c,d) VALUES (10,'a','f'),(20,'b','e'),(30,'c','d'),(40,'d','c'),(50,'e','b'),(60,'f','a');

SELECT * FROM t1;

REPLACE INTO t1 VALUES (1, 20, 'd','x');

SELECT * FROM t1;

DROP TABLE t1;

We get this transaction_log output (via print_transaction_message)
+SELECT PRINT_TRANSACTION_MESSAGE('transaction.log',(select max(entry_offset) from DATA_DICTIONARY.TRANSACTION_LOG_TRANSACTIONS));
+PRINT_TRANSACTION_MESSAGE('transaction.log',(select max(entry_offset) from DATA_DICTIONARY.TRANSACTION_LOG_TRANSACTIONS))
+transaction_context {
+ server_id: 1
+ transaction_id: 4
+ START_TIMESTAMP
+ END_TIMESTAMP
+}
+statement {
+ type: DELETE
+ START_TIMESTAMP
+ END_TIMESTAMP
+ delete_header {
+ table_metadata {
+ schema_name: "test"
+ table_name: "t1"
+ }
+ key_field_metadata {
+ type: INTEGER
+ name: "a"
+ }
+ }
+ delete_data {
+ segment_id: 1
+ end_segment: true
+ record {
+ key_value: "1"
+ }
+ }
+}
+statement {
+ type: DELETE
+ start_timestamp: 1277762315337862
+ end_timestamp: 1277762315337872
+ delete_header {
+ table_metadata {
+ schema_name: "test"
+ table_name: "t1"
+ }
+ key_field_metadata {
+ type: INTEGER
+ name: "a"
+ }
+ }
+ delete_data {
+ segment_id: 1
+ end_segment: true
+ record {
+ key_value: "1"
+ }
+ }
+}
+statement {
+ type: UPDATE
+ start_timestamp: 1277762315338263
+ end_timestamp: 1277762315370096
+ update_header {
+ table_metadata {
+ schema_name: "test"
+ table_name: "t1"
+ }
+ key_field_metadata {
+ type: INTEGER
+ name: "a"
+ }
+ set_field_metadata {
+ type: INTEGER
+ name: "a"
+ }
+ set_field_metadata {
+ type: INTEGER
+ name: "b"
+ }
+ set_field_metadata {
+ type: VARCHAR
+ name: "c"
+ }
+ set_field_metadata {
+ type: VARCHAR
+ name: "d"
+ }
+ }
+ update_data {
+ segment_id: 1
+ end_segment: true
+ record {
+ key_value: "4"
+ after_value: "1"
+ after_value: "20"
+ after_value: "d"
+ after_value: "x"
+ }
+ }

Related branches

Revision history for this message
Patrick Crews (patrick-crews) wrote :

./test-run --suite=transaction_log transaction_log_replace

It is the last test case in that test.

Changed in drizzle:
importance: Undecided → Medium
status: New → Confirmed
Changed in drizzle:
assignee: nobody → David Shrewsbury (dshrews)
Changed in drizzle:
status: Confirmed → In Progress
Changed in drizzle:
status: In Progress → Confirmed
Revision history for this message
David Shrewsbury (dshrews) wrote :

It looks like one of these DELETEs (note that there are two for the same row) should be for row #2. Sequence is supposed to be:

DELETE row #1
DELETE row #2
UPDATE row #4 to be row #1

Changed in drizzle:
status: Confirmed → In Progress
milestone: none → 2010-08-30
Revision history for this message
David Shrewsbury (dshrews) wrote :

This looks like a fundamental flaw in Cursor::deleteRecord() and TransactionServices::deleteRecord().

The TransactionServices::deleteRecord() code makes the assumption that for a REPLACE, there will only ever be a single DELETE to remove the conflicting row, followed by an INSERT for the new row. So deleteRecord() assumes the primary key causes the conflict and just deletes the row with the same primary key as the one we want to insert. However, if the new record has another value that conflicts with an additional unique key on the table, another DELETE is attempted. The transaction log will simply log another DELETE with the same primary key. It should instead attempt to delete the row with the other conflicting value.

I'll need to study this some more to figure out the best way to fix this. When Cursor::deleteRecord() calls doDeleteRecord(), we don't seem to have enough information from the storage engine to know which row was actually deleted.

Revision history for this message
Jay Pipes (jaypipes) wrote :

Yes, this is exactly correct, but it's not a fundamental flaw in TransactionServices::deleteRecord(). The flaw is that there is no way to determine what key on the original record caused the deletion.

Revision history for this message
Jay Pipes (jaypipes) wrote :

To be clear, I think this REPLACE statement, in this context, is retarded. It's not clear which records should be replaced since 2 keys will be violated by the resulting insert...

Revision history for this message
Jay Pipes (jaypipes) wrote :

According to the MySQL manual the "algorithm" for REPLACE attempts to insert the new record, and *while it receives any key violation* on a primary or unique key, it issues a DELETE for that row.

So, in order to keep MYSQL's behaviour, you will need to create DeleteRecord messages until the InsertRecord is finally created I suppose.

Revision history for this message
David Shrewsbury (dshrews) wrote :

Thanks for the feedback, Jay! I agree with your assessment.

After studying this, I think this is actually a simple fix, with one slight complication.

The storage engine is sent the conflicting record to delete, which is the row stored in table->record[1]. This record has the primary key of the row we want to log the delete for. However, TransactionServices::deleteRecord() makes the assumption that the row in table->record[0] is the one being deleted, which is usually correct except for REPLACE. We can change this method to take a flag to say use the row in record[1] instead of record[0], then we should get the proper DELETE used by REPLACE.

The only complication is that we don't seem to have any mechanisms in place to parse out the values for the update row (record[1]). The table structure has Field pointers for the row in record[0], but not for record[1]. Have to figure out some way to do this...

Revision history for this message
Jobin Augustine (jobinau) wrote : Re: [Bug 599582] Re: Transaction log does not appear to be collecting all information for multi-row REPLACE statements

IMHO
REPLACE is one of the silly evil remaining in Drizzle.
Many times it came for discussions.
but never showed the courage to go for a public election.

this simple evil can give birth to many many bugs in future.
wishing them all happy life...even inside application. :)

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers