Thread: [BUGS] Commenting a FK crashes ALTER TABLE statements
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
+385 91 2646 342
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 -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Attachment
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 -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Attachment
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
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