Re: review: Deparsing DDL command strings

Hello

2012/11/22 Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr>:
> Hi,
>
> Thank you Pavel for reviewing my patch! :)
>
> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
>> * issues
>>
>> ** missing doc
>
> Yes. I wanted to reach an agreement on why we need the de parsing for.
> You can read the Last comment we made about it at the following link, I
> didn't have any answer to my proposal.
>
> http://archives.postgresql.org/message-id/m2391fu79m.fsf@2ndQuadrant.fr
>
> In that email, I said:
>> Also, I'm thinking that publishing the normalized command string is
>> something that must be maintained in the core code, whereas the external
>> stable format might be done as a contrib extension, coded in C, working
>> with the Node *parsetree. Because it needs to ouput a compatible format
>> when applied to different major versions of PostgreSQL, I think it suits
>> quite well the model of C coded extensions.
>
> I'd like to hear what people think about that position. I could be
> working on the docs meanwhile I guess, but a non trivial amount of what
> I'm going to document is to be thrown away if we end up deciding that we
> don't want the normalized command string…
>

I agree with you, so for PL languages just plain text is best format -
it is enough for all tasks that, I can imagine, can be solved by all
supported PL. And for complex tasks - exported parsetree is perfect.

My starting point is strategy, so everything should by possible from
PL/pgSQL, that is our most used PL - and there any complex format is
bad - the most work is doable via plan text and regular expressions.

There is precedent - EXPLAIN - we support more formats, but in PL, we
use usually just plain format.

>> ** statements are not deparsd for ddl_command_start event
>
> Yes, that's by design, and that's why we have the ddl_command_trace
> event alias too. Tom is right in saying that until the catalogs are
> fully populated we can not rewrite most of the command string if we want
> normalized output (types, constraints, namespaces, all the jazz).
>

ok

>> ** CREATE FUNCTION is not supported
>
> Not yet. The goal of this patch in this CF is to get a green light on
> providing a full implementation of what information to add to event
> triggers and in particular about rewriting command strings.

I don't have a objection. Any other ???

Regards

Pavel

>
> That said, if you think it would help you as a reviewer, I may attack
> the CREATE FUNCTION support right now.
>
>> * some wrong in deparsing - doesn't support constraints
>>
>> postgres=# alter table a add column bbbsss int check (bbbsss > 0);
>> NOTICE: event: ddl_command_start, context: TOPLEVEL, tag: ALTER
>> TABLE, operation: ALTER, type: TABLE, schema: <NULL>, name: <NULL>
>> NOTICE: command: <NULL>
>> NOTICE: event: ddl_command_end, context: TOPLEVEL, tag: ALTER TABLE,
>> operation: ALTER, type: TABLE, schema: public, name: a
>> NOTICE: command: ALTER TABLE public.a ADD COLUMN bbbsss pg_catalog.int4, ;
>> ALTER TABLE
>
> Took me some time to figure out how the constraint processing works in
> CREATE TABLE, and apparently I didn't finish ALTER TABLE support yet.
> Working on that, thanks.
>

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