maria-developers team mailing list archive

[Alexander Barkov <bar@xxxxxxxxxxx>]

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