diff options
author | Alexander Barkov <bar@mariadb.com> | 2020-08-21 19:41:51 +0400 |
---|---|---|
committer | Alexander Barkov <bar@mariadb.com> | 2020-08-22 07:53:44 +0400 |
commit | ae33ebe5b32a82629a40e51c8d6c6611842fbd03 (patch) | |
tree | afe830989d22b8f9bc3af3dccb8c8013f4614d11 /sql/item.h | |
parent | aa6cb7ed03bb41b7ba59b6d7c9197cf24d65a36d (diff) | |
download | mariadb-git-ae33ebe5b32a82629a40e51c8d6c6611842fbd03.tar.gz |
MDEV-23525 Wrong result of MIN(time_expr) and MAX(time_expr) with GROUP BY
Problem:
When calculatung MIN() and MAX() in a query with GROUP BY, like this:
SELECT MIN(time_expr), MAX(time_expr) FROM t1 GROUP BY i;
the code in Item_sum_min_max::update_field() erroneosly used
string format comparison, therefore '100:20:30' was considered as
smaller than '10:20:30'.
Fix:
1. Implementing low level "native" related methods in class Time:
Time::Time(const Native &native) - convert native to Time
Time::to_native(Native *to, uint decimals) - convert Time to native
The "native" binary representation for TIME is equal to
the binary data format of Field_timef, which is used to
store TIME when mysql56_temporal_format is ON (default).
2. Implementing Type_handler_time_common "native" related methods:
Type_handler_time_common::cmp_native()
Type_handler_time_common::Item_val_native_with_conversion()
Type_handler_time_common::Item_val_native_with_conversion_result()
Type_handler_time_common::Item_param_val_native()
3. Implementing missing "native representation" related methods
in Field_time and Field_timef:
Field_time::store_native()
Field_time::val_native()
Field_timef::store_native()
Field_timef::val_native()
4. Implementing missing "native" related methods in all Items
that can have the TIME data type:
Item_timefunc::val_native()
Item_name_const::val_native()
Item_time_literal::val_native()
Item_cache_time::val_native()
Item_handled_func::val_native()
5. Marking Type_handler_time_common as "native ready".
So now Item_sum_min_max::update_field() calculates
values using min_max_update_native_field(),
which uses native binary representation rather than string representation.
Before this change, only the TIMESTAMP data type used native
representation to calculate MIN() and MAX().
Benchmarks (see more details in MDEV):
This change not only fixes the wrong result, but also
makes a "SELECT .. MAX.. GROUP BY .." query faster:
# TIME(0)
CREATE TABLE t1 (id INT, time_col TIME) ENGINE=HEAP;
INSERT INTO t1 VALUES (1,'10:10:10'); -- repeat this 1m times
SELECT id, MAX(time_col) FROM t1 GROUP BY id;
MySQL80: 0.159 sec
10.3: 0.108 sec
10.4: 0.094 sec (fixed)
# TIME(6):
CREATE TABLE t1 (id INT, time_col TIME(6)) ENGINE=HEAP;
INSERT INTO t1 VALUES (1,'10:10:10.999999'); -- repeat this 1m times
SELECT id, MAX(time_col) FROM t1 GROUP BY id;
My80: 0.154
10.3: 0.135
10.4: 0.093 (fixed)
Diffstat (limited to 'sql/item.h')
-rw-r--r-- | sql/item.h | 16 |
1 files changed, 12 insertions, 4 deletions
diff --git a/sql/item.h b/sql/item.h index 2d5cedbee76..aced2ec5f0d 100644 --- a/sql/item.h +++ b/sql/item.h @@ -1311,16 +1311,13 @@ public: { /* The default implementation for the Items that do not need native format: - - Item_basic_value + - Item_basic_value (default implementation) - Item_copy - Item_exists_subselect - Item_sum_field - Item_sum_or_func (default implementation) - Item_proc - Item_type_holder (as val_xxx() are never called for it); - - TODO: Item_name_const will need val_native() in the future, - when we add this syntax: - TIMESTAMP WITH LOCAL TIMEZONE'2001-01-01 00:00:00' These hybrid Item types override val_native(): - Item_field @@ -1331,6 +1328,8 @@ public: - Item_direct_ref - Item_direct_view_ref - Item_ref_null_helper + - Item_name_const + - Item_time_literal - Item_sum_or_func Note, these hybrid type Item_sum_or_func descendants override the default implementation: @@ -3139,6 +3138,7 @@ public: String *val_str(String *sp); my_decimal *val_decimal(my_decimal *); bool get_date(THD *thd, MYSQL_TIME *ltime, date_mode_t fuzzydate); + bool val_native(THD *thd, Native *to); bool is_null(); virtual void print(String *str, enum_query_type query_type); @@ -4897,6 +4897,10 @@ public: String *val_str(String *to) { return Time(this).to_string(to, decimals); } my_decimal *val_decimal(my_decimal *to) { return Time(this).to_decimal(to); } bool get_date(THD *thd, MYSQL_TIME *res, date_mode_t fuzzydate); + bool val_native(THD *thd, Native *to) + { + return Time(thd, this).to_native(to, decimals); + } Item *get_copy(THD *thd) { return get_item_copy<Item_time_literal>(thd, this); } }; @@ -6872,6 +6876,10 @@ public: { return has_value() ? Time(this).to_decimal(to) : NULL; } + bool val_native(THD *thd, Native *to) + { + return has_value() ? Time(thd, this).to_native(to, decimals) : true; + } }; |