Re: [Maria-developers] MDEV-5450 Assertion ... mysql_type_to_time_type(cached_field_type) == ltime.time_type' fails ...

[Alexander Barkov]

On 02/03/2014 07:51 PM, Alexander Barkov wrote:

Hi Sergei,

On 02/03/2014 07:44 PM, Sergei Golubchik wrote:

Hi, Alexander!


Ah, so at the end you've decided to fix the type, not the assert.
Fine.
Ok to push.

There were two problems actually:

- the type
- the assert

I fixed both.

Sorry, I misunderstood something when replying to the previous letter.
Let me clarify.


There were two problems.

1. Item_func_add_time::get_date() might erroneously return
MYSQL_TIMESTAMP_TIME in cases when field_type() is actually DATETIME.

This is fixed by this change:

-  if (is_date)                        // TIMESTAMP function
+  if (cached_field_type == MYSQL_TYPE_DATETIME)



2. The problem with a crash on DBUG_ASSERT,
which was causes by a wrong time_type  returned from
Item_temporal_hybrid_func::get_date().
This is fixed by adding a new method
Item_temporal_hybrid_func::fix_temporal_type().

But the condition itself inside the DBUG_ASSERT() was not changed.

Thanks for review!


Btw, the problem with type can be back ported to 5.5 or 5.3.

On Jan 31, Alexander Barkov wrote:

Hi Sergei,

Please review a patch for mdev-5450.

Thanks.

=== modified file 'sql/item_timefunc.cc'
--- sql/item_timefunc.cc    2013-12-16 12:02:21 +0000
+++ sql/item_timefunc.cc    2014-01-31 12:35:17 +0000
@@ -1480,12 +1480,42 @@ String *Item_temporal_func::val_str(Stri
  }


+bool Item_temporal_hybrid_func::fix_temporal_type(MYSQL_TIME *ltime)
+{
+  if (ltime->time_type < 0) /* MYSQL_TIMESTAMP_NONE,
MYSQL_TIMESTAMP_ERROR */
+    return false;
+  switch (field_type())
+  {
+  case MYSQL_TYPE_TIME:
+    ltime->year= ltime->month= ltime->day= 0;
+    ltime->time_type= MYSQL_TIMESTAMP_TIME;
+    return false;
+  case MYSQL_TYPE_DATETIME:
+  case MYSQL_TYPE_TIMESTAMP:
+    ltime->neg= 0;
+    ltime->time_type= MYSQL_TIMESTAMP_DATETIME;
+    return false;
+  case MYSQL_TYPE_DATE:
+    ltime->neg= 0;
+    ltime->hour= ltime->minute= ltime->second= ltime->second_part= 0;
+    ltime->time_type= MYSQL_TIMESTAMP_DATE;
+    return false;
+  case MYSQL_TYPE_STRING: /* DATE_ADD, ADDTIME can return VARCHAR */
+    return false;
+  default:
+    DBUG_ASSERT(0);
+    return true;
+  }
+  return false;
+}
+
+
  String *Item_temporal_hybrid_func::val_str_ascii(String *str)
  {
    DBUG_ASSERT(fixed == 1);
    MYSQL_TIME ltime;

-  if (get_date(&ltime, 0) ||
+  if (get_date(&ltime, 0) || fix_temporal_type(&ltime) ||
        (null_value= my_TIME_to_str(&ltime, str, decimals)))
      return (String *) 0;

Regards,
Sergei

_______________________________________________
Mailing list: https://launchpad.net/~maria-developers
Post to     : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help   : https://help.launchpad.net/ListHelp