Re: deparsing utility commands

On 4/7/15 4:32 PM, Alvaro Herrera wrote:
> Executive summary:
>
> There is now a CommandDeparse_hook;
> deparse_utility_command is provided as an extension, intended for 9.6;
> rest of patch would be pushed to 9.5.
>
>
> Long version:
>
> I've made command deparsing hookable.  Attached there are three patches:
> the first patch contains changes to core that just add the "command
> list" stuff, so that on ddl_command_end there is access to what has been
> executed.  This includes the OID of the object just created, the command
> tag, and assorted other details; the deparsed command in JSON format is
> not immediately part of the result.

I'm looking forward to testing this with pg_audit.  I think the first
patch alone will give us the bulk of the desired functionality.

> The third patch contains all the deparse code, packaged as a contrib
> module and extension named ddl_deparse.  Essentially, it's everything
> that was previously in tcop/deparse_utility.c and utils/adt/ddl_json.c:
> the stuff that takes the parsenode and OID of a command execution and
> turns it into a JSON blob, and also the support function that takes the
> JSON blob and converts back into the plain text rendering of the
> command.
>
> The second patch contains some changes to core code that support the
> ddl_deparse extension; mainly some ruleutils.c changes.

> What links patches 0001 and 0003 is a hook, CommandDeparse_hook.  If
> unset, the pg_event_trigger_ddl_commands function returns some
> boilerplate text like "no deparse function installed"; if the extension
> is installed, the JSON rendering is returned instead and can be used
> with the ddl_deparse_expand_command() function.

I would prefer for the column to be NULL or to have a boolean column
that indicates if JSON output is available (or both).  I'd rather not do
string matching if I can help it (or test for invalid JSON).

> The rationale for doing things this way is that it will be useful to
> have 9.5 expose the pg_event_trigger_ddl_commands() function for various
> uses, while we refine the JSON bits some more and get it committed for
> 9.6.  In reviews, it's clear that there's some more bits to fiddle so
> that it can be as general as possible.  I think we should label the
> whole DDL command reporting as experimental in 9.5 and subject to
> change, so that we can just remove the hook in 9.6 when the ddl_deparse
> thing becomes part of core.

I'm completely on board with this.  I think deparse is a cool piece of
code and I see a bunch of potential uses for it, but at the same time
I'm not sure it has finished baking yet.  This is a smart and safe choice.

Is there a particular commit you'd recommend for applying the deparse
patches (especially the first set)?

--
- David Steele
david@pgmasters.net