Vladimir
Vladimir

Reputation: 1622

Triggers are calculating the wrong sum, giving unexpected results

I have a weird issue with my Trigger. There are 2 tables: Table A and Table B.

Whenever a row is inserted to Table A sum of a column in this table is inserted into Table B

It was working fine at first, but recently I noticed when >1 rows are inserted at the exact time for a user, the trigger returns sum in a weird way.

CREATE TRIGGER `update_something` AFTER INSERT ON `Table_A` 

FOR EACH ROW BEGIN

    DECLARE sum BIGINT(20);

    SELECT IFNULL(SUM(number), 0) INTO sum FROM Table_A WHERE `user` = NEW.user;

    UPDATE Table_B SET sum_number = sum WHERE id = NEW.id;

END

Example:

Table A

User X has a sum of 15 currently, then (with almost no delay in between):

  1. Number 5 is inserted for him
  2. Number 7 is inserted for him

Table B

On this table where we hold the sum, sum for this user was 15

Trigger updates this table in this way:

  1. 20
  2. 22 <--- Wrong, this should be 27

As you can see there isn't any number 2 inserted, it adds 7-5 = 2 for some reason.

How is that possible and why does it subtract 5 from 7 and add 2 to the sum instead of normally adding 7?

Edit 1:

Warning: This won't work, check the accepted answer instead

One of answers suggested select for update method.

Will this SELECT ... FOR UPDATE affect the performance negatively in a huge way?

CREATE TRIGGER `update_something` AFTER INSERT ON `Table_A` 

FOR EACH ROW BEGIN

    DECLARE sum BIGINT(20);

    SELECT IFNULL(SUM(number), 0) INTO sum FROM Table_A WHERE `user` = NEW.user FOR UPDATE; 

    UPDATE Table_B SET sum_number = sum WHERE id = NEW.id;

END

Basically we only add FOR UPDATE to the end of SELECT line like this and it will perform Row Lock in InnoDB to fix the issue?

SELECT IFNULL(SUM(number), 0) INTO sum FROM Table_A WHERE user = NEW.user FOR UPDATE;

Edit 2 (Temporary Fix):

In case some one needs a very quick temporary fix for this before doing the actual & logical suggested fix: What I did was to put a random usleep(rand(1,500000)) before INSERT query in PHP to reduce the chance of simultaneous inserts.

Upvotes: 1

Views: 484

Answers (2)

piotrgajow
piotrgajow

Reputation: 2950

You get this because of the race condition in the trigger. Both triggeres are fired at the same time, thus SELECT returns the same value for both of them - 15. Then first trigger updates tha value adding 5 and resulting in 20, and then the second update is run with 15 + 7 = 22.

What you should do is use SELECT ... FOR UPDATE instead. This way if first trigger issues the select, then the second one will have to wait until first one finishes.

EDIT:

Your question made me think, and maybe using FOR UPDATE is not the best solution. According to documentation:

For index records the search encounters, SELECT ... FOR UPDATE locks the rows and any associated index entries, the same as if you issued an UPDATE statement for those rows.

And because you are selecting the sum of entries from Table A it will lock those entries, but will still allow inserting new ones, so the problem would not be solved.

It would be better to operate only on data from Table B inside the trigger, as suggested by trincot.

Upvotes: 1

trincot
trincot

Reputation: 350365

The reason for this behaviour is that the inserted data is only committed to the database when the trigger finishes executing. So when both insert operations (5 and 7) execute the trigger in parallel, they read the data as it is in their transaction, i.e. the committed data with the changes made in their own transaction, but not the changes made in any other ongoing transaction.

The committed data in table A sums up to 20 for both transactions, and to that is added the record that is inserted in their own transaction. For the one this is 5, for the other it is 7, but as these records were not yet committed, the other transaction does not see this value.

That is why the sum is 20+5 for the one, and 20+7 for the other. The transactions then both update table B, one after the other (because table B will be locked during an update and until the end of the transaction), and the one that is latest "wins".

To solve this, don't read the sum from table A, but keep a running sum in Table B:

CREATE TRIGGER `update_something` AFTER INSERT ON `Table_A` 
FOR EACH ROW BEGIN
    UPDATE Table_B SET sum_number = sum_number + NEW.number WHERE id = NEW.id;
END;
/

I suppose you already have triggers for delete and update on Table_B, as otherwise you'd have another source of inconsistencies.

So these need to be (re)written too:

CREATE TRIGGER `delete_something` AFTER DELETE ON `Table_A` 
FOR EACH ROW BEGIN
    UPDATE Table_B SET sum_number = sum_number - OLD.number WHERE id = OLD.id;
END;
/
CREATE TRIGGER `update_something` AFTER UPDATE ON `Table_A` 
FOR EACH ROW BEGIN
    UPDATE Table_B SET sum_number = sum_number - OLD.number WHERE id = OLD.id;
    UPDATE Table_B SET sum_number = sum_number + NEW.number WHERE id = NEW.id;
END;
/

This way you prevent to lock potentially many rows in your triggers.

Then, after you have done the above, you can fix the issues from the past, and do a one-shot update:

update Table_B
join   (select   id, user, ifnull(sum(number),0) sum_number 
        from     Table_A 
        group by id, user) A
     on Table_B.id = A.id
    and Table_B.sum_number <> A.sum_number
set     Table_B.sum_number = A.sum_number;

Upvotes: 3

Related Questions