[MDEV-26646] SFORMAT Does not allow @variable use - Jira

Details

Type: Bug
Status: Closed (View Workflow)
Priority: Critical
Resolution: Fixed
Affects Version/s: 10.7(EOL)
Fix Version/s: 10.7.1
Component/s: Character Sets, Server
Labels:
None

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

is caused by

MDEV-25015 Custom formatting of strings in MariaDB queries

Closed

Activity

Ascending order - Click to sort in descending order

Roel Van de Paar created issue - 2021-09-20 07:02

Roel Van de Paar made changes - 2021-09-20 07:02

Field	Original Value	New Value
Link		This issue is caused by ~~MDEV-25015~~ [ ~~MDEV-25015~~ ]

Roel Van de Paar made changes - 2021-09-20 07:03

Description

{noformat:title=10.7.0 53b2c1f4664a3cb90f583979d9aa2771b7e1c98d (Debug)}
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)
{noformat}

{noformat:title=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)
{noformat}

Roel Van de Paar made changes - 2021-09-20 07:37

Component/s

Server [ 13907 ]

Sergei Golubchik made changes - 2021-09-20 18:51

Priority

Major [ 3 ]

Critical [ 2 ]

Sergei Golubchik made changes - 2021-10-09 13:30

Summary

SFORMAT Does not allow @variable use (Warning 4183 SFORMAT error: invalid type specifier)

SFORMAT Does not allow @variable use

Sergei Golubchik made changes - 2021-10-09 14:04

Status

Open [ 1 ]

Confirmed [ 10101 ]

Sergei Golubchik added a comment - 2021-10-09 14:06 - 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.

Sergei Golubchik added a comment - 2021-10-09 14:06 - 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.

Sergei Golubchik made changes - 2021-10-09 14:06

Assignee	Sergei Golubchik [ serg ]	Alexander Barkov [ bar ]
Status	Confirmed [ 10101 ]	In Review [ 10002 ]

Alexander Barkov added a comment - 2021-10-12 08:24 - 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.

Alexander Barkov added a comment - 2021-10-12 08:24 - 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.

Alexander Barkov made changes - 2021-10-12 08:24

Assignee	Alexander Barkov [ bar ]	Sergei Golubchik [ serg ]
Status	In Review [ 10002 ]	Stalled [ 10000 ]

Sergei Golubchik added a comment - 2021-10-12 09:05

pushed into a preview branch

Sergei Golubchik added a comment - 2021-10-12 09:05 pushed into a preview branch

Sergei Golubchik made changes - 2021-10-12 09:05

Fix Version/s		10.7.1 [ 26120 ]
Fix Version/s	10.7 [ 24805 ]
Resolution		Fixed [ 1 ]
Status	Stalled [ 10000 ]	Closed [ 6 ]

Sergei Golubchik made changes - 2021-10-12 09:05

Component/s

Character Sets [ 10801 ]

Sergei Golubchik made changes - 2021-12-06 21:53

Workflow

MariaDB v3 [ 125166 ]

MariaDB v4 [ 159700 ]

People

Assignee:: Sergei Golubchik

Reporter:: Roel Van de Paar

Votes:: 0 Vote for this issue

Watchers:: 4 Start watching this issue

Dates

Created:: 2021-09-20 07:02

Updated:: 2021-10-12 11:28

Resolved:: 2021-10-12 09:05

Git Integration

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

MariaDB Server

Details

Description

Attachments

Issue Links

Activity

People

Dates

Git Integration