Bug #70598 Premature expression evaluation prevents short-circuited conditionals
Submitted: 10 Oct 2013 22:19 Modified: 13 Oct 2013 11:23
Reporter: Christopher Shumake Email Updates:
Status: Verified Impact on me:
None 
Category:MySQL Server: Optimizer Severity:S3 (Non-critical)
Version:5.5.32,5.5.34, 5.6.14, 5.7.2 OS:Linux (.6.18-348.16.1.el5)
Assigned to: CPU Architecture:Any
Tags: regression, trigger

[10 Oct 2013 22:19] Christopher Shumake
Description:
The 5.5 upgrade led me to rewriting our triggers instead of setting the NO_UNSIGNED_SUBSTRACTION flag or using CAST and losing half the range of the integers. Testing them is showing that I cannot avoid evaluating a sometimes-invalid expression despite the short-circuiting of conditionals that I've seen elsewhere.

How to repeat:
DROP TABLE IF EXISTS foo;
DROP TABLE IF EXISTS bar;
DROP TRIGGER IF EXISTS short_circuit_failure;
CREATE TABLE foo (a BIGINT UNSIGNED NOT NULL);
CREATE TABLE bar (b BIGINT UNSIGNED NOT NULL);
INSERT INTO foo VALUES (1);
INSERT INTO bar VALUES (0);
DELIMITER ||
CREATE TRIGGER short_circuit_failure AFTER UPDATE ON foo FOR EACH ROW BEGIN
    UPDATE bar SET bar.b =
        IF (OLD.a > NEW.a AND (bar.b > (OLD.a - NEW.a)),0,0);
END;
||
DELIMITER ;
UPDATE foo SET a = 2; -- Errors: "ERROR 1690 (22003) at line 15: BIGINT UNSIGNED value is out of range in '(OLD.a - NEW.a)'"

-- Alternatively:
DROP TABLE IF EXISTS foo;
DROP TABLE IF EXISTS bar;
DROP TRIGGER IF EXISTS short_circuit_failure;
CREATE TABLE foo (a BIGINT UNSIGNED NOT NULL);
CREATE TABLE bar (a BIGINT UNSIGNED NOT NULL);
INSERT INTO foo VALUES (1);
INSERT INTO bar VALUES (0);
DELIMITER ||
CREATE TRIGGER short_circuit_failure AFTER UPDATE ON foo FOR EACH ROW BEGIN
    UPDATE bar SET bar.a =
        (CASE
            WHEN (OLD.a > NEW.a AND (bar.a > (OLD.a - NEW.a))) THEN 0
        END);
END;
||
DELIMITER ;
UPDATE foo SET a = 2; -- Errors: "ERROR 1690 (22003) at line 15: BIGINT UNSIGNED value is out of range in '(OLD.a - NEW.a)'"

Suggested fix:
Allow for short-circuiting expressions in triggers.
[11 Oct 2013 6:07] MySQL Verification Team
Hello Christopher,

Thank you for the report and test case.
Verified as described.

Thanks,
Umesh
[11 Oct 2013 6:08] MySQL Verification Team
// Seems 5.1.72 is not affected

mysql> DROP TABLE IF EXISTS foo;
Query OK, 0 rows affected, 1 warning (0.00 sec)

mysql> DROP TABLE IF EXISTS bar;
Query OK, 0 rows affected, 1 warning (0.00 sec)

mysql> DROP TRIGGER IF EXISTS short_circuit_failure;
Query OK, 0 rows affected, 1 warning (0.00 sec)

mysql> CREATE TABLE foo (a BIGINT UNSIGNED NOT NULL);
Query OK, 0 rows affected (0.01 sec)

mysql> CREATE TABLE bar (b BIGINT UNSIGNED NOT NULL);
Query OK, 0 rows affected (0.01 sec)

mysql> INSERT INTO foo VALUES (1);
Query OK, 1 row affected (0.00 sec)

mysql> INSERT INTO bar VALUES (0);
Query OK, 1 row affected (0.00 sec)

mysql> DELIMITER ||
mysql> CREATE TRIGGER short_circuit_failure AFTER UPDATE ON foo FOR EACH ROW BEGIN
    ->     UPDATE bar SET bar.b =
    ->         IF (OLD.a > NEW.a AND (bar.b > (OLD.a - NEW.a)),0,0);
    -> END;
    -> ||
Query OK, 0 rows affected (0.01 sec)

mysql> DELIMITER ;
mysql> UPDATE foo SET a = 2;
Query OK, 1 row affected (0.02 sec)
Rows matched: 1  Changed: 1  Warnings: 0

mysql> select * from foo;
+---+
| a |
+---+
| 2 |
+---+
1 row in set (0.00 sec)

mysql> select * from bar;
+---+
| b |
+---+
| 0 |
+---+
1 row in set (0.00 sec)

mysql> select version();
+-------------------------------------------+
| version()                                 |
+-------------------------------------------+
| 5.1.72-enterprise-commercial-advanced-log |
+-------------------------------------------+
1 row in set (0.00 sec)

mysql> show variables like 'sql_mode';
+---------------+--------------------------------------------+
| Variable_name | Value                                      |
+---------------+--------------------------------------------+
| sql_mode      | STRICT_TRANS_TABLES,NO_ENGINE_SUBSTITUTION |
+---------------+--------------------------------------------+
1 row in set (0.00 sec)
[11 Oct 2013 8:05] Roy Lyseng
The problem is that when doing arithmetic on unsigned values, any sub-expression must be non-negative too, but this is an inherent property of these kinds of operations.

I will close this as not a bug, as there are two good workarounds for this problem.

Workaround with reordered expressions:

CREATE TRIGGER short_circuit_failure AFTER UPDATE ON foo FOR EACH ROW BEGIN
    UPDATE bar SET bar.b =
        IF (OLD.a > NEW.a AND (bar.b + NEW.a > OLD.a),0,0);
END;

The only problem with this workaround is when bar.b + NEW.a exceeds the value range of unsigned bigint.

We have a second workaround for this case, using CAST to a signed datatype with greater value range:

CREATE TRIGGER short_circuit_failure AFTER UPDATE ON foo FOR EACH ROW BEGIN
    UPDATE bar SET bar.b =
        IF (OLD.a > NEW.a AND (bar.b > (CAST(OLD.a AS DECIMAL(20,0)) -
                                        CAST(NEW.a AS DECIMAL(20,0)))),0,0);
END;
[11 Oct 2013 15:50] Christopher Shumake
It's nice that there's a workaround for my specific case, but why does that make my bug invalid? My use-case is valid. My bug's about short-circuiting conditionals, it's not about the inability to subtract bigints.
Is it unacceptable to short-circuit conditionals in triggers? Did I miss something?
[13 Oct 2013 11:23] Roy Lyseng
My apologies, I was too focused on finding a workaround than the actual bug.

The error occurs because the expression in the trigger is evaluated prematurely,
probably when no actual data are available.