From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Michael Paquier <michael.paquier@gmail.com>
Cc: Kevin Grittner <kgrittn@ymail.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Subject: Re: [COMMITTERS] pgsql: Retain comments on indexes and constraints at ALTER TABLE ... TY
Date: Sat, 18 Jul 2015 16:15:28 +0300
Message-ID: <20150718131528.GV2301@postgresql.org> (raw)
In-Reply-To: <55A94C2D.1010204@iki.fi>
References: <E1ZEvoc-0002RV-Vh@gemulon.postgresql.org>
<81620362.883169.1437142581761.JavaMail.yahoo@mail.yahoo.com>
<CAB7nPqTnAyTwrnXfyJdmovu0B991KK3tymgW5NNtPWyU=fhNzQ@mail.gmail.com>
<55A94C2D.1010204@iki.fi>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
Heikki Linnakangas wrote:
> On 07/17/2015 05:40 PM, Michael Paquier wrote:
> >On Fri, Jul 17, 2015 at 11:16 PM, Kevin Grittner <kgrittn@ymail.com> wrote:
> >>Heikki Linnakangas <heikki.linnakangas@iki.fi> wrote:
> >>
> >>>This fixes bug #13126, reported by Kirill Simonov.
> >>
> >>It looks like you missed something with the addition of
> >>AT_ReAddComment:
> >>
> >>test_ddl_deparse.c:80:11: warning: enumeration value 'AT_ReAddComment' not handled in switch [-Wswitch]
> >> switch (subcmd->subtype)
> >> ^
> >
> >Oops. If someone could pick up the attached (backpatch to 9.5 needed)...
>
> Hmm, that function is pretty fragile, it will segfault on any AT_* type that
> it doesn't recognize. Thankfully you get that compiler warning, but we have
> added AT_* type codes before in minor releases.
Yeah, that module was put together in a bit of a rush when I decided to
remove the JSON deparsing part of the DDL patch.
> I couldn't quite figure out what the purpose of that module is, as
> there is no documentation or README or file-header comments on it.
Well, since it's in src/test/modules I thought it was clear that the
intention is just to be able to test the pg_ddl_command type --
obviously not. I will add a README or something.
> If it's there just to so you can run the regression tests that come
> with it, it might make sense to just add a "default" case to that
> switch to handle any unrecognized commands, and perhaps even remove
> the cases for the currently untested subcommands as it's just dead
> code.
Well, I would prefer to have an output that says "unrecognized" and then
add more test cases to the SQL files so that there's not so much dead
code. I prefer that to removing the C support code, because then as
we add extra tests we don't need to modify the C source.
> If it's supposed to act like a sample that you can copy-paste and
> modify, then perhaps that would still be the best option, and add a
> comment there explaining that it only cares about the most common
> subtypes but you can add handlers for others if necessary.
I wasn't thinking in having it be useful for copy-paste. My longer-term
plan is to have the JSON deparsing extension live in src/extensions.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-committers@postgresql.org
Cc: alvherre@2ndquadrant.com, hlinnaka@iki.fi, michael.paquier@gmail.com, kgrittn@ymail.com, pgsql-hackers@postgresql.org
Subject: Re: [COMMITTERS] pgsql: Retain comments on indexes and constraints at ALTER TABLE ... TY
In-Reply-To: <20150718131528.GV2301@postgresql.org>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox