Bug #70598 Premature expression evaluation prevents short-circuited conditionals

Submitted: 10 Oct 2013 22:19 Modified: 13 Oct 2013 11:23
Reporter: Chris 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] Chris 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] Chris 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.

Portions of this website are copyright © 2001, 2002 The PHP Group

Page generated in 0.041 sec. using MySQL 8.4.7-u6-cloud

Timestamp references displayed by the system are UTC.
Content reproduced on this site is the property of the respective copyright holders.
It is not reviewed in advance by Oracle and does not necessarily represent the opinion of Oracle or any other party.