Skip site navigation (1) Skip section navigation (2)

Re: Revised Patch to allow multiple table locks in "Unison"

From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Neil Padgett <npadgett(at)redhat(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Fernando Nasser <fnasser(at)cygnus(dot)com>, "pgsql-patches(at)postgresql(dot)org" <pgsql-patches(at)postgresql(dot)org>
Subject: Re: Revised Patch to allow multiple table locks in "Unison"
Date: 2001-08-10 14:07:43
Message-ID: 200108101407.f7AE7h104055@candle.pha.pa.us (view raw or flat)
Thread:
Lists: pgsql-hackerspgsql-patches
Patch applied.  Thanks.


> Tom Lane wrote:
> > 
> > Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)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.
> 
> Thanks for having a look, Tom. I've taken you advice into account, and
> I've reworked the patch.
> 
> A new patch is below.
> 
> Neil
> 
> -- 
> Neil Padgett
> Red Hat Canada Ltd.                       E-Mail:  npadgett(at)redhat(dot)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.26
> diff -c -p -r1.26 lock.sgml
> *** doc/src/sgml/ref/lock.sgml	2001/08/04 22:01:38	1.26
> --- doc/src/sgml/ref/lock.sgml	2001/08/07 18:34:23
> *************** 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.138
> diff -c -p -r1.138 command.c
> *** src/backend/commands/command.c	2001/08/04 22:01:38	1.138
> --- src/backend/commands/command.c	2001/08/07 18:34:24
> *************** 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,2030 ----
>   void
>   LockTableCommand(LockStmt *lockstmt)
>   {
> ! 	List *p;
> ! 	Relation rel;
> ! 	
> ! 	/* Iterate over the list and open, lock, and close the relations
> ! 	   one at a time
> ! 	 */
> ! 
> ! 		foreach(p, lockstmt->rellist)
> ! 		{
> ! 			char* relname = strVal(lfirst(p));
> ! 			int			aclresult;
> ! 			
> ! 			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);
> ! 			
> ! 			heap_close(rel, NoLock);	/* close rel, keep lock */
> ! 		}
>   }
>   
>   
> Index: src/backend/nodes/copyfuncs.c
> ===================================================================
> RCS file:
> /home/projects/pgsql/cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v
> retrieving revision 1.150
> diff -c -p -r1.150 copyfuncs.c
> *** src/backend/nodes/copyfuncs.c	2001/08/04 22:01:38	1.150
> --- src/backend/nodes/copyfuncs.c	2001/08/07 18:34:24
> *************** _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.98
> diff -c -p -r1.98 equalfuncs.c
> *** src/backend/nodes/equalfuncs.c	2001/08/04 22:01:38	1.98
> --- src/backend/nodes/equalfuncs.c	2001/08/07 18:34:24
> *************** _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.241
> diff -c -p -r2.241 gram.y
> *** src/backend/parser/gram.y	2001/08/06 05:42:48	2.241
> --- src/backend/parser/gram.y	2001/08/07 18:34:30
> *************** DeleteStmt:  DELETE FROM relation_expr w
> *** 3281,3291 ****
>   				}
>   		;
>   
> ! LockStmt:	LOCK_P opt_table relation_name opt_lock
>   				{
>   					LockStmt *n = makeNode(LockStmt);
>   
> ! 					n->relname = $3;
>   					n->mode = $4;
>   					$$ = (Node *)n;
>   				}
> --- 3281,3291 ----
>   				}
>   		;
>   
> ! 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.138
> diff -c -p -r1.138 parsenodes.h
> *** src/include/nodes/parsenodes.h	2001/08/04 22:01:39	1.138
> --- src/include/nodes/parsenodes.h	2001/08/07 18:34:30
> *************** 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.148
> diff -c -p -r1.148 preproc.y
> *** src/interfaces/ecpg/preproc/preproc.y	2001/08/04 22:01:39	1.148
> --- src/interfaces/ecpg/preproc/preproc.y	2001/08/07 18:34:32
> *************** 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);
>   				}
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org
> 

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman(at)candle(dot)pha(dot)pa(dot)us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

In response to

pgsql-hackers by date

Next:From: Tom LaneDate: 2001-08-10 14:15:32
Subject: Re: PL/pgSQL bug?
Previous:From: Tom LaneDate: 2001-08-10 14:06:54
Subject: Re: PL/pgSQL bug?

pgsql-patches by date

Next:From: Bruce MomjianDate: 2001-08-10 14:41:54
Subject: Re: [PATCHES] JDBC Statement cleanup patch
Previous:From: Bruce MomjianDate: 2001-08-10 12:37:40
Subject: Re: Libpq ssl docs duplicated

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group