Thread: [PATCH] Editing table properties drops all foreign keys and recreates them

[PATCH] Editing table properties drops all foreign keys and recreates them

From
Michael Banck
Date:
Hi,

(resent to list as per discussion with dpage at FOSDEM)

Editing the properties of a table containing foreign key constraints
leads (without any further action) to those foreign key constraints
being dropped and recreated (see Trac #378).  Recreating the foreign key
constraint will hold an exclusive lock on both the local and the foreign
table while the foreign key constraint is verified[1]. Depending on how
big the tables are, the locks can be held for a long time.

At one client, we had a production database grind to a halt because the
DBA wanted to add a GRANT to a table and did not inspect the SQL tab
closely, resulting in the above scenario as the foreign key constraint
referenced a central table.

Thankfully, the client allowed us to look at the problem on their time.
I concluded that this bug is a regression introduced with commit
9280f965[2].  I debugged PGAdmin3 (version 1.14.2) and it appears the
problem is as follows: when comparing the constraints around line 820 of
pgadmin/dlg/dlgTable.cpp, the content of the wxStringArray
constraintsDefinition contains additional whitespace and newlines
compared to the content of previousConstraints (and thus tmpDef, in
which the definition is searched for), resulting in no match (index =
-1) and the subsequent deletion of the constraint.

The culprit is when adding the existing constraints to
constraintsDefinition and lstConstraints in dlgTable::Go. For the
PGM_FOREIGNKEY case around line 355 in pgadmin/dlg/dlgTable.cpp,
whitespace and newlines get stripped off for lstConstraints but not for
constraintsDefinition, resulting in different strings. It is unclear why
the whitespace is removed for PGM_FOREIGNKEY but not for any of the
other constraints types, and git annotate does not help either as this
code got last touched during a reindent run.

I propose to remove said stripping of whitespace from lstConstraints,
this fixes the problem for me, see attached patch. However, I do not
have a Windows development environment in order to check whether the
original bug as reported by Vjacheslav Vjacheslav in
http://archives.postgresql.org/pgadmin-support/2011-12/msg00010.php is
still fixed, but I suppose so.


Best regards,

Michael

[1] http://archives.postgresql.org/pgsql-hackers/2012-11/msg00497.php

[2]
http://git.postgresql.org/gitweb/?p=pgadmin3.git;a=commit;h=9280f9654714872816ac3b1a40455536c754ea4d

--
Michael Banck
Tel.: +49 (0) 2161 / 4643-171

credativ GmbH, HRB Mönchengladbach 12080
Hohenzollernstr. 133, 41061 Mönchengladbach
Geschäftsführung: Dr. Michael Meskes, Jörg Folz

Attachment
Thanks - patch applied to master and the 1.16 branch.

On Mon, Feb 4, 2013 at 2:41 PM, Michael Banck <michael.banck@credativ.de> wrote:
> Hi,
>
> (resent to list as per discussion with dpage at FOSDEM)
>
> Editing the properties of a table containing foreign key constraints
> leads (without any further action) to those foreign key constraints
> being dropped and recreated (see Trac #378).  Recreating the foreign key
> constraint will hold an exclusive lock on both the local and the foreign
> table while the foreign key constraint is verified[1]. Depending on how
> big the tables are, the locks can be held for a long time.
>
> At one client, we had a production database grind to a halt because the
> DBA wanted to add a GRANT to a table and did not inspect the SQL tab
> closely, resulting in the above scenario as the foreign key constraint
> referenced a central table.
>
> Thankfully, the client allowed us to look at the problem on their time.
> I concluded that this bug is a regression introduced with commit
> 9280f965[2].  I debugged PGAdmin3 (version 1.14.2) and it appears the
> problem is as follows: when comparing the constraints around line 820 of
> pgadmin/dlg/dlgTable.cpp, the content of the wxStringArray
> constraintsDefinition contains additional whitespace and newlines
> compared to the content of previousConstraints (and thus tmpDef, in
> which the definition is searched for), resulting in no match (index =
> -1) and the subsequent deletion of the constraint.
>
> The culprit is when adding the existing constraints to
> constraintsDefinition and lstConstraints in dlgTable::Go. For the
> PGM_FOREIGNKEY case around line 355 in pgadmin/dlg/dlgTable.cpp,
> whitespace and newlines get stripped off for lstConstraints but not for
> constraintsDefinition, resulting in different strings. It is unclear why
> the whitespace is removed for PGM_FOREIGNKEY but not for any of the
> other constraints types, and git annotate does not help either as this
> code got last touched during a reindent run.
>
> I propose to remove said stripping of whitespace from lstConstraints,
> this fixes the problem for me, see attached patch. However, I do not
> have a Windows development environment in order to check whether the
> original bug as reported by Vjacheslav Vjacheslav in
> http://archives.postgresql.org/pgadmin-support/2011-12/msg00010.php is
> still fixed, but I suppose so.
>
>
> Best regards,
>
> Michael
>
> [1] http://archives.postgresql.org/pgsql-hackers/2012-11/msg00497.php
>
> [2]
> http://git.postgresql.org/gitweb/?p=pgadmin3.git;a=commit;h=9280f9654714872816ac3b1a40455536c754ea4d
>
> --
> Michael Banck
> Tel.: +49 (0) 2161 / 4643-171
>
> credativ GmbH, HRB Mönchengladbach 12080
> Hohenzollernstr. 133, 41061 Mönchengladbach
> Geschäftsführung: Dr. Michael Meskes, Jörg Folz



--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company