maria-developers team mailing list archive

[Alexander Barkov <bar@xxxxxxxxxxx>]

Hi Alexey,


On 11/25/2018 09:05 AM, Alexander Barkov wrote:
>>
>> What do you think if we do this:
>>   - add THD* memver to the MYSQL_TIME_STATUS structure,
>>     so we can get rid of the THD * parameter to many functions?
>> - send one MYSQL_TIME_STATUS* to the add_nanoseconds()
>>   instead of three {thd, &status->warnings, and status->nanoseconds} ?
> 
> Changing all functions that accept both &warn and nsec to
> accept a pointer to MYSQL_TIME_STATUS instead, like this:
> 
> < xxxx(thd, &status->warnings, and status->nanoseconds);
>> xxxx(thd, &status);
> 
> is a very good idea.
> 
> 
> I'd avoid mix "thd" to MYSQL_TIME_STATUS though.
> It's a pure C structure.
> 
> Does it sound OK?
> 

I tried this, but this does not save anything.
See the attached patch.

git diff --stat
 sql/sql_time.cc | 6 +++---
 sql/sql_type.cc | 2 +-
 sql/sql_type.h  | 8 ++++++++
 3 files changed, 12 insertions(+), 4 deletions(-)


In all other places "*warn" and "nsec" come from different
places (not from the same MYSQL_TIME_STATUS).


Btw, MYSQL_TIME_STATUS is actually an interface to low level
pure C conversion functions like str_to_xxx() and number_to_xxx().
Let's not propagate it all around the code.

Can we leave the relevant code as is?

Thanks.

diff --git a/sql/sql_time.cc b/sql/sql_time.cc
index 3977df6..79e0b39 100644
--- a/sql/sql_time.cc
+++ b/sql/sql_time.cc
@@ -379,7 +379,7 @@ bool Temporal::str_to_datetime_or_date_or_time(THD *thd, MYSQL_TIME_STATUS *st,
 {
   TemporalAsciiBuffer tmp(str, length, cs);
   return ascii_to_datetime_or_date_or_time(st, tmp.str, tmp.length, fuzzydate)||
-         add_nanoseconds(thd, &st->warnings, fuzzydate, st->nanoseconds);
+         add_nanoseconds(thd, st, fuzzydate);
 }
 
 
@@ -391,7 +391,7 @@ bool Temporal::str_to_datetime_or_date(THD *thd, MYSQL_TIME_STATUS *status,
 {
   TemporalAsciiBuffer tmp(str, length, cs);
   return ascii_to_datetime_or_date(status, tmp.str, tmp.length, flags) ||
-         add_nanoseconds(thd, &status->warnings, flags, status->nanoseconds);
+         add_nanoseconds(thd, status, flags);
 }
 
 
@@ -402,7 +402,7 @@ bool Temporal::str_to_temporal(THD *thd, MYSQL_TIME_STATUS *status,
 {
   TemporalAsciiBuffer tmp(str, length, cs);
   return ascii_to_temporal(status, tmp.str, tmp.length, flags) ||
-         add_nanoseconds(thd, &status->warnings, flags, status->nanoseconds);
+         add_nanoseconds(thd, status, flags);
 }
 
 
diff --git a/sql/sql_type.cc b/sql/sql_type.cc
index 429c122..9acfcb7 100644
--- a/sql/sql_type.cc
+++ b/sql/sql_type.cc
@@ -409,7 +409,7 @@ Interval_DDhhmmssff::Interval_DDhhmmssff(THD *thd, Status *st,
       else
       {
         if (mode == TIME_FRAC_ROUND)
-          time_round_or_set_max(dec, &st->warnings, max_hour, st->nanoseconds);
+          time_round_or_set_max(st, dec, max_hour);
         if (hour > max_hour)
         {
           st->warnings|= MYSQL_TIME_WARN_OUT_OF_RANGE;
diff --git a/sql/sql_type.h b/sql/sql_type.h
index 6799159..3c06967 100644
--- a/sql/sql_type.h
+++ b/sql/sql_type.h
@@ -823,6 +823,10 @@ class Temporal: protected MYSQL_TIME
     return true;
   }
   void time_round_or_set_max(uint dec, int *warn, ulong max_hour, ulong nsec);
+  void time_round_or_set_max(MYSQL_TIME_STATUS *st, uint dec, ulong max_hour)
+  {
+    time_round_or_set_max(dec, &st->warnings, max_hour, (ulong)st->nanoseconds);
+  }
   bool datetime_add_nanoseconds_or_invalidate(THD *thd, int *warn, ulong nsec);
   bool datetime_round_or_invalidate(THD *thd, uint dec, int *warn, ulong nsec);
   bool add_nanoseconds_with_round(THD *thd, int *warn,
@@ -833,6 +837,10 @@ class Temporal: protected MYSQL_TIME
     return time_round_mode_t(mode) == TIME_FRAC_ROUND ?
            add_nanoseconds_with_round(thd, warn, cmode, nsec) : false;
   }
+  bool add_nanoseconds(THD *thd, MYSQL_TIME_STATUS *st, date_mode_t mode)
+  {
+    return add_nanoseconds(thd, &st->warnings, mode, (ulong) st->nanoseconds);
+  }
 public:
   static void *operator new(size_t size, MYSQL_TIME *ltime) throw()
   {