19 Dec
2013
19 Dec
'13
8:57 a.m.
Hi!
10.12.2013 17:58, Michael Widenius пишет:
[skip]
>>>> + if (m_table_id == ptr->table_id)
>>>> + master_had_triggers= ((RPL_TABLE_LIST*)ptr)->master_had_triggers;
>>> <cut>
>>>> if (table)
>>>> {
>>>> bool transactional_table= table->file->has_transactions();
>>>> @@ -8618,6 +8656,9 @@ int Rows_log_event::do_apply_event(Relay
>>>> */
> thd-> reset_current_stmt_binlog_format_row();
> thd-> is_slave_error= 1;
>>>> + /* remove trigger's tables */
>>>> + if (slave_run_triggers_for_rbr)
>>>> + restore_empty_query_table_list(thd->lex);
>>>> DBUG_RETURN(error);
>>>> }
>>> You do the above 'restore_empty_query_table_list(thd->lex);' before
>>> every return in this function, except the last return.
> Oleksandr> it is done before very last return but not so close to it.
>>> - Isn't it better to do a 'error:' tag at the end of the function
>>> to ensure we don't miss any cases ?
>>>
>>> - Add a comment why we don't need to do this at the very end of
>>> Rows_log_event::do_apply_event().
>>> Something like:
>>> /*
>>> We don't have to call restore_empty_query_table_list() here as
>>> it's called in rows_event_stmt_cleanup().
>>> */
> Did you do the above?
>
>
yes. (postreview patch)
[skip]
>>>> + if (slave_run_triggers_for_rbr && !master_had_triggers && m_table->triggers &&
>>>> + m_table->triggers->has_triggers(TRG_EVENT_UPDATE,
>>>> + TRG_ACTION_AFTER))
>>>> + {
>>>> + /*
>>>> + The table has AFTER UPDATE triggers that might access to subject
>>>> + table and therefore might need update to be done immediately.
>>>> + So we turn-off the batching.
>>>> + */
>>>> + (void) m_table->file->extra(HA_EXTRA_UPDATE_CANNOT_BATCH);
>>>> + }
>>>> +
>>> Would be better if we could do a function 'prepare_triggers_for_update_stmt()'
>>> and use the same code here and in sql_update.cc
> Oleksandr> see above.
>
> Agree with having this as a table function.
>
> One last question:
>
> Do you happen to know why we in select_create::prepare() call
> table->mark_columns_needed_for_insert();
> but not
> prepare_triggers_for_insert_stmt() ?
>
> Does this mean:
> 1) This is a bug, we should instead call prepare_triggers_for_insert_stmt().
> 2) prepare_triggers_for_insert_stmt() is called later
> (and thus table->mark_columns_needed_for_insert() is called twice).
> 3) prepare_triggers_for_insert_stmt() is not needed in this case ???
>
> My guess would be that we are calling mark_columns_needed_for_insert()
> twice and we can delete one of the calls, probably in select_create::prepare(),
> but I didn't have time to verify this.
I hope that I have understand the question correctly.
I checked mark_columns_needed_for_insert() it always called only once.
SELECT ... CREATE has no trigger preparation because there is no way to
get triggers on the table we are creating in the same statement.
[skip]