Hi, Alexander! On Oct 20, Alexander Barkov wrote:
=== modified file 'mysql-test/suite/binlog/r/binlog_mysqlbinlog_row.result' --- mysql-test/suite/binlog/r/binlog_mysqlbinlog_row.result 2013-09-14 01:09:36 +0000 +++ mysql-test/suite/binlog/r/binlog_mysqlbinlog_row.result 2014-08-29 06:20:22 +0000 @@ -1625,7 +1625,7 @@ BEGIN #010909 4:46:40 server id 1 end_log_pos # Write_rows: table id # flags: STMT_END_F ### INSERT INTO `test`.`t1` ### SET -### @1=2001-02-03 10:20:30 /* DATETIME meta=0 nullable=1 is_null=0 */ +### @1='2001-02-03 10:20:30' /* DATETIME(0) meta=0 nullable=1 is_null=0 */
Why is that? 1. quoting 2. (0)
hmm, quoting is ok, I suppose. time values were (and are) quoted, timestamp values were (and are) not quoted, so it's kind of ok to quote datetime values here
I added '(0)' when implemented the FSP data types MySQL, so one can distinguish between the old format and the new format when reading the mysqlbinlog output.
If you don't like this, I can suppress printing the precision in case of FSP=0.
Let's keep your '(0)' - it seems useful. It users will complain, we can reconsider it.
=== modified file 'mysql-test/suite/innodb/r/data_types.result' --- mysql-test/suite/innodb/r/data_types.result 2013-11-13 21:58:19 +0000 +++ mysql-test/suite/innodb/r/data_types.result 2014-08-29 05:46:28 +0000 @@ -112,7 +112,7 @@ t1_BLOB DATA_BLOB t1_CHAR_100 DATA_CHAR t1_CHAR_100_BINARY DATA_MYSQL t1_DATE DATA_INT -t1_DATETIME DATA_INT +t1_DATETIME DATA_FIXBINARY
Hmm. The comment in the test file says
This test records what *internal type codes* innodb is using for every MariaDB data type. THEY MUST ALWAYS BE THE SAME AND NEVER CHANGE! Otherwise we create a compatibility problem and possible silent data corruption too, see MDEV-5248
I suspect in your case the change is ok, as you've also changed the server type code for a field. But still please MDEV-5248, and test a scenario from there (upgrade + online alter) to make sure you didn't break it. And the same scenario for upgrade from mysql-5.5 and from mysql-5.6
Can you suggest how to test this? I have never done this kind of tests with InnoDB yet.
I mean to test it manually. Normally the data_types.test ensures that type codes don't change, so there's no need to put the whole upgrade test into mysql-test.
=== added file 'mysql-test/suite/rpl/t/rpl_temporal_format_mariadb53_to_mariadb53.test' --- mysql-test/suite/rpl/t/rpl_temporal_format_mariadb53_to_mariadb53.test 1970-01-01 00:00:00 +0000 +++ mysql-test/suite/rpl/t/rpl_temporal_format_mariadb53_to_mariadb53.test 2014-08-29 10:10:47 +0000 @@ -0,0 +1,4 @@ +--let $force_master_mysql56_temporal_format=false; +--let $force_slave_mysql56_temporal_format=false; + +--source rpl_temporal_format_default_to_default.test
If you need to run one test with different settings, you can, of course, include it many times in different files, but there's also an alternative approach - combinations. It's not always applicable, but when it is, it's pretty convenient and concise.
Thanks for the tip. Would combinations work for my purposes? Do you want me to switch the tests to use combinations?
No, wait... Sorry. I'm afraid, currently combinations don't support different settings for master and slave. So, they wouldn't help here. I didn't think of that use case before...
=== modified file 'sql/field.cc' --- sql/field.cc 2014-06-11 08:08:08 +0000 +++ sql/field.cc 2014-08-29 10:45:32 +0000 @@ -5042,7 +5042,8 @@ int Field_timestamp_with_dec::set_time() { THD *thd= get_thd(); set_notnull(); - store_TIME(thd->query_start(), thd->query_start_sec_part()); + // Avoid writing microseconds into binlog for FSP=0 + store_TIME(thd->query_start(), decimals() ? thd->query_start_sec_part() : 0);
I'd expect that when decimals() == 0, the object would be Field_timestamp, not Field_timestamp_with_dec. How do you end up here with decimals() == 0?
The actual object that is created is Field_timestampf, which is derived from Field_timestamp_with_dec.
It is implemented this way in MySQL, and the idea is to deprecate the old type codes asap. Why have two type codes?
That's not about type codes. We used to have Field_timestamp that handles TIMESTAMP(0) and Field_timestamp_hires that handles TIMESTAMP(N) for N>0. And the old class Field_timestamp was optimized for old timestamps without microseconds. Now you seem to create a generic Field_timestampf for all TIMESTAMP fields.
=== modified file 'sql/item.cc' --- sql/item.cc 2014-08-07 16:06:56 +0000 +++ sql/item.cc 2014-08-27 07:27:31 +0000 @@ -1585,6 +1585,7 @@ Item_splocal::Item_splocal(const LEX_STR { maybe_null= TRUE;
+ sp_var_type= real_type_to_type(sp_var_type);
What's that for? Another bug fix? Do you have a test case for it?
You seem to have missed this question.
m_type= sp_map_item_type(sp_var_type); m_field_type= sp_var_type; m_result_type= sp_map_result_type(sp_var_type);
=== modified file 'sql/sys_vars.cc' --- sql/sys_vars.cc 2014-08-07 16:06:56 +0000 +++ sql/sys_vars.cc 2014-08-28 14:16:31 +0000 @@ -4815,3 +4815,8 @@ static Sys_var_mybool Sys_pseudo_slave_m SESSION_ONLY(pseudo_slave_mode), NO_CMD_LINE, DEFAULT(FALSE), NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(check_pseudo_slave_mode));
+static Sys_var_mybool Sys_mysql56_temporal_format( + "mysql56_temporal_format", + "Use MySQL-5.6 (instead of MariaDB-5.3) format for TIME, DATETIME, TIMESTAMP columns.", + GLOBAL_VAR(opt_mysql56_temporal_format),
why is it global, not session?
Are you sure we want this as a session variable?
From my understanding, it has a very low impact on the end user, because there are no visible behaviour change other than the column sizes in "EXPLAIN SELECT" output. So the end users does not really care.
Hmm. Generally I prefer to have session variables for anything that can be session local behavior. Only variables that have global effect, like change global buffers, are created as global-only variables. But that's a guideline, not a strict rule. If you think this variable makes no sense as a session variables, okay, make it global. Regards, Sergei