Great work.
Patch approved.
Cheers.
Daogang Qu wrote:
> Hi Alfranio,
> Thanks for the good comments! Please review the newest patch. Thanks!
>
> Best Regards,
>
> Daogang
>
> Alfranio Correia wrote:
>> Hi Daogang,
>>
>> The patch seems ok, although I still don't like the use of the
>> create_info.
>> When Jasonh mentioned it I did not check it and only bought the idea
>> by the name. However, after reviewing your patch I've notice that it was
>> designed to store information on tables/handlers.
>>
>> So, please keep your original approach that changes the lex directly by
>> changing or augmenting what follows (sql/sql_lex.h):
>>
> OK.
>> ----------------------
>> /*
>> stmt_definition_begin is intended to point to the next word after
>> DEFINER-clause in the following statements:
>> - CREATE TRIGGER (points to "TRIGGER");
>> - CREATE PROCEDURE (points to "PROCEDURE");
>> - CREATE FUNCTION (points to "FUNCTION" or "AGGREGATE");
>>
>> This pointer is required to add possibly omitted DEFINER-clause
>> to the
>> DDL-statement before dumping it to the binlog.
>> */
>> const char *stmt_definition_begin;
>>
>> const char *stmt_definition_end;
>> ----------------------
What I've suggested above is not considering comments between the CREATE
and KEYWORD.
So forget about it.
I have just one more request.
I think we should file a new bug report to make the CREATE PROC,
FUNCTION, TRIGGER compatible
with the CREATE EVENT. This should be handled as a feature request.
>
>>> +#
>>> +CREATE TABLE test.t1(details CHAR(30));
>>> +
>>> +/* comment befor create */ CREATE /* comment after create */ EVENT
>>> /* comment after event */ event44331_1
>>> + ON SCHEDULE AT CURRENT_TIMESTAMP
>>> + ON COMPLETION PRESERVE DISABLE
>>> + DO INSERT INTO test.t1 VALUES('event event44331_1 fired - no
>>> definer');
>>>
>> Why CURRENT_USER EVENT?
>>
> The EVENT is keyword and the CURRENT_USER is assigned DEFINER.
> The sentence looks fine if we get rid of some comments in-line.
>
> CREATE DEFINER=CURRENT_USER EVENT event44331_2
> ON SCHEDULE AT CURRENT_TIMESTAMP
> ...
You are just checking different syntaxes.
Good.
Cheers.
