Re: deparsing utility commands

On Tue, May 12, 2015 at 12:25 AM, Alvaro Herrera <alvherre@2ndquadrant.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.