Re: [COMMITTERS] pgsql: Retain comments on indexes and constraints at ALTER TABLE ... TY

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Kevin Grittner <kgrittn(at)ymail(dot)com>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Subject: Re: [COMMITTERS] pgsql: Retain comments on indexes and constraints at ALTER TABLE ... TY
Date: 2015-07-17 18:40:45
Message-ID: 55A94C2D.1010204@iki.fi
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

On 07/17/2015 05:40 PM, Michael Paquier wrote:
> On Fri, Jul 17, 2015 at 11:16 PM, Kevin Grittner <kgrittn(at)ymail(dot)com> wrote:
>> Heikki Linnakangas <heikki(dot)linnakangas(at)iki(dot)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

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2015-07-17 19:53:21 pgsql: Repair mishandling of cached cast-expression trees in plpgsql.
Previous Message Tom Lane 2015-07-17 18:10:57 pgsql: Fix entirely broken permissions test in new alter_operator regre

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2015-07-17 18:48:22 Re: Further issues with jsonb semantics, documentation
Previous Message Robert Haas 2015-07-17 18:37:39 Re: Further issues with jsonb semantics, documentation