On 2015-05-27 15:10, Michael Paquier wrote:
On Wed, Apr 29, 2015 at 1:30 PM, Michael Paquier
<michael.paqu...@gmail.com> wrote:
On Sun, Apr 26, 2015 at 6:05 AM, Tom Lane <t...@sss.pgh.pa.us> wrote:
x...@resolvent.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.
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?
- I think the changes to ATPostAlterTypeParse should follow more closely
the coding of transformTableLikeClause - namely use the idxcomment
- 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.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
