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

+#
+# 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;

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;

mysql-test/std_data/loaddata/mdev-15497.txt

+1

mysql-test/t/gis-loaddata.test

+--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;

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;

sql/field.cc

@@ -1253,6 +1253,26 @@ int Field::store_hex_hybrid(const char *str, size_t length)
 }
 
 
+/**
+  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();

sql/field.h

@@ -1161,6 +1161,9 @@ class Field: public Value_source
   { 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 @@ class Field_timestamp :public Field_temporal {
     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 @@ class Field_geom :public Field_blob {
    */
   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);

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);
       }
     }