sql: pg_catalog.pg_constraint broken after swapping PK and altering column type

[Marcuzz]

We're currently working on a rather large database migration where we're migrating ids to uuids in all of our old tables. During this migration process we first encountered #71089, and now we're back with yet another exciting bug for you guys 😂

This bug is actually two different parts:

When creating two simple tables, where one references the other via a FK, and swapping the primary keys around on the references table in order to alter the column type of the id column (first bug: this should be impossible due to the FK) we encounter a bug where selecting from pg_catalog.pg_constraint; completely breaks with the following error: ERROR: column-id "1" does not exist

To Reproduce

1. Set up a fresh CockroachDB cluster
2. Send the following queries:

CREATE TABLE a(id int8 primary key, legacy_id int8 not null);
CREATE TABLE b(a_id int8 references a (id));

BEGIN;
ALTER TABLE a DROP CONSTRAINT "primary";
ALTER TABLE a ADD CONSTRAINT "primary" PRIMARY KEY (legacy_id);
COMMIT;

SET enable_experimental_alter_column_type_general = true;
-- You may have to wait a few seconds for the asynchronous job to complete before running this query:
-- This query should also be impossible because table b.a_id references a.id
ALTER TABLE a ALTER COLUMN id TYPE uuid USING gen_random_uuid();

-- This query will now return an error
SELECT * FROM pg_catalog.pg_constraint;

3. See error:

(1 row)
(error encountered after some results were delivered)
ERROR: column-id "1" does not exist

Expected behavior

For SELECT * FROM pg_catalog.pg_constraint; to not return an error, and for changing the data type on a column that is referenced by a foreign key to be impossible. Could this possibly be related to enable_experimental_alter_column_type_general?

Environment:

• CockroachDB version: 21.1.11
• Server OS: docker image, cockroachdb/cockroach:v21.1.11
• Client app cockroach sql

Additional context
Introspection in tools like DataGrip is now fully broken, it's impossible to select from pg_catalog.pg_constraint and we've also noticed that it is impossible in some circumstances to check if a table has another colum via pg_catalog.pg_tables so there might be more to it.

[blathers-crl]

Hello, I am Blathers. I am here to help you get the issue triaged.

Hoot - a bug! Though bugs are the bane of my existence, rest assured the wretched thing will get the best of care here.

I have CC'd a few people who may be able to assist you:

• @cockroachdb/sql-schema (found keywords: ALTER TABLE)
• @postamar (assigned to sql: unable to alter column type for a column that has (but actually doesn't have) a constraint #71089)
• @cockroachdb/sql-experience (found keywords: pg_)

If we have not gotten back to your issue within a few business days, you can try the following:

• Join our community slack channel and ask on #cockroachdb.
• Try find someone from here if you know they worked closely on the area and CC them.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.}

[ajwerner]

Well, for some sort of good news, we "fixed" this on master. Let me track down why we allowed it in 21.1 and earlier.

ERROR: unimplemented: ALTER COLUMN TYPE requiring rewrite of on-disk data is currently not supported for columns that are part of an index

[ajwerner]

Indeed it seems that the fix for #71089 (#71097) is what prevents this from happening. Let me see if @postamar remembers what is going on here.

[postamar]

This error you're seeing is the direct consequence of ALTER TABLE a ALTER COLUMN id TYPE uuid USING gen_random_uuid(); having run when indeed it shouldn't have been allowed to. I'm looking into why.

[Marcuzz]

This error you're seeing is the direct consequence of ALTER TABLE a ALTER COLUMN id TYPE uuid USING gen_random_uuid(); having run when indeed it shouldn't have been allowed to. I'm looking into why.

What about the following?

CREATE TABLE a(id int8 primary key, legacy_id int8 not null);
CREATE TABLE b(a_id int8 references a (id));

BEGIN;
ALTER TABLE a DROP CONSTRAINT "primary";
ALTER TABLE a ADD CONSTRAINT "primary" PRIMARY KEY (legacy_id);
COMMIT;

ALTER TABLE a DROP COLUMN id;

SELECT * FROM pg_catalog.pg_constraint;

Here we are dropping the column instead of altering the column data type. This also seems to trigger the same behaviour.

Shouldn't it be impossible to drop the primary constraint on id given that it referenced by a foreign key?

I don't know if this is fixed on master, however.

[postamar]

I'm not able to repro the error with the above DROP COLUMN example, I'm afraid.

I'm sorry to say the reason you're seeing this error is that the fix for #71089 hasn't actually been merged to the 21.1 release branch yet: #71166. This is because we usually wait a couple of weeks for a change to "bake" in the master branch before actually backporting it.

[postamar]

That being said, there's something else at play here. I just checked out the branch for my backport and it also allows the ALTER TYPE in there when it shouldn't! As it turns out, ALTER TYPE never actually checks foreign key back-references.

[Marcuzz]

We just grabbed build (CCL v22.1.0-alpha.00000000-481-g0e2e6454a3-dirty @ 2021/10/19 16:14:21 (go1.16.6)) from TeamCity and the following appears to produce the error:

CREATE TABLE a(id int8 primary key, legacy_id int8 not null );
CREATE TABLE b(a_id int8 references a (id));

BEGIN;
ALTER TABLE a DROP CONSTRAINT "a_pkey";
ALTER TABLE a ADD CONSTRAINT "a_pkey" PRIMARY KEY (legacy_id);
COMMIT;

ALTER TABLE a DROP COLUMN id;

SELECT * FROM pg_catalog.pg_constraint;

[postamar]

Thanks, I got it to repro. This proved very useful. The real issue here is that its ALTER TABLE a DROP CONSTRAINT "a_pkey" which should fail, because there's a foreign key referencing a column in the PK, and there's nothing enforcing the uniqueness of the column values once the PK is gone.

Note that this is correctly enforced with unique constraints:

[email protected]:26257/defaultdb> CREATE TABLE a (id INT NOT NULL UNIQUE, legacy_id INT NOT NULL);
CREATE TABLE


Time: 4ms total (execution 3ms / network 0ms)

[email protected]:26257/defaultdb> CREATE TABLE b (a_id INT REFERENCES a (id));
CREATE TABLE


Time: 30ms total (execution 30ms / network 0ms)

[email protected]:26257/defaultdb> ALTER TABLE a DROP COLUMN id;
ERROR: "a_id_key" is referenced by foreign key from table "b"

In the code this means we should be calling something like tryRemoveFKBackReferences when dropping PKs, just like we do when dropping unique constraints.

[ajwerner]

@fqazi do you know what this is about?

[fqazi]

Only recently picked this up from Marius because it was in a similar area to #75155. It's on my backlog for this week, only did a very superficial look in terms of what needs to be done.