1

To prevent user's balance to ever go negative (besides from the checks on the PHP side) I used the following BEFORE INSERT Mysql Trigger:

BEGIN

    UPDATE `table` SET `user_balance` = user_balance - NEW.amount WHERE `uid` = NEW.id;

    -- Some other inserts etc...

END

I thought as this is a BEFORE INSERT trigger, it would prevent insertion of any new invoice that might make the balance negative but it doesn't.

Even though the STRICT MODE is ON and the user_balance column is UNSIGED DECIMAL, the trigger doesn't stop the insertion and the field is simply turned into 0 if it's negative...

I tried running an UPDATE query manually, but it throws an error as expected, the trigger doesn't act the same.

Edit:

Does any one know why it works fine on a manual UPDATE, but not with the trigger?

T. Rex
  • 61
  • 8
  • What part of the code makes you think that this would check for a negative amount? – Jacob H Jul 14 '17 at 14:11
  • @JacobH The Mysql STRICT MODE will prevent insert or update if the amount is out of range, when I try the UPDATE query manually, it works as expected (throws an error that value is out of range and doesn't execute if the result is negative) – T. Rex Jul 14 '17 at 14:16
  • It appears to be pretty bad design. Why can't you just check if the balance is going to be negative before inserting? – Allen King Jul 14 '17 at 14:19
  • I would avoid this kind of design and do it all in your application layer. If you decide to change your database in future it won't be as trivial as it should be if you are relying on mysql triggers.. – Peter Featherstone Jul 14 '17 at 14:21
  • @AllenKing I have that check on PHP side as I've mentioned, but I've seen rare cases that multiple inserts happen at the same time on the PHP side and the value goes negative. (for example 2 scripts check at the same time that value is fine, but then they both update and the amount goes negative) – T. Rex Jul 14 '17 at 14:22

1 Answers1

3

You have to check the result of the subtraction before perform the update.

 UPDATE `table` 
 SET `user_balance` = user_balance - NEW.amount 
 WHERE `uid` = NEW.id
   AND user_balance - NEW.amount >= 0

Or better yet get the value first and raise an error :

Throw an error in a MySQL trigger

 SET @user_balance := 0;
 SELECT @user_balance := `user_balance`
 FROM `table` 
 WHERE `uid` = NEW.id;

IF @user_balance - user_balance >= 0 THEN
     UPDATE `table` 
     SET `user_balance` = user_balance - NEW.amount 
     WHERE `uid` = NEW.id
       AND user_balance - NEW.amount >= 0
ELSE 
    set @msg = concat('MyTriggerError: Trying to insert a negative value in trigger: '
                     , cast(@user_balance - NEW.amount as varchar));
    signal sqlstate '45000' set message_text = @msg;
END IF;
Juan Carlos Oropeza
  • 47,252
  • 12
  • 78
  • 118
  • Thank you, but this way the invoice is inserted anyway as the trigger will run just fine (it just won't affect any rows), doesn't it? even if I remove the trigger and run this `UPDATE` in a database transaction along with the invoice `INSERT` it still doesn't count as failure and INSERT is performed while the UPDATE has not. – T. Rex Jul 14 '17 at 14:20
  • Thank you very much, I didn't know that... In your professional opinion do you think the Trigger Approach is better or I should do this `UPDATE` along with `INSERT` in a Database Transaction on PHP side (as others are suggesting)? – T. Rex Jul 14 '17 at 14:25
  • 1
    I'm not sure what is your situation. But if is an user interface, I wouldn't send the insert to db. I will collect the db value first and validate the amount on the client side. And anyway you still may need the trigger if you are in a multiuser enviroment. In that case you can cancel the transction and report to user. – Juan Carlos Oropeza Jul 14 '17 at 14:42
  • Just to be clear. You probably need both. You do the validation on client side to improve user interface, but you also include it on the db to assure db integrity. – Juan Carlos Oropeza Jul 14 '17 at 15:17
  • Thank you very much for the explanation. – T. Rex Jul 14 '17 at 16:12