Re: Revised Patch to allow multiple table locks in "Unison"
От | Bruce Momjian |
---|---|
Тема | Re: Revised Patch to allow multiple table locks in "Unison" |
Дата | |
Msg-id | 200108042203.f74M3Vx26380@candle.pha.pa.us обсуждение исходный текст |
Ответ на | Re: Revised Patch to allow multiple table locks in "Unison" (Tom Lane <tgl@sss.pgh.pa.us>) |
Список | pgsql-patches |
I have backed out the entire patch. I think it is best for the author to make the suggested changes and resubmit. Reversed patch attached. > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Patch applied. > > Idly looking this over again, I noticed a big OOPS: > > >> ! freeList(lockstmt->rellist); > > >> ! pfree(relname); > > >> ! pfree(relname); > > It is most definitely NOT the executor's business to release pieces of > the querytree; this will certainly break plpgsql functions, for example, > where the same querytree is executed repeatedly. > Bruce, please remove those lines. > > Another thing I am concerned about now that I look more closely is that > the multi-rel case code opens the relations without any lock, and then > assumes they'll stick around while it opens and access-checks the rest. > This will fail if someone else drops one of the rels meanwhile. I think > the entire routine should be reduced to a simple loop that opens, locks, > and closes the rels one at a time. The extra code bulk to do it this > way isn't buying us anything at all. > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026 Tom Lane wrote: > > Neil Padgett <npadgett@redhat.com> writes: > >> This seems fine to me (and in fact I thought we'd already agreed to it). > > > Here's the patch then. =) > > [ quick mental checklist... ] You forgot the documentation updates. > Otherwise it looks good. Ok -- I made a go at the documentation. I'm no SGML wizard, so if someone could check my changes that would be helpful. Neil -- Neil Padgett Red Hat Canada Ltd. E-Mail: npadgett@redhat.com 2323 Yonge Street, Suite #300, Toronto, ON M4P 2C9 Index: doc/src/sgml/ref/lock.sgml =================================================================== RCS file: /home/projects/pgsql/cvsroot/pgsql/doc/src/sgml/ref/lock.sgml,v retrieving revision 1.24 diff -c -p -r1.24 lock.sgml *** doc/src/sgml/ref/lock.sgml 2001/07/09 22:18:33 1.24 --- doc/src/sgml/ref/lock.sgml 2001/08/02 21:39:42 *************** Postgres documentation *** 15,21 **** LOCK </refname> <refpurpose> ! Explicitly lock a table inside a transaction </refpurpose> </refnamediv> <refsynopsisdiv> --- 15,21 ---- LOCK </refname> <refpurpose> ! Explicitly lock a table / tables inside a transaction </refpurpose> </refnamediv> <refsynopsisdiv> *************** Postgres documentation *** 23,30 **** <date>2001-07-09</date> </refsynopsisdivinfo> <synopsis> ! LOCK [ TABLE ] <replaceable class="PARAMETER">name</replaceable> ! LOCK [ TABLE ] <replaceable class="PARAMETER">name</replaceable> IN <replaceable class="PARAMETER">lockmode</replaceable>MODE where <replaceable class="PARAMETER">lockmode</replaceable> is one of: --- 23,30 ---- <date>2001-07-09</date> </refsynopsisdivinfo> <synopsis> ! LOCK [ TABLE ] <replaceable class="PARAMETER">name</replaceable> [,...] ! LOCK [ TABLE ] <replaceable class="PARAMETER">name</replaceable> [,...] IN <replaceable class="PARAMETER">lockmode</replaceable>MODE where <replaceable class="PARAMETER">lockmode</replaceable> is one of: *************** ERROR <replaceable class="PARAMETER">nam *** 373,378 **** --- 373,379 ---- An example for this rule was given previously when discussing the use of SHARE ROW EXCLUSIVE mode rather than SHARE mode. </para> + </listitem> </itemizedlist> *************** ERROR <replaceable class="PARAMETER">nam *** 383,388 **** --- 384,395 ---- </para> </note> + <para> + When locking multiple tables, the command LOCK a, b; is equivalent to LOCK + a; LOCK b;. The tables are locked one-by-one in the order specified in the + <command>LOCK</command> command. + </para> + <refsect2 id="R2-SQL-LOCK-3"> <refsect2info> <date>1999-06-08</date> *************** ERROR <replaceable class="PARAMETER">nam *** 406,411 **** --- 413,419 ---- <para> <command>LOCK</command> works only inside transactions. </para> + </refsect2> </refsect1> Index: src/backend/commands/command.c =================================================================== RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/commands/command.c,v retrieving revision 1.136 diff -c -p -r1.136 command.c *** src/backend/commands/command.c 2001/07/16 05:06:57 1.136 --- src/backend/commands/command.c 2001/08/02 21:39:43 *************** needs_toast_table(Relation rel) *** 1984,1991 **** MAXALIGN(data_length); return (tuple_length > TOAST_TUPLE_THRESHOLD); } ! ! /* * * LOCK TABLE --- 1984,1990 ---- MAXALIGN(data_length); return (tuple_length > TOAST_TUPLE_THRESHOLD); } ! /* * * LOCK TABLE *************** needs_toast_table(Relation rel) *** 1994,2019 **** void LockTableCommand(LockStmt *lockstmt) { ! Relation rel; ! int aclresult; ! rel = heap_openr(lockstmt->relname, NoLock); ! if (rel->rd_rel->relkind != RELKIND_RELATION) ! elog(ERROR, "LOCK TABLE: %s is not a table", lockstmt->relname); ! if (lockstmt->mode == AccessShareLock) ! aclresult = pg_aclcheck(lockstmt->relname, GetUserId(), ACL_SELECT); ! else ! aclresult = pg_aclcheck(lockstmt->relname, GetUserId(), ! ACL_UPDATE | ACL_DELETE); ! if (aclresult != ACLCHECK_OK) ! elog(ERROR, "LOCK TABLE: permission denied"); ! LockRelation(rel, lockstmt->mode); ! heap_close(rel, NoLock); /* close rel, keep lock */ } --- 1993,2096 ---- void LockTableCommand(LockStmt *lockstmt) { ! int relCnt; ! ! relCnt = length(lockstmt -> rellist); ! ! /* Handle a single relation lock specially to avoid overhead on likely the ! most common case */ ! ! if(relCnt == 1) ! { ! ! /* Locking a single table */ ! ! Relation rel; ! int aclresult; ! char *relname; ! ! relname = strVal(lfirst(lockstmt->rellist)); ! ! freeList(lockstmt->rellist); ! ! rel = heap_openr(relname, NoLock); ! ! if (rel->rd_rel->relkind != RELKIND_RELATION) ! elog(ERROR, "LOCK TABLE: %s is not a table", relname); ! ! if (lockstmt->mode == AccessShareLock) ! aclresult = pg_aclcheck(relname, GetUserId(), ! ACL_SELECT); ! else ! aclresult = pg_aclcheck(relname, GetUserId(), ! ACL_UPDATE | ACL_DELETE); ! ! if (aclresult != ACLCHECK_OK) ! elog(ERROR, "LOCK TABLE: permission denied"); ! ! LockRelation(rel, lockstmt->mode); ! ! pfree(relname); ! ! heap_close(rel, NoLock); /* close rel, keep lock */ ! } ! else ! { ! List *p; ! Relation *RelationArray; ! Relation *pRel; ! ! /* Locking multiple tables */ ! ! /* Create an array of relations */ ! ! RelationArray = palloc(relCnt * sizeof(Relation)); ! pRel = RelationArray; ! ! /* Iterate over the list and populate the relation array */ ! ! foreach(p, lockstmt->rellist) ! { ! char* relname = strVal(lfirst(p)); ! int aclresult; ! ! *pRel = heap_openr(relname, NoLock); ! ! if ((*pRel)->rd_rel->relkind != RELKIND_RELATION) ! elog(ERROR, "LOCK TABLE: %s is not a table", ! relname); ! ! if (lockstmt->mode == AccessShareLock) ! aclresult = pg_aclcheck(relname, GetUserId(), ! ACL_SELECT); ! else ! aclresult = pg_aclcheck(relname, GetUserId(), ! ACL_UPDATE | ACL_DELETE); ! ! if (aclresult != ACLCHECK_OK) ! elog(ERROR, "LOCK TABLE: permission denied"); ! pRel++; ! pfree(relname); ! } ! /* Now, lock all the relations, closing each after it is locked ! (Keeping the locks) ! */ ! for(pRel = RelationArray; ! pRel < RelationArray + relCnt; ! pRel++) ! { ! LockRelation(*pRel, lockstmt->mode); ! heap_close(*pRel, NoLock); ! } ! /* Free the relation array */ ! pfree(RelationArray); ! } } Index: src/backend/nodes/copyfuncs.c =================================================================== RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v retrieving revision 1.148 diff -c -p -r1.148 copyfuncs.c *** src/backend/nodes/copyfuncs.c 2001/07/16 19:07:37 1.148 --- src/backend/nodes/copyfuncs.c 2001/08/02 21:39:44 *************** _copyLockStmt(LockStmt *from) *** 2425,2432 **** { LockStmt *newnode = makeNode(LockStmt); ! if (from->relname) ! newnode->relname = pstrdup(from->relname); newnode->mode = from->mode; return newnode; --- 2425,2432 ---- { LockStmt *newnode = makeNode(LockStmt); ! Node_Copy(from, newnode, rellist); ! newnode->mode = from->mode; return newnode; Index: src/backend/nodes/equalfuncs.c =================================================================== RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/nodes/equalfuncs.c,v retrieving revision 1.96 diff -c -p -r1.96 equalfuncs.c *** src/backend/nodes/equalfuncs.c 2001/07/16 19:07:38 1.96 --- src/backend/nodes/equalfuncs.c 2001/08/02 21:39:44 *************** _equalDropUserStmt(DropUserStmt *a, Drop *** 1283,1289 **** static bool _equalLockStmt(LockStmt *a, LockStmt *b) { ! if (!equalstr(a->relname, b->relname)) return false; if (a->mode != b->mode) return false; --- 1283,1289 ---- static bool _equalLockStmt(LockStmt *a, LockStmt *b) { ! if (!equal(a->rellist, b->rellist)) return false; if (a->mode != b->mode) return false; Index: src/backend/parser/gram.y =================================================================== RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/parser/gram.y,v retrieving revision 2.238 diff -c -p -r2.238 gram.y *** src/backend/parser/gram.y 2001/07/16 19:07:40 2.238 --- src/backend/parser/gram.y 2001/08/02 21:39:46 *************** DeleteStmt: DELETE FROM relation_expr w *** 3280,3290 **** } ; ! LockStmt: LOCK_P opt_table relation_name opt_lock { LockStmt *n = makeNode(LockStmt); ! n->relname = $3; n->mode = $4; $$ = (Node *)n; } --- 3280,3290 ---- } ; ! LockStmt: LOCK_P opt_table relation_name_list opt_lock { LockStmt *n = makeNode(LockStmt); ! n->rellist = $3; n->mode = $4; $$ = (Node *)n; } Index: src/include/nodes/parsenodes.h =================================================================== RCS file: /home/projects/pgsql/cvsroot/pgsql/src/include/nodes/parsenodes.h,v retrieving revision 1.136 diff -c -p -r1.136 parsenodes.h *** src/include/nodes/parsenodes.h 2001/07/16 19:07:40 1.136 --- src/include/nodes/parsenodes.h 2001/08/02 21:39:46 *************** typedef struct VariableResetStmt *** 760,766 **** typedef struct LockStmt { NodeTag type; ! char *relname; /* relation to lock */ int mode; /* lock mode */ } LockStmt; --- 760,766 ---- typedef struct LockStmt { NodeTag type; ! List *rellist; /* relations to lock */ int mode; /* lock mode */ } LockStmt; Index: src/interfaces/ecpg/preproc/preproc.y =================================================================== RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/ecpg/preproc/preproc.y,v retrieving revision 1.146 diff -c -p -r1.146 preproc.y *** src/interfaces/ecpg/preproc/preproc.y 2001/07/16 05:07:00 1.146 --- src/interfaces/ecpg/preproc/preproc.y 2001/08/02 21:39:49 *************** DeleteStmt: DELETE FROM relation_expr w *** 2421,2427 **** } ; ! LockStmt: LOCK_P opt_table relation_name opt_lock { $$ = cat_str(4, make_str("lock"), $2, $3, $4); } --- 2421,2427 ---- } ; ! LockStmt: LOCK_P opt_table relation_name_list opt_lock { $$ = cat_str(4, make_str("lock"), $2, $3, $4); }
В списке pgsql-patches по дате отправления:
Предыдущее
От: Tom LaneДата:
Сообщение: Re: Revised Patch to allow multiple table locks in "Unison"
Следующее
От: "Serguei Mokhov"Дата:
Сообщение: Re: libpq.pot -> ru.po : Russian - Cyrillic - KOI8-R