Re: Add CREATE support to event triggers - Mailing list pgsql-hackers

On 2014-01-15 02:11:11 -0300, Alvaro Herrera wrote:
> Then execute commands to your liking.

So, I am giving a quick look, given that I very likely will have to
write a consumer for it...

* deparse_utility_command returns payload via parameter, not return value?
* functions beginning with an underscore formally are reserved, we shouldn't add new places using such a convention.
* I don't think dequote_jsonval is correct as is, IIRC that won't correctly handle unicode escapes and such. I think
json.cneeds to expose functionality for this.
 
* I wonder if expand_jsonval_identifier shouldn't truncate as well? It should get handled by the individual created
commands,right?
 
* So, if I read things correctly, identifiers in json are never downcased, is that correct? I.e. they are implicitly
quoted?
* Why must we not schema qualify system types (c.f. expand_jsonval_typename)? It seems to be perfectly sensible to me
tojust use pg_catalog there.
 
* It looks like pg_event_trigger_expand_command will misparse nested {, error out instead?
* I wonder if deparseColumnConstraint couldn't somehow share a bit more code with ruleutils.c's
pg_get_constraintdef_worker().
* I guess you know, but deparseColumnConstraint() doesn't handle foreign keys yet.
* Is tcop/ the correct location for deparse_utility.c? I wonder if it shouldn't be alongside ruleutils.c instead.
* shouldn't pg_event_trigger_get_creation_commands return "command" as json instead of text?

* Not your department, but a builtin json pretty printer would be really, really handy. Something like
CREATE FUNCTION json_prettify(j json)
RETURNS TEXT AS $$   import json   return json.dumps(json.loads(j), sort_keys=True, indent=4)
$$ LANGUAGE PLPYTHONU;makes the json so much more readable.

Some minimal tests:
* CREATE SEQUENCE errors out with:
NOTICE:  JSON blob: {"sequence":{"relation":"frakbar2","schema":"public"},"persistence":"","fmt":"CREATE
%{persistence}sSEQUENCE %{identity}D"}
 
ERROR:  non-existant element "identity" in JSON formatting object
*CREATE TABLE frakkbar2(id int); error out with:
postgres=# CREATE TABLE frakkbar2(id int);
NOTICE:  JSON blob: {"on_commit":{"present":false,"on_commit_value":null,"fmt":"ON COMMIT
%{on_commit_value}s"},"tablespace":{"present":false,"tablespace":null,"fmt":"TABLESPACE
%{tablespace}I"},"inherits":{"present":false,"parents":null,"fmt":"INHERITS(%{parents:,
}D)"},"table_elements":[{"collation":{"present":false,"fmt":"COLLATE
%{name}I"},"type":{"typmod":"","typename":"integer","is_system":true,"is_array":false},"name":"id","fmt":"%{name}I
%{type}T%{collation}s"}],"of_type":{"present":false,"of_type":null,"fmt":"OF
%{of_type}T"},"if_not_exists":"","identity":{"relation":"frakkbar2","schema":"public"},"persistence":"","fmt":"CREATE
%{persistence}sTABLE %{identity}D %{if_not_exists}s %{of_type}s (%{table_elements:, }s) %{inherits}s %{on_commit}s
%{tablespace}s"}
ERROR:  invalid NULL is_system flag in %T element
CONTEXT:  PL/pgSQL function snitch() line 8 at RAISE

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services