Re: deparsing utility commands

Here's an updated version of this series. I have fixed many issues; in
particular the supported command set is now complete, with the only
exception being CREATE TABLE AS EXECUTE.

I went to all the prior reviews and compiled a list of unhandled issues,
on which I will be working shortly. I copied it below. I wanted to
post an updated version so in case someone wants to give this another
look while I do so.

I have changed the ALTER TABLE preparatory patch: Stephen had previously
commented that it'd be better to have it return ObjAddress rather than
Oid/attnum. I originally thought this wasn't an improvement, but after
looking again, it turns out that it is, so I changed it that way.
Things look better this way.

Note patch 0004: this modifies pg_event_trigger_dropped_objects to
report temporary objects as they are dropped. In the original coding,
all temp objects are silently ignored, but for the purposes of the
testing module of this patch, this is a problem; consider the case where
a test create a temp object, then drops it, then creates another object
with the same name. We need the sql_drop event trigger in the test
harness to report the dropped object, so that it can be dropped in the
regression_deparse database as well -- otherwise the second create
command fails because the first object still exists.

I have applied numerous other fixes.

For the first time I also include the regression test harness. To run
this, execute "make deparsecheck" in src/test/regress. This runs a
special schedule file, corresponding to serial_schedule minus the
tablespace test, and adds one test at the beginning and one test after
the end; those add event triggers to capture all commands and then run
the commands so captured in another database. The output of that is
grepped for errors; there's a very short list of errors that are always
present. Later, pg_dump is run on both databases, and the dumps are
compared. Currently there are some differences in the dumps; those
correspond to remaining issues in the deparse code. Regarding CREATE
TABLE AS EXECUTE, I had to add two "expected" files for two tests that
have an extra WARNING line when that command fails to deparse. I would
love to, instead, be able to deparse the command to something sensible.
(This is a bit brittle, because it's run from a makefile rather than
inside pg_regress; it's not pretty when things fail. Must be improved,
perhaps as a separate pg_regress binary that does all the process
control internally.)

Known remaining issues:

* Rename pg_event_trigger_get_creation_command(). Some ideas:
- pg_event_trigger_commands()
- pg_event_trigger_ddl_commands()
- pg_event_trigger_deparsed_commands()
- pg_event_trigger_deparsed_ddl_commands()

* CREATE SEQUENCE OWNED BY is not handled properly for freestanding
sequences.

* Handle default_tablespace GUC in CREATE TABLE and others.

* Handle the extension version in some way to CREATE EXTENSION.

* RENAME and others need to handle the inheritance flags (ONLY, etc)

* Many constant strings in SQL syntax ought to use a JSON object
representation with the "present" boolean thingy, instead of an empty
string as currently. That makes it easier to turn them on/off from
event triggers without forcing the user to be aware of the exact
spelling of each option.

* Should we prohibit DDL from within event triggers?

* Handle subxact aborts in ALTER TABLE processing.

* Maybe stash of commands should be an slist rather than List, just like
the dropped objects list.

* For CREATE TABLE AS, the generated command does not have a column list,
and the deparsed version might depend on temp tables for instance;
maybe we need some way to control that one from producing a normal
CREATE TABLE instead.

* The "provider" bit in SECURITY LABEL is messy. There is no Oid for
security providers, they're just loadable modules.

* event trigger currentEventTriggerContext was only being set up when
there were sql_drop/table_rewrite events; we remove that and set it up
always now. That should be restored.

* There are several XXX/FIXME/TODO comments in deparse_utility.c.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services