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