← Back to team overview

maria-developers team mailing list archive

Re: 10.0 sprint: Please advise re MDEV-8109 unexpected CAST result

 

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


References