MDEV-19761 Before Trigger not processed for Not Null Columns if no explicit value and no DEFAULT

it's incorrect to zero out table->triggers->extra_null_bitmap
before a statement, because if insert uses an explicit field list
and omits a field that has no default value, the field should
get NULL implicitly. So extra_null_bitmap should have 1s for all
fields that have no defaults

* create extra_null_bitmap_init and initialize it as above
* copy extra_null_bitmap_init to extra_null_bitmap for inserts
* still zero out extra_null_bitmap for updates/deletes where
  all fields definitely have a value
* make not_null_fields_have_null_values() to send
  ER_NO_DEFAULT_FOR_FIELD for fields with no default and no value,
  otherwise creation of a trigger with an empty body would change the
  error message

mysql-test/main/trigger_no_defaults-11698.result

@@ -15,10 +15,10 @@ insert t1 (b) values (10);
 insert t1 (b) values (20);
 ERROR HY000: Field 'a' doesn't have a default value
 insert t1 (b) values (30);
+ERROR 23000: Column 'a' cannot be null
 select * from t1;
 a	b
 10	10
-0	30
 drop table t1;
 set sql_mode=default;
 set sql_mode='';

mysql-test/main/trigger_no_defaults-11698.test

@@ -19,7 +19,7 @@ delimiter ;|
 insert t1 (b) values (10);
 --error ER_NO_DEFAULT_FOR_FIELD
 insert t1 (b) values (20);
-# arguably the statement below should fail too
+--error ER_BAD_NULL_ERROR
 insert t1 (b) values (30);
 select * from t1;
 drop table t1;

mysql-test/main/trigger_null-8605.result

@@ -364,3 +364,28 @@ create trigger tr before update on t1 for each row set @a = 1;
 insert into t1 (pk, i) values (null, null);
 ERROR 23000: Column 'pk' cannot be null
 drop table t1;
+#
+# MDEV-19761 Before Trigger not processed for Not Null Columns if no explicit value and no DEFAULT
+#
+create table t1( id int, rate int not null);
+create trigger test_trigger before insert on t1 for each row
+set new.rate=if(new.rate is null,10,new.rate);
+insert into t1 (id) values (1);
+insert into t1 values (2,3);
+select * from t1;
+id	rate
+1	10
+2	3
+create or replace trigger test_trigger before insert on t1 for each row
+if new.rate is null then set new.rate = 15; end if;
+$$
+insert into t1 (id) values (3);
+insert into t1 values (4,5);
+select * from t1;
+id	rate
+1	10
+2	3
+3	15
+4	5
+drop table t1;
+# End of 10.5 tests

mysql-test/main/trigger_null-8605.test

@@ -391,3 +391,28 @@ create trigger tr before update on t1 for each row set @a = 1;
 --error ER_BAD_NULL_ERROR
 insert into t1 (pk, i) values (null, null);
 drop table t1;
+
+--echo #
+--echo # MDEV-19761 Before Trigger not processed for Not Null Columns if no explicit value and no DEFAULT
+--echo #
+create table t1( id int, rate int not null);
+create trigger test_trigger before insert on t1 for each row
+	set new.rate=if(new.rate is null,10,new.rate);
+
+insert into t1 (id) values (1);
+insert into t1 values (2,3);
+select * from t1;
+
+delimiter $$;
+create or replace trigger test_trigger before insert on t1 for each row
+	if new.rate is null then set new.rate = 15; end if;
+$$
+delimiter ;$$
+
+insert into t1 (id) values (3);
+insert into t1 values (4,5);
+select * from t1;
+
+drop table t1;
+
+--echo # End of 10.5 tests

sql/field.h

@@ -5846,7 +5846,7 @@ uint pack_length_to_packflag(uint type);
 enum_field_types get_blob_type_from_length(ulong length);
 int set_field_to_null(Field *field);
 int set_field_to_null_with_conversions(Field *field, bool no_conversions);
-int convert_null_to_field_value_or_error(Field *field);
+int convert_null_to_field_value_or_error(Field *field, uint err);
 bool check_expression(Virtual_column_info *vcol, const LEX_CSTRING *name,
                       enum_vcol_info_type type, Alter_info *alter_info= NULL);
 

sql/field_conv.cc

@@ -126,7 +126,7 @@ static int set_bad_null_error(Field *field, int err)
     return 0;
   case CHECK_FIELD_ERROR_FOR_NULL:
     if (!field->table->in_use->no_errors)
-      my_error(ER_BAD_NULL_ERROR, MYF(0), field->field_name.str);
+      my_error(err, MYF(0), field->field_name.str);
     return -1;
   }
   DBUG_ASSERT(0); // impossible
@@ -164,7 +164,7 @@ int set_field_to_null(Field *field)
     If no_conversion was not set, an error message is printed
 */
 
-int convert_null_to_field_value_or_error(Field *field)
+int convert_null_to_field_value_or_error(Field *field, uint err)
 {
   if (field->type() == MYSQL_TYPE_TIMESTAMP)
   {
@@ -179,7 +179,7 @@ int convert_null_to_field_value_or_error(Field *field)
     field->table->auto_increment_field_not_null= FALSE;
     return 0;                             // field is set in fill_record()
   }
-  return set_bad_null_error(field, ER_BAD_NULL_ERROR);
+  return set_bad_null_error(field, err);
 }
 
 /**
@@ -216,7 +216,7 @@ set_field_to_null_with_conversions(Field *field, bool no_conversions)
   if (no_conversions)
     return -1;
 
-  return convert_null_to_field_value_or_error(field);
+  return convert_null_to_field_value_or_error(field, ER_BAD_NULL_ERROR);
 }
 
 

sql/sql_base.cc

@@ -8622,7 +8622,6 @@ void switch_to_nullable_trigger_fields(List<Item> &items, TABLE *table)
 
     while ((item= it++))
       item->walk(&Item::switch_to_nullable_fields_processor, 1, field);
-    table->triggers->reset_extra_null_bitmap();
   }
 }
 
@@ -8676,8 +8675,14 @@ static bool not_null_fields_have_null_values(TABLE *table)
         swap_variables(uint32, of->flags, ff->flags);
         if (ff->is_real_null())
         {
+          uint err= ER_BAD_NULL_ERROR;
+          if (ff->flags & NO_DEFAULT_VALUE_FLAG && !ff->has_explicit_value())
+          {
+            err= ER_NO_DEFAULT_FOR_FIELD;
+            table->in_use->count_cuted_fields= CHECK_FIELD_WARN;
+          }
           ff->set_notnull(); // for next row WHERE condition in UPDATE
-          if (convert_null_to_field_value_or_error(of) || thd->is_error())
+          if (convert_null_to_field_value_or_error(of, err) || thd->is_error())
             return true;
         }
       }

sql/sql_delete.cc

@@ -722,8 +722,8 @@ bool mysql_delete(THD *thd, TABLE_LIST *table_list, COND *conds,
     table->mark_columns_needed_for_delete();
   }
 
-  if ((table->file->ha_table_flags() & HA_CAN_FORCE_BULK_DELETE) &&
+  if (!table->prepare_triggers_for_delete_stmt_or_event() &&
-      !table->prepare_triggers_for_delete_stmt_or_event())
+      table->file->ha_table_flags() & HA_CAN_FORCE_BULK_DELETE)
     will_batch= !table->file->start_bulk_delete();
 
   /*

sql/sql_insert.cc

@@ -1019,7 +1019,7 @@ bool mysql_insert(THD *thd, TABLE_LIST *table_list,
           INSERT INTO t1 (fields) VALUES ...
           INSERT INTO t1 VALUES ()
         */
-        restore_record(table,s->default_values);	// Get empty record
+        restore_default_record_for_insert(table);
         table->reset_default_fields();
 
         if (unlikely(fill_record_n_invoke_before_triggers(thd, table, fields,
@@ -1048,7 +1048,7 @@ bool mysql_insert(THD *thd, TABLE_LIST *table_list,
         */
         if (thd->lex->used_tables || // Column used in values()
             table->s->visible_fields != table->s->fields)
-	  restore_record(table,s->default_values);	// Get empty record
+          restore_default_record_for_insert(table);
         else
         {
           TABLE_SHARE *share= table->s;
@@ -1085,24 +1085,6 @@ bool mysql_insert(THD *thd, TABLE_LIST *table_list,
         }
       }
 
-      /*
-        with triggers a field can get a value *conditionally*, so we have to
-        repeat has_no_default_value() check for every row
-      */
-      if (table->triggers &&
-          table->triggers->has_triggers(TRG_EVENT_INSERT, TRG_ACTION_BEFORE))
-      {
-        for (Field **f=table->field ; *f ; f++)
-        {
-          if (unlikely(!(*f)->has_explicit_value() &&
-                       has_no_default_value(thd, *f, table_list)))
-          {
-            error= 1;
-            goto values_loop_end;
-          }
-        }
-      }
-
       if ((res= table_list->view_check_option(thd,
                                               (values_list.elements == 1 ?
                                                0 :
@@ -4081,7 +4063,7 @@ select_insert::prepare(List<Item> &values, SELECT_LEX_UNIT *u)
     */
     table->file->ha_start_bulk_insert((ha_rows) 0);
   }
-  restore_record(table,s->default_values);		// Get empty record
+  restore_default_record_for_insert(table);
   table->reset_default_fields();
   table->next_number_field=table->found_next_number_field;
 
@@ -4226,7 +4208,7 @@ int select_insert::send_data(List<Item> &values)
         originally touched by INSERT ... SELECT, so we have to restore
         their original values for the next row.
       */
-      restore_record(table, s->default_values);
+      restore_default_record_for_insert(table);
     }
     if (table->next_number_field)
     {

sql/sql_insert.h

@@ -46,6 +46,13 @@ void kill_delayed_threads(void);
 bool binlog_create_table(THD *thd, TABLE *table, bool replace);
 bool binlog_drop_table(THD *thd, TABLE *table);
 
+static inline void restore_default_record_for_insert(TABLE *t)
+{
+  restore_record(t,s->default_values);
+  if (t->triggers)
+    t->triggers->default_extra_null_bitmap();
+}
+
 #ifdef EMBEDDED_LIBRARY
 inline void kill_delayed_threads(void) {}
 #endif

sql/sql_load.cc

@@ -23,7 +23,6 @@
 #include "sql_priv.h"
 #include "unireg.h"
 #include "sql_load.h"
-#include "sql_load.h"
 #include "sql_cache.h"                          // query_cache_*
 #include "sql_base.h"          // fill_record_n_invoke_before_triggers
 #include <my_dir.h>
@@ -1004,8 +1003,7 @@ read_fixed_length(THD *thd, COPY_INFO &info, TABLE_LIST *table_list,
     read_info.row_end[0]=0;
 #endif
 
-    restore_record(table, s->default_values);
+    restore_default_record_for_insert(table);
-
     while ((item= it++))
     {
       Load_data_outvar *dst= item->get_load_data_outvar();
@@ -1118,8 +1116,8 @@ read_sep_field(THD *thd, COPY_INFO &info, TABLE_LIST *table_list,
                             thd->progress.max_counter);
       }
     }
-    restore_record(table, s->default_values);
 
+    restore_default_record_for_insert(table);
     while ((item= it++))
     {
       uint length;
@@ -1273,8 +1271,7 @@ read_xml_field(THD *thd, COPY_INFO &info, TABLE_LIST *table_list,
     }
 #endif
     
-    restore_record(table, s->default_values);
+    restore_default_record_for_insert(table);
-    
     while ((item= it++))
     {
       /* If this line is to be skipped we don't want to fill field or var */

sql/sql_trigger.cc

@@ -1297,8 +1297,9 @@ bool Table_triggers_list::prepare_record_accessors(TABLE *table)
 
   {
     int null_bytes= (table->s->fields - table->s->null_fields + 7)/8;
-    if (!(extra_null_bitmap= (uchar*)alloc_root(&table->mem_root, null_bytes)))
+    if (!(extra_null_bitmap= (uchar*)alloc_root(&table->mem_root, 2*null_bytes)))
       return 1;
+    extra_null_bitmap_init= extra_null_bitmap + null_bytes;
     if (!(record0_field= (Field **)alloc_root(&table->mem_root,
                                               (table->s->fields + 1) *
                                               sizeof(Field*))))
@@ -1323,13 +1324,17 @@ bool Table_triggers_list::prepare_record_accessors(TABLE *table)
           null_ptr++, null_bit= 1;
         else
           null_bit*= 2;
+        if (f->flags & NO_DEFAULT_VALUE_FLAG)
+          f->set_null();
+        else
+          f->set_notnull();
       }
       else
         *trg_fld= *fld;
     }
     *trg_fld= 0;
     DBUG_ASSERT(null_ptr <= extra_null_bitmap + null_bytes);
-    bzero(extra_null_bitmap, null_bytes);
+    memcpy(extra_null_bitmap_init, extra_null_bitmap, null_bytes);
   }
   else
   {

sql/sql_trigger.h

@@ -147,7 +147,7 @@ class Table_triggers_list: public Sql_alloc
     BEFORE INSERT/UPDATE triggers.
   */
   Field             **record0_field;
-  uchar              *extra_null_bitmap;
+  uchar              *extra_null_bitmap, *extra_null_bitmap_init;
   /**
     Copy of TABLE::Field array with field pointers set to TABLE::record[1]
     buffer instead of TABLE::record[0] (used for OLD values in on UPDATE
@@ -211,8 +211,8 @@ public:
   /* End of character ser context. */
 
   Table_triggers_list(TABLE *table_arg)
-    :record0_field(0), extra_null_bitmap(0), record1_field(0),
+    :record0_field(0), extra_null_bitmap(0), extra_null_bitmap_init(0),
-    trigger_table(table_arg),
+    record1_field(0), trigger_table(table_arg),
     m_has_unparseable_trigger(false), count(0)
   {
     bzero((char *) triggers, sizeof(triggers));
@@ -276,11 +276,15 @@ public:
                                             TABLE_LIST *table_list);
 
   Field **nullable_fields() { return record0_field; }
-  void reset_extra_null_bitmap()
+  void clear_extra_null_bitmap()
   {
-    size_t null_bytes= (trigger_table->s->fields -
+    if (size_t null_bytes= extra_null_bitmap_init - extra_null_bitmap)
-                        trigger_table->s->null_fields + 7)/8;
+      bzero(extra_null_bitmap, null_bytes);
-    bzero(extra_null_bitmap, null_bytes);
+  }
+  void default_extra_null_bitmap()
+  {
+    if (size_t null_bytes= extra_null_bitmap_init - extra_null_bitmap)
+      memcpy(extra_null_bitmap, extra_null_bitmap_init, null_bytes);
   }
 
   Trigger *find_trigger(const LEX_CSTRING *name, bool remove_from_list);

sql/sql_update.cc

@@ -996,8 +996,8 @@ update_begin:
     goto update_end;
   }
 
-  if ((table->file->ha_table_flags() & HA_CAN_FORCE_BULK_UPDATE) &&
+  if (!table->prepare_triggers_for_update_stmt_or_event() &&
-      !table->prepare_triggers_for_update_stmt_or_event())
+      table->file->ha_table_flags() & HA_CAN_FORCE_BULK_UPDATE)
     will_batch= !table->file->start_bulk_update();
 
   /*

sql/table.cc

@@ -9140,8 +9140,8 @@ void TABLE::prepare_triggers_for_insert_stmt_or_event()
 {
   if (triggers)
   {
-    if (triggers->has_triggers(TRG_EVENT_DELETE,
+    triggers->clear_extra_null_bitmap();
-                               TRG_ACTION_AFTER))
+    if (triggers->has_triggers(TRG_EVENT_DELETE, TRG_ACTION_AFTER))
     {
       /*
         The table has AFTER DELETE triggers that might access to
@@ -9150,8 +9150,7 @@ void TABLE::prepare_triggers_for_insert_stmt_or_event()
       */
       (void) file->extra(HA_EXTRA_DELETE_CANNOT_BATCH);
     }
-    if (triggers->has_triggers(TRG_EVENT_UPDATE,
+    if (triggers->has_triggers(TRG_EVENT_UPDATE, TRG_ACTION_AFTER))
-                               TRG_ACTION_AFTER))
     {
       /*
         The table has AFTER UPDATE triggers that might access to subject
@@ -9166,17 +9165,19 @@ void TABLE::prepare_triggers_for_insert_stmt_or_event()
 
 bool TABLE::prepare_triggers_for_delete_stmt_or_event()
 {
-  if (triggers &&
+  if (triggers)
-      triggers->has_triggers(TRG_EVENT_DELETE,
-                             TRG_ACTION_AFTER))
   {
-    /*
+    triggers->clear_extra_null_bitmap();
-      The table has AFTER DELETE triggers that might access to subject table
+    if (triggers->has_triggers(TRG_EVENT_DELETE, TRG_ACTION_AFTER))
-      and therefore might need delete to be done immediately. So we turn-off
+    {
-      the batching.
+      /*
-    */
+        The table has AFTER DELETE triggers that might access to subject table
-    (void) file->extra(HA_EXTRA_DELETE_CANNOT_BATCH);
+        and therefore might need delete to be done immediately. So we turn-off
-    return TRUE;
+        the batching.
+      */
+      (void) file->extra(HA_EXTRA_DELETE_CANNOT_BATCH);
+      return TRUE;
+    }
   }
   return FALSE;
 }
@@ -9184,17 +9185,19 @@ bool TABLE::prepare_triggers_for_delete_stmt_or_event()
 
 bool TABLE::prepare_triggers_for_update_stmt_or_event()
 {
-  if (triggers &&
+  if (triggers)
-      triggers->has_triggers(TRG_EVENT_UPDATE,
-                             TRG_ACTION_AFTER))
   {
-    /*
+    triggers->clear_extra_null_bitmap();
-      The table has AFTER UPDATE triggers that might access to subject
+    if (triggers->has_triggers(TRG_EVENT_UPDATE, TRG_ACTION_AFTER))
-      table and therefore might need update to be done immediately.
+    {
-      So we turn-off the batching.
+      /*
-    */ 
+        The table has AFTER UPDATE triggers that might access to subject
-    (void) file->extra(HA_EXTRA_UPDATE_CANNOT_BATCH);
+        table and therefore might need update to be done immediately.
-    return TRUE;
+        So we turn-off the batching.
+      */
+      (void) file->extra(HA_EXTRA_UPDATE_CANNOT_BATCH);
+      return TRUE;
+    }
   }
   return FALSE;
 }