database/mariadb
1
0
Fork 0
mirror of https://github.com/MariaDB/server.git synced 2025-07-30 16:24:05 +03:00
Code Activity

MDEV-17216 Assertion `!dt->fraction_remainder(decimals())' failed in Field_temporal_with_date::store_TIME_with_warning

Browse Source 
The problem happened because {{Field_xxx::store(longlong nr, bool unsigned_val)}} erroneously passed {{unsigned_flag}} to the {{usec}} parameter of this constructor:
{code:cpp}
Datetime(int *warn, longlong sec, ulong usec, date_conv_mode_t flags)
{code}

1. Changing Time and Datetime constructors to accept data as Sec6 rather than as
longlong/double/my_decimal, so it's not possible to do such mistakes
in the future. Additional good effect of these changes:
- This reduced some amount of similar code (minus ~35 lines).
- The code now does not rely on the fact that "unsigned_flag" is
   not important inside Datetime().
  The constructor always gets all three parts: sign, integer part,
  fractional part. The simple the better.

2. Fixing Field_xxx::store() to use the new Datetime constructor format.
   This change actually fixes the problem.

3. Adding "explicit" keyword to all Sec6 constructors,
to avoid automatic hidden conversion from double/my_decimal to Sec6,
as well as from longlong/ulonglong through double to Sec6.

4. Change#1 caused (as a dependency) changes in a few places
   with code like this:

  bool neg= nr < 0 && !unsigned_val;
  ulonglong value= m_neg ? (ulonglong) -nr : (ulonglong) nr;

These fragments relied on a non-standard behavior with
the operator "minus" applied to the lowest possible negative
signed long long value. This can lead to different results
depending on the platform and compilation flags.
We have fixed such bugs a few times already.
So instead of modifying the old wrong code to a new wrong code,
replacing all such fragments to use Longlong_hybrid,
which correctly handles this special case with -LONGLONG_MIN
in its method abs().
This also reduced the amount of similar code
(1 or 2 new lines instead 3 old lines in all 6 such fragments).

5. Removing ErrConvInteger(longlong nr, bool unsigned_flag= false)
   and adding ErrConvInteger(Longlong_hybrid) instead, to encourage
   use of safe Longlong_hybrid instead of unsafe pairs nr+neg.

6. Removing unused ErrConvInteger from Item_cache_temporal::get_date()
This commit is contained in:
Alexander Barkov
2018-10-09 12:02:35 +04:00
parent f4cdf90d73
commit 5646c43159
15 changed files with 146 additions and 106 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
mysql-test/main/type_date.result
View File

@ -988,5 +988,15 @@ Note 1265 Data truncated for column 'a' at row 1
DROP TABLE t1;
SET sql_mode=DEFAULT;
#
# MDEV-17216 Assertion `!dt->fraction_remainder(decimals())' failed in Field_temporal_with_date::store_TIME_with_warning
#
SET sql_mode='';
CREATE TABLE t1 (i1 date );
CREATE TABLE t2 (i2 int unsigned );
INSERT INTO t2 VALUES (0);
INSERT INTO t1 SELECT * FROM t2;
DROP TABLE t1,t2;
SET sql_mode=DEFAULT;
#
# End of 10.4 tests
#

12
mysql-test/main/type_date.test
View File

@ -665,6 +665,18 @@ DROP TABLE t1;
SET sql_mode=DEFAULT;
--echo #
--echo # MDEV-17216 Assertion `!dt->fraction_remainder(decimals())' failed in Field_temporal_with_date::store_TIME_with_warning
--echo #
SET sql_mode='';
CREATE TABLE t1 (i1 date );
CREATE TABLE t2 (i2 int unsigned );
INSERT INTO t2 VALUES (0);
INSERT INTO t1 SELECT * FROM t2;
DROP TABLE t1,t2;
SET sql_mode=DEFAULT;
--echo #
--echo # End of 10.4 tests
--echo #

11
mysql-test/main/type_datetime.result
View File

@ -1400,5 +1400,16 @@ Warnings:
Note 1003 select `test`.`t1`.`a` AS `a` from `test`.`t1` where octet_length(coalesce(TIME'00:00:00.0',`test`.`t1`.`a`)) <=> octet_length(coalesce(<cache>(TIMESTAMP'2001-01-01 00:00:00.00'),`test`.`t1`.`a`))
DROP TABLE t1;
#
# MDEV-17216 Assertion `!dt->fraction_remainder(decimals())' failed in Field_temporal_with_date::store_TIME_with_warning
#
CREATE TABLE t1 (b BIT(20));
CREATE TABLE t2 (t DATETIME);
INSERT IGNORE INTO t1 VALUES (b'000001001100000');
INSERT INTO t2 SELECT * FROM t1;
DROP TABLE t1, t2;
CREATE TABLE t1 (a DATETIME);
INSERT INTO t1 SELECT CAST(20010101 AS UNSIGNED);
DROP TABLE t1;
#
# End of 10.4 tests
#

15
mysql-test/main/type_datetime.test
View File

@ -898,6 +898,21 @@ EXECUTE IMMEDIATE 'EXPLAIN EXTENDED SELECT * FROM t1 WHERE LENGTH(COALESCE(TIME'
DROP TABLE t1;
--echo #
--echo # MDEV-17216 Assertion `!dt->fraction_remainder(decimals())' failed in Field_temporal_with_date::store_TIME_with_warning
--echo #
CREATE TABLE t1 (b BIT(20));
CREATE TABLE t2 (t DATETIME);
INSERT IGNORE INTO t1 VALUES (b'000001001100000');
INSERT INTO t2 SELECT * FROM t1;
DROP TABLE t1, t2;
CREATE TABLE t1 (a DATETIME);
INSERT INTO t1 SELECT CAST(20010101 AS UNSIGNED);
DROP TABLE t1;
--echo #
--echo # End of 10.4 tests
--echo #

17
mysql-test/main/type_timestamp.result
View File

@ -1018,3 +1018,20 @@ DROP TABLE t1;
#
# End of 10.3 tests
#
#
# Start of 10.4 tests
#
#
# MDEV-17216 Assertion `!dt->fraction_remainder(decimals())' failed in Field_temporal_with_date::store_TIME_with_warning
#
CREATE TABLE t1 (b BIT(20));
CREATE TABLE t2 (t TIMESTAMP);
INSERT IGNORE INTO t1 VALUES (b'000001001100000');
INSERT INTO t2 SELECT * FROM t1;
DROP TABLE t1, t2;
CREATE TABLE t1 (a TIMESTAMP);
INSERT INTO t1 SELECT CAST(20010101 AS UNSIGNED);
DROP TABLE t1;
#
# End of 10.4 tests
#

22
mysql-test/main/type_timestamp.test
View File

@ -606,3 +606,25 @@ DROP TABLE t1;
--echo #
--echo # End of 10.3 tests
--echo #
--echo #
--echo # Start of 10.4 tests
--echo #
--echo #
--echo # MDEV-17216 Assertion `!dt->fraction_remainder(decimals())' failed in Field_temporal_with_date::store_TIME_with_warning
--echo #
CREATE TABLE t1 (b BIT(20));
CREATE TABLE t2 (t TIMESTAMP);
INSERT IGNORE INTO t1 VALUES (b'000001001100000');
INSERT INTO t2 SELECT * FROM t1;
DROP TABLE t1, t2;
CREATE TABLE t1 (a TIMESTAMP);
INSERT INTO t1 SELECT CAST(20010101 AS UNSIGNED);
DROP TABLE t1;
--echo #
--echo # End of 10.4 tests
--echo #

40
sql/field.cc
View File

@ -2067,9 +2067,8 @@ my_decimal* Field_int::val_decimal(my_decimal *decimal_value)
bool Field_int::get_date(MYSQL_TIME *ltime,date_mode_t fuzzydate)
{
ASSERT_COLUMN_MARKED_FOR_READ;
longlong nr= val_int();
bool neg= !(flags & UNSIGNED_FLAG) && nr < 0;
return int_to_datetime_with_warn(get_thd(), neg, neg ? -nr : nr, ltime,
Longlong_hybrid nr(val_int(), (flags & UNSIGNED_FLAG));
return int_to_datetime_with_warn(get_thd(), nr, ltime,
fuzzydate, field_name.str);
}
@ -5112,7 +5111,7 @@ int Field_timestamp::store(double nr)
int error;
ErrConvDouble str(nr);
THD *thd= get_thd();
Datetime dt(&error, nr, sql_mode_for_timestamp(thd), decimals());
Datetime dt(&error, Sec6(nr), sql_mode_for_timestamp(thd), decimals());
return store_TIME_with_warning(thd, &dt, &str, error);
}
@ -5120,9 +5119,9 @@ int Field_timestamp::store(double nr)
int Field_timestamp::store(longlong nr, bool unsigned_val)
{
int error;
ErrConvInteger str(nr, unsigned_val);
ErrConvInteger str(Longlong_hybrid(nr, unsigned_val));
THD *thd= get_thd();
Datetime dt(&error, nr, unsigned_val, sql_mode_for_timestamp(thd));
Datetime dt(&error, Sec6(nr, unsigned_val), sql_mode_for_timestamp(thd));
return store_TIME_with_warning(thd, &dt, &str, error);
}
@ -5422,7 +5421,7 @@ int Field_timestamp::store_decimal(const my_decimal *d)
int error;
THD *thd= get_thd();
ErrConvDecimal str(d);
Datetime dt(&error, d, sql_mode_for_timestamp(thd), decimals());
Datetime dt(&error, Sec6(d), sql_mode_for_timestamp(thd), decimals());
return store_TIME_with_warning(thd, &dt, &str, error);
}
@ -5572,7 +5571,7 @@ int Field_datetime::store(double nr)
{
int error;
ErrConvDouble str(nr);
Datetime dt(&error, nr, sql_mode_for_dates(get_thd()), decimals());
Datetime dt(&error, Sec6(nr), sql_mode_for_dates(get_thd()), decimals());
return store_TIME_with_warning(&dt, &str, error);
}
@ -5580,8 +5579,8 @@ int Field_datetime::store(double nr)
int Field_datetime::store(longlong nr, bool unsigned_val)
{
int error;
ErrConvInteger str(nr, unsigned_val);
Datetime dt(&error, nr, unsigned_val, sql_mode_for_dates(get_thd()));
ErrConvInteger str(Longlong_hybrid(nr, unsigned_val));
Datetime dt(&error, Sec6(nr, unsigned_val), sql_mode_for_dates(get_thd()));
return store_TIME_with_warning(&dt, &str, error);
}
@ -5599,7 +5598,7 @@ int Field_datetime::store_decimal(const my_decimal *d)
{
int error;
ErrConvDecimal str(d);
Datetime tm(&error, d, sql_mode_for_dates(get_thd()), decimals());
Datetime tm(&error, Sec6(d), sql_mode_for_dates(get_thd()), decimals());
return store_TIME_with_warning(&tm, &str, error);
}
@ -5734,17 +5733,17 @@ int Field_time::store(double nr)
{
ErrConvDouble str(nr);
int was_cut;
Time tm(get_thd(), &was_cut, nr, decimals());
Time tm(get_thd(), &was_cut, Sec6(nr), decimals());
return store_TIME_with_warning(&tm, &str, was_cut);
}
int Field_time::store(longlong nr, bool unsigned_val)
{
ErrConvInteger str(nr, unsigned_val);
ErrConvInteger str(Longlong_hybrid(nr, unsigned_val));
int was_cut;
// Need fractional digit truncation if nr overflows to '838:59:59.999999'
Time tm(get_thd(), &was_cut, nr, unsigned_val, decimals());
Time tm(get_thd(), &was_cut, Sec6(nr, unsigned_val), decimals());
return store_TIME_with_warning(&tm, &str, was_cut);
}
@ -5902,7 +5901,7 @@ int Field_time::store_decimal(const my_decimal *d)
{
ErrConvDecimal str(d);
int was_cut;
Time tm(get_thd(), &was_cut, d, decimals());
Time tm(get_thd(), &was_cut, Sec6(d), decimals());
return store_TIME_with_warning(&tm, &str, was_cut);
}
@ -6222,7 +6221,8 @@ bool Field_year::get_date(MYSQL_TIME *ltime,date_mode_t fuzzydate)
int tmp= (int) ptr[0];
if (tmp || field_length != 4)
tmp+= 1900;
return int_to_datetime_with_warn(get_thd(), false, tmp * 10000,
return int_to_datetime_with_warn(get_thd(),
Longlong_hybrid(tmp * 10000, true),
ltime, fuzzydate, field_name.str);
}
@ -6266,15 +6266,15 @@ int Field_date_common::store(double nr)
{
int error;
ErrConvDouble str(nr);
Datetime dt(&error, nr, sql_mode_for_dates(get_thd()));
Datetime dt(&error, Sec6(nr), sql_mode_for_dates(get_thd()));
return store_TIME_with_warning(&dt, &str, error);
}
int Field_date_common::store(longlong nr, bool unsigned_val)
{
int error;
ErrConvInteger str(nr, unsigned_val);
Datetime dt(&error, nr, unsigned_val, sql_mode_for_dates(get_thd()));
ErrConvInteger str(Longlong_hybrid(nr, unsigned_val));
Datetime dt(&error, Sec6(nr, unsigned_val), sql_mode_for_dates(get_thd()));
return store_TIME_with_warning(&dt, &str, error);
}
@ -6291,7 +6291,7 @@ int Field_date_common::store_decimal(const my_decimal *d)
{
int error;
ErrConvDecimal str(d);
Datetime tm(&error, d, sql_mode_for_dates(get_thd()));
Datetime tm(&error, Sec6(d), sql_mode_for_dates(get_thd()));
return store_TIME_with_warning(&tm, &str, error);
}

7
sql/item.cc
View File

@ -1276,9 +1276,8 @@ Item *Item_param::safe_charset_converter(THD *thd, CHARSET_INFO *tocs)
bool Item::get_date_from_int(THD *thd, MYSQL_TIME *ltime, date_mode_t fuzzydate)
{
longlong value= val_int();
bool neg= !unsigned_flag && value < 0;
if (null_value || int_to_datetime_with_warn(thd, neg, neg ? -value : value,
Longlong_hybrid value(val_int(), unsigned_flag);
if (null_value || int_to_datetime_with_warn(thd, value,
ltime, fuzzydate,
field_name_or_null()))
return null_value|= make_zero_date(ltime, fuzzydate);
@ -9701,8 +9700,6 @@ bool Item_cache_time::cache_value()
bool Item_cache_temporal::get_date(THD *thd, MYSQL_TIME *ltime, date_mode_t fuzzydate)
{
ErrConvInteger str(value);
if (!has_value())
{
bzero((char*) ltime,sizeof(*ltime));

5
sql/item_func.cc
View File

@ -838,9 +838,8 @@ bool Item_func_hybrid_field_type::get_date_from_int_op(THD *thd,
MYSQL_TIME *ltime,
date_mode_t fuzzydate)
{
longlong value= int_op();
bool neg= !unsigned_flag && value < 0;
if (null_value || int_to_datetime_with_warn(thd, neg, neg ? -value : value,
Longlong_hybrid value(int_op(), unsigned_flag);
if (null_value || int_to_datetime_with_warn(thd, value,
ltime, fuzzydate, NULL))
return make_zero_mysql_time(ltime, fuzzydate);
return (null_value= 0);

4
sql/item_strfunc.cc
View File

@ -5134,9 +5134,7 @@ bool Item_dyncol_get::get_date(THD *thd, MYSQL_TIME *ltime, date_mode_t fuzzydat
if (signed_value || val.x.ulong_value <= LONGLONG_MAX)
{
longlong llval = (longlong)val.x.ulong_value;
bool neg = llval < 0;
if (int_to_datetime_with_warn(thd, neg, (ulonglong)(neg ? -llval :
llval),
if (int_to_datetime_with_warn(thd, Longlong_hybrid(llval, !signed_value),
ltime, fuzzydate, 0 /* TODO */))
goto null;
return 0;

4
sql/sql_error.h
View File

@ -846,8 +846,8 @@ public:
class ErrConvInteger : public ErrConv, public Longlong_hybrid
{
public:
ErrConvInteger(longlong num_arg, bool unsigned_flag= false) :
ErrConv(), Longlong_hybrid(num_arg, unsigned_flag) {}
ErrConvInteger(const Longlong_hybrid &nr)
: ErrConv(), Longlong_hybrid(nr) { }
const char *ptr() const
{
return m_unsigned ? ullstr(m_value, err_buffer) :

6
sql/sql_time.cc
View File

@ -467,17 +467,17 @@ bool decimal_to_datetime_with_warn(THD *thd, const my_decimal *value,
}
bool int_to_datetime_with_warn(THD *thd, bool neg, ulonglong value,
bool int_to_datetime_with_warn(THD *thd, const Longlong_hybrid &nr,
MYSQL_TIME *ltime,
date_mode_t fuzzydate, const char *field_name)
{
const ErrConvInteger str(neg ? - (longlong) value : (longlong) value, !neg);
const ErrConvInteger str(nr);
/*
Note: conversion from an integer to TIME can overflow to '838:59:59.999999',
so the conversion result can have fractional digits.
*/
Temporal_hybrid *t= new (ltime)
Temporal_hybrid(thd, Sec6(neg, value, 0),
Temporal_hybrid(thd, Sec6(nr),
fuzzydate, &str, field_name);
return !t->is_valid_temporal();
}

2
sql/sql_time.h
View File

@ -50,7 +50,7 @@ bool decimal_to_datetime_with_warn(THD *thd,
const my_decimal *value, MYSQL_TIME *ltime,
date_mode_t fuzzydate,
const char *name);
bool int_to_datetime_with_warn(THD *thd, bool neg, ulonglong value,
bool int_to_datetime_with_warn(THD *thd, const Longlong_hybrid &nr,
MYSQL_TIME *ltime,
date_mode_t fuzzydate,
const char *name);

6
sql/sql_type.cc
View File

@ -253,14 +253,14 @@ VSec6::VSec6(THD *thd, Item *item, const char *type_str, ulonglong limit)
{
if (item->decimals == 0)
{ // optimize for an important special case
longlong nr= item->val_int();
make_from_int(nr, item->unsigned_flag);
Longlong_hybrid nr(item->val_int(), item->unsigned_flag);
make_from_int(nr);
m_is_null= item->null_value;
if (!m_is_null && m_sec > limit)
{
m_sec= limit;
m_truncated= true;
ErrConvInteger err(nr, item->unsigned_flag);
ErrConvInteger err(nr);
thd->push_warning_truncated_wrong_value(type_str, err.ptr());
}
}

91
sql/sql_type.h
View File

@ -225,10 +225,10 @@ protected:
bool m_truncated; // Indicates if the constructor truncated the value
void make_from_decimal(const my_decimal *d);
void make_from_double(double d);
void make_from_int(longlong nr, bool unsigned_val)
void make_from_int(const Longlong_hybrid &nr)
{
m_neg= nr < 0 && !unsigned_val;
m_sec= m_neg ? (ulonglong) -nr : (ulonglong) nr;
m_neg= nr.neg();
m_sec= nr.abs();
m_usec= 0;
m_truncated= false;
}
@ -238,20 +238,21 @@ protected:
}
Sec6() { }
public:
Sec6(bool neg, ulonglong nr, ulong frac)
:m_sec(nr), m_usec(frac), m_neg(neg), m_truncated(false)
{ }
Sec6(double nr)
explicit Sec6(double nr)
{
make_from_double(nr);
}
Sec6(const my_decimal *d)
explicit Sec6(const my_decimal *d)
{
make_from_decimal(d);
}
Sec6(longlong nr, bool unsigned_val)
explicit Sec6(const Longlong_hybrid &nr)
{
make_from_int(nr, unsigned_val);
make_from_int(nr);
}
explicit Sec6(longlong nr, bool unsigned_val)
{
make_from_int(Longlong_hybrid(nr, unsigned_val));
}
bool neg() const { return m_neg; }
bool truncated() const { return m_truncated; }
@ -396,9 +397,10 @@ protected:
bool to_mysql_time_with_warn(THD *thd, MYSQL_TIME *to, date_mode_t fuzzydate,
const char *field_name) const
{
longlong value= m_year * 10000; // Make it YYYYMMDD
const ErrConvInteger str(value, true);
Sec6 sec(false, value, 0);
// Make it YYYYMMDD
Longlong_hybrid value(static_cast<ulonglong>(m_year) * 10000, true);
const ErrConvInteger str(value);
Sec6 sec(value);
return sec.convert_to_mysql_time(thd, to, fuzzydate, &str, field_name);
}
uint year_precision(const Item *item) const;
@ -846,14 +848,8 @@ public:
time_type= MYSQL_TIMESTAMP_NONE;
xxx_to_time_result_to_valid_value(thd, warn, opt);
}
Time(THD *thd, int *warn, double nr)
:Time(thd, warn, Sec6(nr), Options())
{ }
Time(THD *thd, int *warn, longlong nr, bool unsigned_val)
:Time(thd, warn, Sec6(nr, unsigned_val), Options())
{ }
Time(THD *thd, int *warn, const my_decimal *d)
:Time(thd, warn, Sec6(d), Options())
Time(THD *thd, int *warn, const Sec6 &nr)
:Time(thd, warn, nr, Options())
{ }
Time(THD *thd, Item *item, const Options opt, uint dec)
@ -873,25 +869,11 @@ public:
{
trunc(dec);
}
Time(THD *thd, int *warn, double nr, uint dec)
Time(THD *thd, int *warn, const Sec6 &nr, uint dec)
:Time(thd, warn, nr)
{
trunc(dec);
}
Time(THD *thd, int *warn, longlong nr, bool unsigned_val, uint dec)
:Time(thd, warn, nr, unsigned_val)
{
/*
Decimal digit truncation is needed here in case if nr was out
of the supported TIME range, so "this" was set to '838:59:59.999999'.
*/
trunc(dec);
}
Time(THD *thd, int *warn, const my_decimal *d, uint dec)
:Time(thd, warn, d)
{
trunc(dec);
}
static date_mode_t flags_for_get_date()
{ return TIME_TIME_ONLY | TIME_INVALID_DATES; }
@ -1242,56 +1224,33 @@ public:
date_to_datetime_if_needed();
DBUG_ASSERT(is_valid_value_slow());
}
Datetime(int *warn, double nr, date_mode_t flags)
:Temporal_with_date(warn, Sec6(nr), flags)
{
date_to_datetime_if_needed();
DBUG_ASSERT(is_valid_value_slow());
}
Datetime(int *warn, const my_decimal *d, date_mode_t flags)
:Temporal_with_date(warn, Sec6(d), flags)
{
date_to_datetime_if_needed();
DBUG_ASSERT(is_valid_value_slow());
}
/*
Create a Datime object from a longlong number.
Note, unlike in Time(), we don't need an "unsigned_val" here,
as it's not important if overflow happened because
of a negative number, or because of a huge positive number.
*/
Datetime(int *warn, longlong sec, ulong usec, date_mode_t flags)
:Temporal_with_date(warn, Sec6(false, (ulonglong) sec, usec), flags)
Datetime(int *warn, const Sec6 &nr, date_mode_t flags)
:Temporal_with_date(warn, nr, flags)
{
date_to_datetime_if_needed();
DBUG_ASSERT(is_valid_value_slow());
}
Datetime(THD *thd, Item *item, date_mode_t flags, uint dec)
:Temporal_with_date(Datetime(thd, item, flags))
:Datetime(thd, item, flags)
{
trunc(dec);
}
Datetime(MYSQL_TIME_STATUS *status,
const char *str, size_t len, CHARSET_INFO *cs,
date_mode_t fuzzydate, uint dec)
:Temporal_with_date(Datetime(status, str, len, cs, fuzzydate))
:Datetime(status, str, len, cs, fuzzydate)
{
trunc(dec);
}
Datetime(int *warn, double nr, date_mode_t fuzzydate, uint dec)
:Temporal_with_date(Datetime(warn, nr, fuzzydate))
{
trunc(dec);
}
Datetime(int *warn, const my_decimal *d, date_mode_t fuzzydate, uint dec)
:Temporal_with_date(Datetime(warn, d, fuzzydate))
Datetime(int *warn, const Sec6 &nr, date_mode_t fuzzydate, uint dec)
:Datetime(warn, nr, fuzzydate)
{
trunc(dec);
}
Datetime(THD *thd, int *warn, const MYSQL_TIME *from,
date_mode_t fuzzydate, uint dec)
:Temporal_with_date(Datetime(thd, warn, from, fuzzydate))
:Datetime(thd, warn, from, fuzzydate)
{
trunc(dec);
}