Hi, Aleksey! First - is commit b97e45651d1 the one I was supposed to review? On May 17, Aleksey Midenkov wrote:
revision-id: b97e45651d1 (mariadb-10.4.7-33-gb97e45651d1) parent(s): 7587975bf06 author: Aleksey Midenkov <midenok@gmail.com> committer: Aleksey Midenkov <midenok@gmail.com> timestamp: 2019-08-19 11:58:07 +0300 message:
MDEV-16937 Strict SQL with system versioned tables causes issues
Vers SQL: respect system fields in NO_ZERO_DATE mode.
This is the subject for refactoring in tempesta-tech/mariadb#379
diff --git a/mysql-test/suite/versioning/engines.combinations b/mysql-test/suite/versioning/engines.combinations index 26b5bab23f1..57e2af6cd06 100644 --- a/mysql-test/suite/versioning/engines.combinations +++ b/mysql-test/suite/versioning/engines.combinations @@ -7,5 +7,10 @@ default-storage-engine=innodb [myisam] default-storage-engine=myisam
+[traditional] +default-storage-engine=myisam +sql-mode=traditional
I'm not sure about it. It's, of course, safe to run all tests with every possible settings, but it really blows up testing times. May be it'd make sense to have a more targeted test here?
+ [heap] default-storage-engine=memory + diff --git a/mysql-test/suite/versioning/r/select.result b/mysql-test/suite/versioning/r/select.result index 3569268ce1d..68df246af6b 100644 --- a/mysql-test/suite/versioning/r/select.result +++ b/mysql-test/suite/versioning/r/select.result @@ -45,7 +45,7 @@ ASOF_x y 7 107 8 108 9 109 -select x as FROMTO_x, y from t1 for system_time from timestamp '0-0-0 0:0:0' to timestamp @t1; +select x as FROMTO_x, y from t1 for system_time from timestamp '1970-01-01 00:00:00' to timestamp @t1;
Does this test use a fixed time zone? If not, is 1970-01-01 00:00:00 a valid timestamp in all time zones? If unsure, better use 1971, that will definitely be fine everywhere.
FROMTO_x y 0 100 1 101 diff --git a/sql/field.cc b/sql/field.cc index 969c32a5180..f5580239a9f 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -11139,6 +11139,13 @@ bool Field::save_in_field_default_value(bool view_error_processing) { THD *thd= table->in_use;
+ /* + TODO: refactor setting the system fields via default_value mechanism. + This condition will go away as well as other conditions with VERS_SYSTEM_FIELD. + */
MDEV? There's a pull request, but it is marked as obsolete
+ if (flags & VERS_SYSTEM_FIELD) + return false; + if (unlikely(flags & NO_DEFAULT_VALUE_FLAG && real_type() != MYSQL_TYPE_ENUM)) { diff --git a/sql/unireg.cc b/sql/unireg.cc index d019b5f8a75..484ad0334e4 100644 --- a/sql/unireg.cc +++ b/sql/unireg.cc @@ -1060,7 +1060,8 @@ static bool make_empty_rec(THD *thd, uchar *buff, uint table_options, !f_bit_as_char(field->pack_flag)) null_count+= field->length & 7;
- error= make_empty_rec_store_default(thd, regfield, field->default_value); + if (!(field->flags & VERS_SYSTEM_FIELD)) + error= make_empty_rec_store_default(thd, regfield, field->default_value);
will the field be initialized at all? It should have something, even bzero will do, but we shouldn't dump uninited memory to frm.
delete regfield; // Avoid memory leaks if (error) goto err;
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org