Skip to content

Commit

Permalink
MDEV-15176 Storing DATETIME-alike VARCHAR data into TIME produces wro…
Browse files Browse the repository at this point in the history
…ng results

When storing '0001-01-01 10:20:30x', execution went throw the last code
branch in Field_time::store_TIME_with_warning(), around the test
for (ltime->year || ltime->month). This then resulted into wrong results
because:

1. Field_time::store_TIME() does not check YYYYMM against zero.
  It assumes that ltime->days and ltime->hours are already properly set.
  So it mixed days to hours, even when YYYYMM was not zero.

2. Field_time_hires::store_TIME() does not check YYYYMM against zero.
  It assumes that ltime->year, ltime->month, ltime->days and ltime->hours
  are already properly set. So it always mixed days and even months(!) and years(!)
  to hours, using pack_time(). This gave even worse results comparing to #2.

3. Field_timef::store_TIME() did not check the entire YYYYMM for being zero.
  It only checked MM, but did not check YYYY. In case of a zero MM,
  it mixed days to hours, even if YYYY was not zero.
  The wrong code was in TIME_to_longlong_time_packed().

In the new reduction Field_time::store_TIME_with_warning() is responsible
to prepare the YYYYYMMDD part properly in all code branches
(with trailing garbage like 'x' and without trailing garbage).
It was reorganized into a more straightforward style.

Field_time:store_TIME(), Field_time_hires::store_TIME() and
TIME_to_longlong_time_packed() were fixed to do a DBUG_ASSERT
on non-zero ltime->year or ltime->month. The code testing ltime->month
was removed from TIME_to_longlong_time_packed(), as it's now
properly done on the caller level.

Truncation was moved from Field_timef::store_TIME() to
Field_time::store_TIME_with_warning().

So now all thee methods Field_time*::store_TIME() assume a properly
set input value:
- Only zero ltime->year and ltime->month are allowed.
- The value must be already properly truncated according to decimals()
  (this will help to add rounding soon, see MDEV-8894)

A "const" qualifier was added to the argument of Field_time*::store_TIME().
  • Loading branch information
Alexander Barkov committed Feb 4, 2018
1 parent 2ecf2f9 commit 28d4cf0
Show file tree
Hide file tree
Showing 5 changed files with 202 additions and 23 deletions.
129 changes: 129 additions & 0 deletions mysql-test/r/type_time.result
Original file line number Diff line number Diff line change
Expand Up @@ -1283,3 +1283,132 @@ a
10:20:32
DROP TABLE t1;
SET timestamp=DEFAULT;
#
# MDEV-15176 Storing DATETIME-alike VARCHAR data into TIME produces wrong results
#
SET sql_mode='';
CREATE OR REPLACE TABLE t0 (d VARCHAR(64));
INSERT INTO t0 VALUES ('0000-00-00 10:20:30');
INSERT INTO t0 VALUES ('0000-00-01 10:20:30');
INSERT INTO t0 VALUES ('0000-01-00 10:20:30');
INSERT INTO t0 VALUES ('0000-01-01 10:20:30');
INSERT INTO t0 VALUES ('0001-00-00 10:20:30');
INSERT INTO t0 VALUES ('0001-00-01 10:20:30');
INSERT INTO t0 VALUES ('0001-01-00 10:20:30');
INSERT INTO t0 VALUES ('0001-01-01 10:20:30');
SET @@global.mysql56_temporal_format=false;
CREATE OR REPLACE TABLE t1 (d VARCHAR(64), t0 TIME(0), t1 TIME(1));
INSERT INTO t1 SELECT d,d,d FROM t0;
Warnings:
Note 1265 Data truncated for column 't0' at row 3
Note 1265 Data truncated for column 't1' at row 3
Note 1265 Data truncated for column 't0' at row 4
Note 1265 Data truncated for column 't1' at row 4
Note 1265 Data truncated for column 't0' at row 5
Note 1265 Data truncated for column 't1' at row 5
Note 1265 Data truncated for column 't0' at row 6
Note 1265 Data truncated for column 't1' at row 6
Note 1265 Data truncated for column 't0' at row 7
Note 1265 Data truncated for column 't1' at row 7
Note 1265 Data truncated for column 't0' at row 8
Note 1265 Data truncated for column 't1' at row 8
SELECT * FROM t1 ORDER BY d;
d t0 t1
0000-00-00 10:20:30 10:20:30 10:20:30.0
0000-00-01 10:20:30 34:20:30 34:20:30.0
0000-01-00 10:20:30 10:20:30 10:20:30.0
0000-01-01 10:20:30 10:20:30 10:20:30.0
0001-00-00 10:20:30 10:20:30 10:20:30.0
0001-00-01 10:20:30 10:20:30 10:20:30.0
0001-01-00 10:20:30 10:20:30 10:20:30.0
0001-01-01 10:20:30 10:20:30 10:20:30.0
DROP TABLE t1;
CREATE OR REPLACE TABLE t1 (d VARCHAR(64), t0 TIME(0), t1 TIME(1));
INSERT INTO t1 SELECT CONCAT(d,'x'),CONCAT(d,'x'),CONCAT(d,'x') FROM t0;
Warnings:
Warning 1265 Data truncated for column 't0' at row 1
Warning 1265 Data truncated for column 't1' at row 1
Warning 1265 Data truncated for column 't0' at row 2
Warning 1265 Data truncated for column 't1' at row 2
Warning 1265 Data truncated for column 't0' at row 3
Warning 1265 Data truncated for column 't1' at row 3
Warning 1265 Data truncated for column 't0' at row 4
Warning 1265 Data truncated for column 't1' at row 4
Warning 1265 Data truncated for column 't0' at row 5
Warning 1265 Data truncated for column 't1' at row 5
Warning 1265 Data truncated for column 't0' at row 6
Warning 1265 Data truncated for column 't1' at row 6
Warning 1265 Data truncated for column 't0' at row 7
Warning 1265 Data truncated for column 't1' at row 7
Warning 1265 Data truncated for column 't0' at row 8
Warning 1265 Data truncated for column 't1' at row 8
SELECT * FROM t1;
d t0 t1
0000-00-00 10:20:30x 10:20:30 10:20:30.0
0000-00-01 10:20:30x 34:20:30 34:20:30.0
0000-01-00 10:20:30x 10:20:30 10:20:30.0
0000-01-01 10:20:30x 10:20:30 10:20:30.0
0001-00-00 10:20:30x 10:20:30 10:20:30.0
0001-00-01 10:20:30x 10:20:30 10:20:30.0
0001-01-00 10:20:30x 10:20:30 10:20:30.0
0001-01-01 10:20:30x 10:20:30 10:20:30.0
DROP TABLE t1;
SET @@global.mysql56_temporal_format=true;
CREATE OR REPLACE TABLE t1 (d VARCHAR(64), t0 TIME(0), t1 TIME(1));
INSERT INTO t1 SELECT d,d,d FROM t0;
Warnings:
Note 1265 Data truncated for column 't0' at row 3
Note 1265 Data truncated for column 't1' at row 3
Note 1265 Data truncated for column 't0' at row 4
Note 1265 Data truncated for column 't1' at row 4
Note 1265 Data truncated for column 't0' at row 5
Note 1265 Data truncated for column 't1' at row 5
Note 1265 Data truncated for column 't0' at row 6
Note 1265 Data truncated for column 't1' at row 6
Note 1265 Data truncated for column 't0' at row 7
Note 1265 Data truncated for column 't1' at row 7
Note 1265 Data truncated for column 't0' at row 8
Note 1265 Data truncated for column 't1' at row 8
SELECT * FROM t1;
d t0 t1
0000-00-00 10:20:30 10:20:30 10:20:30.0
0000-00-01 10:20:30 34:20:30 34:20:30.0
0000-01-00 10:20:30 10:20:30 10:20:30.0
0000-01-01 10:20:30 10:20:30 10:20:30.0
0001-00-00 10:20:30 10:20:30 10:20:30.0
0001-00-01 10:20:30 10:20:30 10:20:30.0
0001-01-00 10:20:30 10:20:30 10:20:30.0
0001-01-01 10:20:30 10:20:30 10:20:30.0
DROP TABLE t1;
CREATE OR REPLACE TABLE t1 (d VARCHAR(64), t0 TIME(0), t1 TIME(1));
INSERT INTO t1 SELECT CONCAT(d,'x'),CONCAT(d,'x'),CONCAT(d,'x') FROM t0;
Warnings:
Warning 1265 Data truncated for column 't0' at row 1
Warning 1265 Data truncated for column 't1' at row 1
Warning 1265 Data truncated for column 't0' at row 2
Warning 1265 Data truncated for column 't1' at row 2
Warning 1265 Data truncated for column 't0' at row 3
Warning 1265 Data truncated for column 't1' at row 3
Warning 1265 Data truncated for column 't0' at row 4
Warning 1265 Data truncated for column 't1' at row 4
Warning 1265 Data truncated for column 't0' at row 5
Warning 1265 Data truncated for column 't1' at row 5
Warning 1265 Data truncated for column 't0' at row 6
Warning 1265 Data truncated for column 't1' at row 6
Warning 1265 Data truncated for column 't0' at row 7
Warning 1265 Data truncated for column 't1' at row 7
Warning 1265 Data truncated for column 't0' at row 8
Warning 1265 Data truncated for column 't1' at row 8
SELECT * FROM t1 ORDER BY d;
d t0 t1
0000-00-00 10:20:30x 10:20:30 10:20:30.0
0000-00-01 10:20:30x 34:20:30 34:20:30.0
0000-01-00 10:20:30x 10:20:30 10:20:30.0
0000-01-01 10:20:30x 10:20:30 10:20:30.0
0001-00-00 10:20:30x 10:20:30 10:20:30.0
0001-00-01 10:20:30x 10:20:30 10:20:30.0
0001-01-00 10:20:30x 10:20:30 10:20:30.0
0001-01-01 10:20:30x 10:20:30 10:20:30.0
DROP TABLE t1;
DROP TABLE t0;
SET sql_mode=DEFAULT;
38 changes: 38 additions & 0 deletions mysql-test/t/type_time.test
Original file line number Diff line number Diff line change
Expand Up @@ -776,3 +776,41 @@ INSERT INTO t1 VALUES ('10:20:30'),('10:20:31'),('10:20:32');
SELECT a FROM t1 WHERE a IN (102030,TIME'10:20:31',TIMESTAMP'2001-01-01 10:20:32') ORDER BY a;
DROP TABLE t1;
SET timestamp=DEFAULT;

--echo #
--echo # MDEV-15176 Storing DATETIME-alike VARCHAR data into TIME produces wrong results
--echo #

SET sql_mode='';
CREATE OR REPLACE TABLE t0 (d VARCHAR(64));
INSERT INTO t0 VALUES ('0000-00-00 10:20:30');
INSERT INTO t0 VALUES ('0000-00-01 10:20:30');
INSERT INTO t0 VALUES ('0000-01-00 10:20:30');
INSERT INTO t0 VALUES ('0000-01-01 10:20:30');
INSERT INTO t0 VALUES ('0001-00-00 10:20:30');
INSERT INTO t0 VALUES ('0001-00-01 10:20:30');
INSERT INTO t0 VALUES ('0001-01-00 10:20:30');
INSERT INTO t0 VALUES ('0001-01-01 10:20:30');

SET @@global.mysql56_temporal_format=false;
CREATE OR REPLACE TABLE t1 (d VARCHAR(64), t0 TIME(0), t1 TIME(1));
INSERT INTO t1 SELECT d,d,d FROM t0;
SELECT * FROM t1 ORDER BY d;
DROP TABLE t1;
CREATE OR REPLACE TABLE t1 (d VARCHAR(64), t0 TIME(0), t1 TIME(1));
INSERT INTO t1 SELECT CONCAT(d,'x'),CONCAT(d,'x'),CONCAT(d,'x') FROM t0;
SELECT * FROM t1;
DROP TABLE t1;

SET @@global.mysql56_temporal_format=true;
CREATE OR REPLACE TABLE t1 (d VARCHAR(64), t0 TIME(0), t1 TIME(1));
INSERT INTO t1 SELECT d,d,d FROM t0;
SELECT * FROM t1;
DROP TABLE t1;
CREATE OR REPLACE TABLE t1 (d VARCHAR(64), t0 TIME(0), t1 TIME(1));
INSERT INTO t1 SELECT CONCAT(d,'x'),CONCAT(d,'x'),CONCAT(d,'x') FROM t0;
SELECT * FROM t1 ORDER BY d;
DROP TABLE t1;

DROP TABLE t0;
SET sql_mode=DEFAULT;
6 changes: 4 additions & 2 deletions sql/compat56.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,10 @@
*/
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) |
DBUG_ASSERT(ltime->year == 0);
DBUG_ASSERT(ltime->month == 0);
// Mix days with hours: "1 00:10:10" -> "24:00:10"
long hms= ((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;
Expand Down
41 changes: 23 additions & 18 deletions sql/field.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5699,34 +5699,38 @@ int Field_time::store_TIME_with_warning(MYSQL_TIME *ltime,
int was_cut,
int have_smth_to_conv)
{
Sql_condition::enum_warning_level trunc_level= Sql_condition::WARN_LEVEL_WARN;
int ret= 2;
ASSERT_COLUMN_MARKED_FOR_WRITE_OR_COMPUTED;

if (!have_smth_to_conv)
{
bzero(ltime, sizeof(*ltime));
was_cut= MYSQL_TIME_WARN_TRUNCATED;
ret= 1;
store_TIME(ltime);
set_warnings(Sql_condition::WARN_LEVEL_WARN, str, MYSQL_TIME_WARN_TRUNCATED);
return 1;
}
else if (!MYSQL_TIME_WARN_HAVE_WARNINGS(was_cut) &&
((ltime->year || ltime->month) ||
MYSQL_TIME_WARN_HAVE_NOTES(was_cut)))
if (ltime->year != 0 || ltime->month != 0)
{
if (ltime->year || ltime->month)
ltime->year= ltime->month= ltime->day= 0;
trunc_level= Sql_condition::WARN_LEVEL_NOTE;
was_cut|= MYSQL_TIME_WARN_TRUNCATED;
ret= 3;
ltime->year= ltime->month= ltime->day= 0;
was_cut|= MYSQL_TIME_NOTE_TRUNCATED;
}
set_warnings(trunc_level, str, was_cut, MYSQL_TIMESTAMP_TIME);
my_time_trunc(ltime, decimals());
store_TIME(ltime);
return was_cut ? ret : 0;
if (!MYSQL_TIME_WARN_HAVE_WARNINGS(was_cut) &&
MYSQL_TIME_WARN_HAVE_NOTES(was_cut))
{
set_warnings(Sql_condition::WARN_LEVEL_NOTE, str,
was_cut | MYSQL_TIME_WARN_TRUNCATED);
return 3;
}
set_warnings(Sql_condition::WARN_LEVEL_WARN, str, was_cut);
return was_cut ? 2 : 0;
}


void Field_time::store_TIME(MYSQL_TIME *ltime)
void Field_time::store_TIME(const MYSQL_TIME *ltime)
{
DBUG_ASSERT(ltime->year == 0);
DBUG_ASSERT(ltime->month == 0);
long tmp= (ltime->day*24L+ltime->hour)*10000L +
(ltime->minute*100+ltime->second);
if (ltime->neg)
Expand Down Expand Up @@ -5960,8 +5964,10 @@ int Field_time_hires::reset()
}


void Field_time_hires::store_TIME(MYSQL_TIME *ltime)
void Field_time_hires::store_TIME(const MYSQL_TIME *ltime)
{
DBUG_ASSERT(ltime->year == 0);
DBUG_ASSERT(ltime->month == 0);
ulonglong packed= sec_part_shift(pack_time(ltime), dec) + zero_point;
store_bigendian(packed, ptr, Field_time_hires::pack_length());
}
Expand Down Expand Up @@ -6143,9 +6149,8 @@ int Field_timef::reset()
return 0;
}

void Field_timef::store_TIME(MYSQL_TIME *ltime)
void Field_timef::store_TIME(const MYSQL_TIME *ltime)
{
my_time_trunc(ltime, decimals());
longlong tmp= TIME_to_longlong_time_packed(ltime);
my_time_packed_to_binary(tmp, ptr, dec);
}
Expand Down
11 changes: 8 additions & 3 deletions sql/field.h
Original file line number Diff line number Diff line change
Expand Up @@ -2682,9 +2682,14 @@ class Field_time :public Field_temporal {
*/
long curdays;
protected:
virtual void store_TIME(MYSQL_TIME *ltime);
virtual void store_TIME(const MYSQL_TIME *ltime);
int store_TIME_with_warning(MYSQL_TIME *ltime, const ErrConv *str,
int was_cut, int have_smth_to_conv);
void set_warnings(Sql_condition::enum_warning_level level,
const ErrConv *str, int was_cut)
{
Field_temporal::set_warnings(level, str, was_cut, MYSQL_TIMESTAMP_TIME);
}
bool check_zero_in_date_with_warn(ulonglong fuzzydate);
static void do_field_time(Copy_field *copy);
public:
Expand Down Expand Up @@ -2766,7 +2771,7 @@ class Field_time_with_dec :public Field_time {
*/
class Field_time_hires :public Field_time_with_dec {
longlong zero_point;
void store_TIME(MYSQL_TIME *ltime);
void store_TIME(const MYSQL_TIME *);
public:
Field_time_hires(uchar *ptr_arg, uchar *null_ptr_arg, uchar null_bit_arg,
enum utype unireg_check_arg, const LEX_CSTRING *field_name_arg,
Expand All @@ -2792,7 +2797,7 @@ class Field_time_hires :public Field_time_with_dec {
TIME(0..6) - MySQL56 version
*/
class Field_timef :public Field_time_with_dec {
void store_TIME(MYSQL_TIME *ltime);
void store_TIME(const MYSQL_TIME *);
int do_save_field_metadata(uchar *metadata_ptr)
{
*metadata_ptr= (uchar) decimals();
Expand Down

0 comments on commit 28d4cf0

Please sign in to comment.