Hi, Alexander, This has waited far too long :(
commit fec3696fef6 Author: Alexander Barkov <bar@mariadb.com> Date: Tue Feb 14 13:27:46 2023 +0400
diff --git a/sql/sql_class.h b/sql/sql_class.h index f0bf695a1c1..7378225eaa2 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -3900,7 +3900,8 @@ class THD: public THD_count, /* this must be first */ } const Type_handler *type_handler_for_datetime() const; bool timestamp_to_TIME(MYSQL_TIME *ltime, my_time_t ts, - ulong sec_part, date_mode_t fuzzydate); + ulong sec_part, date_mode_t fuzzydate, + bool zero_timesteamp_means_zero_datetime);
I don't understand it. NOW() cannot return a zero timestamp, how this feature can possibly need changes to zero timestamp treatment?
inline my_time_t query_start() { return start_time; } inline ulong query_start_sec_part() { used|= QUERY_START_SEC_PART_USED; return start_time_sec_part; } diff --git a/sql/item_timefunc.h b/sql/item_timefunc.h index a5f6d9307c6..1c463c8a332 100644 --- a/sql/item_timefunc.h +++ b/sql/item_timefunc.h @@ -707,6 +707,49 @@ class Item_datetimefunc :public Item_func };
+class Item_timestampfunc: public Item_func +{ +protected: + Datetime to_datetime(THD *thd) + { + return Timestamp_or_zero_datetime_native_null(thd, this).to_datetime(thd);
how does it differ from Datetime(this) ?
+ } +public: + Item_timestampfunc(THD *thd): Item_func(thd) {} + Item_timestampfunc(THD *thd, Item *a): Item_func(thd, a) {} + const Type_handler *type_handler() const override + { return &type_handler_timestamp2; } + bool get_date(THD *thd, MYSQL_TIME *ltime, date_mode_t fuzzydate) override + { + return null_value= to_datetime(thd).copy_to_mysql_time(ltime); + } + double val_real() override + { + Datetime dt= to_datetime(current_thd); + null_value= !dt.is_valid_datetime(); + return dt.to_double();
may be `return null_value ? 0 : dt.to_double()` ? why to call to_double if it isn't a valid datetime? (same everywhere below)
+ } + longlong val_int() override + { + Datetime dt= to_datetime(current_thd); + null_value= !dt.is_valid_datetime(); + return dt.to_longlong(); + } + my_decimal *val_decimal(my_decimal *to) override + { + Datetime dt= to_datetime(current_thd); + null_value= !dt.is_valid_datetime(); + return dt.to_decimal(to); + } + String *val_str(String *to) override + { + Datetime dt= to_datetime(current_thd); + null_value= !dt.is_valid_datetime(); + return dt.to_string(to, decimals); + } +}; + + /* Abstract CURTIME function. Children should define what time zone is used */
class Item_func_curtime :public Item_timefunc @@ -837,20 +880,48 @@ class Item_func_now :public Item_datetimefunc };
-class Item_func_now_local :public Item_func_now +/* + This is a replacement for Item_func_now_local, + returning TIMESTAMP instead of DATETIME.
why did you rename it? why didn't you change Item_func_now_utc?
+*/ +class Item_func_current_timestamp: public Item_timestampfunc { public: - Item_func_now_local(THD *thd, uint dec): Item_func_now(thd, dec) {} + Item_func_current_timestamp(THD *thd, uint dec) + :Item_timestampfunc(thd) + { decimals= dec; }
why not Item_timestampfunc(thd, dec) ?
LEX_CSTRING func_name_cstring() const override { static LEX_CSTRING name= {STRING_WITH_LEN("current_timestamp") }; return name; } - int save_in_field(Field *field, bool no_conversions) override; - void store_now_in_TIME(THD *thd, MYSQL_TIME *now_time) override; + void print(String *str, enum_query_type query_type) override + { + str->append(func_name_cstring()); + str->append('('); + if (decimals) + str->append_ulonglong(decimals); + str->append(')'); + } + bool fix_length_and_dec(THD *thd) override + { + if (check_decimal_scale_or_error(TIME_SECOND_PART_DIGITS)) + return true; + fix_attributes_datetime(decimals); + return false; + } + bool val_native(THD *thd, Native *to) override; + bool check_vcol_func_processor(void *arg) override + { + /* + NOW is safe for replication as slaves will run with same time as + master + */
what does this replication comment do in check_vcol_func_processor? (I know it was in the old code too, but "old code did it" never stopped your cleanups before :)
+ return mark_unsupported_function(func_name(), "()", arg, VCOL_TIME_FUNC); + } enum Functype functype() const override { return NOW_FUNC; } Item *get_copy(THD *thd) override - { return get_item_copy<Item_func_now_local>(thd, this); } + { return get_item_copy<Item_func_current_timestamp>(thd, this); } };
diff --git a/sql/item_timefunc.cc b/sql/item_timefunc.cc index 52b9ae7a682..3e224e3e6f7 100644 --- a/sql/item_timefunc.cc +++ b/sql/item_timefunc.cc @@ -1642,32 +1633,10 @@ void Item_func_now::print(String *str, enum_query_type query_type) }
-int Item_func_now_local::save_in_field(Field *field, bool no_conversions) +bool Item_func_current_timestamp::val_native(THD *thd, Native *to) { - if (field->type() == MYSQL_TYPE_TIMESTAMP) - { - THD *thd= field->get_thd(); - my_time_t ts= thd->query_start(); - ulong sec_part= decimals ? thd->query_start_sec_part() : 0; - sec_part-= my_time_fraction_remainder(sec_part, decimals); - field->set_notnull(); - field->store_timestamp(ts, sec_part); - return 0; - } - else - return Item_datetimefunc::save_in_field(field, no_conversions); -} - - -/** - Converts current time in my_time_t to MYSQL_TIME representation for local - time zone. Defines time zone (local) used for whole NOW function. -*/ -void Item_func_now_local::store_now_in_TIME(THD *thd, MYSQL_TIME *now_time) -{ - thd->variables.time_zone->gmt_sec_to_TIME(now_time, thd->query_start()); - set_sec_part(thd->query_start_sec_part(), now_time, this); - thd->used|= THD::TIME_ZONE_USED; + Timestamp ts(Timeval(thd->query_start(), thd->query_start_sec_part())); + return null_value= ts.trunc(decimals).to_native(to, decimals);
can this possibly fail?
}
@@ -2732,7 +2693,6 @@ String *Item_func_tochar::val_str(String* str)
bool Item_func_from_unixtime::fix_length_and_dec(THD *thd) { - thd->used|= THD::TIME_ZONE_USED;
would be nice to have a test case for this. Like direct insert into a timestamp column no longer uses timezone - e.g. timestamp within dst switch time is inserted correctly. and vice versa, when from_unixtime is converted to a string, like `insert ... values(concat(from_unixtime(...)))` then TIME_ZONE_USED is still set and show binlog events (or mysqlbinlog) should show that the timezone was correctly binlogged, as before.
tz= thd->variables.time_zone; Type_std_attributes::set( Type_temporal_attributes_not_fixed_dec(MAX_DATETIME_WIDTH,
Regards, Sergei Chief Architect, MariaDB Server and security@mariadb.org