Transaction oddity with list partition of a list partition

Started by David Fetterabout 9 years ago4 messages
#1David Fetter
david@fetter.org
1 attachment(s)

Folks,

I'm having some trouble understanding what's going on here. When I \i
the file in 55caaaeba877eac1feb6481fb413fa04ae9046ac without starting
a transaction explicitly, it produces the expected results. When I \i
it after a BEGIN, not so much.

What's going on?

Best,
David.

shackle@shackle=# BEGIN;
BEGIN
shackle@shackle=# \i ten_plus.sql
CREATE TABLE
CREATE TABLE
CREATE TABLE
CREATE FUNCTION
CREATE TABLE
CREATE TRIGGER
psql:ten_plus.sql:66: ERROR: no partition of relation "the_log" found for row
DETAIL: Failing row contains (2016-12-15 00:17:46.579357-08, shackle, INSERT, public, city, null, {"id": 1, "name": "Oakland", "population": 419267}).
CONTEXT: SQL statement "INSERT INTO the_log(
action,
table_schema,
table_name,
old_row,
new_row)
VALUES (
TG_OP,
TG_TABLE_SCHEMA,
TG_TABLE_NAME,
CASE TG_OP WHEN 'INSERT' THEN NULL ELSE row_to_json(OLD)::jsonb END,
CASE TG_OP WHEN 'DELETE' THEN NULL ELSE row_to_json(NEW)::jsonb END
)"
PL/pgSQL function log_change() line 3 at SQL statement
shackle@shackle=# ROLLBACK;

--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Attachments:

ten_plus.sqlapplication/sqlDownload
#2David Fetter
david@fetter.org
In reply to: David Fetter (#1)
Re: Transaction oddity with list partition of a list partition

On Thu, Dec 15, 2016 at 12:23:24AM -0800, David Fetter wrote:

Folks,

I'm having some trouble understanding what's going on here. When I \i
the file in 55caaaeba877eac1feb6481fb413fa04ae9046ac without starting
a transaction explicitly, it produces the expected results. When I \i
it after a BEGIN, not so much.

I've managed to get a shorter repro for the issue:

BEGIN;
CREATE TABLE the_log (
ts TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP,
"user" TEXT NOT NULL DEFAULT current_user,
action TEXT NOT NULL,
table_schema TEXT NOT NULL,
table_name TEXT NOT NULL,
old_row JSONB,
new_row JSONB,
CHECK(
CASE action
WHEN 'INSERT' THEN old_row IS NULL AND new_row IS NOT NULL
WHEN 'UPDATE' THEN old_row IS NOT NULL AND new_row IS NOT NULL
ELSE /*DELETE, and maybe TRUNCATE, if that's supported by access to old rows */
old_row IS NOT NULL AND new_row IS NULL
END
)
) PARTITION BY LIST(table_schema);
CREATE TABLE public_log
PARTITION OF the_log FOR VALUES IN ('public');
INSERT INTO the_log (action, table_schema, table_name, new_row)
VALUES ('INSERT','public','city','{"name": "Oakland", "population": 419267}');

leads to:

ERROR: no partition of relation "the_log" found for row
DETAIL: Failing row contains (2016-12-15 00:59:17.980094-08, shackle, INSERT, public, city, null, {"name": "Oakland", "population": 419267}).

Per Thomas Munro, could it be that the CREATE ... PARTITION OF ... code
fails to run CacheInvalidateRelcache on its parent(s)?

Best,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: David Fetter (#2)
1 attachment(s)
Re: Transaction oddity with list partition of a list partition

Hi David,

On 2016/12/15 18:09, David Fetter wrote:

Per Thomas Munro, could it be that the CREATE ... PARTITION OF ... code
fails to run CacheInvalidateRelcache on its parent(s)?

Thomas's right. There is a patch posted for this issue [1]/messages/by-id/CA+TgmoZ86v1G+zx9etMiSQaBBvYMKfU-iitqZArSh5z0n8Q4cA@mail.gmail.com; I'm sending
an updated version of the patch later today in reply to [1]/messages/by-id/CA+TgmoZ86v1G+zx9etMiSQaBBvYMKfU-iitqZArSh5z0n8Q4cA@mail.gmail.com. Meanwhile,
could you try and see if the problem is fixed with the attached patch.

Thanks,
Amit

[1]: /messages/by-id/CA+TgmoZ86v1G+zx9etMiSQaBBvYMKfU-iitqZArSh5z0n8Q4cA@mail.gmail.com
/messages/by-id/CA+TgmoZ86v1G+zx9etMiSQaBBvYMKfU-iitqZArSh5z0n8Q4cA@mail.gmail.com

Attachments:

0001-Invalidate-the-parent-s-relcache-after-partition-cre.patchtext/x-diff; name=0001-Invalidate-the-parent-s-relcache-after-partition-cre.patchDownload
From 4553aa4588a3d18aba3d0aa8d07627ff8654f436 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Tue, 13 Dec 2016 15:07:06 +0900
Subject: [PATCH 1/6] Invalidate the parent's relcache after partition
 creation.

---
 src/backend/catalog/heap.c       |  7 ++++++-
 src/backend/commands/tablecmds.c | 13 ++++---------
 src/include/catalog/heap.h       |  2 +-
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index c09c9f28a7..e5d6aecc3f 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -3230,9 +3230,12 @@ RemovePartitionKeyByRelId(Oid relid)
  * StorePartitionBound
  *		Update pg_class tuple of rel to store the partition bound and set
  *		relispartition to true
+ *
+ * Also, invalidate the parent's relcache, so that the next rebuild will load
+ * the new partition's info into its partition descriptor.
  */
 void
-StorePartitionBound(Relation rel, Node *bound)
+StorePartitionBound(Relation rel, Relation parent, Node *bound)
 {
 	Relation	classRel;
 	HeapTuple	tuple,
@@ -3273,4 +3276,6 @@ StorePartitionBound(Relation rel, Node *bound)
 	CatalogUpdateIndexes(classRel, newtuple);
 	heap_freetuple(newtuple);
 	heap_close(classRel, RowExclusiveLock);
+
+	CacheInvalidateRelcache(parent);
 }
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 7a574dc50d..1c219b03dd 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -777,10 +777,11 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 		 * it does not return on error.
 		 */
 		check_new_partition_bound(relname, parent, bound);
-		heap_close(parent, NoLock);
 
 		/* Update the pg_class entry. */
-		StorePartitionBound(rel, bound);
+		StorePartitionBound(rel, parent, bound);
+
+		heap_close(parent, NoLock);
 
 		/*
 		 * The code that follows may also update the pg_class tuple to update
@@ -13141,7 +13142,7 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
 							  cmd->bound);
 
 	/* Update the pg_class entry. */
-	StorePartitionBound(attachRel, cmd->bound);
+	StorePartitionBound(attachRel, rel, cmd->bound);
 
 	/*
 	 * Generate partition constraint from the partition bound specification.
@@ -13352,12 +13353,6 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
 		}
 	}
 
-	/*
-	 * Invalidate the parent's relcache so that the new partition is now
-	 * included its partition descriptor.
-	 */
-	CacheInvalidateRelcache(rel);
-
 	ObjectAddressSet(address, RelationRelationId, RelationGetRelid(attachRel));
 
 	/* keep our lock until commit */
diff --git a/src/include/catalog/heap.h b/src/include/catalog/heap.h
index 77dc1983e8..0e4262f611 100644
--- a/src/include/catalog/heap.h
+++ b/src/include/catalog/heap.h
@@ -143,6 +143,6 @@ extern void StorePartitionKey(Relation rel,
 					Oid *partopclass,
 					Oid *partcollation);
 extern void RemovePartitionKeyByRelId(Oid relid);
-extern void StorePartitionBound(Relation rel, Node *bound);
+extern void StorePartitionBound(Relation rel, Relation parent, Node *bound);
 
 #endif   /* HEAP_H */
-- 
2.11.0

#4David Fetter
david@fetter.org
In reply to: Amit Langote (#3)
Re: Transaction oddity with list partition of a list partition

On Thu, Dec 15, 2016 at 06:20:04PM +0900, Amit Langote wrote:

Hi David,

On 2016/12/15 18:09, David Fetter wrote:

Per Thomas Munro, could it be that the CREATE ... PARTITION OF ...
code fails to run CacheInvalidateRelcache on its parent(s)?

Thomas's right. There is a patch posted for this issue [1]; I'm
sending an updated version of the patch later today in reply to [1].
Meanwhile, could you try and see if the problem is fixed with the
attached patch.

That fixed both cases. Thanks!

Best,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers