From 6ec3de5d2d62400593ee0583e554f1f26187cfcb Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Wed, 7 Mar 2018 19:52:00 +0400 Subject: MDEV-15497 Wrong empty value in a GEOMETRY column on LOAD DATA - Adding a new virtual method Field::load_data_set_no_data(). - Overriding Field_timestamp::load_data_set_no_data() and moving the TIMESTAMP specific code there. - Overriding Field_geom::load_data_set_no_data() and implementing GEOMETRY specific behavior, to prevent writing empty strings when the loaded file ends unexpectedly. This fixes the bug. - Adding a new test gis-loaddaata.test. - The test in loaddata.test for CHAR was added simply to record behavior. The CHAR data type did not change its behaviour (only GEOMRYRY did). - Additionally, moving duplicate code into a new method Field::load_data_set_value() and reusing it in three places. --- mysql-test/r/gis-loaddata.result | 25 +++++++++++++ mysql-test/r/loaddata.result | 34 +++++++++++++++++ mysql-test/std_data/loaddata/mdev-15497.txt | 1 + mysql-test/t/gis-loaddata.test | 21 +++++++++++ mysql-test/t/loaddata.test | 24 ++++++++++++ sql/field.cc | 57 +++++++++++++++++++++++++++++ sql/field.h | 5 +++ sql/sql_load.cc | 40 +++----------------- 8 files changed, 173 insertions(+), 34 deletions(-) create mode 100644 mysql-test/r/gis-loaddata.result create mode 100644 mysql-test/std_data/loaddata/mdev-15497.txt create mode 100644 mysql-test/t/gis-loaddata.test diff --git a/mysql-test/r/gis-loaddata.result b/mysql-test/r/gis-loaddata.result new file mode 100644 index 00000000000..1d25c0a496d --- /dev/null +++ b/mysql-test/r/gis-loaddata.result @@ -0,0 +1,25 @@ +# +# MDEV-15497 Wrong empty value in a GEOMETRY column on LOAD DATA +# +SET sql_mode=''; +CREATE TABLE t1 (id INT, a GEOMETRY NOT NULL); +LOAD DATA INFILE '../../std_data/loaddata/mdev-15497.txt' INTO TABLE t1; +ERROR 22004: Column set to default value; NULL supplied to NOT NULL column 'a' at row 1 +LOAD DATA INFILE '../../std_data/loaddata/mdev-15497.txt' INTO TABLE t1 FIELDS TERMINATED BY ''; +ERROR 22004: Column set to default value; NULL supplied to NOT NULL column 'a' at row 1 +DROP TABLE t1; +CREATE TABLE t1 (id INT, a GEOMETRY); +LOAD DATA INFILE '../../std_data/loaddata/mdev-15497.txt' INTO TABLE t1; +Warnings: +Warning 1261 Row 1 doesn't contain data for all columns +SELECT * FROM t1; +id a +1 NULL +TRUNCATE TABLE t1; +LOAD DATA INFILE '../../std_data/loaddata/mdev-15497.txt' INTO TABLE t1 FIELDS TERMINATED BY ''; +Warnings: +Warning 1261 Row 1 doesn't contain data for all columns +SELECT * FROM t1; +id a +1 NULL +DROP TABLE t1; diff --git a/mysql-test/r/loaddata.result b/mysql-test/r/loaddata.result index 4f59e9cdbe3..7a189f36cc6 100644 --- a/mysql-test/r/loaddata.result +++ b/mysql-test/r/loaddata.result @@ -580,3 +580,37 @@ SELECT HEX(a) FROM t1; HEX(a) C3A4 DROP TABLE t1; +# +# MDEV-15497 Wrong empty value in a GEOMETRY column on LOAD DATA +# +SET sql_mode=''; +CREATE TABLE t1 (a CHAR(1), b CHAR(1) NOT NULL); +LOAD DATA INFILE '../../std_data/loaddata/mdev-15497.txt' INTO TABLE t1; +Warnings: +Warning 1261 Row 1 doesn't contain data for all columns +SELECT * FROM t1; +a b +1 +TRUNCATE TABLE t1; +LOAD DATA INFILE '../../std_data/loaddata/mdev-15497.txt' INTO TABLE t1 FIELDS TERMINATED BY ''; +Warnings: +Warning 1261 Row 1 doesn't contain data for all columns +SELECT * FROM t1; +a b +1 +DROP TABLE t1; +CREATE TABLE t1 (a CHAR(1), b CHAR(1)); +LOAD DATA INFILE '../../std_data/loaddata/mdev-15497.txt' INTO TABLE t1; +Warnings: +Warning 1261 Row 1 doesn't contain data for all columns +SELECT * FROM t1; +a b +1 NULL +TRUNCATE TABLE t1; +LOAD DATA INFILE '../../std_data/loaddata/mdev-15497.txt' INTO TABLE t1 FIELDS TERMINATED BY ''; +Warnings: +Warning 1261 Row 1 doesn't contain data for all columns +SELECT * FROM t1; +a b +1 +DROP TABLE t1; diff --git a/mysql-test/std_data/loaddata/mdev-15497.txt b/mysql-test/std_data/loaddata/mdev-15497.txt new file mode 100644 index 00000000000..d00491fd7e5 --- /dev/null +++ b/mysql-test/std_data/loaddata/mdev-15497.txt @@ -0,0 +1 @@ +1 diff --git a/mysql-test/t/gis-loaddata.test b/mysql-test/t/gis-loaddata.test new file mode 100644 index 00000000000..ad6a2c56b08 --- /dev/null +++ b/mysql-test/t/gis-loaddata.test @@ -0,0 +1,21 @@ +--echo # +--echo # MDEV-15497 Wrong empty value in a GEOMETRY column on LOAD DATA +--echo # + +SET sql_mode=''; + +CREATE TABLE t1 (id INT, a GEOMETRY NOT NULL); +--error ER_WARN_NULL_TO_NOTNULL +LOAD DATA INFILE '../../std_data/loaddata/mdev-15497.txt' INTO TABLE t1; +--error ER_WARN_NULL_TO_NOTNULL +LOAD DATA INFILE '../../std_data/loaddata/mdev-15497.txt' INTO TABLE t1 FIELDS TERMINATED BY ''; +DROP TABLE t1; + + +CREATE TABLE t1 (id INT, a GEOMETRY); +LOAD DATA INFILE '../../std_data/loaddata/mdev-15497.txt' INTO TABLE t1; +SELECT * FROM t1; +TRUNCATE TABLE t1; +LOAD DATA INFILE '../../std_data/loaddata/mdev-15497.txt' INTO TABLE t1 FIELDS TERMINATED BY ''; +SELECT * FROM t1; +DROP TABLE t1; diff --git a/mysql-test/t/loaddata.test b/mysql-test/t/loaddata.test index 4e355e5c3a7..722074b34d4 100644 --- a/mysql-test/t/loaddata.test +++ b/mysql-test/t/loaddata.test @@ -676,3 +676,27 @@ CREATE TABLE t1 (a VARCHAR(10) CHARACTER SET utf8); LOAD DATA INFILE '../../std_data/loaddata/mdev-11631.txt' INTO TABLE t1 CHARACTER SET utf8; SELECT HEX(a) FROM t1; DROP TABLE t1; + + +--echo # +--echo # MDEV-15497 Wrong empty value in a GEOMETRY column on LOAD DATA +--echo # + +SET sql_mode=''; + +CREATE TABLE t1 (a CHAR(1), b CHAR(1) NOT NULL); +LOAD DATA INFILE '../../std_data/loaddata/mdev-15497.txt' INTO TABLE t1; +SELECT * FROM t1; +TRUNCATE TABLE t1; +LOAD DATA INFILE '../../std_data/loaddata/mdev-15497.txt' INTO TABLE t1 FIELDS TERMINATED BY ''; +SELECT * FROM t1; +DROP TABLE t1; + + +CREATE TABLE t1 (a CHAR(1), b CHAR(1)); +LOAD DATA INFILE '../../std_data/loaddata/mdev-15497.txt' INTO TABLE t1; +SELECT * FROM t1; +TRUNCATE TABLE t1; +LOAD DATA INFILE '../../std_data/loaddata/mdev-15497.txt' INTO TABLE t1 FIELDS TERMINATED BY ''; +SELECT * FROM t1; +DROP TABLE t1; diff --git a/sql/field.cc b/sql/field.cc index e47b6e9c989..447061d70b7 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -1253,6 +1253,26 @@ warn: } +/** + If a field does not have a corresponding data, it's behavior can vary: + - In case of the fixed file format + it's set to the default value for the data type, + such as 0 for numbers or '' for strings. + - In case of a non-fixed format + it's set to NULL for nullable fields, and + it's set to the default value for the data type for NOT NULL fields. + This seems to be by design. +*/ +bool Field::load_data_set_no_data(THD *thd, bool fixed_format) +{ + reset(); // Do not use the DEFAULT value + if (fixed_format) + set_notnull(); + set_has_explicit_value(); // Do not auto-update this field + return false; +} + + bool Field::load_data_set_null(THD *thd) { reset(); @@ -1267,6 +1287,21 @@ bool Field::load_data_set_null(THD *thd) } +void Field::load_data_set_value(const char *pos, uint length, + CHARSET_INFO *cs) +{ + /* + Mark field as not null, we should do this for each row because of + restore_record... + */ + set_notnull(); + if (this == table->next_number_field) + table->auto_increment_field_not_null= true; + store(pos, length, cs); + set_has_explicit_value(); // Do not auto-update this field +} + + bool Field::sp_prepare_and_store_item(THD *thd, Item **value) { DBUG_ENTER("Field::sp_prepare_and_store_item"); @@ -5314,6 +5349,22 @@ int Field_timestamp::set_time() } +bool Field_timestamp::load_data_set_no_data(THD *thd, bool fixed_format) +{ + if (!maybe_null()) + { + /* + Timestamp fields that are NOT NULL are autoupdated if there is no + corresponding value in the data file. + */ + set_time(); + set_has_explicit_value(); + return false; + } + return Field::load_data_set_no_data(thd, fixed_format); +} + + bool Field_timestamp::load_data_set_null(THD *thd) { if (!maybe_null()) @@ -8927,6 +8978,12 @@ bool Field_geom::can_optimize_range(const Item_bool_func *cond, } +bool Field_geom::load_data_set_no_data(THD *thd, bool fixed_format) +{ + return Field_geom::load_data_set_null(thd); +} + + bool Field_geom::load_data_set_null(THD *thd) { Field_blob::reset(); diff --git a/sql/field.h b/sql/field.h index 7b473ba7094..370054c850f 100644 --- a/sql/field.h +++ b/sql/field.h @@ -1161,6 +1161,9 @@ public: { return null_ptr != 0 || table->maybe_null; } // Set to NULL on LOAD DATA or LOAD XML virtual bool load_data_set_null(THD *thd); + // Reset when a LOAD DATA file ended unexpectedly + virtual bool load_data_set_no_data(THD *thd, bool fixed_format); + void load_data_set_value(const char *pos, uint length, CHARSET_INFO *cs); /* @return true if this field is NULL-able (even if temporarily) */ inline bool real_maybe_null(void) const { return null_ptr != 0; } @@ -2520,6 +2523,7 @@ public: return get_equal_const_item_datetime(thd, ctx, const_item); } bool load_data_set_null(THD *thd); + bool load_data_set_no_data(THD *thd, bool fixed_format); uint size_of() const { return sizeof(*this); } }; @@ -3725,6 +3729,7 @@ public: */ int reset(void) { return Field_blob::reset() || !maybe_null(); } bool load_data_set_null(THD *thd); + bool load_data_set_no_data(THD *thd, bool fixed_format); 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 9d367149eaa..d0cfb717a08 100644 --- a/sql/sql_load.cc +++ b/sql/sql_load.cc @@ -976,26 +976,15 @@ read_fixed_length(THD *thd, COPY_INFO &info, TABLE_LIST *table_list, { Field *field= sql_field->field; table->auto_increment_field_not_null= auto_increment_field_not_null; - /* - No fields specified in fields_vars list can be null in this format. - Mark field as not null, we should do this for each row because of - restore_record... - */ - field->set_notnull(); - if (pos == read_info.row_end) { + if (field->load_data_set_no_data(thd, true)) + DBUG_RETURN(1); thd->cuted_fields++; /* Not enough fields */ push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN, ER_WARN_TOO_FEW_RECORDS, ER_THD(thd, ER_WARN_TOO_FEW_RECORDS), thd->get_stmt_da()->current_row_for_warning()); - /* - Timestamp fields that are NOT NULL are autoupdated if there is no - corresponding value in the data file. - */ - if (!field->maybe_null() && field->type() == FIELD_TYPE_TIMESTAMP) - field->set_time(); } else { @@ -1005,13 +994,11 @@ read_fixed_length(THD *thd, COPY_INFO &info, TABLE_LIST *table_list, field->field_length) length=field->field_length; save_chr=pos[length]; pos[length]='\0'; // Safeguard aganst malloc - field->store((char*) pos,length,read_info.read_charset); + field->load_data_set_value((char*) pos,length,read_info.read_charset); pos[length]=save_chr; if ((pos+=length) > read_info.row_end) pos= read_info.row_end; /* Fills rest with space */ } - /* Do not auto-update this field. */ - field->set_has_explicit_value(); } if (pos != read_info.row_end) { @@ -1162,12 +1149,8 @@ read_sep_field(THD *thd, COPY_INFO &info, TABLE_LIST *table_list, else { Field *field= real_item->field; - field->set_notnull(); read_info.row_end[0]=0; // Safe to change end marker - if (field == table->next_number_field) - table->auto_increment_field_not_null= TRUE; - field->store((char*) pos, length, read_info.read_charset); - field->set_has_explicit_value(); + field->load_data_set_value((char*) pos, length, read_info.read_charset); } } @@ -1202,15 +1185,8 @@ 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()); + if (field->load_data_set_no_data(thd, false)) DBUG_RETURN(1); - } - if (!field->maybe_null() && field->type() == FIELD_TYPE_TIMESTAMP) - field->set_time(); - field->set_has_explicit_value(); /* TODO: We probably should not throw warning for each field. But how about intention to always have the same number @@ -1363,11 +1339,7 @@ read_xml_field(THD *thd, COPY_INFO &info, TABLE_LIST *table_list, { Field *field= ((Item_field *)item)->field; - field->set_notnull(); - if (field == table->next_number_field) - table->auto_increment_field_not_null= TRUE; - field->store((char *) tag->value.ptr(), tag->value.length(), cs); - field->set_has_explicit_value(); + field->load_data_set_value(tag->value.ptr(), tag->value.length(), cs); } } -- cgit v1.2.1