Transaction log retaining partial information from an aborted transaction

Bug #660779 reported by Patrick Crews
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Drizzle
Fix Released
High
David Shrewsbury
7.0
Fix Released
High
David Shrewsbury

Bug Description

The transaction log is retaining partial information for a failed query that didn't change the data of the master server.
This appears to be related to:
https://bugs.launchpad.net/drizzle/+bug/644760

But to the opposite extreme.

In the example below (from the test case), we have a query which will fail:
UPDATE t1 SET col_int_not_null = col_int1 WHERE col_int2 = 1;
This is because there is a NULL value for col_int1 and col_int_not_null is NOT NULL.

CREATE TABLE t1
(pk INT NOT NULL AUTO_INCREMENT,
col_int1 INT,
col_int2 INT,
col_int_not_null INT NOT NULL,
PRIMARY KEY (pk) );
INSERT INTO t1 (col_int1, col_int2, col_int_not_null) VALUES (1,1,1);
INSERT INTO t1 (col_int1, col_int2, col_int_not_null) VALUES (NULL,1,1);
INSERT INTO t1 (col_int1, col_int2, col_int_not_null) VALUES (2,1,3);

# transaction
BEGIN;
UPDATE t1 SET col_int1 = (col_int1 + 1) WHERE col_int2 = 1;
--ERROR ER_BAD_NULL_ERROR
UPDATE t1 SET col_int_not_null = col_int1 WHERE col_int2 = 1;
COMMIT;

After running this query, we get the 1048 (ER_BAD_NULL_ERROR) as expected, however, the log contains this - the first row that would have been affected by the failed update.
statement {
  type: UPDATE
  START_TIMESTAMP
  END_TIMESTAMP
  update_header {
    table_metadata {
      schema_name: "test"
      table_name: "t1"
    }
    key_field_metadata {
      type: INTEGER
      name: "pk"
    }
    set_field_metadata {
      type: INTEGER
      name: "col_int_not_null"
    }
  }
  update_data {
    segment_id: 1
    end_segment: true
    record {
      key_value: "1"
      after_value: "2"
      is_null: false
    }
  }
}

Related branches

Changed in drizzle:
status: New → Incomplete
status: Incomplete → Confirmed
assignee: nobody → David Shrewsbury (dshrews)
Revision history for this message
Patrick Crews (patrick-crews) wrote :

Attached branch contains a test case
./test-run --suite=transaction_log bug660779

The test will pass, but examining the log output in the .result file shows the issue.

Revision history for this message
Joe Daly (skinny.moey) wrote :

the error is thrown in dispatch_command()

  switch (session->main_da.status())
  {
  case Diagnostics_area::DA_ERROR:
    /* The query failed, send error to log and abort bootstrap. */
    session->client->sendError(session->main_da.sql_errno(),
                               session->main_da.message());

nothing is done to remove the failed statement from the session after this failure. I think a fix for this may be similar to what was done with rollback to a savepoint essentially make a copy of the statement before a new statement is added to it, if it fails reset the statement to the copy that was made. There is no remove last statement function in the GPB api to my knowledge.

Changed in drizzle:
status: Confirmed → In Progress
Revision history for this message
David Shrewsbury (dshrews) wrote :

I *think* this can be done without the need for keeping a copy of the Statement. We can access the repeated elements with a google::protobuf::RepeatedPtrField object and simply truncate off the records we don't want.

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

I have a fix for this particular case (UPDATE), but I need to handle other multi-row affecting statements that could fail in a similar manner. The only one that I can think of that would be similar to this is INSERT..SELECT. Unfortunately, the logic of the code is not the same as UPDATE, so I need to figure that out.

I have a test case for INSERT..SELECT that confirms the problem exists for this statement:

CREATE TABLE t1
(pk INT NOT NULL AUTO_INCREMENT,
col_int1 INT,
col_int2 INT,
col_int_not_null INT NOT NULL,
PRIMARY KEY (pk) );

INSERT INTO t1 (col_int1, col_int2, col_int_not_null) VALUES (1,1,1);
INSERT INTO t1 (col_int1, col_int2, col_int_not_null) VALUES (NULL,1,1);
INSERT INTO t1 (col_int1, col_int2, col_int_not_null) VALUES (2,1,3);

CREATE TABLE t2 (pk INT NOT NULL AUTO_INCREMENT PRIMARY KEY, a INT);
INSERT INTO t2 (a) VALUES (1), (2), (NULL);

BEGIN;
--ERROR ER_BAD_NULL_ERROR
INSERT INTO t1 (col_int_not_null) SELECT a FROM t2;
COMMIT;

The Transaction statement will have INSERTS for the '1' and '2' values.

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

Attached a branch that fixes both UPDATE and INSERT..SELECT. Need to run this through some heavy testing.

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

Try this test case -
The call to PRINT_TRANSACTION_MESSAGE at the end of the test reveals a transaction log record for a change to col_int_not_null, despite that query dying due to ER_BAD_NULL_ERROR (1048)

--disable_warnings
DROP TABLE IF EXISTS A;
--enable_warnings

CREATE TABLE `a` (
  `col_int` INT DEFAULT NULL,
  `col_int_not_null` INT NOT NULL,
  `col_int_not_null_key` INT NOT NULL,
  `pk` INT NOT NULL AUTO_INCREMENT,
  `col_int_key` INT DEFAULT NULL,
  PRIMARY KEY (`pk`),
  KEY `col_int_not_null_key` (`col_int_not_null_key`),
  KEY `col_int_key` (`col_int_key`)
) ENGINE=InnoDB COLLATE = utf8_general_ci;

INSERT INTO `a` VALUES (NULL,1,5,1,NULL);
INSERT INTO `a` VALUES (6,1,7,2,6);
INSERT INTO `a` VALUES (NULL,3,7,3,NULL);
INSERT INTO `a` VALUES (NULL,9,0,4,2);
INSERT INTO `a` VALUES (3,5,4,5,2);
INSERT INTO `a` VALUES (4,6,0,6,5);
INSERT INTO `a` VALUES (NULL,7,9,7,NULL);
INSERT INTO `a` VALUES (NULL,6,6,8,NULL);
INSERT INTO `a` VALUES (5,8,2,9,0);
INSERT INTO `a` VALUES (7,8,2,10,NULL);
INSERT INTO `a` VALUES (6,4,1,11,6);
INSERT INTO `a` VALUES (NULL,4,2,12,NULL);

BEGIN;
UPDATE a SET col_int = col_int + 1 WHERE col_int_not_null_key = 2;
--error 1048
UPDATE a SET col_int_not_null = col_int WHERE col_int_not_null_key = 2;
COMMIT;

SELECT * FROM a;

SELECT PRINT_TRANSACTION_MESSAGE('transaction.log',(select max(entry_offset) from DATA_DICTIONARY.TRANSACTION_LOG_TRANSACTIONS));

DROP TABLE a;

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

Re-pushed the bug fix branch with a fix for the latest problem.

Changed in drizzle:
status: In Progress → Fix Committed
milestone: none → 2010-11-08
importance: Undecided → High
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers