From 0140bfac5e216bd7ba8ad324bd914d596bf59a1f Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Tue, 2 Oct 2018 22:50:28 +0200 Subject: [PATCH] MDEV-16127 mroonga/storage.* tests fail with GCC 8 Tests were failing because in TIME_from_longlong_datetime_packed() GCC8 at -O2 assumed that tmp is always positive and used mul and shr while it used imul and sar at -O1 (where tests passed). GCC8 used multiplication (by 0x4ec4ec4ec4ec4ec5) and shift to implement division by 13. It could assume that tmp is always positive, because the function starts with `if (tmp < 0) tmp= -tmp;` But this assumption breaks if tmp=0x8000000000000000; This is invalid value and TIME_from_longlong_datetime_packed() should never see it, garbage in - garbage out. It was getting this invalid value because mroonga tried to convert a NULL key part to MYSQL_TIME. If the key part value is NULL, datetime2 value of it happens to be bzero-ed, which is invalid binary datetime2 value. The correct behavior is not to try to interpret the key part value, if it is marked as NULL. But this minimal fix only covers the datetime2 type. --- sql/compat56.cc | 3 +++ storage/mroonga/ha_mroonga.cpp | 9 ++++++--- storage/mroonga/ha_mroonga.hpp | 2 +- storage/mroonga/lib/mrn_multiple_column_key_codec.cpp | 4 +++- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/sql/compat56.cc b/sql/compat56.cc index 3bd6b21a154..357b4bcf78b 100644 --- a/sql/compat56.cc +++ b/sql/compat56.cc @@ -252,6 +252,9 @@ void TIME_from_longlong_datetime_packed(MYSQL_TIME *ltime, longlong tmp) { longlong ymd, hms; longlong ymdhms, ym; + + DBUG_ASSERT(tmp != LONGLONG_MIN); + if ((ltime->neg= (tmp < 0))) tmp= -tmp; diff --git a/storage/mroonga/ha_mroonga.cpp b/storage/mroonga/ha_mroonga.cpp index d317fef44fb..02213f0f179 100644 --- a/storage/mroonga/ha_mroonga.cpp +++ b/storage/mroonga/ha_mroonga.cpp @@ -11800,7 +11800,8 @@ int ha_mroonga::storage_encode_key_timestamp2(Field *field, const uchar *key, #endif #ifdef MRN_HAVE_MYSQL_TYPE_DATETIME2 -int ha_mroonga::storage_encode_key_datetime2(Field *field, const uchar *key, +int ha_mroonga::storage_encode_key_datetime2(Field *field, bool is_null, + const uchar *key, uchar *buf, uint *size) { MRN_DBUG_ENTER_METHOD(); @@ -11808,7 +11809,7 @@ int ha_mroonga::storage_encode_key_datetime2(Field *field, const uchar *key, bool truncated = false; Field_datetimef *datetime2_field = (Field_datetimef *)field; - longlong packed_time = + longlong packed_time = is_null ? 0 : my_datetime_packed_from_binary(key, datetime2_field->decimals()); MYSQL_TIME mysql_time; TIME_from_longlong_datetime_packed(&mysql_time, packed_time); @@ -11935,6 +11936,7 @@ int ha_mroonga::storage_encode_key(Field *field, const uchar *key, MRN_DBUG_ENTER_METHOD(); int error; bool truncated = false; + bool is_null = false; const uchar *ptr = key; error = mrn_change_encoding(ctx, field->charset()); @@ -11942,6 +11944,7 @@ int ha_mroonga::storage_encode_key(Field *field, const uchar *key, DBUG_RETURN(error); if (field->null_bit) { + is_null = *ptr; ptr += 1; } @@ -12039,7 +12042,7 @@ int ha_mroonga::storage_encode_key(Field *field, const uchar *key, #endif #ifdef MRN_HAVE_MYSQL_TYPE_DATETIME2 case MYSQL_TYPE_DATETIME2: - error = storage_encode_key_datetime2(field, ptr, buf, size); + error = storage_encode_key_datetime2(field, is_null, ptr, buf, size); break; #endif #ifdef MRN_HAVE_MYSQL_TYPE_TIME2 diff --git a/storage/mroonga/ha_mroonga.hpp b/storage/mroonga/ha_mroonga.hpp index b769583d434..71ec40ee63a 100644 --- a/storage/mroonga/ha_mroonga.hpp +++ b/storage/mroonga/ha_mroonga.hpp @@ -796,7 +796,7 @@ private: uchar *buf, uint *size); #endif #ifdef MRN_HAVE_MYSQL_TYPE_DATETIME2 - int storage_encode_key_datetime2(Field *field, const uchar *key, + int storage_encode_key_datetime2(Field *field, bool is_null, const uchar *key, uchar *buf, uint *size); #endif #ifdef MRN_HAVE_MYSQL_TYPE_TIME2 diff --git a/storage/mroonga/lib/mrn_multiple_column_key_codec.cpp b/storage/mroonga/lib/mrn_multiple_column_key_codec.cpp index 0038a7fe34f..fa3c49a4236 100644 --- a/storage/mroonga/lib/mrn_multiple_column_key_codec.cpp +++ b/storage/mroonga/lib/mrn_multiple_column_key_codec.cpp @@ -89,11 +89,13 @@ namespace mrn { for (int i = 0; i < n_key_parts && current_mysql_key < mysql_key_end; i++) { KEY_PART_INFO *key_part = &(key_info_->key_part[i]); Field *field = key_part->field; + bool is_null = false; DBUG_PRINT("info", ("mroonga: key_part->length=%u", key_part->length)); if (field->null_bit) { DBUG_PRINT("info", ("mroonga: field has null bit")); *current_grn_key = 0; + is_null = *current_mysql_key; current_mysql_key += 1; current_grn_key += 1; (*grn_key_length)++; @@ -164,7 +166,7 @@ namespace mrn { { Field_datetimef *datetimef_field = static_cast(field); - long long int mysql_datetime_packed = + long long int mysql_datetime_packed = is_null ? 0 : my_datetime_packed_from_binary(current_mysql_key, datetimef_field->decimals()); MYSQL_TIME mysql_time;