Re: bug in ALTER TABLE / TYPE

I realize this needs review, but I will put it the queue so we don't
forget it.

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.

---------------------------------------------------------------------------

Neil Conway wrote:
> A coworker of mine reported a subtle issue in ATExecAlterColumnType() in
> tablecmds.c. Suppose we have the following DDL:
>
> CREATE TABLE pktable (a int primary key, b int);
> CREATE TABLE fktable (fk int references pktable, c int);
> ALTER TABLE pktable ALTER COLUMN a TYPE bigint;
>
> Circa line 4891 in current sources, we begin a system table scan to look
> for pg_depend rows that depend on the column we're modifying. In this
> case, there are two dependencies on the column: the pg_constraint row
> for pktable's PK constraint, and the pg_constraint row for fktable's FK
> constraint. The bug is that we require the systable scan to return the
> FK constraint before the PK constraint -- if we attempt to remove the PK
> constraint first, it will fail because the FK constraint still exists
> and depends on the PK constraint.
>
> This bug is difficult to reproduce with a normal postmaster and vanilla
> sources, as the systable scan is usually implemented via a B+-tree scan
> on (refclassid, refobjid, refobjsubid). For this particular test case
> these three fields have equal values for the PK and FK constraint
> pg_depend rows, so the order in which the two constraints are returned
> is undefined. It just so happens that the Postgres b+-tree
> implementation *usually* returns the FK constraint first (per comments
> in nbtinsert.c, we usually place an equal key value at the end of a
> chain of equal keys, but stop looking for the end of the chain with a
> probability of 1%). And of course there is no ordering constraint if the
> systable scan is implemented via a heap scan (which would happen if,
> say, we're ignoring indexes on system catalogs in a standalone backend).
>
> To reproduce, any of:
>
> (1) Run the above SQL in a standalone backend started with the "-P" flag
>
> (2) Change "true" to "false" in the third argument to
> systable_beginscan() at tablecmds.c:4891, which means a heap scan will
> be used by a normal backend.
>
> (3) I've attached a dirty kludge of a patch that shuffles the results of
> the systable scan with probability 50%. Applying the patch should repro
> the bug with a normal backend (approx. 1 in 2 times, naturally).
>
> I'm not too familiar with the pg_depend code, so I'm not sure the right
> fix. Comments?
>
> -Neil
>

>
> ---------------------------(end of broadcast)---------------------------
> TIP 9: In versions below 8.0, the planner will ignore your desire to
> choose an index scan if your joining column's datatypes do not
> match

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073