Re: review: Deparsing DDL command strings

Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
> 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.

Thanks.

> 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

We could have an event trigger that stores the statements in a table,
does some object description like \d or \df+, then drops them all and
replay all the statements from the table and describe the objects again.

I'm not sure how to check that the descriptions are the same from the
regression tests themselves, but once that's manually/externally sorted
out the current facilities will sure provide guards against any
regression.

> Please, can others to write an agreement or any rejection now?
> Does somebody know why this concept is not acceptable? Speak now, please.

+1 :)

>> 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.

Cool. The goal still is to support all functions, but I need aproval
first. It's too much work to be wasted 2 months from now.

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

I will look into that. I know I had to switch in between -O0 and -O2 to
be able to debug some really strange things that happened to have been
due to our build infrastructure not being able to cope with some header
changes. Days like that I really hate the C developper tooling.

> 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

That might be due to some refactoring I just did for the ALTER TABLE.
Sharing code means "action at a distance" sometimes. I need to begin
adding regression tests, but they will look a lot like the ones Robert
did insist on removing last time…

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

My guess is that you didn't initdb the version you tested. As usual I
didn't include the catalog bump in my patch, but initdb still is needed.

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

Works for me (from memory). Sequels of the initdb problem?

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support