[Maria-developers] 10.0 sprint: Please advise re MDEV-8109 unexpected CAST result
Hi Sergei, I'm thinking about what to do with MDEV-8109 unexpected CAST result. Observations: 1. An empty string in VALUES SET sql_mode='STRICT_ALL_TABLES'; DROP TABLE IF EXISTS t1; CREATE TABLE t1 (a DECIMAL); INSERT INTO t1 VALUES (''); ERROR 1292 (22007): Truncated incorrect DECIMAL value: '' This is OK. In strict mode INSERT with a bad value correctly returns the error. 2. CAST('' AS DECIMAL) SET sql_mode='STRICT_ALL_TABLES'; SELECT CAST('' AS DECIMAL); +---------------------+ | CAST('' AS DECIMAL) | +---------------------+ | 0 | +---------------------+ 1 row in set, 1 warning (0.00 sec) This is also OK. There are no tables involved, and CAST itself does not convert warnings to errors. 3. CAST('' AS DECIMAL) in VALUES: INSERT INTO t1 VALUES(CAST('' AS DECIMAL)); ERROR 1292 (22007): Truncated incorrect DECIMAL value: '' I think this is not OK. I wrote an explicit CAST, so I expect: - CAST to return 0 with a warning, like in #2 - INSERT to write 0 into the table normally, as I'm actually inserting the CAST result, which is 0. 4. COLUMN_GET(AS DECIMAL) in VALUES: SET @aaa = COLUMN_CREATE('price', ''); INSERT INTO t1 VALUES(COLUMN_GET(@aaa, 'price' AS DECIMAL)); ERROR 1918 (22007): Encountered illegal value '' when converting to DECIMAL This is also not OK. I wrote explicit column conversion to DECIMAL. I expect: - COLUMN_GET() to return 0 with a warning, like in #2 - INSERT to write 0 into the table normally, as I'm actually inserting the COLUMN_GET result, which is 0. Do you agree? Thanks.
Hi, Alexander! Summary: agree with 1,2,3, not so sure about 4.
I'm thinking about what to do with MDEV-8109 unexpected CAST result. Observations:
1. An empty string in VALUES
SET sql_mode='STRICT_ALL_TABLES'; DROP TABLE IF EXISTS t1; CREATE TABLE t1 (a DECIMAL); INSERT INTO t1 VALUES ('');
ERROR 1292 (22007): Truncated incorrect DECIMAL value: ''
This is OK. In strict mode INSERT with a bad value correctly returns the error.
2. CAST('' AS DECIMAL)
SET sql_mode='STRICT_ALL_TABLES'; SELECT CAST('' AS DECIMAL); +---------------------+ | CAST('' AS DECIMAL) | +---------------------+ | 0 | +---------------------+ 1 row in set, 1 warning (0.00 sec)
This is also OK. There are no tables involved, and CAST itself does not convert warnings to errors.
3. CAST('' AS DECIMAL) in VALUES:
INSERT INTO t1 VALUES(CAST('' AS DECIMAL)); ERROR 1292 (22007): Truncated incorrect DECIMAL value: ''
I think this is not OK.
I wrote an explicit CAST, so I expect: - CAST to return 0 with a warning, like in #2 - INSERT to write 0 into the table normally, as I'm actually inserting the CAST result, which is 0.
Agree
4. COLUMN_GET(AS DECIMAL) in VALUES:
SET @aaa = COLUMN_CREATE('price', ''); INSERT INTO t1 VALUES(COLUMN_GET(@aaa, 'price' AS DECIMAL)); ERROR 1918 (22007): Encountered illegal value '' when converting to DECIMAL
This is also not OK.
I wrote explicit column conversion to DECIMAL. I expect: - COLUMN_GET() to return 0 with a warning, like in #2 - INSERT to write 0 into the table normally, as I'm actually inserting the COLUMN_GET result, which is 0.
That wouldv'e been ok if there was some "native" type of COLUMN_GET(@aaa, 'price'). Like, you can insert the value in its "native" type (relying on implicit type conversion): INSERT INTO t1 VALUES (''); or cast it explicitly: INSERT INTO t1 VALUES (CAST('' AS DECIMAL)) But with dynamic columns there is no "native" type, you always *must* cast it to something. If strict mode won't turn these warnings into errors, there will be no way to do that. Normally you add a cast to insert with warnings and remove a cast to let strict mode turn warnings into errors (here I assume the case 3 from above is already fixed :). But with dynamic columns there is no cast to remove. So I agree with you here, because logically there is an explicit conversion and strict mode should not turn this warning into an error. Still, if we do that, there will be no way to catch conversion warnings in dynamic columns. Unless you introduce STRICT_DYNAMIC_COLUMNS mode :) Regards, Sergei
Hi Sergei, On 06/10/2015 06:47 PM, Sergei Golubchik wrote:
Hi, Alexander!
Summary: agree with 1,2,3, not so sure about 4.
I'm thinking about what to do with MDEV-8109 unexpected CAST result. Observations:
1. An empty string in VALUES
SET sql_mode='STRICT_ALL_TABLES'; DROP TABLE IF EXISTS t1; CREATE TABLE t1 (a DECIMAL); INSERT INTO t1 VALUES ('');
ERROR 1292 (22007): Truncated incorrect DECIMAL value: ''
This is OK. In strict mode INSERT with a bad value correctly returns the error.
2. CAST('' AS DECIMAL)
SET sql_mode='STRICT_ALL_TABLES'; SELECT CAST('' AS DECIMAL); +---------------------+ | CAST('' AS DECIMAL) | +---------------------+ | 0 | +---------------------+ 1 row in set, 1 warning (0.00 sec)
This is also OK. There are no tables involved, and CAST itself does not convert warnings to errors.
3. CAST('' AS DECIMAL) in VALUES:
INSERT INTO t1 VALUES(CAST('' AS DECIMAL)); ERROR 1292 (22007): Truncated incorrect DECIMAL value: ''
I think this is not OK.
I wrote an explicit CAST, so I expect: - CAST to return 0 with a warning, like in #2 - INSERT to write 0 into the table normally, as I'm actually inserting the CAST result, which is 0.
Agree
I noticed some other problems: 1. DECIMAL/INT vs DOUBLE work differently on CAST
SET sql_mode='STRICT_ALL_TABLES'; DROP TABLE IF EXISTS t1; CREATE TABLE t1 (a DOUBLE); INSERT INTO t1 VALUES (CAST('' AS DOUBLE)); Query OK, 1 row affected (0.03 sec)
I.e. DOUBLE, unlike DECIMAL and INT, accepts empty string values silently with no any warnings. This is because the CAST alone does not produce any warnings: SELECT CAST('' AS DOUBLE); 2. DECIMAL/INT vs DOUBLE work differently on trailing spaces:
SET sql_mode='STRICT_ALL_TABLES'; DROP TABLE IF EXISTS t1; CREATE TABLE t1 (a DECIMAL); INSERT INTO t1 VALUES ('1 '); Query OK, 1 row affected (0.04 sec)
SET sql_mode='STRICT_ALL_TABLES'; DROP TABLE IF EXISTS t1; CREATE TABLE t1 (a INT); INSERT INTO t1 VALUES ('1 '); Query OK, 1 row affected (0.03 sec)
SET sql_mode='STRICT_ALL_TABLES'; DROP TABLE IF EXISTS t1; CREATE TABLE t1 (a DOUBLE); INSERT INTO t1 VALUES ('1 '); ERROR 1265 (01000): Data truncated for column 'a' at row 1
Compare to the behavior exposed by CAST, it's exactly the opposite: - CAST is stricter for DECIMAL/INT comparing to DOUBLE (on empty strings) - INSERT is weaker for DECINAL/INT comparing to DOUBLE (on trailing spaces) I guess silently converting empty strings or silently ignoring trailing spaces is not correct. I have a proposal, but not 100% sure yet: Why not to use a NOTE rather than a WARNING for both empty values and trailing spaces, in all above cases: - for all query parts: INSERT, CAST, COLUMN_GET - for all numeric data types: INT, DECIMAL, DOUBLE Unlike warnings, notes are not converted to errors in strict mode. This will fix the problem for MDEV-8109, because the reported is concerned about empty strings. See also more comments below.
4. COLUMN_GET(AS DECIMAL) in VALUES:
SET @aaa = COLUMN_CREATE('price', ''); INSERT INTO t1 VALUES(COLUMN_GET(@aaa, 'price' AS DECIMAL)); ERROR 1918 (22007): Encountered illegal value '' when converting to DECIMAL
This is also not OK.
I wrote explicit column conversion to DECIMAL. I expect: - COLUMN_GET() to return 0 with a warning, like in #2 - INSERT to write 0 into the table normally, as I'm actually inserting the COLUMN_GET result, which is 0.
That wouldv'e been ok if there was some "native" type of COLUMN_GET(@aaa, 'price'). Like, you can insert the value in its "native" type (relying on implicit type conversion):
INSERT INTO t1 VALUES ('');
or cast it explicitly:
INSERT INTO t1 VALUES (CAST('' AS DECIMAL))
But with dynamic columns there is no "native" type, you always *must* cast it to something. If strict mode won't turn these warnings into errors, there will be no way to do that. Normally you add a cast to insert with warnings and remove a cast to let strict mode turn warnings into errors (here I assume the case 3 from above is already fixed :). But with dynamic columns there is no cast to remove.
So I agree with you here, because logically there is an explicit conversion and strict mode should not turn this warning into an error. Still, if we do that, there will be no way to catch conversion warnings in dynamic columns. Unless you introduce STRICT_DYNAMIC_COLUMNS mode :)
Does this mean that we should also introduce STRICT_CAST, STRICT_OPERATORS for the things like: SELECT CAST('' AS DECIMAL); SELECT ''+1; ? :) Thanks.
Regards, Sergei
Hi, Alexander! On Jun 11, Alexander Barkov wrote:
I noticed some other problems:
1. DECIMAL/INT vs DOUBLE work differently on CAST
SET sql_mode='STRICT_ALL_TABLES'; DROP TABLE IF EXISTS t1; CREATE TABLE t1 (a DOUBLE); INSERT INTO t1 VALUES (CAST('' AS DOUBLE)); Query OK, 1 row affected (0.03 sec)
I.e. DOUBLE, unlike DECIMAL and INT, accepts empty string values silently with no any warnings.
This is because the CAST alone does not produce any warnings:
SELECT CAST('' AS DOUBLE);
2. DECIMAL/INT vs DOUBLE work differently on trailing spaces:
SET sql_mode='STRICT_ALL_TABLES'; DROP TABLE IF EXISTS t1; CREATE TABLE t1 (a DECIMAL); INSERT INTO t1 VALUES ('1 '); Query OK, 1 row affected (0.04 sec)
SET sql_mode='STRICT_ALL_TABLES'; DROP TABLE IF EXISTS t1; CREATE TABLE t1 (a INT); INSERT INTO t1 VALUES ('1 '); Query OK, 1 row affected (0.03 sec)
SET sql_mode='STRICT_ALL_TABLES'; DROP TABLE IF EXISTS t1; CREATE TABLE t1 (a DOUBLE); INSERT INTO t1 VALUES ('1 '); ERROR 1265 (01000): Data truncated for column 'a' at row 1
Compare to the behavior exposed by CAST, it's exactly the opposite: - CAST is stricter for DECIMAL/INT comparing to DOUBLE (on empty strings) - INSERT is weaker for DECINAL/INT comparing to DOUBLE (on trailing spaces)
I guess silently converting empty strings or silently ignoring trailing spaces is not correct.
I have a proposal, but not 100% sure yet:
Why not to use a NOTE rather than a WARNING for both empty values and trailing spaces, in all above cases:
- for all query parts: INSERT, CAST, COLUMN_GET - for all numeric data types: INT, DECIMAL, DOUBLE
When I was reading about these more cases I also thought of notes instead of warnings. But only for extra spaces, not for the empty value. Usually trailing spaces are always ok for a number. A parser ignores them: select 1 + 1; select 1 + 1; as you see, trailing spaces don't matter, so it's kind of reasonable when they won't matter in string-to-number conversion. But a completely empty string is not the same as 0, so a warning would be appropriate, I'd say.
I expect: - COLUMN_GET() to return 0 with a warning, like in #2 - INSERT to write 0 into the table normally, as I'm actually inserting the COLUMN_GET result, which is 0.
That wouldv'e been ok if there was some "native" type of COLUMN_GET(@aaa, 'price'). Like, you can insert the value in its "native" type (relying on implicit type conversion):
INSERT INTO t1 VALUES ('');
or cast it explicitly:
INSERT INTO t1 VALUES (CAST('' AS DECIMAL))
But with dynamic columns there is no "native" type, you always *must* cast it to something. If strict mode won't turn these warnings into errors, there will be no way to do that. Normally you add a cast to insert with warnings and remove a cast to let strict mode turn warnings into errors (here I assume the case 3 from above is already fixed :). But with dynamic columns there is no cast to remove.
So I agree with you here, because logically there is an explicit conversion and strict mode should not turn this warning into an error. Still, if we do that, there will be no way to catch conversion warnings in dynamic columns. Unless you introduce STRICT_DYNAMIC_COLUMNS mode :)
Does this mean that we should also introduce STRICT_CAST, STRICT_OPERATORS for the things like:
SELECT CAST('' AS DECIMAL); SELECT ''+1;
?
Well, I didn't say we *should* introduce STRICT_DYNAMIC_COLUMNS :) I could see of other approaches, like COLUMN_GET(@aaa, 'price' AS DECIMAL STRICT) COLUMN_GET_STRICT(@aaa, 'price' AS DECIMAL) STRICT(COLUMN_GET(@aaa, 'price' AS DECIMAL)) and I don't say we should do any of that either (although STRICT "function" is kinda cool). But I say that there will be no way to catch dynamic column conversion errors, if INSERT won't do that in strict mode. Regards, Sergei
Hi Sergei, On 06/10/2015 06:47 PM, Sergei Golubchik wrote:
SET sql_mode='STRICT_ALL_TABLES'; DROP TABLE IF EXISTS t1; CREATE TABLE t1 (a DECIMAL); ... INSERT INTO t1 VALUES(CAST('' AS DECIMAL)); ERROR 1292 (22007): Truncated incorrect DECIMAL value: ''
I think this is not OK.
I wrote an explicit CAST, so I expect: - CAST to return 0 with a warning, like in #2 - INSERT to write 0 into the table normally, as I'm actually inserting the CAST result, which is 0.
Agree
I created a separate issue for this: MDEV-8300 CAST('' AS DECIMAL) is too strict on INSERT in strict mode
participants (2)
-
Alexander Barkov -
Sergei Golubchik