From a53e087ea9d865bbe1f3a53a90b1683196feb5ba Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Wed, 13 Dec 2017 13:22:45 +0400 Subject: [PATCH] MDEV-14628 Wrong autoinc value assigned by LOAD XML in the NO_AUTO_VALUE_ON_ZERO mode The fixes for these bugs: Bug#27586 Wrong autoinc value assigned by LOAD DATA in the NO_AUTO_VALUE_ON_ZERO mode Bug#22372 Disable spatial key, load data, enable spatial key, crashes table fixed only LOAD DATA INFILE, but did not fix LOAD XML INFILE. This patch does for LOAD XML FILE what patches for Bug#27586 and Bug#22372 earlier did for LOAD DATA INFILE. 1. Fixing the auto_increment problem: a. table->auto_increment_field_not_null is not set to TRUE anymore when a column does not have a corresponding XML tag. b. Adding "table->auto_increment_field_not_null= false" in the end of read_xml_field(). These two changes resemble the patch for Bug#27586. 2. Fixing the GEOMETRY problem: The result for "reset()" was not tested for errors in read_xml_field(), which made it possible for empty string to sneak into a "GEOMETRY NOT NULL" column when this column does not have a corresponding XML tag with data. After this patch the result of reset() is tested and and an error is returned in such cases. This change effectively resembles the patch for Bug#22372 3. Spliting the code into a new virtual method Field::load_data_set_null(). Rationale: a. To avoid duplicate code in read_sep_field() and read_xml_field(): Changes #1 and #2 made the code handling NULL values for Field exactly the same in read_sep_field() and read_xml_field(). b. To avoid tests for field_type(), which is not friendly to upcoming data type plugins. This change makes it possible for data type plugins to implement their own special way for handling NULL values in LOAD DATA by overriding Field_xxx::load_data_set_null(), like Field_geom and Field_timestamp do. --- mysql-test/r/loadxml.result | 20 ++++++++ mysql-test/std_data/loaddata/mdev14628a.xml | 4 ++ mysql-test/std_data/loaddata/mdev14628b.xml | 3 ++ mysql-test/t/loadxml.test | 19 ++++++++ sql/field.cc | 51 +++++++++++++++++++++ sql/field.h | 4 ++ sql/sql_load.cc | 47 ++++--------------- 7 files changed, 110 insertions(+), 38 deletions(-) create mode 100644 mysql-test/std_data/loaddata/mdev14628a.xml create mode 100644 mysql-test/std_data/loaddata/mdev14628b.xml diff --git a/mysql-test/r/loadxml.result b/mysql-test/r/loadxml.result index 1d7af4f8b38..b0fb867a676 100644 --- a/mysql-test/r/loadxml.result +++ b/mysql-test/r/loadxml.result @@ -134,3 +134,23 @@ LOAD XML INFILE '../../std_data/loaddata/mdev12696.xml' INTO TABLE v1 (c2); ERROR HY000: Column 'c2' is not updatable DROP VIEW v1; DROP TABLE t1; +# +# MDEV-14628 Wrong autoinc value assigned by LOAD XML in the NO_AUTO_VALUE_ON_ZERO mode +# +SET sql_mode=NO_AUTO_VALUE_ON_ZERO; +CREATE TABLE t1 (a INT AUTO_INCREMENT PRIMARY KEY, b TEXT); +LOAD XML INFILE '../../std_data/loaddata/mdev14628a.xml' INTO TABLE t1 ROWS IDENTIFIED BY ''; +SELECT * FROM t1 ORDER BY b; +a b +1 bbb1 +2 bbb2 +DROP TABLE t1; +SET sql_mode=DEFAULT; +SET sql_mode=''; +CREATE TABLE t1 (id INT, g GEOMETRY NOT NULL); +LOAD XML INFILE '../../std_data/loaddata/mdev14628b.xml' INTO TABLE t1 ROWS IDENTIFIED BY ''; +ERROR 22004: Column set to default value; NULL supplied to NOT NULL column 'g' at row 1 +SELECT * FROM t1; +id g +DROP TABLE t1; +SET sql_mode=DEFAULT; diff --git a/mysql-test/std_data/loaddata/mdev14628a.xml b/mysql-test/std_data/loaddata/mdev14628a.xml new file mode 100644 index 00000000000..34ee7336a5a --- /dev/null +++ b/mysql-test/std_data/loaddata/mdev14628a.xml @@ -0,0 +1,4 @@ + + + + diff --git a/mysql-test/std_data/loaddata/mdev14628b.xml b/mysql-test/std_data/loaddata/mdev14628b.xml new file mode 100644 index 00000000000..2ea02d2a35f --- /dev/null +++ b/mysql-test/std_data/loaddata/mdev14628b.xml @@ -0,0 +1,3 @@ + + + diff --git a/mysql-test/t/loadxml.test b/mysql-test/t/loadxml.test index 0bd97a81649..623deea6ec6 100644 --- a/mysql-test/t/loadxml.test +++ b/mysql-test/t/loadxml.test @@ -143,3 +143,22 @@ LOAD XML INFILE '../../std_data/loaddata/mdev12696.xml' INTO TABLE v1 (c1); LOAD XML INFILE '../../std_data/loaddata/mdev12696.xml' INTO TABLE v1 (c2); DROP VIEW v1; DROP TABLE t1; + +--echo # +--echo # MDEV-14628 Wrong autoinc value assigned by LOAD XML in the NO_AUTO_VALUE_ON_ZERO mode +--echo # + +SET sql_mode=NO_AUTO_VALUE_ON_ZERO; +CREATE TABLE t1 (a INT AUTO_INCREMENT PRIMARY KEY, b TEXT); +LOAD XML INFILE '../../std_data/loaddata/mdev14628a.xml' INTO TABLE t1 ROWS IDENTIFIED BY ''; +SELECT * FROM t1 ORDER BY b; +DROP TABLE t1; +SET sql_mode=DEFAULT; + +SET sql_mode=''; +CREATE TABLE t1 (id INT, g GEOMETRY NOT NULL); +--error ER_WARN_NULL_TO_NOTNULL +LOAD XML INFILE '../../std_data/loaddata/mdev14628b.xml' INTO TABLE t1 ROWS IDENTIFIED BY ''; +SELECT * FROM t1; +DROP TABLE t1; +SET sql_mode=DEFAULT; diff --git a/sql/field.cc b/sql/field.cc index cfa9623fe17..b8d19c92cda 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -1253,6 +1253,20 @@ warn: } +bool Field::load_data_set_null(THD *thd) +{ + reset(); + set_null(); + if (!maybe_null()) + { + if (this != table->next_number_field) + set_warning(Sql_condition::WARN_LEVEL_WARN, ER_WARN_NULL_TO_NOTNULL, 1); + } + set_has_explicit_value(); // Do not auto-update this field + return false; +} + + /** Numeric fields base class constructor. */ @@ -5152,6 +5166,27 @@ int Field_timestamp::set_time() return 0; } + +bool Field_timestamp::load_data_set_null(THD *thd) +{ + if (!maybe_null()) + { + /* + Timestamp fields that are NOT NULL are autoupdated if there is no + corresponding value in the data file. + */ + set_time(); + } + else + { + reset(); + set_null(); + } + set_has_explicit_value(); // Do not auto-update this field + return false; +} + + #ifdef NOT_USED static void store_native(ulonglong num, uchar *to, uint bytes) { @@ -8528,6 +8563,22 @@ bool Field_geom::can_optimize_range(const Item_bool_func *cond, return item->cmp_type() == STRING_RESULT; } + +bool Field_geom::load_data_set_null(THD *thd) +{ + Field_blob::reset(); + if (!maybe_null()) + { + my_error(ER_WARN_NULL_TO_NOTNULL, MYF(0), field_name.str, + thd->get_stmt_da()->current_row_for_warning()); + return true; + } + set_null(); + set_has_explicit_value(); // Do not auto-update this field + return false; +} + + #endif /*HAVE_SPATIAL*/ /**************************************************************************** diff --git a/sql/field.h b/sql/field.h index 3147374d496..c9df9138b19 100644 --- a/sql/field.h +++ b/sql/field.h @@ -1146,6 +1146,8 @@ public: { if (null_ptr) null_ptr[row_offset]&= (uchar) ~null_bit; } inline bool maybe_null(void) const { return null_ptr != 0 || table->maybe_null; } + // Set to NULL on LOAD DATA or LOAD XML + virtual bool load_data_set_null(THD *thd); /* @return true if this field is NULL-able (even if temporarily) */ inline bool real_maybe_null(void) const { return null_ptr != 0; } @@ -2448,6 +2450,7 @@ public: { return get_equal_const_item_datetime(thd, ctx, const_item); } + bool load_data_set_null(THD *thd); uint size_of() const { return sizeof(*this); } }; @@ -3509,6 +3512,7 @@ public: but the underlying blob must still be reset. */ int reset(void) { return Field_blob::reset() || !maybe_null(); } + bool load_data_set_null(THD *thd); geometry_type get_geometry_type() { return geom_type; }; static geometry_type geometry_type_merge(geometry_type, geometry_type); diff --git a/sql/sql_load.cc b/sql/sql_load.cc index 6e3396f0fcf..dfb7ede04f4 100644 --- a/sql/sql_load.cc +++ b/sql/sql_load.cc @@ -1015,7 +1015,6 @@ continue_loop:; } - static int read_sep_field(THD *thd, COPY_INFO &info, TABLE_LIST *table_list, List &fields_vars, List &set_fields, @@ -1094,28 +1093,9 @@ read_sep_field(THD *thd, COPY_INFO &info, TABLE_LIST *table_list, } else { - Field *field= real_item->field; - if (field->reset()) - { - my_error(ER_WARN_NULL_TO_NOTNULL, MYF(0), field->field_name.str, - thd->get_stmt_da()->current_row_for_warning()); + DBUG_ASSERT(real_item->field->table == table); + if (real_item->field->load_data_set_null(thd)) DBUG_RETURN(1); - } - field->set_null(); - if (!field->maybe_null()) - { - /* - Timestamp fields that are NOT NULL are autoupdated if there is no - corresponding value in the data file. - */ - if (field->type() == MYSQL_TYPE_TIMESTAMP) - field->set_time(); - else if (field != table->next_number_field) - field->set_warning(Sql_condition::WARN_LEVEL_WARN, - ER_WARN_NULL_TO_NOTNULL, 1); - } - /* Do not auto-update this field. */ - field->set_has_explicit_value(); } continue; @@ -1262,6 +1242,7 @@ read_xml_field(THD *thd, COPY_INFO &info, TABLE_LIST *table_list, for ( ; ; it.rewind()) { + bool err; if (thd->killed) { thd->send_kill_message(); @@ -1313,21 +1294,9 @@ read_xml_field(THD *thd, COPY_INFO &info, TABLE_LIST *table_list, } else { - Field *field= real_item->field; - field->reset(); - field->set_null(); - if (field == table->next_number_field) - table->auto_increment_field_not_null= TRUE; - if (!field->maybe_null()) - { - if (field->type() == FIELD_TYPE_TIMESTAMP) - field->set_time(); - else if (field != table->next_number_field) - field->set_warning(Sql_condition::WARN_LEVEL_WARN, - ER_WARN_NULL_TO_NOTNULL, 1); - } - /* Do not auto-update this field. */ - field->set_has_explicit_value(); + DBUG_ASSERT(real_item->field->table == table); + if (real_item->field->load_data_set_null(thd)) + DBUG_RETURN(1); } continue; } @@ -1410,7 +1379,9 @@ read_xml_field(THD *thd, COPY_INFO &info, TABLE_LIST *table_list, DBUG_RETURN(-1); } - if (write_record(thd, table, &info)) + err= write_record(thd, table, &info); + table->auto_increment_field_not_null= false; + if (err) DBUG_RETURN(1); /*