Hello,
thank you for the patch.
The patch is Ok to push after fixing minor issues and considering
a couple of suggestions.
On 04/19/11 16:01, Dmitry Shulga wrote:
> #At file:///Users/shulga/projects/mysql/mysql-5.1-bug11764334/ based on
> revid:sergey.glukhov@stripped
>
> 3662 Dmitry Shulga 2011-04-19
> Fixed bug#11764334 (formerly bug#57156): ALTER EVENT CHANGES
> THE EVENT STATUS.
>
> The problem was that when ALTER EVENT statement being processed
> a value of status field is saved uncoditionnaly even if it wasn't
s/uncoditionnaly/unconditionally/
> specified in ALTER STATEMENT. As a consequence this field
> is set to default value that corresponds to ENABLE value.
>
> The solution is to check if status field was specified in
> ALTER EVENT statememt before make assignment of new value to
s/statememt/statement/
> this field.
> === modified file 'sql/event_db_repository.cc'
> --- a/sql/event_db_repository.cc 2011-03-21 16:02:47 +0000
> +++ b/sql/event_db_repository.cc 2011-04-19 12:01:16 +0000
> @@ -226,9 +226,15 @@ mysql_event_fill_row(THD *thd,
> if (fields[f_num= ET_FIELD_NAME]->store(et->name.str, et->name.length,
> scs))
> goto err_truncate;
>
> - /* both ON_COMPLETION and STATUS are NOT NULL thus not calling set_notnull()*/
> + /* ON_COMPLETION are NOT NULL thus not calling set_notnull()*/
set_notnull()*/ -> set_notnull(). */
> + /*
> + set STATUS value unconditionally in case of CREATE EVENT processing.
> + For ALTER EVENT sets this field only if it was changed.
How about change the wording to be more consistent/clear:
For CREATE EVENT: the STATUS value is set unconditionally.
For ALTER EVENT: the STATUS value is set if it has been specified
in the statement.
> === modified file 'sql/event_parse_data.cc'
> --- a/sql/event_parse_data.cc 2009-02-13 16:41:47 +0000
> +++ b/sql/event_parse_data.cc 2011-04-19 12:01:16 +0000
> @@ -46,9 +46,8 @@ Event_parse_data::new_instance(THD *thd)
>
> Event_parse_data::Event_parse_data()
> :on_completion(Event_parse_data::ON_COMPLETION_DEFAULT),
> - status(Event_parse_data::ENABLED),
> - do_not_create(FALSE),
> - body_changed(FALSE),
> + status(Event_parse_data::ENABLED), status_changed(false),
> + do_not_create(FALSE), body_changed(FALSE),
I've never understood the idea of minimizing the number of lines
in the source code, but it's up to you.
> item_starts(NULL), item_ends(NULL), item_execute_at(NULL),
> starts_null(TRUE), ends_null(TRUE), execute_at_null(TRUE),
> item_expression(NULL), expression(0)
