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:
@ -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='';
|
||||
|
@ -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;
|
||||
|
@ -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
|
||||
|
@ -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
|
||||
|
@ -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);
|
||||
|
||||
|
@ -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);
|
||||
}
|
||||
|
||||
|
||||
|
@ -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;
|
||||
}
|
||||
}
|
||||
|
@ -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();
|
||||
|
||||
/*
|
||||
|
@ -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)
|
||||
{
|
||||
|
@ -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
|
||||
|
@ -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 */
|
||||
|
@ -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
|
||||
{
|
||||
|
@ -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);
|
||||
|
||||
|
@ -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();
|
||||
|
||||
/*
|
||||
|
23
sql/table.cc
23
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,
|
||||
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;
|
||||
}
|
||||
|
||||
|
Reference in New Issue
Block a user