Re: BUG #13126: table constraint loses its comment

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Petr Jelinek <petr(at)2ndquadrant(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, xi(at)resolvent(dot)net, PostgreSQL mailing lists <pgsql-bugs(at)postgresql(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: BUG #13126: table constraint loses its comment
Date: 2015-07-03 13:50:11
Message-ID: CAB7nPqRXLGYh4HZbW2_m5kJNjNtnt1zwAndkZGRdHuaj7d-jHQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

On Thu, Jul 2, 2015 at 11:16 PM, Petr Jelinek <petr(at)2ndquadrant(dot)com> wrote:
> I was going through the code and have few comments:
> - Why do you change the return value of TryReuseIndex? Can't we use reuse
> the same OidIsValid(stmt->oldNode) check that ATExecAddIndex is doing
> instead?

As pointed out by Heikki previously, that is actually unnecessary,
comments are still lost even if the index is reused for constraints.
So perhaps for simplicity we could just unconditionally recreate the
comments all the time if they are available.

> - I think the changes to ATPostAlterTypeParse should follow more closely the
> coding of transformTableLikeClause - namely use the idxcomment

I am not sure I follow here. Could you elaborate?

> - Also the changes to ATPostAlterTypeParse move the code somewhat
> uncomfortably over the 80 char width, I don't really see a way to fix that
> except for moving some of the nesting out to another function.

Yes, I did some refactoring here by adding a set of new routines
dedicated to attach generated commands to the correct queue. This way
the code is empty of large switch/case blocks.

Update patch is attached, with the issue reported by Heikki upthread
fixed as well.
Regards,
--
Michael

Attachment Content-Type Size
0001-Ensure-COMMENT-persistency-of-indexes-and-constraint.patch application/x-patch 19.3 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Tom Lane 2015-07-03 13:55:08 Re: BUG #13481: No config folder upon installation
Previous Message Michael Paquier 2015-07-03 12:56:43 Re: BUG #13126: table constraint loses its comment

Browse pgsql-hackers by date

  From Date Subject
Next Message Fabien COELHO 2015-07-03 13:52:23 Re: pgbench - allow backslash-continuations in custom scripts
Previous Message Michael Paquier 2015-07-03 12:56:43 Re: BUG #13126: table constraint loses its comment