From 530f3f7cfc912ce9ff8282c520aa055b768cbe2a Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Fri, 11 Oct 2019 23:16:01 +0400 Subject: [PATCH] MDEV-20806 Federated does not work with INET6, returns NULL with warning ER_TRUNCATED_WRONG_VALUE --- .../federated/federated_type_inet6.result | 24 ++++++++++++ .../suite/federated/federated_type_inet6.test | 17 ++++++++ plugin/type_inet/sql_type_inet.cc | 39 ++++++++++++------- sql/field.cc | 17 ++++---- sql/field.h | 19 +++++++++ sql/sql_class.h | 17 ++++++++ storage/csv/ha_tina.cc | 14 +------ storage/federated/ha_federated.cc | 2 +- storage/federatedx/ha_federatedx.cc | 3 +- 9 files changed, 118 insertions(+), 34 deletions(-) create mode 100644 mysql-test/suite/federated/federated_type_inet6.result create mode 100644 mysql-test/suite/federated/federated_type_inet6.test diff --git a/mysql-test/suite/federated/federated_type_inet6.result b/mysql-test/suite/federated/federated_type_inet6.result new file mode 100644 index 00000000000..625a7f3353f --- /dev/null +++ b/mysql-test/suite/federated/federated_type_inet6.result @@ -0,0 +1,24 @@ +connect master,127.0.0.1,root,,test,$MASTER_MYPORT,; +connect slave,127.0.0.1,root,,test,$SLAVE_MYPORT,; +connection master; +CREATE DATABASE federated; +connection slave; +CREATE DATABASE federated; +# +# MDEV-20806 Federated does not work with INET6, returns NULL with warning ER_TRUNCATED_WRONG_VALUE +# +connection master; +CREATE TABLE t1 (a INET6); +INSERT INTO t1 VALUES ('::'),('f::f'); +CREATE TABLE t2 (a INET6) ENGINE=Federated CONNECTION='mysql://root@127.0.0.1:MASTER_PORT/test/t1'; +SELECT * FROM t2; +a +:: +f::f +DROP TABLE t1, t2; +connection master; +DROP TABLE IF EXISTS federated.t1; +DROP DATABASE IF EXISTS federated; +connection slave; +DROP TABLE IF EXISTS federated.t1; +DROP DATABASE IF EXISTS federated; diff --git a/mysql-test/suite/federated/federated_type_inet6.test b/mysql-test/suite/federated/federated_type_inet6.test new file mode 100644 index 00000000000..857f0726689 --- /dev/null +++ b/mysql-test/suite/federated/federated_type_inet6.test @@ -0,0 +1,17 @@ +source include/federated.inc; + +--echo # +--echo # MDEV-20806 Federated does not work with INET6, returns NULL with warning ER_TRUNCATED_WRONG_VALUE +--echo # + +connection master; + +CREATE TABLE t1 (a INET6); +INSERT INTO t1 VALUES ('::'),('f::f'); + +--replace_result $MASTER_MYPORT MASTER_PORT +eval CREATE TABLE t2 (a INET6) ENGINE=Federated CONNECTION='mysql://root@127.0.0.1:$MASTER_MYPORT/test/t1'; +SELECT * FROM t2; +DROP TABLE t1, t2; + +source include/federated_cleanup.inc; diff --git a/plugin/type_inet/sql_type_inet.cc b/plugin/type_inet/sql_type_inet.cc index 7f9adf1d299..9eba3f4c032 100644 --- a/plugin/type_inet/sql_type_inet.cc +++ b/plugin/type_inet/sql_type_inet.cc @@ -631,6 +631,16 @@ class Field_inet6: public Field set_max_value((char*) ptr); return 1; } + int store_inet6_null_with_warn(const Inet6_null &inet6, + const ErrConvString &err) + { + DBUG_ASSERT(marked_for_write_or_computed()); + if (inet6.is_null()) + return maybe_null() ? set_null_with_warn(err) : + set_min_value_with_warn(err); + inet6.to_binary((char *) ptr, Inet6::binary_length()); + return 0; + } public: Field_inet6(const LEX_CSTRING *field_name_arg, const Record_addr &rec) @@ -749,23 +759,26 @@ public: int store(const char *str, size_t length, CHARSET_INFO *cs) override { - DBUG_ASSERT(marked_for_write_or_computed()); - Inet6_null tmp= cs == &my_charset_bin ? - Inet6_null(str, length) : - Inet6_null(str, length, cs); - if (tmp.is_null()) - { - return maybe_null() ? - set_null_with_warn(ErrConvString(str, length, cs)) : - set_min_value_with_warn(ErrConvString(str, length, cs)); - } - tmp.to_binary((char *) ptr, Inet6::binary_length()); - return 0; + return cs == &my_charset_bin ? store_binary(str, length) : + store_text(str, length, cs); + } + + int store_text(const char *str, size_t length, CHARSET_INFO *cs) override + { + return store_inet6_null_with_warn(Inet6_null(str, length, cs), + ErrConvString(str, length, cs)); + } + + int store_binary(const char *str, size_t length) override + { + return store_inet6_null_with_warn(Inet6_null(str, length), + ErrConvString(str, length, + &my_charset_bin)); } int store_hex_hybrid(const char *str, size_t length) override { - return store(str, length, &my_charset_bin); + return Field_inet6::store_binary(str, length); } int store_decimal(const my_decimal *num) override diff --git a/sql/field.cc b/sql/field.cc index 01220d7205a..b74c95909e8 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -1887,13 +1887,16 @@ bool Field::compatible_field_size(uint field_metadata, int Field::store(const char *to, size_t length, CHARSET_INFO *cs, enum_check_fields check_level) { - int res; - THD *thd= get_thd(); - enum_check_fields old_check_level= thd->count_cuted_fields; - thd->count_cuted_fields= check_level; - res= store(to, length, cs); - thd->count_cuted_fields= old_check_level; - return res; + Check_level_instant_set tmp_level(get_thd(), check_level); + return store(to, length, cs); +} + + +int Field::store_text(const char *to, size_t length, CHARSET_INFO *cs, + enum_check_fields check_level) +{ + Check_level_instant_set tmp_level(get_thd(), check_level); + return store_text(to, length, cs); } diff --git a/sql/field.h b/sql/field.h index 3d9cfe92346..1af55487704 100644 --- a/sql/field.h +++ b/sql/field.h @@ -916,6 +916,23 @@ public: reset(); } virtual int store(const char *to, size_t length,CHARSET_INFO *cs)=0; + /* + This is used by engines like CSV and Federated to signal the field + that the data is going to be in text (rather than binary) representation, + even if cs points to &my_charset_bin. + + If a Field distinguishes between text and binary formats (e.g. INET6), + we cannot call store(str,length,&my_charset_bin), + to avoid "field" mis-interpreting the data format as binary. + */ + virtual int store_text(const char *to, size_t length, CHARSET_INFO *cs) + { + return store(to, length, cs); + } + virtual int store_binary(const char *to, size_t length) + { + return store(to, length, &my_charset_bin); + } virtual int store_hex_hybrid(const char *str, size_t length); virtual int store(double nr)=0; virtual int store(longlong nr, bool unsigned_val)=0; @@ -940,6 +957,8 @@ public: { return store_time_dec(ltime, TIME_SECOND_PART_DIGITS); } int store(const char *to, size_t length, CHARSET_INFO *cs, enum_check_fields check_level); + int store_text(const char *to, size_t length, CHARSET_INFO *cs, + enum_check_fields check_level); int store(const LEX_STRING *ls, CHARSET_INFO *cs) { DBUG_ASSERT(ls->length < UINT_MAX32); diff --git a/sql/sql_class.h b/sql/sql_class.h index 77059e7aab0..8ca74457273 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -6884,6 +6884,23 @@ public: }; +class Check_level_instant_set +{ + THD *m_thd; + enum_check_fields m_check_level; +public: + Check_level_instant_set(THD *thd, enum_check_fields temporary_value) + :m_thd(thd), m_check_level(thd->count_cuted_fields) + { + thd->count_cuted_fields= temporary_value; + } + ~Check_level_instant_set() + { + m_thd->count_cuted_fields= m_check_level; + } +}; + + /** This class resembles the SQL Standard schema qualified object name: ::= [ ] diff --git a/storage/csv/ha_tina.cc b/storage/csv/ha_tina.cc index e5c980e0c28..ac5a0eeb850 100644 --- a/storage/csv/ha_tina.cc +++ b/storage/csv/ha_tina.cc @@ -820,16 +820,6 @@ int ha_tina::find_current_row(uchar *buf) if (read_all || bitmap_is_set(table->read_set, (*field)->field_index)) { bool is_enum= ((*field)->real_type() == MYSQL_TYPE_ENUM); - /* - If "field" distinguishes between text and binary formats (e.g. INET6), - we cannot pass buffer.char() (which is &my_charset_bin) to store(), - to avoid "field" mis-interpreting the data format as binary. - Let's pass my_charset_latin1 to tell the field that we're storing - in text format. - */ - CHARSET_INFO *storecs= - (*field)->type_handler()->convert_to_binary_using_val_native() ? - &my_charset_latin1 : buffer.charset(); /* Here CHECK_FIELD_WARN checks that all values in the csv file are valid which is normally the case, if they were written by @@ -838,8 +828,8 @@ int ha_tina::find_current_row(uchar *buf) Thus, for enums we silence the warning, as it doesn't really mean an invalid value. */ - if ((*field)->store(buffer.ptr(), buffer.length(), storecs, - is_enum ? CHECK_FIELD_IGNORE : CHECK_FIELD_WARN)) + if ((*field)->store_text(buffer.ptr(), buffer.length(), buffer.charset(), + is_enum ? CHECK_FIELD_IGNORE : CHECK_FIELD_WARN)) { if (!is_enum) goto err; diff --git a/storage/federated/ha_federated.cc b/storage/federated/ha_federated.cc index a479d14ab6c..6f231ef2c36 100644 --- a/storage/federated/ha_federated.cc +++ b/storage/federated/ha_federated.cc @@ -959,7 +959,7 @@ uint ha_federated::convert_row_to_internal_format(uchar *record, if (bitmap_is_set(table->read_set, (*field)->field_index)) { (*field)->set_notnull(); - (*field)->store(*row, *lengths, &my_charset_bin); + (*field)->store_text(*row, *lengths, &my_charset_bin); } } (*field)->move_field_offset(-old_ptr); diff --git a/storage/federatedx/ha_federatedx.cc b/storage/federatedx/ha_federatedx.cc index 1dd4aacee49..7db6fe9d541 100644 --- a/storage/federatedx/ha_federatedx.cc +++ b/storage/federatedx/ha_federatedx.cc @@ -893,7 +893,8 @@ uint ha_federatedx::convert_row_to_internal_format(uchar *record, if (bitmap_is_set(table->read_set, (*field)->field_index)) { (*field)->set_notnull(); - (*field)->store(io->get_column_data(row, column), lengths[column], &my_charset_bin); + (*field)->store_text(io->get_column_data(row, column), lengths[column], + &my_charset_bin); } } (*field)->move_field_offset(-old_ptr);