Hi Dmitry. The proposal is great and IMHO the only way to fix this. I
have prepared a working patch but please consider it 'work in progress'.
It needs more commenting, probably some DBUG_ASSERTs and you probably
will find some problems with it that I didn't even think of. :-)
What worries me the most is creating an inconsistent Table_triggers_list
object and not making this fix look like a gigantic hack. The use of the
Internal_error_handler class makes it much nicer.
> Since trigger name is stored at the beginning of trigger definition
> it is quite probable that we will be able to successfully parse it
> and later extract from LEX::spname member even if parsing of trigger
> definition as whole has failed (and if it is not possible then it is
> likely that .TRG file was corrupted so we are better to ask user to
> drop triggers by using manual system operations).
>
Yes, this is in fact possible, and the fix is possible thanks to this.
But we must take care to handle the other case as well, i.e. not core dump.
> And knowing correct trigger name we can produce Table_triggers_list
> object (particularly properly fill names_list member in it)
> which won't be suitable for trigger execution but which still can
> be used for performing DROP TRIGGER operation in usual fashion
> (i.e. dropping only specified trigger and removing .TRN file).
>
Yes, but we have to be careful. I can see the following problems.
- This extends the use for a Table_triggers_list and we have to document
it carefully in the code. Preferrably there should be a flag for each
trigger whether it contains parse errors or not, and any other operation
than DROP TRIGGER should be refused with an obvious error.
- Table_triggers_list isn't exactly implemented in a way that lets us do
this smoothly. First, there is no Trigger class, but a series of lists
of properties where implicitly the position of an element within the
list tells which trigger it belongs to. This invariant is now painfully
maintained by peppering the code with DBUG_ASSERTs and warning comments.
Preferrably there should be *one* list containing Trigger objects, but
this seems a major change which is outside the scope of a bug fix.
Best Regards
Martin
