Robert Haas <robertmhaas@gmail.com> writes:
> And here is another incremental patch, this one doing some more
> cleanup. Some of this is cosmetic, but it also:
Thanks, applied in my github repository!
> I'm feeling pretty good about this at this point, although I think
> there is still some more work to do before we call it done and go
> home.
Nice reading that :)
> I have a large remaining maintainability concern about the way we're
> mapping back and forth between node tags, event tags, and command
> tags. Right now we've got parse_event_tag, which parses something
[…valid concern…]
> If you don't have a brilliant idea I'll hack on it and see what I can
> come up with.
I think we might be able to install a static array for the setup where
we would find the different elements, and then code up some procedures
doing different kind of look ups in that array.
> like 'ALTER AGGREGATE' into E_AlterAggregate; and then we've got
> command_to_string, which turns E_AlterAggregate back into 'ALTER
> AGGREGATE', and then we've got InitEventContext(), which turns
> T_RenameStmt or T_AlterObjectSchemaStmt with OBJECT_AGGREGATE into
> E_AlterAggregate. I can't easily verify that all three of these
{ E_AlterAggregate, // TrigEventCommand "ALTER AGGREGATE", // command tag T_RenameStmt,
// nodeTag -1 // object type
},
{ E_AlterAggregate, "ALTER AGGREGATE", T_AlterObjectSchemaStmt, OBJECT_AGGREGATE
}
The problem is coming up with a way of writing the code that does not
incur a full array scan for each step of parsing or rewriting. And I
don't see that it merits yet another cache. Given the existing event
trigger cache it might be that we don't care about having a full scan of
this table twice per event trigger related commands, as I don't think it
would happen when executing other DDLs.
Scratch that we need to parse command tags when we build the event
cache, so scanning the full array each time would make that O(n²) and we
want to avoid that. So we could install the contents of the array in
another hash table in BuildEventTriggerCache() then use that to lookup
the TrigEventCommand from the command tag…
Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support