maria-developers team mailing list archive

[Alexander Barkov <bar@xxxxxxxxxxx>]

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