1
0
mirror of https://github.com/MariaDB/server.git synced 2025-07-11 15:22:09 +03:00

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
This commit is contained in:
Sergei Golubchik
2024-12-23 21:27:00 +01:00
parent 350cc77fee
commit a69da0c31e
15 changed files with 127 additions and 74 deletions

View File

@ -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='';

View File

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

View File

@ -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

View File

@ -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

View File

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

View File

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

View File

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

View File

@ -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) &&
!table->prepare_triggers_for_delete_stmt_or_event())
if (!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();
/*

View File

@ -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)
{

View File

@ -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

View File

@ -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 */

View File

@ -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
{

View File

@ -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),
trigger_table(table_arg),
:record0_field(0), extra_null_bitmap(0), extra_null_bitmap_init(0),
record1_field(0), trigger_table(table_arg),
m_has_unparseable_trigger(false), count(0)
{
bzero((char *) triggers, sizeof(triggers));
@ -276,12 +276,16 @@ 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 -
trigger_table->s->null_fields + 7)/8;
if (size_t null_bytes= extra_null_bitmap_init - extra_null_bitmap)
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);

View File

@ -996,8 +996,8 @@ update_begin:
goto update_end;
}
if ((table->file->ha_table_flags() & HA_CAN_FORCE_BULK_UPDATE) &&
!table->prepare_triggers_for_update_stmt_or_event())
if (!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();
/*

View File

@ -9140,8 +9140,8 @@ void TABLE::prepare_triggers_for_insert_stmt_or_event()
{
if (triggers)
{
if (triggers->has_triggers(TRG_EVENT_DELETE,
TRG_ACTION_AFTER))
triggers->clear_extra_null_bitmap();
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,
TRG_ACTION_AFTER))
if (triggers->has_triggers(TRG_EVENT_UPDATE, TRG_ACTION_AFTER))
{
/*
The table has AFTER UPDATE triggers that might access to subject
@ -9166,9 +9165,10 @@ void TABLE::prepare_triggers_for_insert_stmt_or_event()
bool TABLE::prepare_triggers_for_delete_stmt_or_event()
{
if (triggers &&
triggers->has_triggers(TRG_EVENT_DELETE,
TRG_ACTION_AFTER))
if (triggers)
{
triggers->clear_extra_null_bitmap();
if (triggers->has_triggers(TRG_EVENT_DELETE, TRG_ACTION_AFTER))
{
/*
The table has AFTER DELETE triggers that might access to subject table
@ -9178,15 +9178,17 @@ bool TABLE::prepare_triggers_for_delete_stmt_or_event()
(void) file->extra(HA_EXTRA_DELETE_CANNOT_BATCH);
return TRUE;
}
}
return FALSE;
}
bool TABLE::prepare_triggers_for_update_stmt_or_event()
{
if (triggers &&
triggers->has_triggers(TRG_EVENT_UPDATE,
TRG_ACTION_AFTER))
if (triggers)
{
triggers->clear_extra_null_bitmap();
if (triggers->has_triggers(TRG_EVENT_UPDATE, TRG_ACTION_AFTER))
{
/*
The table has AFTER UPDATE triggers that might access to subject
@ -9196,6 +9198,7 @@ bool TABLE::prepare_triggers_for_update_stmt_or_event()
(void) file->extra(HA_EXTRA_UPDATE_CANNOT_BATCH);
return TRUE;
}
}
return FALSE;
}