Message-ID: <55A94C2D.1010204@iki.fi>
Date: Fri, 17 Jul 2015 21:40:45 +0300
From: Heikki Linnakangas <hlinnaka@iki.fi>
Reply-To: hlinnaka@iki.fi
User-Agent: Mozilla/5.0 (X11; Linux x86_64;
 rv:31.0) Gecko/20100101 Icedove/31.7.0
MIME-Version: 1.0
To: Michael Paquier <michael.paquier@gmail.com>,
 Kevin Grittner <kgrittn@ymail.com>
CC: "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>,
 Alvaro Herrera <alvherre@2ndquadrant.com>
Subject: Re: [COMMITTERS] pgsql: Retain comments on indexes and constraints
 at ALTER TABLE ... TY
References: <E1ZEvoc-0002RV-Vh@gemulon.postgresql.org>
	<81620362.883169.1437142581761.JavaMail.yahoo@mail.yahoo.com>
 <CAB7nPqTnAyTwrnXfyJdmovu0B991KK3tymgW5NNtPWyU=fhNzQ@mail.gmail.com>
In-Reply-To: 
 <CAB7nPqTnAyTwrnXfyJdmovu0B991KK3tymgW5NNtPWyU=fhNzQ@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

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. 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. 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. 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.

Alvaro, you want to comment on that?

- Heikki


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers