diff options
author | Alexander Barkov <bar@mariadb.com> | 2018-10-09 12:02:35 +0400 |
---|---|---|
committer | Alexander Barkov <bar@mariadb.com> | 2018-10-09 12:02:35 +0400 |
commit | 5646c4315918c1e4127bfb68b70d1c5333bfc2c7 (patch) | |
tree | 8b894deecb6371aa530ecebcd879dd5c2d52cf81 /sql/sql_time.cc | |
parent | f4cdf90d73aeffae37b400fa060bece5917cf4c2 (diff) | |
download | mariadb-git-5646c4315918c1e4127bfb68b70d1c5333bfc2c7.tar.gz |
MDEV-17216 Assertion `!dt->fraction_remainder(decimals())' failed in Field_temporal_with_date::store_TIME_with_warning
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()
Diffstat (limited to 'sql/sql_time.cc')
-rw-r--r-- | sql/sql_time.cc | 6 |
1 files changed, 3 insertions, 3 deletions
diff --git a/sql/sql_time.cc b/sql/sql_time.cc index c3019668497..2bbe7257eb0 100644 --- a/sql/sql_time.cc +++ b/sql/sql_time.cc @@ -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(); } |