Commenting a FK crashes ALTER TABLE statements

Started by Marko Elezovicalmost 9 years ago5 messagesbugs
Jump to latest
#1Marko Elezovic
marko.elezovic@oradian.com

Affected versions: 9.5.0 up to 9.6.3 (regression from 9.4.12)
Affected OS: All

Script to reproduce:
psql -Upostgres -c"DROP DATABASE IF EXISTS cod;" postgres
psql -Upostgres -c"CREATE DATABASE cod;" postgres
psql -Upostgres -c"CREATE TABLE foo(id int PRIMARY KEY);" cod
psql -Upostgres -c"CREATE TABLE bar(id int CONSTRAINT baz REFERENCES foo);" cod
psql -Upostgres -c"COMMENT ON CONSTRAINT baz ON bar IS 'Fubar';" cod
psql -Upostgres -c"ALTER TABLE foo ALTER COLUMN id TYPE int;" cod

*IMPORTANT NOTE*: The example above is 100% reproducible if executed verbatim, from command line of 9.5+ while on either Windows/Linux
On the other hand, when ran from psql by pasting all the commands or running them within the same script it is not always reproducible.
I also haven't been able to reproduce the issue by adding a new test into src/test/regress/sql/comments.sql

Expected output after last alter should be a message with random garbage instead of foreign key name:

ERROR: relation "public.¬" does not exist
ERROR: relation "public." does not exist

I've traced the issue into Re-adding an existing comment (starting in tablecmds.c):

case AT_ReAddComment: /* Re-add existing comment */
address = CommentObject((CommentStmt *) cmd->def);

... which fails in pg_constraint.c while traversing the constraints:

scan = systable_beginscan(pg_constraint, ConstraintRelidIndexId, true,
NULL, 1, skey);

while (HeapTupleIsValid(tuple = systable_getnext(scan)))

Here tuple sometimes gets garbage memory addresses, so it explains at least 50% of the craziness I've seen while reproducing.

The same example also has a chance to produce these as well:

ERROR: constraint "baz" for table "foo" does not exist <-- due to not being found in the above while loop
ERROR: "foo_pkey" is an index <-- when trying to open the heap for pkey constraint

ERROR: relation "public.pg_class_tblspc_relfilenode_index" does not exist <-- ??? Heisenbugs, frequently visible in production SQL migrations
ERROR: relation "public.pg_constraint_conname_nsp_index" does not exist <-- ??? but could never reproduce when in debug mode / tracing

I'll be happy to continue the tracing effort if need be, but hoping this is reproducible on your machines

With heartfelt regards,
Marko

Oradian Ltd., Head of Technology
marko.elezovic@oradian.com<mailto:marko.elezovic@oradian.com>
+385 91 2646 342

#2David Rowley
dgrowleyml@gmail.com
In reply to: Marko Elezovic (#1)
Re: Commenting a FK crashes ALTER TABLE statements

On 15 May 2017 at 03:43, Marko Elezovic <marko.elezovic@oradian.com> wrote:

Script to reproduce:

psql -Upostgres -c"DROP DATABASE IF EXISTS cod;" postgres

psql -Upostgres -c"CREATE DATABASE cod;" postgres

psql -Upostgres -c"CREATE TABLE foo(id int PRIMARY KEY);" cod

psql -Upostgres -c"CREATE TABLE bar(id int CONSTRAINT baz REFERENCES
foo);" cod

psql -Upostgres -c"COMMENT ON CONSTRAINT baz ON bar IS 'Fubar';" cod

psql -Upostgres -c"ALTER TABLE foo ALTER COLUMN id TYPE int;" cod

Thanks for detailing out the method to reproduce.

It can be simplified a bit to become:

CREATE TABLE foo(id int PRIMARY KEY);
CREATE TABLE bar(id int CONSTRAINT baz REFERENCES foo);
COMMENT ON CONSTRAINT baz ON bar IS 'Fubar';
\c
ALTER TABLE foo ALTER COLUMN id TYPE int;

It seems there's just some missing pstrdup() calls in
RebuildConstraintComment().

The attached should fix it.

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

Attachments:

recomment_fix.patchapplication/octet-stream; name=recomment_fix.patchDownload+2-2
#3Michael Paquier
michael@paquier.xyz
In reply to: David Rowley (#2)
Re: Commenting a FK crashes ALTER TABLE statements

On Mon, May 15, 2017 at 1:05 PM, David Rowley
<david.rowley@2ndquadrant.com> wrote:

It can be simplified a bit to become:

CREATE TABLE foo(id int PRIMARY KEY);
CREATE TABLE bar(id int CONSTRAINT baz REFERENCES foo);
COMMENT ON CONSTRAINT baz ON bar IS 'Fubar';
\c
ALTER TABLE foo ALTER COLUMN id TYPE int;

It seems there's just some missing pstrdup() calls in
RebuildConstraintComment().

The attached should fix it.

I was just finishing to debug it :)
It is surprising that we have not caught this earlier as comment
re-creation handling in ALTER TABLE has been reworked some time ago
already. This means that we did not stress this code enough. Attached
is the previous fix, completed with a set of regression tests.
--
Michael

Attachments:

recomment_fix_v2.patchapplication/octet-stream; name=recomment_fix_v2.patchDownload+53-2
#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Michael Paquier (#3)
Re: Commenting a FK crashes ALTER TABLE statements

Michael Paquier <michael.paquier@gmail.com> writes:

On Mon, May 15, 2017 at 1:05 PM, David Rowley
<david.rowley@2ndquadrant.com> wrote:

It seems there's just some missing pstrdup() calls in
RebuildConstraintComment().

The attached should fix it.

I was just finishing to debug it :)
It is surprising that we have not caught this earlier as comment
re-creation handling in ALTER TABLE has been reworked some time ago
already. This means that we did not stress this code enough. Attached
is the previous fix, completed with a set of regression tests.

Yeah, there should at least be a test case to cover the known crash.

I'll take this one, unless some other committer is already on it.

regards, tom lane

--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

#5Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#4)
Re: Commenting a FK crashes ALTER TABLE statements

On Mon, May 15, 2017 at 11:37 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Michael Paquier <michael.paquier@gmail.com> writes:

On Mon, May 15, 2017 at 1:05 PM, David Rowley
<david.rowley@2ndquadrant.com> wrote:

It seems there's just some missing pstrdup() calls in
RebuildConstraintComment().

The attached should fix it.

I was just finishing to debug it :)
It is surprising that we have not caught this earlier as comment
re-creation handling in ALTER TABLE has been reworked some time ago
already. This means that we did not stress this code enough. Attached
is the previous fix, completed with a set of regression tests.

Yeah, there should at least be a test case to cover the known crash.

I'll take this one, unless some other committer is already on it.

Thanks. (committed as 12590c5d)
--
Michael

--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs