MySQL Bugs: #70598: Premature expression evaluation prevents short-circuited conditionals

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

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.

Hello Christopher,

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

Thanks,
Umesh

// 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)

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;

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?

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.