On Wed, 2020-05-13 at 19:29 +0530, Amit Kapila wrote:
> > > > > On Fri, May 8, 2020 at 4:58 AM Laurenz Albe 
> > > > > <laurenz.a...@cybertec.at> wrote:
> > > > > > I happened to notice that COPY TO releases the ACCESS SHARE lock
> > > > > > on the table right when the command ends rather than holding it
> > > > > > until the end of the transaction:
> > 
> > Here is a patch.
> > 
> 
> - /*
> - * Close the relation. If reading, we can release the AccessShareLock we
> - * got; if writing, we should hold the lock until end of transaction to
> - * ensure that updates will be committed before lock is released.
> - */
> - if (rel != NULL)
> - table_close(rel, (is_from ? NoLock : AccessShareLock));
> + table_close(rel, NoLock);
> 
> I wonder why you have removed (rel != NULL) check?

That was just unexcusable sloppiness, nothing more.

Here is a fixed patch.

Yours,
Laurenz Albe
From 01923efdcf5546859728a8d4d4d59c76b8a89781 Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.a...@cybertec.at>
Date: Wed, 13 May 2020 22:29:39 +0200
Subject: [PATCH] Make COPY TO keep locks until transaction end

COPY TO released the ACCESS SHARE lock immediately when it
was done rather than holding on to it until the end of the
transaction.

This violates the two-phase lock protocol and breaks MCVV.
For example, a REPEATABLE READ transaction could see an
empty table if it repeats a COPY statement and somebody
truncated the table in the meantime.

Before 4dded12faad the lock was also released after
COPY FROM, but the commit failed to notice the irregularity
in COPY TO.

This is old behavior, but doesn't seem important enough
to backpatch.

Author: Laurenz Albe
Reviewed-by: Robert Haas, Tom Lane, Amit Kapila
Discussion: https://postgr.es/m/7bcfc39d4176faf85ab317d0c26786953646a411.ca...@cybertec.at
---
 src/backend/commands/copy.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index ac07f75bc3..6d53dc463c 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -1081,13 +1081,8 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt,
 		EndCopyTo(cstate);
 	}
 
-	/*
-	 * Close the relation. If reading, we can release the AccessShareLock we
-	 * got; if writing, we should hold the lock until end of transaction to
-	 * ensure that updates will be committed before lock is released.
-	 */
 	if (rel != NULL)
-		table_close(rel, (is_from ? NoLock : AccessShareLock));
+		table_close(rel, NoLock);
 }
 
 /*
-- 
2.21.3

Reply via email to