From: Dmitry Shulga Date: October 18 2012 11:15am Subject: bzr push into mysql-trunk branch (Dmitry.Shulga:4227 to 4228) List-Archive: http://lists.mysql.com/commits/145096 Message-Id: <20121018111508.31644.24892.4228@dhcp-uk-twvpn-1-vpnpool-10-175-2-92.vpn.oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 4228 Dmitry Shulga 2012-10-18 It's addressed some remarks found by reviewrs. modified: mysql-test/t/wl6030.test sql/field.cc sql/field.h sql/sql_base.cc sql/sql_insert.cc sql/sql_load.cc sql/sql_trigger.cc sql/sql_update.cc 4227 Dmitry Shulga 2012-10-10 Some unittests were modified: replaced explicit assignement of pointer value to Field::null_ptr by call of Field::set_null_ptr(). modified: unittest/gunit/field_date-t.cc unittest/gunit/field_datetime-t.cc unittest/gunit/field_long-t.cc unittest/gunit/field_newdecimal-t.cc unittest/gunit/mock_field_timestamp.h unittest/gunit/mock_field_timestampf.h === modified file 'mysql-test/t/wl6030.test' --- a/mysql-test/t/wl6030.test 2012-10-04 15:09:30 +0000 +++ b/mysql-test/t/wl6030.test 2012-10-18 09:04:39 +0000 @@ -1423,7 +1423,6 @@ DELETE FROM t2; --echo #------------------------------------------------------------------ --error ER_WARN_NULL_TO_NOTNULL -# FIXME: LOAD DATA inserts wrong data. LOAD DATA INFILE '../../std_data/wl6030_2.dat' INTO TABLE t2 FIELDS TERMINATED BY ',' ENCLOSED BY '"'; @@ -1434,7 +1433,6 @@ SELECT * FROM t2; DELETE FROM t2; --error ER_WARN_NULL_TO_NOTNULL -# FIXME: LOAD DATA inserts wrong data. LOAD DATA INFILE '../../std_data/wl6030_2.dat' INTO TABLE v2 FIELDS TERMINATED BY ',' ENCLOSED BY '"'; === modified file 'sql/field.cc' --- a/sql/field.cc 2012-10-10 07:05:52 +0000 +++ b/sql/field.cc 2012-10-18 09:04:39 +0000 @@ -924,6 +924,18 @@ static enum_field_types field_types_merg /** + Set field to temporary value NULL. +*/ +void Field::set_tmp_null() +{ + m_is_tmp_null= true; + + m_count_cuted_fields_saved= table ? table->in_use->count_cuted_fields : + current_thd->count_cuted_fields; +} + + +/** Return type of which can carry value of both given types in UNION result. @param a type for merging @@ -1378,7 +1390,7 @@ Field::Field(uchar *ptr_arg,uint32 lengt @return TYPE_OK if the value is Ok, or corresponding error code from the type_conversion_status enum. */ -type_conversion_status Field::check_constraints(int warning_no) +type_conversion_status Field::check_constraints(int mysql_errno) { /* Ensure that Field::check_constraints() is called only when temporary @@ -1413,7 +1425,7 @@ type_conversion_status Field::check_cons switch (m_count_cuted_fields_saved) { case CHECK_FIELD_WARN: - set_warning(Sql_condition::WARN_LEVEL_WARN, warning_no, 1); + set_warning(Sql_condition::WARN_LEVEL_WARN, mysql_errno, 1); /* fall through */ case CHECK_FIELD_IGNORE: return TYPE_OK; @@ -1448,26 +1460,6 @@ void Field::set_null(my_ptrdiff_t row_of /** - Reset temporary NULL value for field -*/ -void Field::reset_tmp_null() -{ - m_is_tmp_null= false; -} - -/** - Set field to temporary value NULL. -*/ -void Field::set_tmp_null() -{ - m_is_tmp_null= true; - - m_count_cuted_fields_saved= table ? table->in_use->count_cuted_fields : - current_thd->count_cuted_fields; -} - - -/** Set field to value NOT NULL. @param row_offset This is the offset between the row being updated @@ -10561,33 +10553,6 @@ uint32 Field_blob::max_display_length() Warning handling *****************************************************************************/ -/** - Produce warning or note about data saved into field. - - @param level - level of message (Note/Warning/Error) - @param code - error code of message to be produced - @param cut_increment - whenever we should increase cut fields count - - @note - This function won't produce warning and increase cut fields counter - if count_cuted_fields == CHECK_FIELD_IGNORE for current thread. - - if count_cuted_fields == CHECK_FIELD_IGNORE then we ignore notes. - This allows us to avoid notes in optimisation, like convert_constant_item(). - - @retval - 1 if count_cuted_fields == CHECK_FIELD_IGNORE and error level is not NOTE - @retval - 0 otherwise -*/ - -bool Field::set_warning(Sql_condition::enum_warning_level level, - uint code, - int cut_increment) -{ - return set_warning(level, code, cut_increment, NULL, NULL); -} - /** Produce warning or note about data saved into field. === modified file 'sql/field.h' --- a/sql/field.h 2012-10-10 07:05:52 +0000 +++ b/sql/field.h 2012-10-18 09:04:39 +0000 @@ -595,12 +595,29 @@ public: { m_warnings_pushed= 0; } /** - Turn on/off temporary nullability for the field. + Turn on temporary nullability for the field. */ - void set_tmp_nullable(bool is_tmp_nullable) - { m_is_tmp_nullable= is_tmp_nullable; } + void set_tmp_nullable() + { + m_is_tmp_nullable= true; + } + + /** + Turn off temporary nullability for the field. + */ + void reset_tmp_nullable() + { + m_is_tmp_nullable= false; + } + + /** + Reset temporary NULL value for field + */ + void reset_tmp_null() + { + m_is_tmp_null= false; + } - void reset_tmp_null(); void set_tmp_null(); /** @@ -1002,7 +1019,7 @@ public: void set_notnull(my_ptrdiff_t row_offset= 0); - type_conversion_status check_constraints(int warning_no); + type_conversion_status check_constraints(int mysql_errno); /** Remember the value of THD::count_cuted_fields to handle possible @@ -1256,8 +1273,33 @@ public: { return DERIVATION_IMPLICIT; } virtual uint repertoire(void) const { return MY_REPERTOIRE_UNICODE30; } virtual void set_derivation(enum Derivation derivation_arg) { } - bool set_warning(Sql_condition::enum_warning_level, unsigned int code, - int cuted_increment); + + /** + Produce warning or note about data saved into field. + + @param level - level of message (Note/Warning/Error) + @param code - error code of message to be produced + @param cut_increment - whenever we should increase cut fields count + + @note + This function won't produce warning and increase cut fields counter + if count_cuted_fields == CHECK_FIELD_IGNORE for current thread. + + if count_cuted_fields == CHECK_FIELD_IGNORE then we ignore notes. + This allows us to avoid notes in optimisation, like convert_constant_item(). + + @retval + 1 if count_cuted_fields == CHECK_FIELD_IGNORE and error level is not NOTE + @retval + 0 otherwise + */ + bool set_warning(Sql_condition::enum_warning_level level, + uint code, + int cut_increment) + { + return set_warning(level, code, cut_increment, NULL, NULL); + } + bool set_warning(Sql_condition::enum_warning_level level, uint code, int cut_increment, const char *view_db, const char *view_name); === modified file 'sql/sql_base.cc' --- a/sql/sql_base.cc 2012-10-10 07:05:52 +0000 +++ b/sql/sql_base.cc 2012-10-18 09:04:39 +0000 @@ -8816,12 +8816,13 @@ err_no_arena: /* Fill fields with given items. - @param thd thread handler - @param fields Item_fields list to be filled - @param values values to fill with - @param ignore_errors TRUE if we should ignore errors - @param bitmap Bitmap over fields to fill - + @param thd thread handler + @param fields Item_fields list to be filled + @param values values to fill with + @param ignore_errors TRUE if we should ignore errors + @param bitmap Bitmap over fields to fill + @param insert_into_fields_bitmap Bitmap for fields that is set + in fill_record @note fill_record() may set table->auto_increment_field_not_null and a caller should make sure that it is reset after their last call to this function. @@ -8944,6 +8945,65 @@ static bool check_record(THD *thd, Field } +/** + Check if SQL-statement is INSERT/INSERT SELECT/REPLACE/REPLACE SELECT + and there is a trigger ON INSERT + + @param event event type for triggers to be invoked + + @return Test result + @retval true SQL-statement is + INSERT/INSERT SELECT/REPLACE/REPLACE SELECT + and there is a trigger ON INSERT + @retval false Either SQL-statement is not + INSERT/INSERT SELECT/REPLACE/REPLACE SELECT + or there isn't a trigger ON INSERT +*/ +inline bool command_invokes_insert_triggers(enum trg_event_type event, + enum_sql_command sql_command) +{ + return event == TRG_EVENT_INSERT && + (sql_command == SQLCOM_INSERT || + sql_command == SQLCOM_INSERT_SELECT || + sql_command == SQLCOM_REPLACE || + sql_command == SQLCOM_REPLACE_SELECT); +} + + +/** + Execute BEFORE INSERT trigger. + + @param thd thread context + @param triggers object holding list of triggers + to be invoked + @param event event type for triggers to be invoked + @param insert_into_fields_bitmap Bitmap for fields that is set + in fill_record + + @return Operation status + @retval false OK + @retval true Error occurred +*/ +inline bool call_before_insert_triggers(THD *thd, + Table_triggers_list *triggers, + enum trg_event_type event, + MY_BITMAP *insert_into_fields_bitmap) +{ + TABLE *tbl= triggers->trigger_table; + + for (Field** f= tbl->field; *f; ++f) + { + if (((*f)->flags & NO_DEFAULT_VALUE_FLAG) && + !bitmap_is_set(insert_into_fields_bitmap, (*f)->field_index)) + { + (*f)->set_tmp_null(); + } + } + + return triggers->process_triggers(thd, event, TRG_ACTION_BEFORE, true); +} + + /* Fill fields in list with values from the list of items and invoke before triggers. @@ -8972,84 +9032,63 @@ fill_record_n_invoke_before_triggers(THD enum trg_event_type event, int num_fields) { - if (triggers) - triggers->enable_fields_temporary_nullability(thd); - - MY_BITMAP insert_into_fields_bitmap; - bool rc; - /* If it's 'INSERT INTO ... ON DUPLICATE KEY UPDATE ...' statement the event is TRG_EVENT_UPDATE and the SQL-command is SQLCOM_INSERT. */ - if (triggers && event == TRG_EVENT_INSERT && - (thd->lex->sql_command == SQLCOM_INSERT || - thd->lex->sql_command == SQLCOM_INSERT_SELECT || - thd->lex->sql_command == SQLCOM_REPLACE || - thd->lex->sql_command == SQLCOM_REPLACE_SELECT)) - { - DBUG_ASSERT(num_fields); - - bitmap_init(&insert_into_fields_bitmap, NULL, num_fields, false); - rc= fill_record(thd, fields, values, ignore_errors, NULL, - &insert_into_fields_bitmap); - } - else - rc= fill_record(thd, fields, values, ignore_errors, NULL, NULL); - - if (rc) + if (triggers) { - // Return error status if fill_record() failed. + bool rc; - if (triggers) - triggers->disable_fields_temporary_nullability(); - - return true; - } + triggers->enable_fields_temporary_nullability(thd); - if (triggers) - { - if (event == TRG_EVENT_INSERT && - (thd->lex->sql_command == SQLCOM_INSERT || - thd->lex->sql_command == SQLCOM_INSERT_SELECT || - thd->lex->sql_command == SQLCOM_REPLACE || - thd->lex->sql_command == SQLCOM_REPLACE_SELECT)) + if (command_invokes_insert_triggers(event, thd->lex->sql_command)) { - TABLE *tbl= triggers->trigger_table; + DBUG_ASSERT(num_fields); - for (Field** f= tbl->field; *f; ++f) - { - if (((*f)->flags & NO_DEFAULT_VALUE_FLAG) && - !bitmap_is_set(&insert_into_fields_bitmap, (*f)->field_index)) - { - (*f)->set_tmp_null(); - } - } - } + MY_BITMAP insert_into_fields_bitmap; + bitmap_init(&insert_into_fields_bitmap, NULL, num_fields, false); - rc= triggers->process_triggers(thd, event, TRG_ACTION_BEFORE, true); + rc= fill_record(thd, fields, values, ignore_errors, NULL, + &insert_into_fields_bitmap); - triggers->disable_fields_temporary_nullability(); - } + if (!rc) + rc= call_before_insert_triggers(thd, triggers, event, + &insert_into_fields_bitmap); - if (rc) - return true; + bitmap_free(&insert_into_fields_bitmap); + } + else + { + rc= fill_record(thd, fields, values, ignore_errors, NULL, NULL) || + triggers->process_triggers(thd, event, TRG_ACTION_BEFORE, true); + } - return triggers ? - check_record(thd, triggers->trigger_table->field) : - check_record(thd, fields, ignore_errors); + triggers->disable_fields_temporary_nullability(); + + return rc || + check_record(thd, triggers->trigger_table->field); + } + else + { + return + fill_record(thd, fields, values, ignore_errors, NULL, NULL) || + check_record(thd, fields, ignore_errors); + } } /** Fill field buffer with values from Field list. - @param thd thread handler - @param ptr pointer on pointer to record - @param values list of fields - @param ignore_errors True if we should ignore errors - @param bitmap Bitmap over fields to fill + @param thd thread handler + @param ptr pointer on pointer to record + @param values list of fields + @param ignore_errors True if we should ignore errors + @param bitmap Bitmap over fields to fill + @param insert_into_fields_bitmap Bitmap for fields that is set + in fill_record @note fill_record() may set table->auto_increment_field_not_null and a caller should make sure that it is reset after their last call to this @@ -9143,10 +9182,6 @@ fill_record_n_invoke_before_triggers(THD enum trg_event_type event, int num_fields) { - if (triggers) - triggers->enable_fields_temporary_nullability(thd); - - MY_BITMAP insert_into_fields_bitmap; bool rc; /* @@ -9154,57 +9189,35 @@ fill_record_n_invoke_before_triggers(THD the event is TRG_EVENT_UPDATE and the SQL-command is SQLCOM_INSERT. */ - if (triggers && event == TRG_EVENT_INSERT && - (thd->lex->sql_command == SQLCOM_INSERT || - thd->lex->sql_command == SQLCOM_INSERT_SELECT || - thd->lex->sql_command == SQLCOM_REPLACE || - thd->lex->sql_command == SQLCOM_REPLACE_SELECT)) + if (triggers) { - DBUG_ASSERT(num_fields); - - bitmap_init(&insert_into_fields_bitmap, NULL, num_fields, false); + triggers->enable_fields_temporary_nullability(thd); - rc= fill_record(thd, ptr, values, ignore_errors, NULL, - &insert_into_fields_bitmap); - } - else - rc= fill_record(thd, ptr, values, ignore_errors, NULL, NULL); + if (command_invokes_insert_triggers(event, thd->lex->sql_command)) + { + DBUG_ASSERT(num_fields); - if (rc) - { - // Return error status if fill_record() failed. + MY_BITMAP insert_into_fields_bitmap; + bitmap_init(&insert_into_fields_bitmap, NULL, num_fields, false); - if (triggers) - triggers->disable_fields_temporary_nullability(); + rc= fill_record(thd, ptr, values, ignore_errors, NULL, + &insert_into_fields_bitmap); - return true; - } + if (!rc) + rc= call_before_insert_triggers(thd, triggers, event, + &insert_into_fields_bitmap); - if (triggers) - { - if (event == TRG_EVENT_INSERT && - (thd->lex->sql_command == SQLCOM_INSERT || - thd->lex->sql_command == SQLCOM_INSERT_SELECT || - thd->lex->sql_command == SQLCOM_REPLACE || - thd->lex->sql_command == SQLCOM_REPLACE_SELECT)) + bitmap_free(&insert_into_fields_bitmap); + } + else { - TABLE *tbl= triggers->trigger_table; - - for (Field** f= tbl->field; *f; ++f) - { - if (((*f)->flags & NO_DEFAULT_VALUE_FLAG) && - !bitmap_is_set(&insert_into_fields_bitmap, (*f)->field_index)) - { - (*f)->set_tmp_null(); - } - } + rc= fill_record(thd, ptr, values, ignore_errors, NULL, NULL) || + triggers->process_triggers(thd, event, TRG_ACTION_BEFORE, true); } - - rc= triggers->process_triggers(thd, event, TRG_ACTION_BEFORE, true); - triggers->disable_fields_temporary_nullability(); } - + else + rc= fill_record(thd, ptr, values, ignore_errors, NULL, NULL); if (rc) return true; === modified file 'sql/sql_insert.cc' --- a/sql/sql_insert.cc 2012-10-10 07:05:52 +0000 +++ b/sql/sql_insert.cc 2012-10-18 09:04:39 +0000 @@ -639,25 +639,6 @@ static bool safely_check_that_all_fields /** - This method cleans up internal structures allocated during running - function mysql_insert(). It must be used with mysql_insert() only. -*/ -inline static void mysql_insert_cleanup_after_error(THD *thd, bool joins_freed, - thr_lock_type lock_type) -{ -#ifndef EMBEDDED_LIBRARY - if (lock_type == TL_WRITE_DELAYED) - end_delayed_insert(thd); -#endif - - if (!joins_freed) - free_underlaid_joins(thd, &thd->lex->select_lex); - - thd->abort_on_warning= 0; -} - - -/** INSERT statement implementation @note Like implementations of other DDL/DML in MySQL, this function @@ -770,25 +751,16 @@ bool mysql_insert(THD *thd,TABLE_LIST *t (fields.elements || !value_count || table_list->view != 0), !ignore && thd->is_strict_mode())) - { - mysql_insert_cleanup_after_error(thd, joins_freed, lock_type); - DBUG_RETURN(true); - } + goto exit_without_my_ok; /* mysql_prepare_insert set table_list->table if it was not set */ table= table_list->table; /* Must be done before can_prune_insert, due to internal initialization. */ if (info.add_function_default_columns(table, table->write_set)) - { - mysql_insert_cleanup_after_error(thd, joins_freed, lock_type); - DBUG_RETURN(true); - } + goto exit_without_my_ok; if (duplic == DUP_UPDATE && update.add_function_default_columns(table, table->write_set)) - { - mysql_insert_cleanup_after_error(thd, joins_freed, lock_type); - DBUG_RETURN(true); - } + goto exit_without_my_ok; context= &thd->lex->select_lex.context; /* @@ -822,11 +794,7 @@ bool mysql_insert(THD *thd,TABLE_LIST *t &can_prune_partitions, &prune_needs_default_values, &used_partitions)) - { - mysql_insert_cleanup_after_error(thd, joins_freed, lock_type); - DBUG_RETURN(true); - } - + goto exit_without_my_ok; if (can_prune_partitions != partition_info::PRUNE_NO) { @@ -859,15 +827,10 @@ bool mysql_insert(THD *thd,TABLE_LIST *t if (values->elements != value_count) { my_error(ER_WRONG_VALUE_COUNT_ON_ROW, MYF(0), counter); - mysql_insert_cleanup_after_error(thd, joins_freed, lock_type); - DBUG_RETURN(true); + goto exit_without_my_ok; } if (setup_fields(thd, Ref_ptr_array(), *values, MARK_COLUMNS_READ, 0, 0)) - { - mysql_insert_cleanup_after_error(thd, joins_freed, lock_type); - DBUG_RETURN(true); - } - + goto exit_without_my_ok; #ifdef WITH_PARTITION_STORAGE_ENGINE /* @@ -913,8 +876,7 @@ bool mysql_insert(THD *thd,TABLE_LIST *t */ err= explain_no_table(thd, "No tables used"); - mysql_insert_cleanup_after_error(thd, joins_freed, lock_type); - DBUG_RETURN(err); + goto exit_without_my_ok; } #ifdef WITH_PARTITION_STORAGE_ENGINE @@ -963,10 +925,7 @@ bool mysql_insert(THD *thd,TABLE_LIST *t if(info.get_duplicate_handling() == DUP_UPDATE && table->next_number_field != NULL && rpl_master_has_bug(active_mi->rli, 24432, TRUE, NULL, NULL)) - { - mysql_insert_cleanup_after_error(thd, joins_freed, lock_type); - DBUG_RETURN(true); - } + goto exit_without_my_ok; } #endif @@ -1261,10 +1220,7 @@ bool mysql_insert(THD *thd,TABLE_LIST *t table->file->extra(HA_EXTRA_WRITE_CANNOT_REPLACE); if (error) - { - mysql_insert_cleanup_after_error(thd, joins_freed, lock_type); - DBUG_RETURN(true); - } + goto exit_without_my_ok; if (values_list.elements == 1 && (!(thd->variables.option_bits & OPTION_WARNINGS) || !thd->cuted_fields)) @@ -1294,6 +1250,16 @@ bool mysql_insert(THD *thd,TABLE_LIST *t } thd->abort_on_warning= 0; DBUG_RETURN(FALSE); + +exit_without_my_ok: +#ifndef EMBEDDED_LIBRARY + if (lock_type == TL_WRITE_DELAYED) + end_delayed_insert(thd); +#endif + if (!joins_freed) + free_underlaid_joins(thd, &thd->lex->select_lex); + thd->abort_on_warning= 0; + DBUG_RETURN(err); } @@ -4257,13 +4223,13 @@ select_create::prepare2() table->file->ha_start_bulk_insert((ha_rows) 0); thd->abort_on_warning= (!ignore_errors && thd->is_strict_mode()); - enum_check_fields save_coiunt_cuted_fields= thd->count_cuted_fields; + enum_check_fields save_count_cuted_fields= thd->count_cuted_fields; thd->count_cuted_fields= CHECK_FIELD_WARN; if (check_that_all_fields_are_given_values(thd, table, table_list)) DBUG_RETURN(1); - thd->count_cuted_fields= save_coiunt_cuted_fields; + thd->count_cuted_fields= save_count_cuted_fields; table->mark_columns_needed_for_insert(); table->file->extra(HA_EXTRA_WRITE_CACHE); === modified file 'sql/sql_load.cc' --- a/sql/sql_load.cc 2012-10-10 06:13:15 +0000 +++ b/sql/sql_load.cc 2012-10-18 09:04:39 +0000 @@ -974,7 +974,7 @@ read_sep_field(THD *thd, COPY_INFO &info to table fields. */ if (real_item->type() == Item::FIELD_ITEM) - ((Item_field *)real_item)->field->set_tmp_nullable(true); + ((Item_field *)real_item)->field->set_tmp_nullable(); if ((!read_info.enclosed && (enclosed_length && length == 4 && @@ -1107,7 +1107,7 @@ read_sep_field(THD *thd, COPY_INFO &info { Item *real_item= item->real_item(); if (real_item->type() == Item::FIELD_ITEM) - ((Item_field *)real_item)->field->set_tmp_nullable(false); + ((Item_field *)real_item)->field->reset_tmp_nullable(); } if (thd->killed || === modified file 'sql/sql_trigger.cc' --- a/sql/sql_trigger.cc 2012-10-10 07:05:52 +0000 +++ b/sql/sql_trigger.cc 2012-10-18 09:04:39 +0000 @@ -2251,7 +2251,7 @@ void Table_triggers_list::enable_fields_ { for (Field** next_field= trigger_table->field; *next_field; ++next_field) { - (*next_field)->set_tmp_nullable(true); + (*next_field)->set_tmp_nullable(); (*next_field)->set_count_cuted_fields(thd->count_cuted_fields); /* @@ -2278,7 +2278,7 @@ void Table_triggers_list::enable_fields_ void Table_triggers_list::disable_fields_temporary_nullability() { for (Field** next_field= trigger_table->field; *next_field; ++next_field) - (*next_field)->set_tmp_nullable(false); + (*next_field)->reset_tmp_nullable(); } === modified file 'sql/sql_update.cc' --- a/sql/sql_update.cc 2012-09-26 07:11:12 +0000 +++ b/sql/sql_update.cc 2012-10-18 09:04:39 +0000 @@ -2194,7 +2194,7 @@ bool multi_update::send_data(List for (Field** modified_fields= tmp_table->field + 1 + unupdated_check_opt_tables.elements; *modified_fields; ++modified_fields) { - (*modified_fields)->set_tmp_nullable(true); + (*modified_fields)->set_tmp_nullable(); } /* Store regular updated fields in the row. */ No bundle (reason: useless for push emails).