← Back to team overview

maria-developers team mailing list archive

Re: Please review: MDEV-5528 Command line variable to choose MariaDB-5.3 vs MySQL-5.6 temporal data formats

 

Hi Sergei,


On 10/23/2014 02:48 PM, Sergei Golubchik wrote:
Hi, Alexander!

On Oct 22, Alexander Barkov wrote:

Also, while experimenting, I noticed that InnoDB complains to stderr
about unexpected format for innodb_table_stats.last_update.

I had to add these lines into scripts/mysql_system_tables_fix.sql
to fix the problem:

+set @str="alter table mysql.innodb_table_stats modify last_update timestamp not null default current_timestamp on update current_timestamp";
+set @str=if(@have_innodb <> 0, @str, "set @dummy = 0");
+prepare stmt from @str;
+execute stmt;

Does it look fine?

Yes.

=== 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.

Well, that's both about type codes and an optimized field.

There are no optimized fields for FSP=0 for the new
formats MYSQL_TYPE_{TIME2|DATETIME2|TIMESTAMP2}.

Okay, I see.

=== 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.

Sorry, I missed this.

This is to avoid crash in Item::tmp_table_field_from_field_type().
It expects type(), it does not expect real_type().

When a SP variable type is parsed in sql_yacc.yy, it returns real_type,
i.e. MYSQL_TYPE_TIME2/DATETIME2/TIMESTAMP2. This is needed to create
proper fields when doing "CREATE TABLE t1 (a TIME(6))".

But as a side effect, we have real_type in Item_splocal::Item::splocal.
This line changes real_type to type.

Is there a test that crashes without this line?

Yes, some sp.*.test fail without this line.


Btw, I wish we had separate enums for type() and real_type().
:)

Agree. May be in a followup patch for your IPv6 patch.

Regards,
Sergei



Follow ups

References