← Back to team overview

maria-developers team mailing list archive

Re: 8c1ad2a9fe9: MDEV-30633 DATETIME to TIMESTAMP conversion to return maximum timestamp on overflow

 

Hi, Alexander,

please add tests with fractional seconds.

remember, that in UTC the max timestamp(2) is 2038-01-19 03:14:07.99
and max timestamp(6) is 2038-01-19 03:14:07.999999

On Mar 08, Alexander Barkov wrote:
> revision-id: 8c1ad2a9fe9 (mariadb-10.11.1-48-g8c1ad2a9fe9)
> parent(s): ce4a289f1c3
> author: Alexander Barkov
> committer: Alexander Barkov
> timestamp: 2023-02-13 17:25:18 +0400
> message:
> 
> MDEV-30633 DATETIME to TIMESTAMP conversion to return maximum timestamp on overflow

a bit more verbose description would be nice

> diff --git a/mysql-test/main/events_bugs.result b/mysql-test/main/events_bugs.result
> --- a/mysql-test/main/events_bugs.result
> +++ b/mysql-test/main/events_bugs.result
> @@ -35,12 +35,34 @@ SET NAMES latin1;
>  set @a=3;
>  CREATE PROCEDURE p_16 () CREATE EVENT e_16 ON SCHEDULE EVERY @a SECOND DO SET @a=5;
>  ERROR HY000: Recursion of EVENT DDL statements is forbidden when body is present
> +SET time_zone='+00:00';
> +SET timestamp=UNIX_TIMESTAMP('2023-02-13 00:00:00');
>  create event e_55 on schedule at 99990101000000 do drop table t;
> -ERROR HY000: Incorrect AT value: '99990101000000'

Hmm, why did you not keep it an error?

> +Warnings:
> +Warning	1292	Truncated incorrect timestamp value: '9999-01-01 00:00:00'
> +Warning	1105	Event scheduler is switched off, use SET GLOBAL event_scheduler=ON to enable it.
> diff --git a/mysql-test/main/func_time.result b/mysql-test/main/func_time.result
> --- a/mysql-test/main/func_time.result
> +++ b/mysql-test/main/func_time.result
> @@ -599,19 +599,25 @@ Warnings:
>  Warning	1292	Truncated incorrect unixtime value: '2147483648'
>  select unix_timestamp('2039-01-20 01:00:00');
>  unix_timestamp('2039-01-20 01:00:00')
> -NULL
> +2147483647
> +Warnings:
> +Warning	1292	Truncated incorrect timestamp value: '2039-01-20 01:00:00'

it's consistent with

  MariaDB [test]> select cast(1e30 as int);
  +---------------------+
  | cast(1e30 as int)   |
  +---------------------+
  | 9223372036854775807 |
  +---------------------+
  1 row in set, 1 warning (0.000 sec)

  Note (Code 1916): Got overflow when converting '1e30' to SIGNED BIGINT. Value truncated

good.
Except that it's Note vs Warning. Want to unify this somehow?

>  select unix_timestamp('1968-01-20 01:00:00');
>  unix_timestamp('1968-01-20 01:00:00')
>  NULL

isn't it strange? That overflow is capped to max value, while underflow
is NULL?

> diff --git a/mysql-test/main/select.result b/mysql-test/main/select.result
> --- a/mysql-test/main/select.result
> +++ b/mysql-test/main/select.result
> @@ -3766,11 +3766,15 @@ AND t1.ts BETWEEN "2006-01-01" AND "2006-12-31";
>  id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
>  1	SIMPLE	t2	const	PRIMARY	PRIMARY	4	const	1	
>  1	SIMPLE	t1	range	ts	ts	4	NULL	2	Using index condition; Using where
> +Warnings:
> +Warning	1292	Truncated incorrect timestamp value: '2999-12-31 00:00:00'

Looks questionable. The query compares timestamp with a datetime. I
think in this case timestamp should be casted to a datetime (so, no
warning), not vice versa.

>  SELECT * FROM t1 LEFT JOIN t2 ON (t1.a=t2.a) WHERE t1.a=30
>  AND t1.ts BETWEEN t2.dt1 AND t2.dt2
>  AND t1.ts BETWEEN "2006-01-01" AND "2006-12-31";
>  a	ts	a	dt1	dt2
>  30	2006-01-03 23:00:00	30	2006-01-01 00:00:00	2999-12-31 00:00:00
> +Warnings:
> +Warning	1292	Truncated incorrect timestamp value: '2999-12-31 00:00:00'
>  DROP TABLE t1,t2;
>  create table t1 (a bigint unsigned);
>  insert into t1 values
> diff --git a/sql/item_timefunc.cc b/sql/item_timefunc.cc
> --- a/sql/item_timefunc.cc
> +++ b/sql/item_timefunc.cc
> @@ -2793,11 +2793,17 @@ bool Item_func_convert_tz::get_date(THD *thd, MYSQL_TIME *ltime,
>      return true;
>  
>    {
> -    uint not_used;
> -    my_time_tmp= from_tz->TIME_to_gmt_sec(ltime, &not_used);
> +    uint error_code;
> +    my_time_tmp= from_tz->TIME_to_gmt_sec(ltime, &error_code);
>      ulong sec_part= ltime->second_part;
> -    /* my_time_tmp is guaranteed to be in the allowed range */
> -    if (my_time_tmp)
> +    /*
> +      my_time_tmp is guaranteed to be in the allowed range.
> +      Don't perform the conversion in case the source DATETIME was above
> +      TIMESTAMP_MAX_VALUE (and was truncated to TIMESTAMP_MAX_VALUE).

why not?

> +    */   
> +    if (my_time_tmp &&
> +        (my_time_tmp != TIMESTAMP_MAX_VALUE ||
> +         error_code != ER_WARN_DATA_OUT_OF_RANGE))

why `my_time_tmp != TIMESTAMP_MAX_VALUE` ? shouldn't error_code check
be sufficient?

>        to_tz->gmt_sec_to_TIME(ltime, my_time_tmp);
>      /* we rely on the fact that no timezone conversion can change sec_part */
>      ltime->second_part= sec_part;

Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx