TODO Alter Table Rename Constraint

Started by Viktor Valyabout 15 years ago6 messages
#1Viktor Valy
vili0121@gmail.com

Hi Everybody!

I looked up this todo, and figured out a plan, how the implementation could
be written.
The main challenge is to keep constraints and indexes (for unique and PK
constraints) consistent.
Fortunately this is already realized in the case of an index. So at ALTER
INDEX RENAME the consistency is given by an extra check in *tablecmds.c
(Line 2246)*, where a function is finally called,* RenameConstraintById
(pg_constraint.c Line 604)*.

My idea is, to do that analog for ALTER CONSTRAINT RENAME too. So renaming
the constraint is going to be done in *tablecmds.c* as for indexes, tables,
sequences, views. And after checking whether the renametype is constraint,
an extra rename has to be done for the index. Getting the index can be done
with the function *get_constraint_index (pg_depend.c Line 475)*. Now it
should be possible to do the same as in *RenameConstraintById.*

Is that so legal? Is anything else to be considered?

I appreciate any suggestion, and maybe some help too, as I'm not so familiar
with the source.
Thanks in advance,

Viktor
(Student of technical unverstity of Vienna)

#2Robert Haas
robertmhaas@gmail.com
In reply to: Viktor Valy (#1)
Re: TODO Alter Table Rename Constraint

On Tue, Nov 9, 2010 at 10:50 AM, Viktor Valy <vili0121@gmail.com> wrote:

Hi Everybody!
I looked up this todo, and figured out a plan, how the implementation could
be written.
The main challenge is to keep constraints and indexes (for unique and PK
constraints) consistent.
Fortunately this is already realized in the case of an index. So at ALTER
INDEX RENAME the consistency is given by an extra check in tablecmds.c (Line
2246), where a function is finally called, RenameConstraintById
(pg_constraint.c Line 604).
My idea is, to do that analog for ALTER CONSTRAINT RENAME too. So renaming
the constraint is going to be done in tablecmds.c as for indexes, tables,
sequences, views. And after checking whether the renametype is constraint,
an extra rename has to be done for the index. Getting the index can be done
with the function get_constraint_index (pg_depend.c Line  475). Now it
should be possible to do the same as in RenameConstraintById.
Is that so legal? Is anything else to be considered?

I think the biggest problem is handling inherited tables properly,
especially in complex inheritance hierarchies where there are
multiple, separate paths from the top of the hierarchy to the bottom.

See here for a couple of relevant test cases:

http://archives.postgresql.org/pgsql-hackers/2010-07/msg01570.php

I believe that the rename needs to fail if any table in the
inheritance hierarchy rooted at the target table also inherits the
constraint from someplace outside that hierarchy; or if any table in
that hierarchy has a local copy of the constraint that got merged with
the inherited one.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#3Viktor Valy
vili0121@gmail.com
In reply to: Robert Haas (#2)
Re: TODO Alter Table Rename Constraint

Thanks for your answer!

I'm not really familiar with inheritance, but I wonder how this issue
is handled in other cases, for instance renaming an index, which invokes
internal a constraint rename too. Is that relevant or is the renaming of
constraints so special?
Is there a solution for the test-cases you have posted? Or is this yet a
problem?

Viktor

2010/11/9 Robert Haas <robertmhaas@gmail.com>

Show quoted text

On Tue, Nov 9, 2010 at 10:50 AM, Viktor Valy <vili0121@gmail.com> wrote:

Hi Everybody!
I looked up this todo, and figured out a plan, how the implementation

could

be written.
The main challenge is to keep constraints and indexes (for unique and PK
constraints) consistent.
Fortunately this is already realized in the case of an index. So at ALTER
INDEX RENAME the consistency is given by an extra check in tablecmds.c

(Line

2246), where a function is finally called, RenameConstraintById
(pg_constraint.c Line 604).
My idea is, to do that analog for ALTER CONSTRAINT RENAME too. So

renaming

the constraint is going to be done in tablecmds.c as for indexes, tables,
sequences, views. And after checking whether the renametype is

constraint,

an extra rename has to be done for the index. Getting the index can be

done

with the function get_constraint_index (pg_depend.c Line 475). Now it
should be possible to do the same as in RenameConstraintById.
Is that so legal? Is anything else to be considered?

I think the biggest problem is handling inherited tables properly,
especially in complex inheritance hierarchies where there are
multiple, separate paths from the top of the hierarchy to the bottom.

See here for a couple of relevant test cases:

http://archives.postgresql.org/pgsql-hackers/2010-07/msg01570.php

I believe that the rename needs to fail if any table in the
inheritance hierarchy rooted at the target table also inherits the
constraint from someplace outside that hierarchy; or if any table in
that hierarchy has a local copy of the constraint that got merged with
the inherited one.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#4Robert Haas
robertmhaas@gmail.com
In reply to: Viktor Valy (#3)
Re: TODO Alter Table Rename Constraint

On Wed, Nov 10, 2010 at 6:32 AM, Viktor Valy <vili0121@gmail.com> wrote:

Thanks for your answer!
I'm not really familiar with inheritance, but I wonder how this issue
is handled in other cases, for instance renaming an index, which invokes
internal a constraint rename too. Is that relevant or is the renaming of
constraints so special?

Indexes can't be inherited, so the problem doesn't arise in that case.

Is there a solution for the test-cases you have posted? Or is this yet a
problem?

We had a bug related to the handling of ALTER TABLE .. ADD/DROP
CONSTRAINT for those test cases, which I fixed. I think we still have
a similar problem with ALTER TABLE .. ADD/DROP ATTRIBUTE, which I
haven't fixed because it's hard and I haven't had time, and no one
seems to care that much. My point was just that whatever patch you
come up with for ALTER TABLE .. RENAME CONSTRAINT should probably be
tested against those cases to see if it behaves correctly.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#5Viktor Valy
vili0121@gmail.com
In reply to: Robert Haas (#4)
Re: TODO Alter Table Rename Constraint

OK, I see. Thanks for mentioning it.
Are there other problems with the suggestion? Or should the work like that?

Viktor

2010/11/10 Robert Haas <robertmhaas@gmail.com>

Show quoted text

On Wed, Nov 10, 2010 at 6:32 AM, Viktor Valy <vili0121@gmail.com> wrote:

Thanks for your answer!
I'm not really familiar with inheritance, but I wonder how this issue
is handled in other cases, for instance renaming an index, which invokes
internal a constraint rename too. Is that relevant or is the renaming of
constraints so special?

Indexes can't be inherited, so the problem doesn't arise in that case.

Is there a solution for the test-cases you have posted? Or is this yet a
problem?

We had a bug related to the handling of ALTER TABLE .. ADD/DROP
CONSTRAINT for those test cases, which I fixed. I think we still have
a similar problem with ALTER TABLE .. ADD/DROP ATTRIBUTE, which I
haven't fixed because it's hard and I haven't had time, and no one
seems to care that much. My point was just that whatever patch you
come up with for ALTER TABLE .. RENAME CONSTRAINT should probably be
tested against those cases to see if it behaves correctly.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#6Robert Haas
robertmhaas@gmail.com
In reply to: Viktor Valy (#5)
Re: TODO Alter Table Rename Constraint

On Fri, Nov 12, 2010 at 4:28 AM, Viktor Valy <vili0121@gmail.com> wrote:

OK, I see. Thanks for mentioning it.
Are there other problems with the suggestion? Or should the work like that?

I think you'll just need to give it a try and see how it goes. I
think we've covered most of the possible sticking points that I know
about, but of course there could be some I don't know about.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company