Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexpected behaviour in "pg_attribute" on column drop and recreate #34710

Closed
AlexMesser opened this issue Feb 7, 2019 · 7 comments · Fixed by #33697
Closed

Unexpected behaviour in "pg_attribute" on column drop and recreate #34710

AlexMesser opened this issue Feb 7, 2019 · 7 comments · Fixed by #33697
Assignees

Comments

@AlexMesser
Copy link

AlexMesser commented Feb 7, 2019

Hello. I'm working on CockroachDB implementation in TypeORM and I faced a strange behaviour when I drop and recreate column again. I tested the same steps on postgres and also show postgres behavior.

First, create a table with some columns and constraints:

CREATE TABLE "post" (
"id" INT NOT NULL,
"version" INT NOT NULL,
"name" VARCHAR NOT NULL DEFAULT 'My post',
"text" VARCHAR NOT NULL,
"tag" VARCHAR NOT NULL,
CONSTRAINT "UQ_d7c82163ac258e5d18d52d0fe16" UNIQUE ( "version" ),
CONSTRAINT "UQ_12e79261329cf680e4e4db3cc0d" UNIQUE ( "text", "tag" ),
CONSTRAINT "PK_be5fda3aac270b134ff9c21cdee" PRIMARY KEY ( "id" ))

columns definition in "pg_attribute" is the same in crdb and postgres:

attname attnum
id 1
version 2
name 3
text 4
tag 5

then, drop the "name" column

ALTER TABLE "post" DROP COLUMN "name"

"pg_attribute" result in crdb:

attname attnum
id 1
version 2
text 3
tag 4

and in postgres:

attname attnum
id 1
version 2
........pg.dropped.3........ 3
text 4
tag 5

then, recreate column again:

ALTER TABLE "post" ADD COLUMN "name" VARCHAR

result in crdb:

attname attnum
id 1
version 2
text 3
tag 4
name 5

and in postgres

attname attnum
id 1
version 2
........pg.dropped.3........ 3
text 4
tag 5
name 6

As we see, in crdb columns attnum recalculating after each column deletion.

Why its a problem?

I'm trying to find all table constraints with such query which relies on columns attnum property. It works perfectly in postgres, but wrong in crdb:

SELECT
	"ns"."nspname" "table_schema",
	"t"."relname" "table_name",
	"cnst"."conname" "constraint_name",
	"a"."attnum",
	"cnst"."conkey",
CASE
	"cnst"."contype" 
	WHEN 'x' THEN pg_get_constraintdef ("cnst"."oid", TRUE) ELSE "cnst"."consrc"
	END AS "expression",
CASE
	"cnst"."contype" 
	WHEN 'p' THEN 'PRIMARY' 
	WHEN 'u' THEN 'UNIQUE' 
	WHEN 'c' THEN 'CHECK' 
	END AS "constraint_type",
	"a"."attname" AS "column_name" 
FROM
	"pg_constraint" "cnst"
	INNER JOIN "pg_class" "t" ON "t"."oid" = "cnst"."conrelid"
	INNER JOIN "pg_namespace" "ns" ON "ns"."oid" = "cnst"."connamespace"
	INNER JOIN "pg_attribute" "a" ON "a"."attrelid" = "cnst"."conrelid" AND "a"."attnum" = ANY ( "cnst"."conkey" ) 
WHERE
	"t"."relkind" = 'r' 
	AND "ns"."nspname" = 'public' 
	AND "t"."relname" = 'post' 

result after table creation:

image
here we see an unique constraint named UQ_12e79261329cf680e4e4db3cc0d with columns text and tag presented on 3 and 4 rows. They have attnum attribute with values 4 for text and 5 for tag which are also defined in 'conkey' column as {4, 5}. The name column does not have any constraint and not presented in selection. Its attnum is equal 3.

"name" column dropped:

image
after column was dropped, attnum property values recalculated. text's attnum becomes 3 and tag's - 4. But conkey was't changed and row with text column disappears from selection result.

"name" column recreated:

image
Now column was recreated, attnum values recalculated again and name becomes 5. Now name column involved in composite unique constraint, but WE NEVER created UNIQUE for it

Query result becomes totally wrong and breaks table synchronization. Is it an expected behaviour of attnum and I must look for other ways to find table constraints, or it is a bug ?

I am using CockroachDB version 2.1.4.

@knz
Copy link
Contributor

knz commented Feb 7, 2019 via email

@knz
Copy link
Contributor

knz commented Feb 8, 2019

@AlexMesser while we're at this, so you know we have ran some investigations about what breaks in TypeORM when ran with CockroachDB 2.1. Are you interested in our findings? so as to save you some time.

@knz
Copy link
Contributor

knz commented Feb 8, 2019

The analysis is here: #22298 (comment)

craig bot pushed a commit that referenced this issue Feb 8, 2019
33697: sql: make pg_catalog OIDs stable and refer to the descriptor IDs directly r=knz a=hueypark

Fixes #32940
Fixes #32963
Fixes #34710
Enables #32964

This patch changes the definition of vtables to assign a unique table
ID to every virtual table. Moreover, it also extends vtable
descriptors to assign proper column IDs to virtual table columns. This
aims to support the logical planning code, the table and plan caches,
and simplify+fix the introspection tables. Incidentally it also makes
it possible to use COMMENT ON on virtual tables and their columns too.

The table IDs are picked descending from MaxUint32 although this may
be refined in future PRs to accommodate the numbering of virtual
schema descriptors.

Incidentally `pg_catalog.pg_attribute` is fixed to properly use the
column ID in column `attnum`, so that `attnum` remains stable across
column drops.

Release note (sql change): Virtual tables in `pg_catalog`,
`information_schema` etc now support `COMMENT ON` like regular tables.

Co-authored-by: Jaewan Park <jaewan.huey.park@gmail.com>
@knz
Copy link
Contributor

knz commented Feb 8, 2019

You can pick up the changes in #33697 to move forward with pg_attribute.

@knz
Copy link
Contributor

knz commented Feb 8, 2019

You can alternatively pick #34734 against the release-2.1 branch.

@craig craig bot closed this as completed in #33697 Feb 8, 2019
@AlexMesser
Copy link
Author

hey @knz, how can I get those changes in my locally installed crdb? Do I need to build a binary manually or do you have some automatic build system? Thanks in advice.

@knz
Copy link
Contributor

knz commented Feb 8, 2019

@AlexMesser you can pull a copy of the repo + branch (ensure it's in your $GOPATH) then build/builder.sh mkrelease linux-gnu (or amd64-darwin not sure what you're building for)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants