Re: BUG #13126: table constraint loses its comment

From: Petr Jelinek <petr(at)2ndquadrant(dot)com>
To: Michael Paquier <michael(dot)paquier(at)gmail(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 14:59:57
Message-ID: 5596A36D.4030008@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

On 2015-07-03 15:50, Michael Paquier wrote:
> 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.
>

Ah ok, I missed Heikki's email.

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

Well for indexes you don't really need to add the new AT command, as
IndexStmt has char *idxcomment which it will automatically uses as
comment if not NULL. While I am not huge fan of the idxcomment it
doesn't seem to be easy to remove it in the future and it's what
transformTableLikeClause uses so it might be good to be consistent with
that.

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Heikki Linnakangas 2015-07-03 16:32:36 Re: PQexec() hangs on OOM
Previous Message roberto.morelli 2015-07-03 14:46:57 BUG #13485: JSONB To recordset not working with CamelCase

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2015-07-03 15:44:42 Re: pgbench - allow backslash-continuations in custom scripts
Previous Message Tom Lane 2015-07-03 14:52:00 Re: WAL logging problem in 9.4.3?