Errors creating partitioned tables from existing using (LIKE <table>) after renaming table constraints
Dear team,
I am using PostGreSQL 11.1, I compiled from source on openSuSE Tumbleweed.
ousa_new=# SELECT version() ; version ---------------------------------------------------------------------------------------------------------------------------- PostgreSQL 11.1 on x86_64-pc-linux-gnu, compiled by gcc (SUSE Linux) 8.2.1 20181025 [gcc-8-branch revision 265488], 64-bit
I imported a table definition from another database using:
pg_dump -d ousa -t knowledge_vectors -s | psql ousa_new
The table inherits from a hierarchy and I imported those parent tables first. I want to test partitioning so in the new db, I renamed the table and all its indexes and constraints so I can create a new table partitioned table based on this design using (like <table>).
ousa_new=# \d+ knowledge_vectors_old
Table "public.knowledge_vectors_old"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
---------------+-----------------------------+-----------+----------+--------------------------------+----------+--------------+-------------
entry_date | timestamp(3) with time zone | | not null | now() | plain | |
revision_date | timestamp(3) with time zone | | | NULL::timestamp with time zone | plain | |
entered_by | text | | not null | "current_user"() | extended | |
revised_by | text | | | ''::text | extended | |
source_id | bigint | | | | plain | |
object_id | bigint | | not null | | plain | |
description | text | | | ''::text | extended | |
vectors | tsvector | | not null | | extended | |
Indexes:
"knowledgevectorsold_pk" PRIMARY KEY, btree (object_id), tablespace "pgindex"
"knowledgevectorsold_vector_idx" gin (vectors), tablespace "pgindex"
Check constraints:
"knowledgevectors_vectors_ck" CHECK (vectors <> ''::tsvector)
Foreign-key constraints:
"knowledgevectorsold_objectid_fk" FOREIGN KEY (object_id) REFERENCES knowledge(object_id)
Triggers:
knowledgevectors_revision_tr BEFORE UPDATE ON knowledge_vectors_old FOR EACH ROW EXECUTE PROCEDURE revised()
Inherits: ousa_objects
ousa_new=# alter table knowledge_vectors_old rename constraint knowledgevectors_vectors_ck to knowledgevectorsold_vectors_ck ;
ALTER TABLE
ousa_new=# \d+ knowledge_vectors_old
Table "public.knowledge_vectors_old"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
---------------+-----------------------------+-----------+----------+--------------------------------+----------+--------------+-------------
entry_date | timestamp(3) with time zone | | not null | now() | plain | |
revision_date | timestamp(3) with time zone | | | NULL::timestamp with time zone | plain | |
entered_by | text | | not null | "current_user"() | extended | |
revised_by | text | | | ''::text | extended | |
source_id | bigint | | | | plain | |
object_id | bigint | | not null | | plain | |
description | text | | | ''::text | extended | |
vectors | tsvector | | not null | | extended | |
Indexes:
"knowledgevectorsold_pk" PRIMARY KEY, btree (object_id), tablespace "pgindex"
"knowledgevectorsold_vector_idx" gin (vectors), tablespace "pgindex"
Check constraints:
"knowledgevectorsold_vectors_ck" CHECK (vectors <> ''::tsvector)
Foreign-key constraints:
"knowledgevectorsold_objectid_fk" FOREIGN KEY (object_id) REFERENCES knowledge(object_id)
Triggers:
knowledgevectors_revision_tr BEFORE UPDATE ON knowledge_vectors_old FOR EACH ROW EXECUTE PROCEDURE revised()
Inherits: ousa_objects
On my attempt to create the new partitioned table using like, I get error that the constraint by the old name doesn't exist:
ousa_new=# create table knowledge_vectors (like knowledge_vectors_old INCLUDING ALL ) PARTITION BY RANGE ( object_id ) ;
ERROR: constraint "knowledgevectors_vectors_ck" for table "knowledge_vectors_old" does not exist
Only after I dropped the new constraint and recreated it, did the create table (like <table>) work.
Ousa_new=# alter table knowledge_vectors_old drop constraint knowledgevectorsold_vectors_ck ;
ALTER TABLE
ousa_new=# alter table knowledge_vectors_old add constraint knowledgevectorsold_vectors_ck CHECK (vectors <> ''::tsvector) ;
ALTER TABLE
ousa_new=# \d+ knowledge_vectors_old
Table "public.knowledge_vectors_old"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
---------------+-----------------------------+-----------+----------+--------------------------------+----------+--------------+-------------
entry_date | timestamp(3) with time zone | | not null | now() | plain | |
revision_date | timestamp(3) with time zone | | | NULL::timestamp with time zone | plain | |
entered_by | text | | not null | "current_user"() | extended | |
revised_by | text | | | ''::text | extended | |
source_id | bigint | | | | plain | |
object_id | bigint | | not null | | plain | |
description | text | | | ''::text | extended | |
vectors | tsvector | | not null | | extended | |
Indexes:
"knowledgevectorsold_pk" PRIMARY KEY, btree (object_id), tablespace "pgindex"
"knowledgevectorsold_vector_idx" gin (vectors), tablespace "pgindex"
Check constraints:
"knowledgevectorsold_vectors_ck" CHECK (vectors <> ''::tsvector)
Foreign-key constraints:
"knowledgevectorsold_objectid_fk" FOREIGN KEY (object_id) REFERENCES knowledge(object_id)
Triggers:
knowledgevectors_revision_tr BEFORE UPDATE ON knowledge_vectors_old FOR EACH ROW EXECUTE PROCEDURE revised()
Inherits: ousa_objects
ousa_new=# create table knowledge_vectors (like knowledge_vectors_old INCLUDING ALL ) PARTITION BY RANGE ( object_id ) ;
CREATE TABLE
ousa_new=# \d+ knowledge_vectors
Table "public.knowledge_vectors"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
---------------+-----------------------------+-----------+----------+--------------------------------+----------+--------------+-------------
entry_date | timestamp(3) with time zone | | not null | now() | plain | |
revision_date | timestamp(3) with time zone | | | NULL::timestamp with time zone | plain | |
entered_by | text | | not null | "current_user"() | extended | |
revised_by | text | | | ''::text | extended | |
source_id | bigint | | | | plain | |
object_id | bigint | | not null | | plain | |
description | text | | | ''::text | extended | |
vectors | tsvector | | not null | | extended | |
Partition key: RANGE (object_id)
Indexes:
"knowledge_vectors_pkey" PRIMARY KEY, btree (object_id), tablespace "pgindex"
"knowledge_vectors_vectors_idx" gin (vectors), tablespace "pgindex"
Check constraints:
"knowledgevectorsold_vectors_ck" CHECK (vectors <> ''::tsvector)
Number of partitions: 0
The original table is:
ousa# \d+ knowledge_vectors
Table "public.knowledge_vectors"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
---------------+-----------------------------+-----------+----------+--------------------------------+----------+--------------+-------------
entry_date | timestamp(3) with time zone | | not null | now() | plain | |
revision_date | timestamp(3) with time zone | | | NULL::timestamp with time zone | plain | |
entered_by | text | | not null | "current_user"() | extended | |
revised_by | text | | | ''::text | extended | |
source_id | bigint | | | | plain | |
object_id | bigint | | not null | | plain | |
description | text | | | ''::text | extended | |
vectors | tsvector | | not null | | extended | |
Indexes:
"knowledgevectors_pk" PRIMARY KEY, btree (object_id), tablespace "pgindex"
"knowledgevectors_vector_idx" gin (vectors), tablespace "pgindex"
Check constraints:
"knowledgevectors_vectors_ck" CHECK (vectors <> ''::tsvector)
Triggers:
knowledgevectors_revision_tr BEFORE UPDATE ON knowledge_vectors FOR EACH ROW EXECUTE PROCEDURE revised()
Inherits: ousa_objects
Hi,
On 2018/12/13 5:00, Stuart wrote:
ousa_new=# \d+ knowledge_vectors_old
Table "public.knowledge_vectors_old"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
---------------+-----------------------------+-----------+----------+--------------------------------+----------+--------------+-------------
entry_date | timestamp(3) with time zone | | not null | now() | plain | |
revision_date | timestamp(3) with time zone | | | NULL::timestamp with time zone | plain | |
entered_by | text | | not null | "current_user"() | extended | |
revised_by | text | | | ''::text | extended | |
source_id | bigint | | | | plain | |
object_id | bigint | | not null | | plain | |
description | text | | | ''::text | extended | |
vectors | tsvector | | not null | | extended | |
Indexes:
"knowledgevectorsold_pk" PRIMARY KEY, btree (object_id), tablespace "pgindex"
"knowledgevectorsold_vector_idx" gin (vectors), tablespace "pgindex"
Check constraints:
"knowledgevectors_vectors_ck" CHECK (vectors <> ''::tsvector)
Foreign-key constraints:
"knowledgevectorsold_objectid_fk" FOREIGN KEY (object_id) REFERENCES knowledge(object_id)
Triggers:
knowledgevectors_revision_tr BEFORE UPDATE ON knowledge_vectors_old FOR EACH ROW EXECUTE PROCEDURE revised()
Inherits: ousa_objectsousa_new=# alter table knowledge_vectors_old rename constraint knowledgevectors_vectors_ck to knowledgevectorsold_vectors_ck ;
ALTER TABLE
ousa_new=# create table knowledge_vectors (like knowledge_vectors_old INCLUDING ALL ) PARTITION BY RANGE ( object_id ) ;
ERROR: constraint "knowledgevectors_vectors_ck" for table "knowledge_vectors_old" does not existOnly after I dropped the new constraint and recreated it, did the create table (like <table>) work.
Thanks for the report.
There is a bug here, but it's not of CREATE TABLE. It is rather of ALTER
TABLE RENAME CONSTRAINT, which fails to reflect the changed constraint
name in the target table's relation info cache. Here is another
reproducer of this behavior:
create table foo (a int, constraint check_a check (a > 0));
alter table foo rename CONSTRAINT check_a to check_a_gt_zero;
-- in the same session
create table bar (like foo including all);
ERROR: constraint "check_a" for table "foo" does not exist
What might be worse is that if you specify INCLUDING CONSTRAINTS (not
ALL), it proceeds with creating the constraint with the outdated name:
create table bar (like foo including constraints);
\d bar
Table "public.bar"
Column │ Type │ Collation │ Nullable │ Default
────────┼─────────┼───────────┼──────────┼─────────
a │ integer │ │ │
Check constraints:
"check_a" CHECK (a > 0)
What's happening here is that when the ALTER TABLE RENAME CONSTRAINT is
followed by CREATE TABLE (LIKE .. INCLUDING ALL) in the same session, the
latter is referring to *stale* information about constraints of the source
table. You said it works correctly after you drop and re-create the
constraint, but that's only because ALTER TABLE DROP/ADD CONSTRAINT will
correctly invalidate the cached information, so that subsequent CREATE
TABLE sees the correct information from the updated cache. The way to fix
it is to teach ALTER TABLE RENAME CONSTRAINT to reset the cached information.
Attached a patch which does that. With the patch:
create table foo (a int, constraint check_a check (a > 0));
alter table foo rename CONSTRAINT check_a to check_a_gt_zero;
create table bar (like foo including all);
\d bar
Table "public.bar"
Column │ Type │ Collation │ Nullable │ Default
────────┼─────────┼───────────┼──────────┼─────────
a │ integer │ │ │
Check constraints:
"check_a_gt_zero" CHECK (a > 0)
-- INCLUDING CONSTRAINTS works correctly too
drop table bar;
create table bar (like foo including constraints);
\d bar
Table "public.bar"
Column │ Type │ Collation │ Nullable │ Default
────────┼─────────┼───────────┼──────────┼─────────
a │ integer │ │ │
Check constraints:
"check_a_gt_zero" CHECK (a > 0)
Thanks,
Amit
Attachments:
v1-0001-Make-rename_constraint_internal-invalidate-relcac.patchtext/plain; charset=UTF-8; name=v1-0001-Make-rename_constraint_internal-invalidate-relcac.patchDownload+21-1
On 2018 Zvita 13, China 12:03:35 +03 Amit Langote wrote:
Hi,
On 2018/12/13 5:00, Stuart wrote:
ousa_new=# \d+ knowledge_vectors_old
Table
"public.knowledge_vec
tors_old"Column | Type | Collation | Nullable |
Default | Storage | Stats target | Description>
---------------+-----------------------------+-----------+----------+-----
---------------------------+----------+--------------+------------->
entry_date | timestamp(3) with time zone | | not null |
now() | plain | | revision_date
| timestamp(3) with time zone | | | NULL::timestamp
with time zone | plain | | entered_by | text
| | not null | "current_user"() |
extended | | revised_by | text |
| | ''::text | extended |
| source_id | bigint | |
| | plain | | object_id
| bigint | | not null |
| plain | | description | text
| | | ''::text
| extended | | vectors | tsvector
| | not null | | extended |
|>
Indexes:
"knowledgevectorsold_pk" PRIMARY KEY, btree (object_id), tablespace
"pgindex" "knowledgevectorsold_vector_idx" gin (vectors), tablespace
"pgindex">
Check constraints:
"knowledgevectors_vectors_ck" CHECK (vectors <> ''::tsvector)Foreign-key constraints:
"knowledgevectorsold_objectid_fk" FOREIGN KEY (object_id) REFERENCES
knowledge(object_id)>
Triggers:
knowledgevectors_revision_tr BEFORE UPDATE ON knowledge_vectors_old
FOR EACH ROW EXECUTE PROCEDURE revised()>
Inherits: ousa_objectsousa_new=# alter table knowledge_vectors_old rename constraint
knowledgevectors_vectors_ck to knowledgevectorsold_vectors_ck ; ALTER
TABLEousa_new=# create table knowledge_vectors (like knowledge_vectors_old
INCLUDING ALL ) PARTITION BY RANGE ( object_id ) ; ERROR: constraint
"knowledgevectors_vectors_ck" for table "knowledge_vectors_old" does not
existOnly after I dropped the new constraint and recreated it, did the create
table (like <table>) work.Thanks for the report.
There is a bug here, but it's not of CREATE TABLE. It is rather of ALTER
TABLE RENAME CONSTRAINT, which fails to reflect the changed constraint
name in the target table's relation info cache. Here is another
reproducer of this behavior:create table foo (a int, constraint check_a check (a > 0));
alter table foo rename CONSTRAINT check_a to check_a_gt_zero;
-- in the same session
create table bar (like foo including all);
ERROR: constraint "check_a" for table "foo" does not existWhat might be worse is that if you specify INCLUDING CONSTRAINTS (not
ALL), it proceeds with creating the constraint with the outdated name:create table bar (like foo including constraints);
\d bar
Table "public.bar"
Column │ Type │ Collation │ Nullable │ Default
────────┼─────────┼───────────┼──────────┼─────────
a │ integer │ │ │
Check constraints:
"check_a" CHECK (a > 0)What's happening here is that when the ALTER TABLE RENAME CONSTRAINT is
followed by CREATE TABLE (LIKE .. INCLUDING ALL) in the same session, the
latter is referring to *stale* information about constraints of the source
table. You said it works correctly after you drop and re-create the
constraint, but that's only because ALTER TABLE DROP/ADD CONSTRAINT will
correctly invalidate the cached information, so that subsequent CREATE
TABLE sees the correct information from the updated cache. The way to fix
it is to teach ALTER TABLE RENAME CONSTRAINT to reset the cached
information.Attached a patch which does that. With the patch:
create table foo (a int, constraint check_a check (a > 0));
alter table foo rename CONSTRAINT check_a to check_a_gt_zero;
create table bar (like foo including all);
\d bar
Table "public.bar"
Column │ Type │ Collation │ Nullable │ Default
────────┼─────────┼───────────┼──────────┼─────────
a │ integer │ │ │
Check constraints:
"check_a_gt_zero" CHECK (a > 0)-- INCLUDING CONSTRAINTS works correctly too
drop table bar;
create table bar (like foo including constraints);
\d bar
Table "public.bar"
Column │ Type │ Collation │ Nullable │ Default
────────┼─────────┼───────────┼──────────┼─────────
a │ integer │ │ │
Check constraints:
"check_a_gt_zero" CHECK (a > 0)Thanks,
Amit
Thanks Amit.
On Thu, Dec 13, 2018 at 06:03:35PM +0900, Amit Langote wrote:
What's happening here is that when the ALTER TABLE RENAME CONSTRAINT is
followed by CREATE TABLE (LIKE .. INCLUDING ALL) in the same session, the
latter is referring to *stale* information about constraints of the source
table. You said it works correctly after you drop and re-create the
constraint, but that's only because ALTER TABLE DROP/ADD CONSTRAINT will
correctly invalidate the cached information, so that subsequent CREATE
TABLE sees the correct information from the updated cache. The way to fix
it is to teach ALTER TABLE RENAME CONSTRAINT to reset the cached
information.
This analysis looks right to me, and that's indeed a bug. And as far as
I can see this is reproducible down to 9.4. I cannot check your patch
in details today unfortunately, but I'll try to look at that in the next
couple of days and see if there are any surrounding issues.
--
Michael
On 2018/12/14 9:20, Michael Paquier wrote:
On Thu, Dec 13, 2018 at 06:03:35PM +0900, Amit Langote wrote:
What's happening here is that when the ALTER TABLE RENAME CONSTRAINT is
followed by CREATE TABLE (LIKE .. INCLUDING ALL) in the same session, the
latter is referring to *stale* information about constraints of the source
table. You said it works correctly after you drop and re-create the
constraint, but that's only because ALTER TABLE DROP/ADD CONSTRAINT will
correctly invalidate the cached information, so that subsequent CREATE
TABLE sees the correct information from the updated cache. The way to fix
it is to teach ALTER TABLE RENAME CONSTRAINT to reset the cached
information.This analysis looks right to me, and that's indeed a bug. And as far as
I can see this is reproducible down to 9.4. I cannot check your patch
in details today unfortunately, but I'll try to look at that in the next
couple of days and see if there are any surrounding issues.
Thank you for looking. I noticed that the previously posted patch doesn't
apply as is to branches before 11, so here are the patches that apply to
9.4 to 10 branches.
Regards,
Amit
Attachments:
pg10-0001-Make-rename_constraint_internal-invalidate-relcache.patchtext/plain; charset=UTF-8; name=pg10-0001-Make-rename_constraint_internal-invalidate-relcache.patchDownload+21-1
pg94-0001-Make-rename_constraint_internal-invalidate-relcache.patchtext/plain; charset=UTF-8; name=pg94-0001-Make-rename_constraint_internal-invalidate-relcache.patchDownload+21-1
pg95-0001-Make-rename_constraint_internal-invalidate-relcache.patchtext/plain; charset=UTF-8; name=pg95-0001-Make-rename_constraint_internal-invalidate-relcache.patchDownload+21-1
pg96-0001-Make-rename_constraint_internal-invalidate-relcache.patchtext/plain; charset=UTF-8; name=pg96-0001-Make-rename_constraint_internal-invalidate-relcache.patchDownload+21-1
On Fri, Dec 14, 2018 at 11:19:16AM +0900, Amit Langote wrote:
Thank you for looking. I noticed that the previously posted patch doesn't
apply as is to branches before 11, so here are the patches that apply to
9.4 to 10 branches.
When renaming an attribute, renameatt_internal is in charge of
invalidating the cache by using CacheInvalidateHeapTuple when running
the catalog update in CatalogTupleUpdate.
For constraint renames, we may finish by using RenameConstraintById or
RenameRelationInternal. And here the constraint is removed by OID where
a invalidation happens for the constraint, but not its parent relation.
In short I agree with your fix and its position, still I think that we
should add a comment why the invalidation needs to happen, with
something simple, like "Invalidate relcache so as others can see the new
constraint name".
I'll try to get that committed tomorrow and back-patched appropriately.
--
Michael
Hi,
Thanks for reviewing.
On 2018/12/16 16:27, Michael Paquier wrote:
On Fri, Dec 14, 2018 at 11:19:16AM +0900, Amit Langote wrote:
Thank you for looking. I noticed that the previously posted patch doesn't
apply as is to branches before 11, so here are the patches that apply to
9.4 to 10 branches.When renaming an attribute, renameatt_internal is in charge of
invalidating the cache by using CacheInvalidateHeapTuple when running
the catalog update in CatalogTupleUpdate.For constraint renames, we may finish by using RenameConstraintById or
RenameRelationInternal. And here the constraint is removed by OID where
a invalidation happens for the constraint, but not its parent relation.
In short I agree with your fix and its position, still I think that we
should add a comment why the invalidation needs to happen, with
something simple, like "Invalidate relcache so as others can see the new
constraint name".
Yeah, a comment is indeed needed.
I'll try to get that committed tomorrow and back-patched appropriately.
Thank you for adding the comment and committing. :)
Regards,
Amit
On Mon, Dec 17, 2018 at 10:41:07AM +0900, Amit Langote wrote:
On 2018/12/16 16:27, Michael Paquier wrote:
I'll try to get that committed tomorrow and back-patched appropriately.
Thank you for adding the comment and committing. :)
Done just a few moments ago. I have also added a test with a primary
key rename which does not matter as long as LIKE INCLUDE INDEXES is
involved but that felt safer in the long-run.
--
Michael
On Mon, Dec 17, 2018 at 10:51:10AM +0900, Michael Paquier wrote:
Done just a few moments ago. I have also added a test with a primary
key rename which does not matter as long as LIKE INCLUDE INDEXES is
involved but that felt safer in the long-run.
Buildfarm member prion is complaining on that:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=prion&dt=2018-12-17%2001%3A46%3A36
This uses -DRELCACHE_FORCE_RELEASE -DCATCACHE_FORCE_RELEASE.
It seems that the call to relation_close should happen after
CacheInvalidateRelcache. Damn..
--
Michael
On 2018/12/17 11:06, Michael Paquier wrote:
On Mon, Dec 17, 2018 at 10:51:10AM +0900, Michael Paquier wrote:
Done just a few moments ago. I have also added a test with a primary
key rename which does not matter as long as LIKE INCLUDE INDEXES is
involved but that felt safer in the long-run.Buildfarm member prion is complaining on that:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=prion&dt=2018-12-17%2001%3A46%3A36
This uses -DRELCACHE_FORCE_RELEASE -DCATCACHE_FORCE_RELEASE.It seems that the call to relation_close should happen after
CacheInvalidateRelcache. Damn..
What a blunder! Sorry. :(
Thanks,
Amit
On Mon, Dec 17, 2018 at 11:20:14AM +0900, Amit Langote wrote:
On 2018/12/17 11:06, Michael Paquier wrote:
It seems that the call to relation_close should happen after
CacheInvalidateRelcache. Damn..
That was the issue and this is actually exactly like what has been fixed
in 8082bea2.
What a blunder! Sorry. :(
No problem. I have committed a fix after reproducing the failure
manually and checking the fix. This should cool down prion now.
--
Michael
On 2018/12/17 12:52, Michael Paquier wrote:
On Mon, Dec 17, 2018 at 11:20:14AM +0900, Amit Langote wrote:
On 2018/12/17 11:06, Michael Paquier wrote:
It seems that the call to relation_close should happen after
CacheInvalidateRelcache. Damn..That was the issue and this is actually exactly like what has been fixed
in 8082bea2.What a blunder! Sorry. :(
No problem. I have committed a fix after reproducing the failure
manually and checking the fix. This should cool down prion now.
Thank you!
Regards,
Amit