maria-developers team mailing list archive
-
maria-developers team
-
Mailing list archive
-
Message #05793
Re: merge for MySQL56 FSP data types
Hi Sergei,
Thanks for review. Please see my comments inline:
On 06/26/2013 11:11 PM, Sergei Golubchik wrote:
Hi, Alexander!
On Jun 19, Alexander Barkov wrote:
Hi Serg,
Please review the patch merging MySQL-5.6 fractional second precision
enabled data types.
Thanks.
Here you are, my comments are below:
=== modified file 'include/my_time.h'
--- include/my_time.h 2013-05-24 15:09:59 +0000
+++ include/my_time.h 2013-06-19 11:13:13 +0000
@@ -107,10 +107,37 @@ ulonglong TIME_to_ulonglong_time(const M
ulonglong TIME_to_ulonglong(const MYSQL_TIME *);
double TIME_to_double(const MYSQL_TIME *my_time);
+/** MySQL56 routines and macros **/
+#define MY_PACKED_TIME_GET_INT_PART(x) ((x) >> 24)
+#define MY_PACKED_TIME_GET_FRAC_PART(x) ((x) % (1LL << 24))
+#define MY_PACKED_TIME_MAKE(i, f) ((((longlong) (i)) << 24) + (f))
+#define MY_PACKED_TIME_MAKE_INT(i) ((((longlong) (i)) << 24))
+
+longlong TIME_to_longlong_datetime_packed(const MYSQL_TIME *);
+longlong TIME_to_longlong_time_packed(const MYSQL_TIME *);
+
+void TIME_from_longlong_datetime_packed(MYSQL_TIME *ltime, longlong nr);
+void TIME_from_longlong_time_packed(MYSQL_TIME *ltime, longlong nr);
+
+void my_datetime_packed_to_binary(longlong nr, uchar *ptr, uint dec);
+longlong my_datetime_packed_from_binary(const uchar *ptr, uint dec);
+uint my_datetime_binary_length(uint dec);
+
+void my_time_packed_to_binary(longlong nr, uchar *ptr, uint dec);
+longlong my_time_packed_from_binary(const uchar *ptr, uint dec);
+uint my_time_binary_length(uint dec);
+
+void my_timestamp_to_binary(const struct timeval *tm, uchar *ptr, uint dec);
+void my_timestamp_from_binary(struct timeval *tm, const uchar *ptr, uint dec);
+uint my_timestamp_binary_length(uint dec);
+/** End of MySQL routines and macros **/
I don't think we need these macros and related functions in include/ and
sql-commont (that is, in the client). These belong in sql/
as they're only used for reading MySQL data files and for RBR.
if you'd like you can even put them in a separate file, sql/compat65.cc
+
longlong pack_time(MYSQL_TIME *my_time);
MYSQL_TIME *unpack_time(longlong packed, MYSQL_TIME *my_time);
int check_time_range(struct st_mysql_time *my_time, uint dec, int *warning);
+my_bool check_datetime_range(const MYSQL_TIME *ltime);
+
Good idea. Will do this.
long calc_daynr(uint year,uint month,uint day);
uint calc_days_in_year(uint year);
=== modified file 'include/mysql_com.h'
--- include/mysql_com.h 2013-06-06 19:32:29 +0000
+++ include/mysql_com.h 2013-06-18 10:35:35 +0000
@@ -400,6 +400,9 @@ enum enum_field_types { MYSQL_TYPE_DECIM
MYSQL_TYPE_DATETIME, MYSQL_TYPE_YEAR,
MYSQL_TYPE_NEWDATE, MYSQL_TYPE_VARCHAR,
MYSQL_TYPE_BIT,
+ MYSQL_TYPE_TIMESTAMP2,
+ MYSQL_TYPE_DATETIME2,
+ MYSQL_TYPE_TIME2,
Add a comment about these types please.
Okey.
MYSQL_TYPE_NEWDECIMAL=246,
MYSQL_TYPE_ENUM=247,
MYSQL_TYPE_SET=248,
=== added file 'mysql-test/t/type_temporal_mysql56.test'
--- mysql-test/t/type_temporal_mysql56.test 1970-01-01 00:00:00 +0000
+++ mysql-test/t/type_temporal_mysql56.test 2013-06-19 09:01:02 +0000
@@ -0,0 +1,23 @@
+
+let $MYSQLD_DATADIR= `select @@datadir`;
+--copy_file std_data/mysql56time.frm $MYSQLD_DATADIR/test/mysql56time.frm
+--copy_file std_data/mysql56time.MYD $MYSQLD_DATADIR/test/mysql56time.MYD
+--copy_file std_data/mysql56time.MYI $MYSQLD_DATADIR/test/mysql56time.MYI
+SHOW CREATE TABLE mysql56time;
+--query_vertical SELECT * FROM mysql56time;
generally, you either start mysqltest command with -- (dash-dash)
OR you end it with a semicolon.
If you do both - as above - you end up with two semicolons,
see the result file
Will fix this. Thanks for noticing.
+DROP TABLE mysql56time;
+
+--copy_file std_data/mysql56datetime.frm $MYSQLD_DATADIR/test/mysql56datetime.frm
+--copy_file std_data/mysql56datetime.MYD $MYSQLD_DATADIR/test/mysql56datetime.MYD
+--copy_file std_data/mysql56datetime.MYI $MYSQLD_DATADIR/test/mysql56datetime.MYI
+SHOW CREATE TABLE mysql56datetime;
+--query_vertical SELECT * FROM mysql56datetime;
+DROP TABLE mysql56datetime;
+
+--copy_file std_data/mysql56timestamp.frm $MYSQLD_DATADIR/test/mysql56timestamp.frm
+--copy_file std_data/mysql56timestamp.MYD $MYSQLD_DATADIR/test/mysql56timestamp.MYD
+--copy_file std_data/mysql56timestamp.MYI $MYSQLD_DATADIR/test/mysql56timestamp.MYI
+SET TIME_ZONE='+00:00';
+SHOW CREATE TABLE mysql56timestamp;
+--query_vertical SELECT * FROM mysql56timestamp;
+DROP TABLE mysql56timestamp;
=== modified file 'sql-common/my_time.c'
--- sql-common/my_time.c 2013-04-07 12:00:16 +0000
+++ sql-common/my_time.c 2013-06-19 11:59:45 +0000
@@ -1346,6 +1396,432 @@ ulonglong TIME_to_ulonglong(const MYSQL_
return 0;
}
+
+/*** MySQL56 TIME low-level memory and disk representation routines ***/
As I wrote above, I'd prefer to remove them from the client library
and move to sql/
+
+/*
+ In-memory format:
+
+ 1 bit sign (Used for sign, when on disk)
+ 1 bit unused (Reserved for wider hour range, e.g. for intervals)
+ 10 bit hour (0-836)
+ 6 bit minute (0-59)
+ 6 bit second (0-59)
+ 24 bits microseconds (0-999999)
+
+ Total: 48 bits = 6 bytes
+ Suhhhhhh.hhhhmmmm.mmssssss.ffffffff.ffffffff.ffffffff
+*/
+
+
+/**
+ Convert time value to MySQL56 numeric packed representation.
+
+ @param ltime The value to convert.
+ @return Numeric packed representation.
+*/
+longlong TIME_to_longlong_time_packed(const MYSQL_TIME *ltime)
+{
+ /* If month is 0, we mix day with hours: "1 00:10:10" -> "24:00:10" */
+ long hms= (((ltime->month ? 0 : ltime->day * 24) + ltime->hour) << 12) |
+ (ltime->minute << 6) | ltime->second;
+ longlong tmp= MY_PACKED_TIME_MAKE(hms, ltime->second_part);
+ return ltime->neg ? -tmp : tmp;
+}
+
+
+
+/**
+ Convert MySQL56 time packed numeric representation to time.
+
+ @param OUT ltime The MYSQL_TIME variable to set.
+ @param tmp The packed numeric representation.
+*/
+void TIME_from_longlong_time_packed(MYSQL_TIME *ltime, longlong tmp)
+{
+ long hms;
+ if ((ltime->neg= (tmp < 0)))
+ tmp= -tmp;
+ hms= MY_PACKED_TIME_GET_INT_PART(tmp);
+ ltime->year= (uint) 0;
+ ltime->month= (uint) 0;
+ ltime->day= (uint) 0;
+ ltime->hour= (uint) (hms >> 12) % (1 << 10); /* 10 bits starting at 12th */
+ ltime->minute= (uint) (hms >> 6) % (1 << 6); /* 6 bits starting at 6th */
+ ltime->second= (uint) hms % (1 << 6); /* 6 bits starting at 0th */
+ ltime->second_part= MY_PACKED_TIME_GET_FRAC_PART(tmp);
+ ltime->time_type= MYSQL_TIMESTAMP_TIME;
+}
+
+
+/**
+ Calculate binary size of MySQL56 packed numeric time representation.
+
+ @param dec Precision.
+*/
+uint my_time_binary_length(uint dec)
+{
+ DBUG_ASSERT(dec <= TIME_SECOND_PART_DIGITS);
+ return 3 + (dec + 1) / 2;
+}
+
+
+/*
+ On disk we convert from signed representation to unsigned
+ representation using TIMEF_OFS, so all values become binary comparable.
+*/
+#define TIMEF_OFS 0x800000000000LL
+#define TIMEF_INT_OFS 0x800000LL
+
+
+/**
+ Convert MySQL56 in-memory numeric time representation to on-disk representation
+
+ @param nr Value in packed numeric time format.
+ @param OUT ptr The buffer to put value at.
+ @param dec Precision.
+*/
+void my_time_packed_to_binary(longlong nr, uchar *ptr, uint dec)
+{
+ DBUG_ASSERT(dec <= TIME_SECOND_PART_DIGITS);
+ /* Make sure the stored value was previously properly rounded or truncated */
+ DBUG_ASSERT((MY_PACKED_TIME_GET_FRAC_PART(nr) %
+ (int) log_10_int[TIME_SECOND_PART_DIGITS - dec]) == 0);
+
+ switch (dec)
+ {
+ case 0:
+ default:
+ mi_int3store(ptr, TIMEF_INT_OFS + MY_PACKED_TIME_GET_INT_PART(nr));
+ break;
+
+ case 1:
+ case 2:
+ mi_int3store(ptr, TIMEF_INT_OFS + MY_PACKED_TIME_GET_INT_PART(nr));
+ ptr[3]= (unsigned char) (char) (MY_PACKED_TIME_GET_FRAC_PART(nr) / 10000);
+ break;
+
+ case 4:
+ case 3:
+ mi_int3store(ptr, TIMEF_INT_OFS + MY_PACKED_TIME_GET_INT_PART(nr));
+ mi_int2store(ptr + 3, MY_PACKED_TIME_GET_FRAC_PART(nr) / 100);
+ break;
+
+ case 5:
+ case 6:
+ mi_int6store(ptr, nr + TIMEF_OFS);
+ break;
+ }
+}
+
+
+/**
+ Convert MySQL56 on-disk time representation to in-memory packed numeric
+ representation.
+
+ @param ptr The pointer to read the value at.
+ @param dec Precision.
+ @return Packed numeric time representation.
+*/
+longlong my_time_packed_from_binary(const uchar *ptr, uint dec)
+{
+ DBUG_ASSERT(dec <= TIME_SECOND_PART_DIGITS);
+
+ switch (dec)
+ {
+ case 0:
+ default:
+ {
+ longlong intpart= mi_uint3korr(ptr) - TIMEF_INT_OFS;
+ return MY_PACKED_TIME_MAKE_INT(intpart);
+ }
+ case 1:
+ case 2:
+ {
+ longlong intpart= mi_uint3korr(ptr) - TIMEF_INT_OFS;
+ int frac= (uint) ptr[3];
+ if (intpart < 0 && frac)
+ {
+ /*
+ Negative values are stored with reverse fractional part order,
+ for binary sort compatibility.
+
+ Disk value intpart frac Time value Memory value
+ 800000.00 0 0 00:00:00.00 0000000000.000000
+ 7FFFFF.FF -1 255 -00:00:00.01 FFFFFFFFFF.FFD8F0
+ 7FFFFF.9D -1 99 -00:00:00.99 FFFFFFFFFF.F0E4D0
+ 7FFFFF.00 -1 0 -00:00:01.00 FFFFFFFFFF.000000
+ 7FFFFE.FF -1 255 -00:00:01.01 FFFFFFFFFE.FFD8F0
+ 7FFFFE.F6 -2 246 -00:00:01.10 FFFFFFFFFE.FE7960
+
+ Formula to convert fractional part from disk format
+ (now stored in "frac" variable) to absolute value: "0x100 - frac".
+ To reconstruct in-memory value, we shift
+ to the next integer value and then substruct fractional part.
+ */
+ intpart++; /* Shift to the next integer value */
+ frac-= 0x100; /* -(0x100 - frac) */
+ }
+ return MY_PACKED_TIME_MAKE(intpart, frac * 10000);
+ }
+
+ case 3:
+ case 4:
+ {
+ longlong intpart= mi_uint3korr(ptr) - TIMEF_INT_OFS;
+ int frac= mi_uint2korr(ptr + 3);
+ if (intpart < 0 && frac)
+ {
+ /*
+ Fix reverse fractional part order: "0x10000 - frac".
+ See comments for FSP=1 and FSP=2 above.
+ */
+ intpart++; /* Shift to the next integer value */
+ frac-= 0x10000; /* -(0x10000-frac) */
+ }
+ return MY_PACKED_TIME_MAKE(intpart, frac * 100);
+ }
+
+ case 5:
+ case 6:
+ return ((longlong) mi_uint6korr(ptr)) - TIMEF_OFS;
+ }
+}
+
+
+/*** MySQL56 DATETIME low-level memory and disk representation routines ***/
+
+/*
+ 1 bit sign (used when on disk)
+ 17 bits year*13+month (year 0-9999, month 0-12)
+ 5 bits day (0-31)
+ 5 bits hour (0-23)
+ 6 bits minute (0-59)
+ 6 bits second (0-59)
+ 24 bits microseconds (0-999999)
+
+ Total: 64 bits = 8 bytes
+
+ SYYYYYYY.YYYYYYYY.YYdddddh.hhhhmmmm.mmssssss.ffffffff.ffffffff.ffffffff
+*/
+
+/**
+ Convert datetime to MySQL56 packed numeric datetime representation.
+ @param ltime The value to convert.
+ @return Packed numeric representation of ltime.
+*/
+longlong TIME_to_longlong_datetime_packed(const MYSQL_TIME *ltime)
+{
+ longlong ymd= ((ltime->year * 13 + ltime->month) << 5) | ltime->day;
+ longlong hms= (ltime->hour << 12) | (ltime->minute << 6) | ltime->second;
+ longlong tmp= MY_PACKED_TIME_MAKE(((ymd << 17) | hms), ltime->second_part);
+ DBUG_ASSERT(!check_datetime_range(ltime)); /* Make sure no overflow */
+ return ltime->neg ? -tmp : tmp;
+}
+
+
+/**
+ Convert MySQL56 packed numeric datetime representation to MYSQL_TIME.
+ @param OUT ltime The datetime variable to convert to.
+ @param tmp The packed numeric datetime value.
+*/
+void TIME_from_longlong_datetime_packed(MYSQL_TIME *ltime, longlong tmp)
+{
+ longlong ymd, hms;
+ longlong ymdhms, ym;
+ if ((ltime->neg= (tmp < 0)))
+ tmp= -tmp;
+
+ ltime->second_part= MY_PACKED_TIME_GET_FRAC_PART(tmp);
+ ymdhms= MY_PACKED_TIME_GET_INT_PART(tmp);
+
+ ymd= ymdhms >> 17;
+ ym= ymd >> 5;
+ hms= ymdhms % (1 << 17);
+
+ ltime->day= ymd % (1 << 5);
+ ltime->month= ym % 13;
+ ltime->year= ym / 13;
+
+ ltime->second= hms % (1 << 6);
+ ltime->minute= (hms >> 6) % (1 << 6);
+ ltime->hour= (hms >> 12);
+
+ ltime->time_type= MYSQL_TIMESTAMP_DATETIME;
+}
+
+
+/**
+ Calculate binary size of MySQL56 packed datetime representation.
+ @param dec Precision.
+*/
+uint my_datetime_binary_length(uint dec)
+{
+ DBUG_ASSERT(dec <= TIME_SECOND_PART_DIGITS);
+ return 5 + (dec + 1) / 2;
+}
+
+
+/*
+ On disk we store as unsigned number with DATETIMEF_INT_OFS offset,
+ for HA_KETYPE_BINARY compatibilty purposes.
+*/
+#define DATETIMEF_INT_OFS 0x8000000000LL
+
+
+/**
+ Convert MySQL56 on-disk datetime representation
+ to in-memory packed numeric representation.
+
+ @param ptr The pointer to read value at.
+ @param dec Precision.
+ @return In-memory packed numeric datetime representation.
+*/
+longlong my_datetime_packed_from_binary(const uchar *ptr, uint dec)
+{
+ longlong intpart= mi_uint5korr(ptr) - DATETIMEF_INT_OFS;
+ int frac;
+ DBUG_ASSERT(dec <= TIME_SECOND_PART_DIGITS);
+ switch (dec)
+ {
+ case 0:
+ default:
+ return MY_PACKED_TIME_MAKE_INT(intpart);
+ case 1:
+ case 2:
+ frac= ((int) (signed char) ptr[5]) * 10000;
+ break;
+ case 3:
+ case 4:
+ frac= mi_sint2korr(ptr + 5) * 100;
+ break;
+ case 5:
+ case 6:
+ frac= mi_sint3korr(ptr + 5);
+ break;
+ }
+ return MY_PACKED_TIME_MAKE(intpart, frac);
+}
+
+
+/**
+ Store MySQL56 in-memory numeric packed datetime representation to disk.
+
+ @param nr In-memory numeric packed datetime representation.
+ @param OUT ptr The pointer to store at.
+ @param dec Precision, 1-6.
+*/
+void my_datetime_packed_to_binary(longlong nr, uchar *ptr, uint dec)
+{
+ DBUG_ASSERT(dec <= TIME_SECOND_PART_DIGITS);
+ /* The value being stored must have been properly rounded or truncated */
+ DBUG_ASSERT((MY_PACKED_TIME_GET_FRAC_PART(nr) %
+ (int) log_10_int[TIME_SECOND_PART_DIGITS - dec]) == 0);
+
+ mi_int5store(ptr, MY_PACKED_TIME_GET_INT_PART(nr) + DATETIMEF_INT_OFS);
+ switch (dec)
+ {
+ case 0:
+ default:
+ break;
+ case 1:
+ case 2:
+ ptr[5]= (unsigned char) (char) (MY_PACKED_TIME_GET_FRAC_PART(nr) / 10000);
+ break;
+ case 3:
+ case 4:
+ mi_int2store(ptr + 5, MY_PACKED_TIME_GET_FRAC_PART(nr) / 100);
+ break;
+ case 5:
+ case 6:
+ mi_int3store(ptr + 5, MY_PACKED_TIME_GET_FRAC_PART(nr));
+ }
+}
+
+
+/*** MySQL56 TIMESTAMP low-level memory and disk representation routines ***/
+
+/**
+ Calculate on-disk size of a timestamp value.
+
+ @param dec Precision.
+*/
+uint my_timestamp_binary_length(uint dec)
+{
+ DBUG_ASSERT(dec <= TIME_SECOND_PART_DIGITS);
+ return 4 + (dec + 1) / 2;
+}
+
+
+/**
+ Convert MySQL56 binary timestamp representation to in-memory representation.
+
+ @param OUT tm The variable to convert to.
+ @param ptr The pointer to read the value from.
+ @param dec Precision.
+*/
+void my_timestamp_from_binary(struct timeval *tm, const uchar *ptr, uint dec)
+{
+ DBUG_ASSERT(dec <= TIME_SECOND_PART_DIGITS);
+ tm->tv_sec= mi_uint4korr(ptr);
+ switch (dec)
+ {
+ case 0:
+ default:
+ tm->tv_usec= 0;
+ break;
+ case 1:
+ case 2:
+ tm->tv_usec= ((int) ptr[4]) * 10000;
+ break;
+ case 3:
+ case 4:
+ tm->tv_usec= mi_sint2korr(ptr + 4) * 100;
+ break;
+ case 5:
+ case 6:
+ tm->tv_usec= mi_sint3korr(ptr + 4);
+ }
+}
+
+
+/**
+ Convert MySQL56 in-memory timestamp representation to on-disk representation.
+
+ @param tm The value to convert.
+ @param OUT ptr The pointer to store the value to.
+ @param dec Precision.
+*/
+void my_timestamp_to_binary(const struct timeval *tm, uchar *ptr, uint dec)
+{
+ DBUG_ASSERT(dec <= TIME_SECOND_PART_DIGITS);
+ /* Stored value must have been previously properly rounded or truncated */
+ DBUG_ASSERT((tm->tv_usec %
+ (int) log_10_int[TIME_SECOND_PART_DIGITS - dec]) == 0);
+ mi_int4store(ptr, tm->tv_sec);
+ switch (dec)
+ {
+ case 0:
+ default:
+ break;
+ case 1:
+ case 2:
+ ptr[4]= (unsigned char) (char) (tm->tv_usec / 10000);
+ break;
+ case 3:
+ case 4:
+ mi_int2store(ptr + 4, tm->tv_usec / 100);
+ break;
+ /* Impossible second precision. Fall through */
+ case 5:
+ case 6:
+ mi_int3store(ptr + 4, tm->tv_usec);
+ }
+}
+
+/****************************************/
+
+
double TIME_to_double(const MYSQL_TIME *my_time)
{
double d= (double)TIME_to_ulonglong(my_time);
=== modified file 'sql/field.cc'
--- sql/field.cc 2013-06-06 19:32:29 +0000
+++ sql/field.cc 2013-06-19 04:51:38 +0000
@@ -87,6 +87,7 @@ const char field_separator=',';
#define FIELDTYPE_NUM (FIELDTYPE_TEAR_FROM + (255 - FIELDTYPE_TEAR_TO))
static inline int field_type2index (enum_field_types field_type)
{
+ field_type= real_type_to_type(field_type);
I don't like that MYSQL_TYPE_*2 types (which are, really, now only a
migration types, for mysql-5.6 data files) are leaking into the server.
I'd prefer if they'd stay in their corresponding Field objects
and the rest of the server wouldn't need to know they exist.
Can you clarify what exactly you suggest?
return (field_type < FIELDTYPE_TEAR_FROM ?
field_type :
((int)FIELDTYPE_TEAR_FROM) + (field_type - FIELDTYPE_TEAR_TO) - 1);
@@ -4690,6 +4767,16 @@ String *Field_timestamp::val_str(String
*to++= (char) ('0'+(char) (temp));
*to= 0;
val_buffer->set_charset(&my_charset_numeric);
+
+ if (uint dec= decimals())
eh? We never declare variables inside if(). A question of a consistent
coding style.
Will fix this.
Btw. It looks convenient. Do you know why we don't use this?
+ {
+ ulong sec_part= (ulong) sec_part_shift(ltime.second_part, dec);
+ char *buf= const_cast<char*>(val_buffer->ptr() + MAX_DATETIME_WIDTH);
+ for (int i= dec; i > 0; i--, sec_part/= 10)
+ buf[i]= (char)(sec_part % 10) + '0';
+ buf[0]= '.';
+ buf[dec + 1]= 0;
+ }
return val_buffer;
}
@@ -5092,7 +5198,18 @@ int Field_temporal::store(double nr)
}
-int Field_temporal::store(longlong nr, bool unsigned_val)
+int Field_temporal::store_decimal(const my_decimal *d)
+{
+ ASSERT_COLUMN_MARKED_FOR_WRITE_OR_COMPUTED;
+ double val;
+ /* TODO: use decimal2string? */
+ int err= warn_if_overflow(my_decimal2double(E_DEC_FATAL_ERROR &
+ ~E_DEC_OVERFLOW, d, &val));
decimal2string is slow, decimal2double is lossy.
Use my_decimal2seconds
decimal2double() should not be lossy for the TIME(0) and DATETIME(0)
types. DATETIME(0) needs 14 digits. The double type provides precision
of 15 digits. Note, This is how it was before my patch.
The zero precision data types TIME(0) and DATETIME(0) used
my_decimal2double() through Field_str::store_decimal().
I just did not change this behaviour, which is well tested
through the years (which is good).
But I had to copy Field_str::store_decimal to
Field_temporal::store_decimal, which introduced
a little bit of duplicate code (which is bad).
What is known to be faster?
my_decimal2seconds() or my_decimal2double()?
I guess we just need to use the faster one.
+ return err | store(val);
+}
+
+
+int Field_temporal_with_date::store(longlong nr, bool unsigned_val)
{
int error;
MYSQL_TIME ltime;
@@ -5229,32 +5375,16 @@ longlong Field_time::val_int(void)
my_charset_bin
*/
-String *Field_time::val_str(String *val_buffer,
- String *val_ptr __attribute__((unused)))
+String *Field_time::val_str(String *str,
+ String *unused __attribute__((unused)))
{
ASSERT_COLUMN_MARKED_FOR_READ;
MYSQL_TIME ltime;
- long tmp=(long) sint3korr(ptr);
- ltime.neg= 0;
- if (tmp < 0)
- {
- tmp= -tmp;
- ltime.neg= 1;
- }
- ltime.year= ltime.month= 0;
- ltime.day= (uint) 0;
- ltime.hour= (uint) (tmp/10000);
- ltime.minute= (uint) (tmp/100 % 100);
- ltime.second= (uint) (tmp % 100);
- ltime.second_part= 0;
-
- val_buffer->alloc(MAX_DATE_STRING_REP_LENGTH);
- uint length= (uint) my_time_to_str(<ime,
- const_cast<char*>(val_buffer->ptr()), 0);
- val_buffer->length(length);
- val_buffer->set_charset(&my_charset_numeric);
-
- return val_buffer;
+ get_date(<ime, TIME_TIME_ONLY);
+ str->alloc(field_length + 1);
+ str->length(my_time_to_str(<ime, const_cast<char*>(str->ptr()), decimals()));
+ str->set_charset(&my_charset_numeric);
+ return str;
Well. I intentionally kept the old "optimized" version.
I'd expect a generic one to be slower.
May be it's ok, though...
Please have a look into Field_time::val_str() and Field_time::get_date()
in the original sources
(before the patch).
They are about 25 lines of code each,
with 20 of them exactly the same.
I thought it was a good idea to remove some duplicate code.
Also, I'm not sure that Field_time::val_str() before this
change was really faster. Why was it?
The difference is:
- the body of get_date() repeated inside val_str() vs
- a call of get_date()
}
@@ -5431,13 +5541,51 @@ void Field_time_hires::sql_type(String &
"time(%u)", dec));
}
-void Field_time_hires::make_field(Send_field *field)
+void Field_time_with_dec::make_field(Send_field *field)
{
Field::make_field(field);
field->decimals= dec;
}
/****************************************************************************
+** time type with fsp (MySQL-5.6 version)
+** In string context: HH:MM:SS.FFFFFF
+** In number context: HHMMSS.FFFFFF
+****************************************************************************/
+
+void Field_timef::sql_type(String &res) const
+{
+ if (dec == 0)
+ {
+ res.set_ascii(STRING_WITH_LEN("time"));
+ return;
+ }
+ const CHARSET_INFO *cs= res.charset();
+ res.length(cs->cset->snprintf(cs, (char*) res.ptr(), res.alloced_length(),
+ "time(%d)", dec));
+}
Why would your mysql-5.6 types need a separate sql_type() method?
It doesn't seem to be anything that couldn't go into the base class
I preserved Field_time::sql_type() and Field_time_hires::sql_type()
for performance purposes.
Because:
- TIME(0) does not have dec and prints "time".
- TIME(N) MariaDB type always has dec>0 and prints "time(N)".
- TIME(N) MySQL56 type supports both dec=0 and dec>0,
and needs to print "time" or "time(N)" depending on dec.
It should be OK to move this code into the base class and remove
Field_time::sql_type() and Field_time_hires::sql_type().
Field_time and Field_time_hires will get a very very little bit slower :)
But sql_type() is not a heavily used method anyway.
Will do this.
+
+int Field_timef::reset()
+{
+ my_time_packed_to_binary(0, ptr, dec);
+ return 0;
+}
+
+void Field_timef::store_TIME(MYSQL_TIME *ltime)
+{
+ my_time_trunc(ltime, decimals());
+ longlong tmp= TIME_to_longlong_time_packed(ltime);
+ my_time_packed_to_binary(tmp, ptr, dec);
+}
+
+bool Field_timef::get_date(MYSQL_TIME *ltime, ulonglong fuzzydate)
+{
+ longlong tmp= my_time_packed_from_binary(ptr, dec);
+ TIME_from_longlong_time_packed(ltime, tmp);
+ return false;
+}
+
+/****************************************************************************
** year type
** Save in a byte the year 0, 1901->2155
** Can handle 2 byte or 4 byte years!
@@ -9413,17 +9494,6 @@ bool Create_field::init(THD *thd, char *
DBUG_RETURN(TRUE);
}
- switch (fld_type) {
- case MYSQL_TYPE_DATE:
- case MYSQL_TYPE_NEWDATE:
- case MYSQL_TYPE_TIME:
- case MYSQL_TYPE_DATETIME:
- case MYSQL_TYPE_TIMESTAMP:
- charset= &my_charset_numeric;
- flags|= BINARY_FLAG;
- default: break;
- }
-
why?
This is the advantage of moving Field_temporal outside of Field_str.
Field_temporal does not need charset to its constructors/initializers
anymore.
DBUG_RETURN(FALSE); /* success */
}
=== modified file 'sql/field.h'
--- sql/field.h 2013-06-06 15:51:28 +0000
+++ sql/field.h 2013-06-19 09:46:34 +0000
@@ -428,6 +464,27 @@ class Field
virtual uint32 key_length() const { return pack_length(); }
virtual enum_field_types type() const =0;
virtual enum_field_types real_type() const { return type(); }
+ virtual enum_field_types binlog_type() const
+ {
+ /*
+ Binlog stores field->type() as type code by default.
+ This puts MYSQL_TYPE_STRING in case of CHAR, VARCHAR, SET and ENUM,
+ with extra data type details put into metadata.
+
+ We cannot store field->type() in case of temporal types with
+ fractional seconds: TIME(n), DATETIME(n) and TIMESTAMP(n),
+ because binlog records with MYSQL_TYPE_TIME, MYSQL_TYPE_DATETIME
+ type codes do not have metadata.
+ So for MySQL-5.6 temporal data types with fractional seconds we'll store
+ real_type() type codes instead, i.e.
+ MYSQL_TYPE_TIME2, MYSQL_TYPE_DATETIME2, MYSQL_TYPE_TIMESTAMP2,
+ and put precision into metatada.
+
+ Note: perhaps binlog should eventually be modified to store
+ real_type() instead of type() for all column types.
+ */
+ return type();
+ }
Eh, I don't like it. So, to replicate with RBR between tables with
TIME columns of different precision, one needs to create tables
in MySQL 5.6? That's very lame thing to say to users :(
I'd rather fix it to work for all temporal types, both 5.6 and ours.
Perhaps, indeed, store real_type() as you write above.
But in a separate changeset. And here let's just say that it doesn't
work at all, and we always put MYSQL_TYPE_TIME/etc in the binlog.
Well. I copied the method, and its comments from MySQL
(and changed the comment slightly).
The phrases:
"We cannot store field->type() in case of temporal types with
fractional seconds"
and
"Note: perhaps binlog should eventually be modified to store
real_type() instead of type() for all column types."
both come from MySQL.
Note: if you start MariaDB on top of MySQL56 tables, RBR will work
even for field of differrent fractional second precision.
So everything should be fine with the code, I believe.
But I agree that the MySQL56 comment might be confusing in
MariaDB sources context.
I will try to rephrase the comment to make it more obvious
that it explains how *MySQL56* works.
inline int cmp(const uchar *str) { return cmp(ptr,str); }
virtual int cmp_max(const uchar *a, const uchar *b, uint max_len)
{ return cmp(a, b); }
@@ -661,6 +718,16 @@ class Field
{ return binary() ? &my_charset_bin : charset(); }
virtual CHARSET_INFO *sort_charset(void) const { return charset(); }
virtual bool has_charset(void) const { return FALSE; }
+ /*
+ match_collation_to_optimize_range() is to distinguish in
+ range optimizer (see opt_range.cc) between real string types:
+ CHAR, VARCHAR, TEXT
+ and the other string-alike types with result_type() == STRING_RESULT:
+ DATE, TIME, DATETIME, TIMESTAMP
+ We need it to decide whether to test if collation of the operation
+ matches collation of the field (needed only for real string types).
+ */
+ virtual bool match_collation_to_optimize_range() const { return false; }
Not needed. temporal types are not string-alike,
they have have cmp_type() == TIME_RESULT,
string types have STRING_RESULT.
This is to get rid of this cast in opt_range.cc:
- ((Field_str*) field)->match_collation_to_optimize_range() &&
+ field->match_collation_to_optimize_range() &&
This kind of casts from a parent to a child
are very harmful and potentially buggy.
I prefer to have publuc methods in the top level class,
to avoid such casts.
The best way to fix this properly would be to add a new method if Field
implementing a bigger part of the condition use in opt_range.cc:
if (field->result_type() == STRING_RESULT &&
((Field_str*) field)->match_collation_to_optimize_range() &&
value->result_type() == STRING_RESULT &&
key_part->image_type == Field::itRAW &&
((Field_str*)field)->charset() != conf_func->compare_collation() &&
!(conf_func->compare_collation()->state & MY_CS_BINSORT &&
(type == Item_func::EQUAL_FUNC || type == Item_func::EQ_FUNC)))
goto end;
The new method should incorporate all parts of the above condition
that use "field":
- field->result_type() == STRING_RESULT
- ((Field_str*) field)->match_collation_to_optimize_range()
- ((Field_str*)field)->charset() != conf_func->compare_collation()
I suggest to do it in a separate patch and use my version now.
virtual void set_charset(CHARSET_INFO *charset_arg) { }
virtual enum Derivation derivation(void) const
{ return DERIVATION_IMPLICIT; }
@@ -1345,7 +1407,67 @@ class Field_null :public Field_str {
};
-class Field_timestamp :public Field_str {
+class Field_temporal: public Field {
+public:
+ Field_temporal(uchar *ptr_arg,uint32 len_arg, uchar *null_ptr_arg,
+ uchar null_bit_arg, utype unireg_check_arg,
+ const char *field_name_arg)
+ :Field(ptr_arg, len_arg, null_ptr_arg, null_bit_arg, unireg_check_arg,
+ field_name_arg)
+ { flags|= BINARY_FLAG; }
+ Item_result result_type () const { return STRING_RESULT; }
+ uint32 max_display_length() { return field_length; }
+ bool str_needs_quotes() { return TRUE; }
+ enum Derivation derivation(void) const { return DERIVATION_NUMERIC; }
+ uint repertoire(void) const { return MY_REPERTOIRE_NUMERIC; }
+ CHARSET_INFO *charset(void) const { return &my_charset_numeric; }
+ const CHARSET_INFO *sort_charset(void) const { return &my_charset_bin; }
+ bool binary() const { return true; }
+ enum Item_result cmp_type () const { return TIME_RESULT; }
+ uint is_equal(Create_field *new_field);
+ bool eq_def(Field *field)
+ {
+ return (Field::eq_def(field) && decimals() == field->decimals());
+ }
+ int store_decimal(const my_decimal *);
+ my_decimal *val_decimal(my_decimal*);
+ void set_warnings(MYSQL_ERROR::enum_warning_level trunc_level,
+ const ErrConv *str, int was_cut, timestamp_type ts_type);
+ double pos_in_interval(Field *min, Field *max)
+ {
+ return pos_in_interval_val_str(min, max, 0);
Really? This would first convert a temporal value to a string,
and then use a quite complicated procedure of placing a string
in the interval. Wouldn't it be much cheaper to use
pos_in_interval_val_real() ?
A native method should obviously be written eventially for
the temporal data types.
Now I just left it how it worked before my patch:
temporal data types used the method from their former
parent Field_str.
I can change to pos_in_interval_val_real().
But it does not have enough precision for all types, e.g. DATETIME(6).
What do you suggest?
+ }
+};
+
+
+/**
+ Abstract class for:
+ - DATE
+ - DATETIME
+ - DATETIME(1..6)
+ - DATETIME(0..6) - MySQL56 version
+*/
+class Field_temporal_with_date: public Field_temporal {
+protected:
+ int store_TIME_with_warning(MYSQL_TIME *ltime, const ErrConv *str,
+ int was_cut, int have_smth_to_conv);
Eh? So you'll have *three* different store_TIME_with_warning() methods?
I didn't like my version, because I had *two*,
there should've been only one.
I don't see any harm with that:
- They are protected (not public).
- They all three do quite different things. No duplicate code.
(well, there are 3 or 4 similar lines out of 20. this doesn't count).
- The old implementation of Field_temporal had conditions like:
if (am I time?)
{
do time related things;
}
else
{
do datetime related things;
}
- The Field_timestamp implementation even has a different set of parameters.
Looks like a very natural change for me.
+ virtual void store_TIME(MYSQL_TIME *ltime) = 0;
+public:
+ Field_temporal_with_date(uchar *ptr_arg, uint32 len_arg,
+ uchar *null_ptr_arg, uchar null_bit_arg,
+ utype unireg_check_arg,
+ const char *field_name_arg)
+ :Field_temporal(ptr_arg, len_arg, null_ptr_arg, null_bit_arg,
+ unireg_check_arg, field_name_arg)
+ {}
+ int store(const char *to, uint length, CHARSET_INFO *charset);
+ int store(double nr);
+ int store(longlong nr, bool unsigned_val);
+ int store_time_dec(MYSQL_TIME *ltime, uint dec);
+};
+
+
+class Field_timestamp :public Field_temporal {
protected:
int store_TIME_with_warning(THD *, MYSQL_TIME *, const ErrConv *,
bool, bool);
=== modified file 'sql/opt_range.cc'
--- sql/opt_range.cc 2013-06-06 19:32:29 +0000
+++ sql/opt_range.cc 2013-06-18 10:35:35 +0000
@@ -8014,10 +8014,10 @@ get_mm_leaf(RANGE_OPT_PARAM *param, COND
*/
if (field->result_type() == STRING_RESULT &&
this and below should probably use cmp_type, not result_type
Probably. Why not to do it in a separate patch?
- ((Field_str*) field)->match_collation_to_optimize_range() &&
+ field->match_collation_to_optimize_range() &&
value->result_type() == STRING_RESULT &&
key_part->image_type == Field::itRAW &&
- ((Field_str*)field)->charset() != conf_func->compare_collation() &&
+ field->charset() != conf_func->compare_collation() &&
!(conf_func->compare_collation()->state & MY_CS_BINSORT &&
(type == Item_func::EQUAL_FUNC || type == Item_func::EQ_FUNC)))
goto end;
=== modified file 'sql/sql_time.h'
--- sql/sql_time.h 2012-08-31 12:15:52 +0000
+++ sql/sql_time.h 2013-06-18 10:35:36 +0000
@@ -110,4 +110,23 @@ extern DATE_TIME_FORMAT global_time_form
extern KNOWN_DATE_TIME_FORMAT known_date_time_formats[];
extern LEX_STRING interval_type_to_name[];
+/* Date/time rounding and truncation functions */
+inline long my_time_fraction_remainder(long nr, uint decimals)
+{
+ DBUG_ASSERT(decimals <= TIME_SECOND_PART_DIGITS);
+ return nr % (long) log_10_int[TIME_SECOND_PART_DIGITS - decimals];
+}
+inline void my_time_trunc(MYSQL_TIME *ltime, uint decimals)
+{
+ ltime->second_part-= my_time_fraction_remainder(ltime->second_part, decimals);
+}
good idea, but
1. put it in my_time.h instead of sec_part_truncate()
2. change item_timefunc.cc (and other files too) to use it
(this will remove all uses of sec_part_truncate())
Okey.
+inline void my_datetime_trunc(MYSQL_TIME *ltime, uint decimals)
+{
+ return my_time_trunc(ltime, decimals);
+}
+inline void my_timeval_trunc(struct timeval *tv, uint decimals)
+{
+ tv->tv_usec-= my_time_fraction_remainder(tv->tv_usec, decimals);
+}
please move all these four functions to my_time.h
Okey.
(although, I'd delete my_datetime_trunc() completely)
:)
+
#endif /* SQL_TIME_INCLUDED */
Regards,
Sergei
Follow ups
References