Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-29159

Patch for MDEV-28918 introduces more inconsistency than it solves, breaks usability

    XMLWordPrintable

Details

    Description

      There has been a recent change in behavior in regard to data type conversion and incompatibility, introduced in MDEV-28918 (patch).

      I think the approach is problematic in many ways and strongly suggest to reconsider before it has been released.

      According to JIRA and the commit comment, what it was trying to solve sounds like relatively minor inconsistencies. It does solve them, but at the cost of introducing bigger ones, adding unnecessary limitations, and hurting usability.

      Let's take the test case from MDEV-28918 description as a basis.

      The complaint there was that under the same circumstances, UPDATE and ALTER produced identical warnings but the automatic cast ended up with different inet6 values, :: vs NULL.

      The achievement is that now they behave identically.
      However:

      Limitations

      Now both UPDATE and ALTER ignore non-strict SQL_MODE and IGNORE modifier in ALTER and UPDATE completely, they don't change the behavior anymore, even though they should and still do for most data types.

      10.7.3, OLD behavior

      MariaDB [test]> UPDATE t1 SET a=b;
      ERROR 1292 (22007): Incorrect inet6 value: '97' for column `test`.`t1`.`a` at row 1
      MariaDB [test]> UPDATE IGNORE t1 SET a=b;
      Query OK, 0 rows affected, 1 warning (0.000 sec)
      Rows matched: 1  Changed: 0  Warnings: 1
       
      MariaDB [test]> show warnings;
      +---------+------+-----------------------------------------------------------------+
      | Level   | Code | Message                                                         |
      +---------+------+-----------------------------------------------------------------+
      | Warning | 1292 | Incorrect inet6 value: '97' for column `test`.`t1`.`a` at row 1 |
      +---------+------+-----------------------------------------------------------------+
      1 row in set (0.000 sec)
      

      10.7 current, NEW behavior

      MariaDB [test]> UPDATE t1 SET a=b;
      ERROR 4078 (HY000): Illegal parameter data types inet6 and int unsigned for operation 'SET'
      MariaDB [test]> UPDATE IGNORE t1 SET a=b;
      ERROR 4078 (HY000): Illegal parameter data types inet6 and int unsigned for operation 'SET'
      

      Usability problem* (in addition to the limitation above, which is also a usability problem)

      The error/warning message has changed from a quite useful, containing the problematic value, table name, column name, (and row number, which may be not always correct, but it was at least trying) to an incomprehensible and useless generic complaint:

      OLD

      ERROR 1292 (22007): Incorrect inet6 value: '97' for column `test`.`t1`.`a` at row 1
      

      ERROR 4078 (HY000): Illegal parameter data types inet6 and int unsigned for operation 'SET'
      

      I personally couldn't understand what it meant until I removed all irrelevant values and columns, got to inserting only one wrong row in one column, and only then, judging by what I was doing, I could guess what the message was trying to say. I cannot imagine how users can understand the problem in a real setup with many rows and many columns.

      More inconsistency

      Now conversion into INET6 and alike from INT and some other types behaves differently comparing to string types/literals, which is a much more visible inconsistency than the one that was being solved:

      NEW

      MariaDB [test]> insert into t values (1);
      ERROR 4078 (HY000): Illegal parameter data types inet6 and int for operation 'SET'
      MariaDB [test]> insert into t values ('1');
      ERROR 1292 (22007): Incorrect inet6 value: '1' for column `test`.`t`.`a` at row 1
      MariaDB [test]> insert ignore into t values (1);
      ERROR 4078 (HY000): Illegal parameter data types inet6 and int for operation 'SET'
      MariaDB [test]> insert ignore into t values ('1');
      Query OK, 1 row affected, 1 warning (0.012 sec)
      

      That is, it has no problem converting an integer passed as a string (or any string, for that matter), but has a problem converting the integer itself. Same with other types, e.g. temporal.


      Discussion

      So far, two arguments were briefly discussed on Slack:

      this isn't completely unheard of. GEOMETRY does the same.

      This is true; however, geometry types have always been somewhat special and notoriously weird, their behavior in this sense appears to be a limitation, more or less adequately explained by the error message itself (which btw has also been damaged by MDEV-28918):

      ERROR 1416 (22003): Cannot get geometry object from data you send to the GEOMETRY field
      

      Well, if it really cannot, then it cannot. There is no reason to extend the legacy deviations onto new data types unless absolutely necessary. We know that INET6 or UUID have no problem casting to a valid value, they did before. Which leads to the 2nd mentioned point:

      It's what you can expect from a data type that doesn't have a natural "default" value. Although, one can use all-zeros as a natural ipv6 value, so may be it shouldn't have GEOMETRY behavior. uuid doesn't have a natural default though

      "Natural" is a subjective term. We use 0000-00-00 as a default value for dates, which is anything but natural. In any case, both INET6 and UUID have a "hard default" value, and still do even after the change, so it's not really an excuse:

      NEW

      create or replace table t (a uuid not null);
      insert ignore into t values (DEFAULT),('x');
      show warnings;
      select * from t;
       
      MariaDB [test]> show warnings;
      +---------+------+--------------------------------------------------------------+
      | Level   | Code | Message                                                      |
      +---------+------+--------------------------------------------------------------+
      | Warning | 1364 | Field 'a' doesn't have a default value                       |
      | Warning | 1292 | Incorrect uuid value: 'x' for column `test`.`t`.`a` at row 2 |
      +---------+------+--------------------------------------------------------------+
      2 rows in set (0.000 sec)
       
      MariaDB [test]> select * from t;
      +--------------------------------------+
      | a                                    |
      +--------------------------------------+
      | 00000000-0000-0000-0000-000000000000 |
      | 00000000-0000-0000-0000-000000000000 |
      +--------------------------------------+
      2 rows in set (0.000 sec)
      

      And with INET6 it is even more naturally ::.

      Btw, it is a weak excuse even for geometry, as it's capable of inserting an empty string when it wants to, but it wants to so inconsistently that it would not be helping my case.

      Attachments

        Issue Links

          Activity

            People

              bar Alexander Barkov
              elenst Elena Stepanova
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.