Re: review: Deparsing DDL command strings

Hello

2012/11/27 Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr>:
> Hi,
>
> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
>> ** missing doc
>
> I still would prefer to have an ack about the premises of this patch
> before spending time on writing docs for it, namely:
>
> - we want a normalized command string (schema, auto naming of some
> objects such as constraints and indexes, exporting default values,
> etc);
>
> - this normalisation can not happen as an extension given the current
> source code organisation and the new ddl_rewrite module is a good fit
> to solve that problem;
>
> - we offer a new event alias "ddl_command_trace" that will get
> activated start of a DROP and end of an ALTER or a CREATE command so
> that it's easy to hook at the right place when you want to trace
> things;
>
> - it's now possible and easy to process GENERATED commands if you wish
> to do so (explicit sequence and index operations for a create table
> containing a serial primary key column).
>

I agree with these premisses. I am sure so normalised commands strings
are very nice feature, that can be helpful for generation clean audit
logs and messages. I see a one issue - testing a consistency between
commands and their deparsed forms.

Do you have some idea, how to do these tests - Maybe we can write
extension, that will try repeated parsing and deparsing - normalised
string should be same. I am not sure, if this test should be part of
regressions test, but we have to thinking about some tool for
checking.

we can take commands via hooks and we can check.... ORIGINAL CMD ==>
tree1 ==> DEPARSED STRING ==> tree2 .... tree1 <=> tree2

Please, can others to write an agreement or any rejection now?

Does somebody know why this concept is not acceptable? Speak now, please.

> I think no complaints is encouraging though, so I will probably begin
> the docs effort soonish.
>
>> ** CREATE FUNCTION is not supported
>
> I've added support for create and drop function in the attached patch.
> Not all commands are rewritten yet though, and not all of them even have
> support for the TG_OBJECTID, TG_OBJECTNAME and TG_SCHEMANAME variables.
>

I have no problem with "defined" list of fully unsupported statements.

> I think we could still apply that patch + docs and a limited set of
> commands, and continue filling the gaps till the end of this cycle. Once
> we agree on how to attack the problem at hand, do we really need support
> for ALTER OPERATOR FAMILY for an intermediate commit? You tell me.
>
>> * some wrong in deparsing - doesn't support constraints
>
> I've been fixing that and reviewing ALTER TABLE rewriting, should be ok
> now. Note that we have to analyze the alter table work queue, not the
> parsetree, if we want to normalize automatic constraints names and such
> like.
>
> Regards,
> --
> Dimitri Fontaine
> http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
>

I had a small problem with patching and compilation produce warnings too

pavel ~/src/postgresql/src $ cat log | grep warning
scan.c:16247:23: warning: unused variable ‘yyg’ [-Wunused-variable]
../../../src/include/utils/ddl_rewrite.h:24:8: warning: extra tokens
at end of #endif directive [enabled by default]
../../../../src/include/utils/ddl_rewrite.h:24:8: warning: extra
tokens at end of #endif directive [enabled by default]
ddl_rewrite.c:1719:9: warning: variable ‘def’ set but not used
[-Wunused-but-set-variable]
ddl_rewrite.c:1777:21: warning: ‘languageOid’ is used uninitialized in
this function [-Wuninitialized]

All 133 tests passed.

when I tested

postgres=# create table bubuaa(a int unique not null check (a > 20));
NOTICE: snitch: ddl_command_end CREATE TABLE CREATE TABLE
public.bubuaa (a pg_catalog.int4 UNIQUE NOT NULL CHECK ((a > 20)),
CHECK ((a > 20)));
NOTICE: snitch: ddl_command_end CREATE INDEX CREATE UNIQUE INDEX ON
public.bubuaa USING btree (a);
CREATE TABLE

constraint is twice time there - it is probably same, but it is confusing

drop table generate command string

postgres=# drop table bubuaa;
NOTICE: snitch: ddl_command_end DROP TABLE <NULL>
DROP TABLE

functions are supported :)

backslash statement \dy doesn't work

postgres=# \dy
ERROR: column "evtctxs" does not exist
LINE 1: ... array_to_string(array(select x from unnest(evtctxs) a...

I was little bit surprised so general event trigger without tag
predicate was not triggered for CREATE FUNCTION statement. Is it
documented somewhere?

Regards

Pavel