Re: review: Deparsing DDL command strings

2012/12/4 Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr>:
> 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?

yes, last two issue are related to missing initdb

Regards

Pavel

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