- Fixing portabibily problems in sql-common/my_time.c
(and additionally in sql/sql_time.cc)
- Re-enabling func_time.test
Now all new chunks added in MDEV-17351 work fine on all platforms.
Problems:
Functions LEAST() and GREATEST() in TIME context, as well as functions
TIMESTAMP(a,b) and ADDTIME(a,b), returned confusing results when the
input TIME-alike value in a number or in a string was out of the TIME
supported range.
In case of TIMESTAMP(a,b) and ADDTIME(a,b), the second argument
value could get extra unexpected digits. For example, in:
ADDTIME('2001-01-01 00:00:00', 10000000) or
ADDTIME('2001-01-01 00:00:00', '1000:00:00')
the second argument was converted to '838:59:59.999999'
with six fractional digits, which contradicted "decimals"
previously set to 0 in fix_length_and_dec().
These unexpected fractional digits led to confusing function results.
Changes:
1. GREATEST(), LEAST()
- fixing Item_func_min_max::get_time_native()
to respect "decimals" set by fix_length_and_dec().
If a value of some numeric or string time-alike argument
goes outside of the TIME range and gets limited to '838:59:59.999999',
it's now right-truncated to the correct fractional precision.
- fixing, Type_handler_temporal_result::Item_func_min_max_fix_attributes()
to take into account arguments' time_precision() or datetime_precision(),
rather than rely on "decimals" calculated by the generic implementation
in Type_handler::Item_func_min_max_fix_attributes(). This makes
GREATEST() and LEAST() return better data types, with the same
fractional precision with what TIMESTAMP(a,b) and ADDTIME(a,b) return
for the same arguments, and with DATE(a) and TIMESTAMP(a).
2. Item_func_add_time and Item_func_timestamp
It was semantically wrong to apply the limit of the TIME data type
to the argument "b", which plays the role of "INTERVAL DAY TO SECOND" here.
Changing the code to fetch the argument "b" as INTERVAL rather than as TIME.
The low level routine calc_time_diff() now gets the interval
value without limiting to '838:59:59.999999', so in these examples:
ADDTIME('2001-01-01 00:00:00', 10000000)
ADDTIME('2001-01-01 00:00:00', '1000:00:00')
calc_time_diff() gets '1000:00:00' as is. The SQL function result
now gets limited to the supported result data type range
(datetime or time) inside calc_time_diff(), which now calculates
the return value using the real fractional digits that
came directly from the arguments (without the effect of limiting
to the TIME range), so the result does not have any unexpected
fractional digits any more.
Detailed changes in TIMESTAMP() and ADDTIME():
- Adding a new class Interval_DDhhmmssff. It's similar to Time, but:
* does not try to parse datetime format, as it's not needed for
functions TIMESTAMP() and ADDTIME().
* does not cut values to '838:59:59.999999'
The maximum supported Interval_DDhhmmssff's hard limit is
'UINT_MAX32:59:59.999999'. The maximum used soft limit is:
- '87649415:59:59.999999' (in 'hh:mm:ss.ff' format)
- '3652058 23:59:59.999999' (in 'DD hh:mm:ss.ff' format)
which is a difference between:
- TIMESTAMP'0001-01-01 00:00:00' and
- TIMESTAMP'9999-12-31 23:59:59.999999'
(the minimum datetime that supports arithmetic, and the
maximum possible datetime value).
- Fixing get_date() methods in the classes related to functions
ADDTIME(a,b) and TIMESTAMP(a,b) to use the new class Interval_DDhhmmssff
for fetching data from the second argument, instead of get_date().
- Fixing fix_length_and_dec() methods in the classes related
to functions ADDTIME(a,b) and TIMESTAMP(a,b) to use
Interval_DDhhmmssff::fsp(item) instead of item->time_precision()
to get the fractional precision of the second argument correctly.
- Splitting the low level function str_to_time() into smaller pieces
to reuse the code. Adding a new function str_to_DDhhmmssff(), to
parse "INTERVAL DAY TO SECOND" values.
After these changes, functions TIMESTAMP() and ADDTIME()
return much more predictable results, in terms of fractional
digits, and in terms of the overall result.
The full ranges of DATETIME and TIME values are now covered by TIMESTAMP()
and ADDTIME(), so the following can now be calculated:
SELECT ADDTIME(TIMESTAMP'0001-01-01 00:00:00', '87649415:59:59.999999');
-> '9999-12-31 23:59:59.999999'
SELECT TIMESTAMP(DATE'0001-01-01', '87649415:59:59.999999')
-> '9999-12-31 23:59:59.999999'
SELECT ADDTIME(TIME'-838:59:59.999999', '1677:59:59.999998');
-> '838:59:59.999999'
- Adding a helper class Sec6 to store (neg,seconds,microseconds)
- Adding a helper class VSec6 (Sec6 with a flag for "IS NULL")
- Wrapping related functions as methods of Sec6;
* number_to_datetime()
* number_to_time()
* my_decimal2seconds()
* Item::get_seconds()
* A big piece of code in Item_func_sec_to_time::get_date()
- Using the new classes in places where second-to-temporal
conversion takes place:
* Field_timestamp::store(double)
* Field_timestamp::store(longlong)
* Field_timestamp_with_dec::store_decimal(my_decimal)
* Field_temporal_with_date::store(double)
* Field_temporal_with_date::store(longlong)
* Field_time::store(double)
* Field_time::store(longlong)
* Field_time::store_decimal(my_decimal)
* Field_temporal_with_date::store_decimal(my_decimal)
* get_interval_value()
* Item_func_sec_to_time::get_date()
* Item_func_from_unixtime::get_date()
* Item_func_maketime::get_date()
This change simplifies these methods and functions a lot.
- Warnings are now sent at VSec6 initialization time, when the source
data is available in its original data type representation.
If Sec6::to_time() or Sec6::to_datetime() truncate data again during
conversion to MYSQL_TIME, they send warnings, but only if no warnings
were sent during VSec6 initialization. This helps prevents double warnings.
The call for val_str() in Item_func_sec_to_time::get_date() is not
needed any more, so it's removed. This change actually fixes the problem.
As a good effect, FROM_UNIXTIME() and MAKETIME() now also send warnings
in case if the seconds arguments is out of range. Previously these
functions returned NULL silently.
- Splitting the code in the global function make_truncated_value_warning()
into a number of methods THD::raise_warning_xxxx().
This was needed to reuse the logic that chooses between:
* ER_TRUNCATED_WRONG_VALUE
* ER_WRONG_VALUE
* ER_TRUNCATED_WRONG_VALUE_FOR_FIELD
for non-temporal data types (Sec6).
- Removing:
* Item::get_seconds()
* number_to_time_with_warn()
as this code now resides inside methods of Sec6.
- Cleanup (changes that are not directly related to the fix):
* Removing calls for field_name_or_null() and passing NULL instead
in Item_func_hybrid_field_type::get_date_from_{int|real}_op,
because Item_func_hybrid_field_type::field_name_or_null()
always returns NULL
* Replacing a number of calls for make_truncated_value_warning()
to calls for THD::raise_warning_xxx(). In these places
we know that the execution went through a certain
branch of make_truncated_value_warning(),
(e.g. the exact error code is known, or field name is always NULL,
or field name is always not-NULL). So calls for the entire
make_truncated_value_warning() after splitting are not necessary.
Being executed under slow_log is ON the test revealed a "side-effect"
in MDEV-8305 implementation which inadvertently made the trigger or
stored function statements to reset the top-level query's
THD::start_time et al. (Details of the test failure analysis are footnoted).
Unlike the SP case the SF and Trigger's internal statement should not
do that.
Fixed with revising the MDEV-8305 decision to backup/reset/restore
the session timestamp inside sp_instr_stmt::execute(). The timestamp
actually remains reset in the SP case by its caller per statement basis by ever
existing logics.
Timestamps related tests are extended to cover the trigger and stored function case.
Note, commit 3395ab7324 is reverted as its struct QUERY_START_TIME_INFO
declaration is not in use anymore after this patch.
Footnote:
--------
Specifically to the failing test, a query on the master was logged
okay with a timestamp of the query's top-level statement but its post
update trigger managed to compute one more (later) timestamp which got
inserted into another table. The latter table master-vs-slave
no fractional part timestamp discrepancy became evident
thanks to different execution time of the trigger combined with the
fact of the logged with micro-second fractional part master timestamp
was truncated on the slave. On master when the fractional part was
close to 1 the trigger execution added up its own latency to overflow
to next second value. That's how the master timestamp surprisingly
turned out to bigger than the slave's one.
