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

JSON_TABLE: Inconsistency in implicit data type conversion

Details

    • Bug
    • Status: Closed (View Workflow)
    • Blocker
    • Resolution: Fixed
    • N/A
    • 10.6.0
    • JSON
    • None

    Description

      This was a part of code review comments / changes. The example is taken from the review comments as is:

      select * from
      json_table(
        '[{"a":"asd"}, 
          {"a":123},
          {"a":[]},
          {"a":{}}
         ]',
        '$[*]' 
        columns (
          id for ordinality,
          intcol int path '$.a' default '1234' on empty default '5678' on error
        )
      ) as tt;
      

      At that point, intcol value in the first row was 0, indicating that the value was retrieved, and an implicit string => number conversion was performed. The question was raised on what grounds this conversion occurs, and why there is no warning about it; eventually the result was changed to the default-on-error, which it is now:

      bb-10.6-mdev17399-psergey2 8b533cc1d5

      +------+--------+
      | id   | intcol |
      +------+--------+
      |    1 |   5678 |
      |    2 |    123 |
      |    3 |   5678 |
      |    4 |   5678 |
      +------+--------+
      4 rows in set (0.001 sec)
      

      Personally, I am fine with this decision; but to my understanding, the implicit conversion is a general MariaDB approach applied in a variety of cases, so this change may contradict the "official line". Here is the basic example:

      MariaDB [test]> select "asd" = 0;
      +-----------+
      | "asd" = 0 |
      +-----------+
      |         1 |
      +-----------+
      1 row in set, 1 warning (0.000 sec)
       
      MariaDB [test]> show warnings;
      +---------+------+-----------------------------------------+
      | Level   | Code | Message                                 |
      +---------+------+-----------------------------------------+
      | Warning | 1292 | Truncated incorrect DOUBLE value: 'asd' |
      +---------+------+-----------------------------------------+
      1 row in set (0.000 sec)
      

      I suggest to make sure that serg is okay with the chosen JSON_TABLE behavior, otherwise there is a big chance that later the result will be re-considered and changed for the sake of consistency. In this case it's better to do it right from the beginning than change the behavior later.

      Of course the review note about a missing warning was also valid and should be taken into account if the logic changes back to returning the converted value.

      Attachments

        Issue Links

          Activity

            If we take this patch and run the example from the bug description, we get

            +------+--------+
            | id   | intcol |
            +------+--------+
            |    1 |   5678 |
            |    2 |    123 |
            |    3 |   5678 |
            |    4 |   5678 |
            +------+--------+
            4 rows in set, 1 warning (0.000 sec)
            

            | Warning | 1366 | Incorrect integer value: 'asd' for column ``.`(temporary)`.`intcol` at row 1 |
            

            That is, converting the first "a":"asd" into integer now produces a warning, but the default '5678' on error rule is still fired.

            This is not what was described in Serg's comment above.

            psergei Sergei Petrunia added a comment - If we take this patch and run the example from the bug description, we get +------+--------+ | id | intcol | +------+--------+ | 1 | 5678 | | 2 | 123 | | 3 | 5678 | | 4 | 5678 | +------+--------+ 4 rows in set, 1 warning (0.000 sec) | Warning | 1366 | Incorrect integer value: 'asd' for column ``.`(temporary)`.`intcol` at row 1 | That is, converting the first "a":"asd" into integer now produces a warning, but the default '5678' on error rule is still fired. This is not what was described in Serg's comment above.
            psergei Sergei Petrunia added a comment - Candidate patch: https://gist.github.com/spetrunia/5f4f13ef5fdf91ca1f343bcb34f3c815
            holyfoot Alexey Botchkov added a comment - Here is my version of the fix. https://github.com/MariaDB/server/commit/cbe2d62b55b1654af35f702861297dc97d713301 so we can compare.

            Review input provided on Slack

            psergei Sergei Petrunia added a comment - Review input provided on Slack

            Ok to push

            psergei Sergei Petrunia added a comment - Ok to push

            People

              holyfoot Alexey Botchkov
              elenst Elena Stepanova
              Votes:
              0 Vote for this issue
              Watchers:
              4 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.