maria-developers team mailing list archive

[Jeremy Cole <jeremycole@xxxxxxxxxx>]

Hi Sergei,


> >    - It is almost certainly worth creating test cases with data files
> from
>
>    older MariaDB and newer MySQL versions to exercise the usage of the
> code
> >    paths that only occur during upgrade; that doesn't need to happen here
> >    necessarily.
>
> Yes, Elena is doing that. But I don't think mtr is suited for this kind
> of tests.


I think mtr is not great for it, but it's also not awful. It just requires
pre-generating the data files.


> > > + case MYSQL_TYPE_NEWDATE:
> > > + return(DATA_INT);
> > > + case MYSQL_TYPE_TIMESTAMP:
> > > + *unsigned_flag = 0;
> > > +                /* fall through */
>
> That's the first change to get_innobase_type_from_mysql_type() as copied
> from 5.6. Because in 5.6 TIMESTAMP is signed, in MariaDB - unsigned.
>

I don't think that's correct. In MySQL 5.6, the Field_timestamp constructor
sets "flags|= ZEROFILL_FLAG | UNSIGNED_FLAG;", which will end up with this
being marked unsigned due to the generic handling of "if (field->flags &
UNSIGNED_FLAG)" at the very top, as long as nothing else *unsets* it. So
this should not be changed relative to MySQL 5.6.


> > You also need to support MYSQL_TYPE_{TIMESTAMP,TIME,DATETIME}2 for MySQL
> > 5.6 here.
>
> I do. That's the second difference from
> get_innobase_type_from_mysql_type().
> In 5.6 they do a switch on field->real_type(). But in MariaDB, temporal
> types with microsecond have the same real_type as without, one need to
> look at the key_type().  Incidentally, this also covers
> {TIMESTAMP,TIME,DATETIME}2 from 5.6, they also have
> key_type() == HA_KEYTYPE_BINARY. So, this if() covers everything.
>

Huh. Sorry, I didn't notice the change to the value being switch'ed on. Why
change this? This is a pretty dangerous (IMHO) deviation from MySQL which
may cause problems, and is unnecessary. Why not just stick with exactly
what MySQL is doing in 5.6, plus whatever minor changes are needed to
support MariaDB-format microsecond time?

Regards,

Jeremy