On Tue, 2020-05-12 at 11:50 -0400, Tom Lane wrote: > Laurenz Albe <laurenz.a...@cybertec.at> writes: > > On Mon, 2020-05-11 at 15:43 -0400, Robert Haas 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: > > > That seems inconsistent with what an INSERT statement would do, and thus > > > bad. > > Well, should we fix the code or the documentation? > > I'd agree with fixing the code. Early lock release is something we do on > system catalog accesses, and while it hasn't bitten us yet, I've been > kind of expecting that someday it will. We should not do it on SQL-driven > accesses to user tables. > > Having said that, I'd vote for just changing it in HEAD, not > back-patching. It's not clear that there are consequences bad enough > to merit a back-patched behavior change.
Agreed. Here is a patch. Yours, Laurenz Albe
From e75be9e8a649548918e675abc0b28a190d41ab8d Mon Sep 17 00:00:00 2001 From: Laurenz Albe <laurenz.a...@cybertec.at> Date: Tue, 12 May 2020 21:47:40 +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 locking 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 the bug doesn't seem important enough to backpatch. Author: Laurenz Albe Reviewed-by: Robert Haas, Tom Lane Discussion: https://postgr.es/m/7bcfc39d4176faf85ab317d0c26786953646a411.ca...@cybertec.at --- src/backend/commands/copy.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index ac07f75bc3..0cd8820696 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -1081,13 +1081,7 @@ 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
