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

SFORMAT Does not allow @variable use

Details

    Description

      10.7.0 53b2c1f4664a3cb90f583979d9aa2771b7e1c98d (Debug) preview-10.7-MDEV-25015-sformat

      10.7.0-dbg>SET @a=3.14;
      Query OK, 0 rows affected (0.000 sec)
       
      10.7.0-dbg>CREATE TABLE t1 SELECT @a;
      Query OK, 1 row affected (0.019 sec)
      Records: 1  Duplicates: 0  Warnings: 0
       
      10.7.0-dbg>SHOW CREATE TABLE t1\G
      *************************** 1. row ***************************
             Table: t1
      Create Table: CREATE TABLE `t1` (
        `@a` decimal(65,38) DEFAULT NULL
      ) ENGINE=InnoDB DEFAULT CHARSET=latin1
      1 row in set (0.000 sec)
       
      10.7.0-dbg>SELECT SFORMAT ('FLOAT {:.5f}',@a);
      +-----------------------------+
      | SFORMAT ('FLOAT {:.5f}',@a) |
      +-----------------------------+
      | NULL                        |
      +-----------------------------+
      1 row in set, 1 warning (0.000 sec)
       
      10.7.0-dbg>SHOW WARNINGS;
      +---------+------+---------------------------------------+
      | Level   | Code | Message                               |
      +---------+------+---------------------------------------+
      | Warning | 4183 | SFORMAT error: invalid type specifier |
      +---------+------+---------------------------------------+
      1 row in set (0.000 sec)
       
      10.7.0-dbg>SELECT SFORMAT ('FLOAT {:.5f}',3.14);
      +-------------------------------+
      | SFORMAT ('FLOAT {:.5f}',3.14) |
      +-------------------------------+
      | FLOAT 3.14000                 |
      +-------------------------------+
      1 row in set (0.000 sec)
      

      Attachments

        Issue Links

          Activity

            serg Sergei Golubchik added a comment - - edited

            please, take a look at the commit

            commit e800625d80c
            Author: Sergei Golubchik <serg@mariadb.org>
            Date:   Sat Oct 9 16:00:13 2021 +0200
             
                MDEV-26646 SFORMAT Does not allow @variable use
                
                If charset aggregation decides to convert arguments to some specific
                charset and some arguments are numbers, they'll be wrapped into a
                Item_func_conv_charset(), that changes result_type() to STRING_RESULT.
                
                Fix: don't convert numbers, sformat needs their numerical value only.
            

            I'm particularly interested in alternative approaches, e.g. pass a flag MY_COLL_SKIP_NUMBERS. Or just always skip numbers everywhere unconditionally.

            serg Sergei Golubchik added a comment - - edited please, take a look at the commit commit e800625d80c Author: Sergei Golubchik <serg@mariadb.org> Date: Sat Oct 9 16:00:13 2021 +0200   MDEV-26646 SFORMAT Does not allow @variable use If charset aggregation decides to convert arguments to some specific charset and some arguments are numbers, they'll be wrapped into a Item_func_conv_charset(), that changes result_type() to STRING_RESULT. Fix: don't convert numbers, sformat needs their numerical value only. I'm particularly interested in alternative approaches, e.g. pass a flag MY_COLL_SKIP_NUMBERS. Or just always skip numbers everywhere unconditionally.
            bar Alexander Barkov added a comment - - edited

            We cannot always skip numbers unconditionally inside all Item_func_xxx. The following examples won't work:

            CONCAT(ucs2arg,1)
            

            SET collation_connection=ucs2_general_ci;
            SELECT CONCAT(1);
            SELECT LEFT(123,1);
            

            I think your patch is good.

            Alternatively, we could try to do the following for Item_func_sformat:

            Don't call Type_std_attributes::agg_item_set_converter() in fix_length_and_dec().
            This won't wrap arguments into Item_func_conv_charset.
            Instead, perform the conversion using this method for the string arguments:

              /*
                Returns the val_str() value converted to the given character set.
              */
              String *val_str(String *str, String *converter, CHARSET_INFO *to);
            

            However, there will be a difference comparing to other functions, e.g. CONCAT():
            1. It won't return "Illegal mix of collations" for potentially lossy charset combinations during fix_fields().
            It will just replace non-convertable characters to question marks during val_str() without warnings.
            2. Item_func_conv_charset implements some caching functionality for constant expressions.
            Caching will disappear.

            I'm slightly inclined to your solution for now.

            bar Alexander Barkov added a comment - - edited We cannot always skip numbers unconditionally inside all Item_func_xxx. The following examples won't work: CONCAT(ucs2arg,1) SET collation_connection=ucs2_general_ci; SELECT CONCAT(1); SELECT LEFT (123,1); I think your patch is good. Alternatively, we could try to do the following for Item_func_sformat: Don't call Type_std_attributes::agg_item_set_converter() in fix_length_and_dec(). This won't wrap arguments into Item_func_conv_charset. Instead, perform the conversion using this method for the string arguments: /* Returns the val_str() value converted to the given character set. */ String *val_str(String *str, String *converter, CHARSET_INFO *to); However, there will be a difference comparing to other functions, e.g. CONCAT(): 1. It won't return "Illegal mix of collations" for potentially lossy charset combinations during fix_fields(). It will just replace non-convertable characters to question marks during val_str() without warnings. 2. Item_func_conv_charset implements some caching functionality for constant expressions. Caching will disappear. I'm slightly inclined to your solution for now.

            pushed into a preview branch

            serg Sergei Golubchik added a comment - pushed into a preview branch

            People

              serg Sergei Golubchik
              Roel Roel Van de Paar
              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.