Re: BUG #13126: table constraint loses its comment

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: xi(at)resolvent(dot)net, PostgreSQL mailing lists <pgsql-bugs(at)postgresql(dot)org>
Subject: Re: BUG #13126: table constraint loses its comment
Date: 2015-07-02 12:28:53
Message-ID: 55952E85.30906@iki.fi
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

On 05/27/2015 04:10 PM, Michael Paquier wrote:
> On Wed, Apr 29, 2015 at 1:30 PM, Michael Paquier
> <michael(dot)paquier(at)gmail(dot)com> wrote:
>> On Sun, Apr 26, 2015 at 6:05 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> xi(at)resolvent(dot)net writes:
>>>> In some circumstances, the comment on a table constraint disappears. Here
>>>> is an example:
>>>
>>> Hm, yeah. The problem is that ATExecAlterColumnType() rebuilds all the
>>> affected indexes from scratch, and it isn't doing anything about copying
>>> their comments to the new objects (either comments on the constraints, or
>>> comments directly on the indexes).
>>>
>>> The least painful way to fix it might be to charter ATPostAlterTypeCleanup
>>> to create COMMENT commands and add those to the appropriate work queue,
>>> rather than complicating the data structure initially emitted by
>>> ATExecAlterColumnType. But it'd still be a fair amount of new code I'm
>>> afraid.
>>>
>>> Not planning to fix this personally, but maybe someone else would like to
>>> take it up.
>>
>> In order to fix this, an idea would be to add a new routine in
>> ruleutils.c that generates the COMMENT query string, and then call it
>> directly from tablecmds.c. On master, I imagine that we could even add
>> some SQL interface if there is some need.
>> Thoughts?
>
> After looking at this problem, I noticed that the test case given
> above does not cover everything: primary key indexes, constraints
> (CHECK for example) and indexes alone have also their comments removed
> after ALTER TABLE when those objects are re-created. I finished with
> the patch attached to fix everything, patch that includes a set of
> regression tests covering all the code paths added.

The problem isn't limited to the cases where the old index is not
reused. This still fails with your patch:

postgres=# create table t(id text primary key);CREATE TABLE
postgres=# comment on constraint t_pkey on t is 'the primary key of
table "t"';COMMENT
postgres=# select d.description from pg_description d, pg_constraint c
where d.classoid = 'pg_constraint'::regclass and c.conname = 't_pkey'
and d.objoid = c.oid;
description
------------------------------
the primary key of table "t"
(1 row)

-- perform an ALTER TABLE that doesn't really do anything
postgres=# ALTER TABLE t ALTER COLUMN id SET DATA TYPE text;
ALTER TABLE

-- The comment is gone:
postgres=# select d.description from pg_description d, pg_constraint c
where d.classoid = 'pg_constraint'::regclass and c.conname = 't_pkey'
and d.objoid = c.oid;
description
-------------
(0 rows)

- Heikki

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Fujii Masao 2015-07-02 13:00:10 Re: BUG #13368: standby cluster immediately promotes after pg_basebackup from previously promoted master
Previous Message joao.antunes 2015-07-02 10:44:59 BUG #13480: RE: #11732 and also possibly #12538 - Possible serializable isolation lepessimistic locking problem

Browse pgsql-hackers by date

  From Date Subject
Next Message Fujii Masao 2015-07-02 12:31:46 Re: Support for N synchronous standby servers - take 2
Previous Message Michael Paquier 2015-07-02 11:59:47 Re: WAL-related tools and .paritial WAL file