Re: [HACKERS] Transaction oddity with list partition of a list partition

[David Fetter]

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  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

Re: [HACKERS] Transaction oddity with list partition of a list partition

[Amit Langote]

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.

Thanks,
Amit

[1]
https://www.postgresql.org/message-id/CA%2BTgmoZ86v1G%2Bzx9etMiSQaBBvYMKfU-iitqZArSh5z0n8Q4cA%40mail.gmail.com
>From 4553aa4588a3d18aba3d0aa8d07627ff8654f436 Mon Sep 17 00:00:00 2001
From: amit 
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


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

Re: [HACKERS] Transaction oddity with list partition of a list partition

[David Fetter]

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  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

[HACKERS] Transaction oddity with list partition of a list partition

[David Fetter]

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  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


ten_plus.sql
Description: application/sql

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