Re: Add CREATE support to event triggers

From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Add CREATE support to event triggers
Date: 2014-01-23 12:57:13
Message-ID: 20140123125713.GC29782@awork2.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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.c needs 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 to just 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}s SEQUENCE %{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}s TABLE %{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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Etsuro Fujita 2014-01-23 12:59:17 [Review] inherit support for foreign tables
Previous Message Simon Riggs 2014-01-23 12:56:49 Re: Hard limit on WAL space used (because PANIC sucks)