fix(tianmu): fix storage of DT type

[duanjr]

Summary about this PR

fix: fix storage of DT type (#1175)

This issue actually only occurs when we use tianmu engine.

In the implemtation of tianmu engine, DATETIME and TIME share same data struct DT

union {
      uint32_t time_hour : 10;  // -838 to 838
      struct {
        uint32_t hour : 5;
        uint32_t day : 5;
        uint32_t month : 4;
        uint32_t year : 14;
        uint32_t padding : 3;
        uint32_t neg : 1;
      };
    };

However, we didn't distinguish between them correctly when storing:

void Store(MYSQL_TIME *my_time, enum_mysql_timestamp_type t) {
    my_time->year = year;
    my_time->month = month;
    my_time->day = day;
    my_time->hour = hour;
    my_time->minute = minute;
    my_time->second = second;
    my_time->second_part = microsecond;
    my_time->neg = neg;
    my_time->time_type = t;

    if (t == MYSQL_TIMESTAMP_TIME)
      my_time->hour = time_hour;
  }

For instace, if we store TIME: 32:00:00, my_time->day will be 1, instead of 0 (hour part takes 5 bit), and the select result will be 56:00:00

I changed the code segment to:

  void Store(MYSQL_TIME *my_time, enum_mysql_timestamp_type t) {
    if (t == MYSQL_TIMESTAMP_TIME) {
      my_time->year = 0;
      my_time->month = 0;
      my_time->day = 0;
      my_time->hour = time_hour;
    } else {
      my_time->year = year;
      my_time->month = month;
      my_time->day = day;
      my_time->hour = hour;
    }
    my_time->minute = minute;
    my_time->second = second;
    my_time->second_part = microsecond;
    my_time->neg = neg;
    my_time->time_type = t;
  }

Tests Check List

• Unit test
• Integration test
• Manual test (add detailed scripts or steps below)
• No code

Manual test scripts :

drop table t1;
CREATE TABLE t1 (a TIME NOT NULL);
INSERT INTO t1 VALUES ('00:00:02.9');
INSERT INTO t1 VALUES ('800:00:02');
INSERT INTO t1 VALUES ('-800:00:02.9');
select * from t1;

output:

+------------+
| a          |
+------------+
| 00:00:03   |
| 800:00:02  |
| -800:00:03 |
+------------+

Changelog

• New Feature
• Bug Fix
• Performance Improvement
• Build/Testing/CI/CD
• Documentation
• Not for changelog (changelog entry is not required)

Documentation

• Affects user behaviors
• Contains syntax changes
• Contains variable changes
• Contains experimental features

[mergify]

Thanks for the contribution!
I have applied any labels matching special text in your PR Changelog.

Please review the labels and make any necessary changes.

[konghaiya]

Very good, this seems like a great solution, but it seems to require some test cases to ensure the correctness of the modifications
You can refer to the MySQL official website, which explains how to write MTR
http://dev.mysql.com/doc/mysqltest/2.0/en/writing-tests-quick-start.html

[duanjr]

I've write a simple MTR test file issue1175.test:

--source include/have_tianmu.inc

--disable_warnings
DROP DATABASE IF EXISTS issue1175_test;
--enable_warnings

CREATE DATABASE issue1175_test;

USE issue1175_test;

--disable_warnings

CREATE TABLE t1 (
  id INT AUTO_INCREMENT PRIMARY KEY,
  t TIME,
  d DATE,
  dt DATETIME,
  ts TIMESTAMP
) ENGINE=TIANMU;


INSERT INTO t1 (t, d, dt, ts) VALUES
('00:00:00', '2000-01-01', '2000-01-01 00:00:00', '2000-01-01 00:00:00'),
('-838:59:59', '2001-02-28', '2001-02-28 00:00:00', '2001-02-28 00:00:00'),
('838:59:59', '2002-03-31', '2002-03-31 23:59:59', '2002-03-31 23:59:59'),
('-100:00:00', '2010-12-31', '2010-12-31 12:34:56', '2010-12-31 12:34:56'),
('200:00:00', '2011-11-30', '2011-11-30 18:00:00', '2011-11-30 18:00:00'),
('-400:00:00', '2012-02-29', '2012-02-29 01:23:45', '2012-02-29 01:23:45'),
('720:00:00', '2020-10-31', '2020-10-31 14:00:00', '2020-10-31 14:00:00'),
('05:30:15', '1985-04-12', '1985-04-12 05:30:15.123', '1985-04-12 05:30:15.123'),
('-23:45:01', '1995-08-24', '1995-08-24 15:45:30.987', '1995-08-24 15:45:30.987'),
('67:15:45', '2005-06-15', '2005-06-15 10:59:59.001', '2005-06-15 10:59:59.001'),
('-115:00:00', '2007-12-25', '2007-12-25 23:30:45.500', '2007-12-25 23:30:45.500'),
('350:30:15', '2015-09-05', '2015-09-05 12:00:00.999', '2015-09-05 12:00:00.999'),
('-500:45:30', '2017-03-01', '2017-03-01 01:23:45.250', '2017-03-01 01:23:45.250'),
('600:15:00', '2021-05-01', '2021-05-01 17:30:00.123', '2021-05-01 17:30:00.123');

SELECT * FROM t1 ORDER BY id;

DROP DATABASE issue1175_test;

If we directly put these cmds into a stonedb instance, the select result is correct.
But if I use ./mtr --suite=tianmu tianmu.issue1175 to produce the result, the result seems very strange:

1       00:00:00        2000-01-01      2000-01-01 00:00:00     2000-01-01 00:00:00
2       06:59:59        2001-02-28      2001-02-28 00:00:00     2001-02-28 00:00:00
3       06:59:59        2002-03-31      2002-03-31 23:59:59     2002-03-31 23:59:59
4       04:00:00        2010-12-31      2010-12-31 12:34:56     2010-12-31 12:34:56
5       08:00:00        2011-11-30      2011-11-30 18:00:00     2011-11-30 18:00:00
6       16:00:00        2012-02-29      2012-02-29 01:23:45     2012-02-29 01:23:45
7       16:00:00        2020-10-31      2020-10-31 14:00:00     2020-10-31 14:00:00
8       05:30:15        1985-04-12      1985-04-12 05:30:15     1985-04-12 05:30:15
9       23:45:01        1995-08-24      1995-08-24 15:45:31     1995-08-24 15:45:31
10      03:15:45        2005-06-15      2005-06-15 10:59:59     2005-06-15 10:59:59
11      19:00:00        2007-12-25      2007-12-25 23:30:46     2007-12-25 23:30:46
12      30:30:15        2015-09-05      2015-09-05 12:00:01     2015-09-05 12:00:01
13      20:45:30        2017-03-01      2017-03-01 01:23:45     2017-03-01 01:23:45
14      24:15:00        2021-05-01      2021-05-01 17:30:00     2021-05-01 17:30:00

This Issue still only occurs when we use tianmu engine, and it performs the same before and after my change. I'm not sure if this worth a new issue or it's a part of the old issue.

[duanjr]

I think the case of incorrect test performance mentioned in my last comment is also part of this issue. And it can be fixed by distinguish between them correctly in tianmu_table.cpp.
I've corrected it and add corresponding MTR in commit 0316185.

[duanjr]

I notice test tianmu.zerofill failed on debian10_1.0.3, and tianmu.alter_table1 failed on centos7_1.0.3.
But these two cases passed in my local test (on Ubuntu20.04), and I notice that some merged PR also failed some tests, so I'm not sure if I need do some further work to pass the CI.

[hustjieke]

I notice test tianmu.zerofill failed on debian10_1.0.3, and tianmu.alter_table1 failed on centos7_1.0.3. But these two cases passed in my local test (on Ubuntu20.04), and I notice that some merged PR also failed some tests, so I'm not sure if I need do some further work to pass the CI.

I try to re-run failed test, mtr in centos is OK now, debian is running.

[codecov]

Codecov Report

Patch coverage: 70.83% and project coverage change: -0.09 ⚠️

Comparison is base (74b5b05) 55.18% compared to head (0316185) 55.09%.

❗ Current head 0316185 differs from pull request most recent head 4436c57. Consider uploading reports for the commit 4436c57 to get more accurate results

Additional details and impacted files

@@                 Coverage Diff                 @@
##           stonedb-5.7-dev    #1752      +/-   ##
===================================================
- Coverage            55.18%   55.09%   -0.09%     
===================================================
  Files                 2031     2031              
  Lines               431436   431448      +12     
===================================================
- Hits                238081   237704     -377     
- Misses              193355   193744     +389     

Impacted Files	Coverage Δ
storage/tianmu/core/tianmu_table.cpp	67.61% <56.25%> (-0.12%)	⬇️
storage/tianmu/types/tianmu_data_types.h	59.72% <100.00%> (+0.85%)	⬆️

... and 88 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[hustjieke]

LGTM

[hustjieke]

LGTM