maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Alexander!

First - I'm afraind this is the kind of a behavior change that we cannot
do in a GA version. Let's push it (when ready) into bb-10.2-bar.

On Sep 24, Alexander Barkov wrote:
> Hi Serg,
> 
> Please review my patch for MDEV-8109.
> 
> Thanks!

> diff --git a/mysql-test/suite/vcol/t/vcol_misc.test b/mysql-test/suite/vcol/t/vcol_misc.test
> index c5b534f..e5951ec 100644
> --- a/mysql-test/suite/vcol/t/vcol_misc.test
> +++ b/mysql-test/suite/vcol/t/vcol_misc.test
> @@ -310,6 +310,23 @@ SELECT COLUMN_GET(@aaa, 'price' AS DECIMAL) aaa;
>  SELECT COLUMN_GET(@aaa, 'price' AS INT) aaa;
>  SELECT COLUMN_GET(@aaa, 'price' AS DOUBLE) aaa;
>  
> +--echo #
> +--echo # MDEV-8109 unexpected CAST result
> +--echo #

why in vcol.vcol_misc? there are no virtual columns in this test.

> +SET sql_mode=STRICT_ALL_TABLES;
> +
> +CREATE FUNCTION test(par_aaa DECIMAL(10,2))
> +  RETURNS DECIMAL(10,2) DETERMINISTIC
> +  RETURN 0;
> +SET @aaa= COLUMN_CREATE('price', '');
> +SELECT COLUMN_GET(@aaa, 'price' AS DECIMAL) AS aaa;

didn't we agree preserve old behavior here? strict applies to
COLUMN_GET? Or do you want a different solution for COLUMN_GET?

> +SELECT COLUMN_GET(@aaa, 'price' AS DECIMAL) + 1 AS aaa;
> +SELECT test(COLUMN_GET(@aaa, 'price' AS DECIMAL)) AS bbb;
> +SELECT test(CAST('' AS DECIMAL(10,2))) AS bbb;
> +SELECT test(''+0) AS bbb;
> +DROP FUNCTION test;
> +
> +SET sql_mode=DEFAULT;
>  
>  --echo #
>  --echo # End of 10.1 tests
> diff --git a/mysql-test/t/strict.test b/mysql-test/t/strict.test
> index 93b3149..4414703 100644
> --- a/mysql-test/t/strict.test
> +++ b/mysql-test/t/strict.test
> @@ -992,11 +992,8 @@ insert into t1 (col1) values (cast(1000 as char(3)));
>  insert into t1 (col1) values (cast(1000E+0 as char(3)));
>  --error 1292
>  insert into t1 (col1) values (cast(1000.0 as char(3)));

why is this still an error?

> ---error 1292
>  insert into t1 (col2) values (cast('abc' as signed integer));
> ---error 1292
>  insert into t1 (col2) values (10E+0 + 'a');
> ---error 1292
>  insert into t1 (col2) values (cast('10a' as unsigned integer));
>  insert into t1 (col2) values (cast('10' as unsigned integer));
>  insert into t1 (col2) values (cast('10' as signed integer));
> @@ -1370,3 +1367,21 @@ SELECT STR_TO_DATE('2001','%Y'),CONCAT(STR_TO_DATE('2001','%Y')), STR_TO_DATE('2
>  --echo #
>  --echo # End of 5.6 tests
>  --echo #
> +
> +--echo #
> +--echo # Start of 10.1 tests
> +--echo #
> +
> +--echo #
> +--echo # MDEV-8300 CAST('' AS DECIMAL) is too strict on INSERT in strict mode
> +--echo #
> +SET sql_mode='STRICT_ALL_TABLES';
> +CREATE TABLE t1 (a DECIMAL,b INT, c DOUBLE);

also add SELECT tests for all these INSERTs.

SELECT CAST('' AS DECIMAL),CAST('' AS SIGNED),CAST('' AS DOUBLE);

> +INSERT INTO t1 VALUES(CAST('' AS DECIMAL),CAST('' AS SIGNED),CAST('' AS DOUBLE));
> +INSERT INTO t1 VALUES(''+0,''+0,''+0);
> +DROP TABLE t1;
> +SET sql_mode=DEFAULT;
> +
> +--echo #
> +--echo # End of 10.1 tests
> +--echo #
> diff --git a/sql/sql_class.cc b/sql/sql_class.cc
> index 0a0d3ca..beaf080 100644
> --- a/sql/sql_class.cc
> +++ b/sql/sql_class.cc
> @@ -1235,10 +1235,6 @@ Sql_condition* THD::raise_condition(uint sql_errno,
>    Sql_condition *cond= NULL;
>    DBUG_ENTER("THD::raise_condition");
>  
> -  if (!(variables.option_bits & OPTION_SQL_NOTES) &&
> -      (level == Sql_condition::WARN_LEVEL_NOTE))
> -    DBUG_RETURN(NULL);
> -

That makes me a bit uneasy. I would keep THD::raise_condition logic as
is, and introduced THD::raise_condition_verbatim (or
THD::raise_condition_no_escalation). That is all existing calls of
THD::raise_condition would've worked as before.

But now I'm not sure that you've replaced every existing invocation of
THD::raise_condition with THD::raise_condition_with_escalation.

And that we won't merge anything from MySQL that uses
THD::raise_condition.

At least you could've change the prototype of THD::raise_condition,
like, swap the arguments. Then old (and merged) code wouldn't compile.

>    da->opt_clear_warning_info(query_id);
>  
>    /*

Regards,
Sergei