Re: deparsing utility commands

From: "Shulgin, Oleksandr" <oleksandr(dot)shulgin(at)zalando(dot)de>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: David Steele <david(at)pgmasters(dot)net>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: deparsing utility commands
Date: 2015-07-29 10:44:36
Message-ID: CACACo5R4VwGddt00qtPmhUhtd=okwu2ECM-j20B6+cmgtZF6mQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, May 12, 2015 at 12:25 AM, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
wrote:

> David Steele wrote:
>
> > I have reviewed and tested this patch and everything looks good to me.
> > It also looks like you added better coverage for schema DDL, which is a
> > welcome addition.
>
> Thanks -- I have pushed this now.
>

Hi,

I've tried compiling the 0003-ddl_deparse-extension part from
http://www.postgresql.org/message-id/20150409161419.GC4369@alvh.no-ip.org
on current master and that has failed because the 0002 part hasn't been
actually pushed (I've asked Alvaro off the list about this, that's how I
know the reason ;-).

I was able to update the 0002 part so it applies cleanly (updated version
attached), and then the contrib module compiles after one minor change and
seems to work.

I've started to look into what it would take to move 0002's code to the
extension itself, and I've got a question about use of printTypmod() in
format_type_detailed():

if (typemod >= 0)
*typemodstr = printTypmod(NULL, typemod, typeform->typmodout);
else
*typemodstr = pstrdup("");

Given that printTypmod() does psprintf("%s%s") one way or the other,
shouldn't we pass an empty string here instead of NULL as typname argument?

My hope is to get this test module extended quite a bit, not only to
> cover existing commands, but also so that it causes future changes to
> cause failure unless command collection is considered. (In a previous
> version of this patch, there was a test mode that ran everything in the
> serial_schedule of regular regression tests. That was IMV a good way to
> ensure that new commands were also tested to run under command
> collection. That hasn't been enabled in the new test module, and I
> think it's necessary.)
>

> If anyone wants to contribute to the test module so that more is
> covered, that would be much appreciated.
>

I'm planning to have a look at this part also.

--
Alex

Attachment Content-Type Size
ddl_deparse-core-support.patch text/x-patch 30.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ashutosh Bapat 2015-07-29 10:58:29 Re: Transactions involving multiple postgres foreign servers
Previous Message Shulgin, Oleksandr 2015-07-29 10:44:15 Re: [PATCH] Generalized JSON output functions