Here is the revised patch to allow the syntax:

LOCK a,b,c;

It uses the method I described as the "new" method in a previous
message.

Neil

-- 
Neil Padgett
Red Hat Canada Ltd.                       E-Mail:  npadgett@redhat.com
2323 Yonge Street, Suite #300, 
Toronto, ON  M4P 2C9

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/07/26 00:02:24
*************** 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 */
  }
  
  
--- 1994,2170 ----
  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;
! 		Relation *blockingLockTarget;
! 		bool allLocked = false;
! 
! 		/* Locking multiple tables */
! 
! 		/* Create an array of relations */
! 
! 		relationArray = palloc(relCnt * sizeof(Relation));
! 		blockingLockTarget = relationArray;
! 
! 		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 acquire locks on all the relations */
! 
! 		while(!allLocked) 
! 		{
! 
! 			allLocked = true;
! 
! 			/* Lock the blocking lock target (initially the first lock
! 			   in the user's list */
! 
! 			LockRelation(*blockingLockTarget, lockstmt->mode);
! 
! 			/* Lock is now obtained on the lock target, now grab locks in a
! 			   non-blocking way on the rest of the list */
! 			
! 			for(pRel = blockingLockTarget + 1;
! 				pRel < relationArray + relCnt;
! 				pRel++)
! 			{
! 				if(!ConditionalLockRelation(*pRel, lockstmt->mode)) 
! 				{
! 					Relation *pRelUnlock;
! 
! 					/* Flag that all relations were not successfully 
! 					   locked */
! 					
! 					allLocked = false;
! 
! 					/* A lock wasn't obtained, so unlock all others */
! 
! 					for(pRelUnlock = blockingLockTarget;
! 						pRelUnlock < pRel;
! 						pRelUnlock++) 
! 						UnlockRelation(*pRelUnlock, lockstmt->mode);
! 					
! 					/* Next time, do our blocking lock on the contended lock */
! 
! 					blockingLockTarget = pRel;
! 
! 					/* Now break out and try again */
! 
! 					break;
! 				}
! 			}
! 
! 			/* Now, lock anything before the blocking lock target in the lock
! 			   target array, if we were successful in getting locks on
! 			   everything after and including the blocking target */
! 
! 			if(allLocked)
! 			{
! 				for(pRel = relationArray;
! 					pRel < blockingLockTarget;
! 					pRel++)
! 				{
! 					if(!ConditionalLockRelation(*pRel, lockstmt->mode)) 
! 					{
! 						Relation *pRelUnlock;
! 						
! 						/* Flag that all relations were not successfully 
! 						   locked */
! 
! 						allLocked = false;
! 
! 						/* Lock wasn't obtained, so unlock all others */
! 
! 						for(pRelUnlock = relationArray;
! 							pRelUnlock < pRel;
! 							pRelUnlock++) 
! 							UnlockRelation(*pRelUnlock, lockstmt->mode);
! 						
! 						/* Next time, do our blocking lock on the contended 
! 						   lock */
! 
! 						blockingLockTarget = pRel;
! 					
! 						/* Now break out and try again */
! 
! 						break;
! 					}
! 				}
! 			}
! 		}
! 
! 		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/07/26 00:02: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.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/07/26 00:02: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.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/07/26 00:02:25
*************** 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/07/26 00:02:26
*************** 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;		/* relation 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/07/26 00:02:27
*************** 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);
  				}

Wed, 25 Jul 2001 20:27:51 -0400
Neil Padgett <npadgett@redhat.com>
Revised Patch to allow multiple table locks in "Unison" 

The problem with this patch is that it doesn't always lock the tables in
the order supplied by the user, leading to possible deadlock.  My guess
is that you need to try locking A, B, C and if B hangs, I think you need
to sleep on B, and when you get it, release the lock on B and try A, B,
C again.  I know this is a pain and could fail multiple times, but I
think we have to do it this ay.


> Here is the revised patch to allow the syntax:
> 
> LOCK a,b,c;
> 
> It uses the method I described as the "new" method in a previous
> message.
> 
> Neil
> 
> -- 
> Neil Padgett
> Red Hat Canada Ltd.                       E-Mail:  npadgett@redhat.com
> 2323 Yonge Street, Suite #300, 
> Toronto, ON  M4P 2C9
> 
> 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/07/26 00:02:24
> *************** 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 */
>   }
>   
>   
> --- 1994,2170 ----
>   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;
> ! 		Relation *blockingLockTarget;
> ! 		bool allLocked = false;
> ! 
> ! 		/* Locking multiple tables */
> ! 
> ! 		/* Create an array of relations */
> ! 
> ! 		relationArray = palloc(relCnt * sizeof(Relation));
> ! 		blockingLockTarget = relationArray;
> ! 
> ! 		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 acquire locks on all the relations */
> ! 
> ! 		while(!allLocked) 
> ! 		{
> ! 
> ! 			allLocked = true;
> ! 
> ! 			/* Lock the blocking lock target (initially the first lock
> ! 			   in the user's list */
> ! 
> ! 			LockRelation(*blockingLockTarget, lockstmt->mode);
> ! 
> ! 			/* Lock is now obtained on the lock target, now grab locks in a
> ! 			   non-blocking way on the rest of the list */
> ! 			
> ! 			for(pRel = blockingLockTarget + 1;
> ! 				pRel < relationArray + relCnt;
> ! 				pRel++)
> ! 			{
> ! 				if(!ConditionalLockRelation(*pRel, lockstmt->mode)) 
> ! 				{
> ! 					Relation *pRelUnlock;
> ! 
> ! 					/* Flag that all relations were not successfully 
> ! 					   locked */
> ! 					
> ! 					allLocked = false;
> ! 
> ! 					/* A lock wasn't obtained, so unlock all others */
> ! 
> ! 					for(pRelUnlock = blockingLockTarget;
> ! 						pRelUnlock < pRel;
> ! 						pRelUnlock++) 
> ! 						UnlockRelation(*pRelUnlock, lockstmt->mode);
> ! 					
> ! 					/* Next time, do our blocking lock on the contended lock */
> ! 
> ! 					blockingLockTarget = pRel;
> ! 
> ! 					/* Now break out and try again */
> ! 
> ! 					break;
> ! 				}
> ! 			}
> ! 
> ! 			/* Now, lock anything before the blocking lock target in the lock
> ! 			   target array, if we were successful in getting locks on
> ! 			   everything after and including the blocking target */
> ! 
> ! 			if(allLocked)
> ! 			{
> ! 				for(pRel = relationArray;
> ! 					pRel < blockingLockTarget;
> ! 					pRel++)
> ! 				{
> ! 					if(!ConditionalLockRelation(*pRel, lockstmt->mode)) 
> ! 					{
> ! 						Relation *pRelUnlock;
> ! 						
> ! 						/* Flag that all relations were not successfully 
> ! 						   locked */
> ! 
> ! 						allLocked = false;
> ! 
> ! 						/* Lock wasn't obtained, so unlock all others */
> ! 
> ! 						for(pRelUnlock = relationArray;
> ! 							pRelUnlock < pRel;
> ! 							pRelUnlock++) 
> ! 							UnlockRelation(*pRelUnlock, lockstmt->mode);
> ! 						
> ! 						/* Next time, do our blocking lock on the contended 
> ! 						   lock */
> ! 
> ! 						blockingLockTarget = pRel;
> ! 					
> ! 						/* Now break out and try again */
> ! 
> ! 						break;
> ! 					}
> ! 				}
> ! 			}
> ! 		}
> ! 
> ! 		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/07/26 00:02: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.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/07/26 00:02: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.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/07/26 00:02:25
> *************** 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/07/26 00:02:26
> *************** 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;		/* relation 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/07/26 00:02:27
> *************** 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 6: Have you searched our list archives?
> 
> http://www.postgresql.org/search.mpl
> 

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

Thu, 26 Jul 2001 17:48:13 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison"
Bruce Momjian wrote:
> 
> The problem with this patch is that it doesn't always lock the tables in
> the order supplied by the user, leading to possible deadlock.  My guess
> is that you need to try locking A, B, C and if B hangs, I think you need
> to sleep on B, and when you get it, release the lock on B and try A, B,
> C again.  I know this is a pain and could fail multiple times, but I
> think we have to do it this ay.
> 

Deadlocks are not possible with this patch. The four conditions needed
for deadlock are (according to Operating Systems: Internals and Design
Principles, 4th Ed. by W. Stallings):

1. Mutual exclusion: Only one process may use a resource at a time.
2. Hold and wait: A process may hold allocated resources while awaiting
assignment of others.
3. No preemption: No resources can be forcibly removed from a process
holding it.
4. Circular wait:  A closed chain of processes exists, such that each
process holds at least one resource needed by the next process in the
chain.

For deadlock prevention one needs only to prevent the existence of
at least one of the four conditions. 


The patch code never holds any of requested locks, while waiting for a 
locked relation to become free. If a lock on all tables in the lock list
cannot be acquired at once, it backs off and releases all locks.

Stallings writes about preventing condition 3: "This condition can be
prevented in several ways. [. . .] [One way is to require that,] if a
process holding certain resources is denied a further request, that
process must release its original resources and, if necessary, request
them again together with the additional resources." 

This is exactly what the patch does. Observe that if one lock is not
available, the patch releases all locks so far acquired, and then
acquires
the locks again. Hence, condition 3 is prevented, and so deadlock is
prevented.

Neil

p.s. Is this mailing list always this slow?

-- 
Neil Padgett
Red Hat Canada Ltd.                       E-Mail:  npadgett@redhat.com
2323 Yonge Street, Suite #300, 
Toronto, ON  M4P 2C9

Fri, 27 Jul 2001 14:36:25 -0400
Neil Padgett <npadgett@redhat.com>
Re: Revised Patch to allow multiple table locks in "Unison"
> Bruce Momjian wrote:
> > 
> > The problem with this patch is that it doesn't always lock the tables in
> > the order supplied by the user, leading to possible deadlock.  My guess
> > is that you need to try locking A, B, C and if B hangs, I think you need
> > to sleep on B, and when you get it, release the lock on B and try A, B,
> > C again.  I know this is a pain and could fail multiple times, but I
> > think we have to do it this ay.
> > 
> 
> Deadlocks are not possible with this patch. The four conditions needed
> for deadlock are (according to Operating Systems: Internals and Design
> Principles, 4th Ed. by W. Stallings):
> 
...
> 
> The patch code never holds any of requested locks, while waiting for a 
> locked relation to become free. If a lock on all tables in the lock list
> cannot be acquired at once, it backs off and releases all locks.
> 
> Stallings writes about preventing condition 3: "This condition can be
> prevented in several ways. [. . .] [One way is to require that,] if a
> process holding certain resources is denied a further request, that
> process must release its original resources and, if necessary, request
> them again together with the additional resources." 
> 
> This is exactly what the patch does. Observe that if one lock is not
> available, the patch releases all locks so far acquired, and then
> acquires
> the locks again. Hence, condition 3 is prevented, and so deadlock is
> prevented.

Excellent point.  I had not considered the fact that you don't hang
waiting for the other locks;  you just release them all and try again.

Looks like a great patch, and it seems better than the OID patch in many
ways.

> p.s. Is this mailing list always this slow?

Not sure.  I have gotten patches stuck in the patches queue recently. 
Not sure on the cause.  Marc may know.

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

Sun, 29 Jul 2001 01:08:02 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison"
Your patch has been added to the PostgreSQL unapplied patches list at:

	http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

> Bruce Momjian wrote:
> > 
> > The problem with this patch is that it doesn't always lock the tables in
> > the order supplied by the user, leading to possible deadlock.  My guess
> > is that you need to try locking A, B, C and if B hangs, I think you need
> > to sleep on B, and when you get it, release the lock on B and try A, B,
> > C again.  I know this is a pain and could fail multiple times, but I
> > think we have to do it this ay.
> > 
> 
> Deadlocks are not possible with this patch. The four conditions needed
> for deadlock are (according to Operating Systems: Internals and Design
> Principles, 4th Ed. by W. Stallings):
> 
> 1. Mutual exclusion: Only one process may use a resource at a time.
> 2. Hold and wait: A process may hold allocated resources while awaiting
> assignment of others.
> 3. No preemption: No resources can be forcibly removed from a process
> holding it.
> 4. Circular wait:  A closed chain of processes exists, such that each
> process holds at least one resource needed by the next process in the
> chain.
> 
> For deadlock prevention one needs only to prevent the existence of
> at least one of the four conditions. 
> 
> 
> The patch code never holds any of requested locks, while waiting for a 
> locked relation to become free. If a lock on all tables in the lock list
> cannot be acquired at once, it backs off and releases all locks.
> 
> Stallings writes about preventing condition 3: "This condition can be
> prevented in several ways. [. . .] [One way is to require that,] if a
> process holding certain resources is denied a further request, that
> process must release its original resources and, if necessary, request
> them again together with the additional resources." 
> 
> This is exactly what the patch does. Observe that if one lock is not
> available, the patch releases all locks so far acquired, and then
> acquires
> the locks again. Hence, condition 3 is prevented, and so deadlock is
> prevented.
> 
> Neil
> 
> p.s. Is this mailing list always this slow?
> 
> -- 
> Neil Padgett
> Red Hat Canada Ltd.                       E-Mail:  npadgett@redhat.com
> 2323 Yonge Street, Suite #300, 
> Toronto, ON  M4P 2C9
> 

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

Sun, 29 Jul 2001 01:08:09 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison"
Bruce Momjian wrote:
> 
> > Bruce Momjian wrote:
> > >

[snip]

> >
> > Deadlocks are not possible with this patch. The four conditions needed
> > for deadlock are (according to Operating Systems: Internals and Design
> > Principles, 4th Ed. by W. Stallings):
> >
> ...
> >
> > The patch code never holds any of requested locks, while waiting for a
> > locked relation to become free. If a lock on all tables in the lock list
> > cannot be acquired at once, it backs off and releases all locks.
> >
> > Stallings writes about preventing condition 3: "This condition can be
> > prevented in several ways. [. . .] [One way is to require that,] if a
> > process holding certain resources is denied a further request, that
> > process must release its original resources and, if necessary, request
> > them again together with the additional resources."
> >
> > This is exactly what the patch does. Observe that if one lock is not
> > available, the patch releases all locks so far acquired, and then
> > acquires
> > the locks again. Hence, condition 3 is prevented, and so deadlock is
> > prevented.
> 
> Excellent point.  I had not considered the fact that you don't hang
> waiting for the other locks;  you just release them all and try again.
> 

I have a question.
What will happen when the second table is locked for a long time 
though the first table isn't locked ?

regards,
Hiroshi Ioue

Mon, 30 Jul 2001 09:11:06 +0900
Hiroshi Inoue <Inoue@tpf.co.jp>
Re: Revised Patch to allow multiple table locks in "Unison"
On Mon, 30 Jul 2001, Hiroshi Inoue wrote:

> I have a question.
> What will happen when the second table is locked for a long time
> though the first table isn't locked ?

Consider the case:

LOCK a,b;

Assume a is free (i.e. not locked), but b is busy (i.e. locked).

First the system will do a blocking lock attempt on a, which will return
immediately, since a was free. Table a is now locked. Now, the system will
try a non-blocking lock on b. But, b is busy so the lock attempt will fail
immediately (since the lock attempt was non-blocking). So, the system will
back off, and the lock on a is released.

Next, a blocking lock attempt will be made on b. (Since it was busy last
time, we want to wait for it to become free.) The lock call will block
until b becomes free. At that time, the lock attempt will return, and b
will be locked. Then, a non-blocking lock attempt will be made on table a.
(Recall that we don't have a lock on it, since we released it during
back-off earlier.) Assuming a is still free, it will be locked and the
LOCK command will complete. Otherwise, if a is busy, the lock attempt will
then restart with a blocking lock attempt on a. The procedure will
continue until all tables are free to lock.

In summary, no locks are held while waiting for tables to become free --
in essence, the tables are locked all at once, once all tables in the
LOCK statement are free.

Neil

-- 
Neil Padgett
Red Hat Canada Ltd.                       E-Mail:  npadgett@redhat.com
2323 Yonge Street, Suite #300,
Toronto, ON  M4P 2C9



Sun, 29 Jul 2001 20:30:57 -0400 (EDT)
Neil Padgett <npadgett@redhat.com>
Re: Revised Patch to allow multiple table locks in "Unison"
> On Mon, 30 Jul 2001, Hiroshi Inoue wrote:
> 
> > I have a question.
> > What will happen when the second table is locked for a long time
> > though the first table isn't locked ?
> 
> Consider the case:
> 
> LOCK a,b;
> 
> Assume a is free (i.e. not locked), but b is busy (i.e. locked).
> 
> First the system will do a blocking lock attempt on a, which will return
> immediately, since a was free. Table a is now locked. Now, the system will
> try a non-blocking lock on b. But, b is busy so the lock attempt will fail
> immediately (since the lock attempt was non-blocking). So, the system will
> back off, and the lock on a is released.
> 
> Next, a blocking lock attempt will be made on b. (Since it was busy last
> time, we want to wait for it to become free.) The lock call will block
> until b becomes free. At that time, the lock attempt will return, and b
> will be locked. Then, a non-blocking lock attempt will be made on table a.
> (Recall that we don't have a lock on it, since we released it during
> back-off earlier.) Assuming a is still free, it will be locked and the
> LOCK command will complete. Otherwise, if a is busy, the lock attempt will
> then restart with a blocking lock attempt on a. The procedure will
> continue until all tables are free to lock.
> 
> In summary, no locks are held while waiting for tables to become free --
> in essence, the tables are locked all at once, once all tables in the
> LOCK statement are free.

The more I think about it the more I like it.  I never would have
thought of the idea myself.

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

Sun, 29 Jul 2001 22:49:28 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison"
> Stallings writes about preventing condition 3: "This condition can be
> prevented in several ways. [. . .] [One way is to require that,] if a
> process holding certain resources is denied a further request, that
> process must release its original resources and, if necessary, request
> them again together with the additional resources."
> 
> This is exactly what the patch does.

No, that is not what it does.  Note that Stallings specifies that the
failed requestor must release ALL previously held resources.  That is
not feasible in Postgres; some of the locks may be held due to previous
commands in the same transaction.  Consider this scenario:

	Process 1			Process 2

	LOCK a;				LOCK b;
	...				...
	LOCK b,c;			LOCK a,c;

The second LOCK statements cannot release the locks already held,
therefore this is a deadlock.  While that's no worse than we had
before, I believe that your patch introduces a possibility of
undetected deadlock.  Consider this:

	Process 1			Process 2

	LOCK a,b;			LOCK b,a;

A possible interleaving of execution is: 1 acquires lock a, 2 acquires
b, 1 tries to acquire b and fails, 2 tries to acquire a and fails,
1 releases a, 2 releases b, 1 acquires b, 2 acquires a, 1 tries to
acquire a and fails, etc etc.  It's implausible that this condition
would persist in perfect lockstep for very long on a uniprocessor
machine ... but not so implausible if you have dual CPUs, each running
one of the two processes at exactly the same speed.

I haven't quite managed to work out a full scenario, but I think it is
possible that the combination of these two effects could result in an
indefinite, never-detected deadlock --- without implausible assumptions
about process speed.  It'd probably take three or more processes
contending for several locks, but it seems possible to construct a
failure scenario.

I think that the only safe way to do something like this is to teach
the lock manager itself about multiple simultaneous requests.

			regards, tom lane

Sun, 29 Jul 2001 23:25:41 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison" 
Neil Padgett wrote:
> 
> On Mon, 30 Jul 2001, Hiroshi Inoue wrote:
> 
> > I have a question.
> > What will happen when the second table is locked for a long time
> > though the first table isn't locked ?
> 
> Consider the case:
> 
> LOCK a,b;
> 
> Assume a is free (i.e. not locked), but b is busy (i.e. locked).
> 
> First the system will do a blocking lock attempt on a, which will return
> immediately, since a was free. Table a is now locked. Now, the system will
> try a non-blocking lock on b. But, b is busy so the lock attempt will fail
> immediately (since the lock attempt was non-blocking). So, the system will
> back off, and the lock on a is released.
> 
> Next, a blocking lock attempt will be made on b. (Since it was busy last
> time, we want to wait for it to become free.) The lock call will block
> until b becomes free. At that time, the lock attempt will return, and b
> will be locked. Then, a non-blocking lock attempt will be made on table a.

Is it paranoid to worry about the followings ?

1) Concurrent 'lock table a, b;' and 'lock table b, a;'
   could last forever in theory ?
2) 'Lock table a,b' could hardly acquire the lock when
   both the table a and b are very frequently accessed.

regards,
Hiroshi Inoue

Mon, 30 Jul 2001 12:45:33 +0900
Hiroshi Inoue <Inoue@tpf.co.jp>
Re: Revised Patch to allow multiple table locks in "Unison"
> > First the system will do a blocking lock attempt on a, which will return
> > immediately, since a was free. Table a is now locked. Now, the system will
> > try a non-blocking lock on b. But, b is busy so the lock attempt will fail
> > immediately (since the lock attempt was non-blocking). So, the system will
> > back off, and the lock on a is released.
> > 
> > Next, a blocking lock attempt will be made on b. (Since it was busy last
> > time, we want to wait for it to become free.) The lock call will block
> > until b becomes free. At that time, the lock attempt will return, and b
> > will be locked. Then, a non-blocking lock attempt will be made on table a.
> 
> Is it paranoid to worry about the followings ?
> 
> 1) Concurrent 'lock table a, b;' and 'lock table b, a;'
>    could last forever in theory ?
> 2) 'Lock table a,b' could hardly acquire the lock when
>    both the table a and b are very frequently accessed.

Well, we do tell people to lock things in the same order.  If they did
this with two lock statements, they would cause a deadlock.  In this
case, they could grab their first lock at the same time, fail on the
second, and wait on the second, get it, fail on the first, and go all
over again.  However, they would have to stay synchronized to do this. 
If one got out of step it would stop.

Actually, with this new code, we could go back to locking in oid order,
which would eliminate the problem.  However, I prefer to do things in
the order specified, at least on the first lock try.

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

Sun, 29 Jul 2001 23:49:37 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison"
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Actually, with this new code, we could go back to locking in oid order,
> which would eliminate the problem.

No it wouldn't.  If anything, locking in a *randomized* order would be
the best bet.  But I have no confidence in this approach, anyway.

			regards, tom lane

Mon, 30 Jul 2001 00:24:14 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison" 
Tom Lane wrote:
> 
> > Stallings writes about preventing condition 3: "This condition can be
> > prevented in several ways. [. . .] [One way is to require that,] if a
> > process holding certain resources is denied a further request, that
> > process must release its original resources and, if necessary, request
> > them again together with the additional resources."
> >
> > This is exactly what the patch does.
> 
> No, that is not what it does.  Note that Stallings specifies that the
> failed requestor must release ALL previously held resources.  That is
> not feasible in Postgres; some of the locks may be held due to previous
> commands in the same transaction.  Consider this scenario:
> 
>         Process 1                       Process 2
> 
>         LOCK a;                         LOCK b;
>         ...                             ...
>         LOCK b,c;                       LOCK a,c;
> 
> The second LOCK statements cannot release the locks already held,
> therefore this is a deadlock.

But that is a programmer's error.  He/she is acquiring the locks in
reversed order.  The fact that the second lock request is in a multiple
lock is not much different from doing it with a single lock like in:

        Process 1                       Process 2

        LOCK a;                         LOCK b;
        ...                             ...
        LOCK b;                         LOCK a;

I guess you've mentioned this scenario because of the undetected
deadlock
concerns, right?



>  While that's no worse than we had
> before, I believe that your patch introduces a possibility of
> undetected deadlock.  Consider this:
> 
>         Process 1                       Process 2
> 
>         LOCK a,b;                       LOCK b,a;
> 
> A possible interleaving of execution is: 1 acquires lock a, 2 acquires
> b, 1 tries to acquire b and fails, 2 tries to acquire a and fails,
> 1 releases a, 2 releases b, 1 acquires b, 2 acquires a, 1 tries to
> acquire a and fails, etc etc.  It's implausible that this condition
> would persist in perfect lockstep for very long on a uniprocessor
> machine ... but not so implausible if you have dual CPUs, each running
> one of the two processes at exactly the same speed.
> 
> I haven't quite managed to work out a full scenario, but I think it is
> possible that the combination of these two effects could result in an
> indefinite, never-detected deadlock --- without implausible assumptions
> about process speed. 


I believe the undetected deadlock is not possible.  To get the multiple
lock statement involved in the deadlock scenario, at least one of the
locks
desired must have been acquired (in a separate statement) by the other
process.

The key property of the algorithm that prevents the undetected deadlocks
is
that it waits on the last failed-to-acquire lock. 

As the algorithm waits on the last non-obtained lock and restarts
from there (in a circular fashion), it will eventually reach the lock
that
the other process has and then stop for good (thus allowing the deadlock
detection to see it).

Even if the algorithm started always from the first specified lock and
then
got in the lockstep mode you've described, the (potential) deadlock 
would
not be detected because it had not happened yet.  It will only happen
when
the 2nd situation you've described ceases to exist and the crossed locks
are
attempted.  But them the processes are really stopped and the deadlock
can be detected. 


-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Mon, 30 Jul 2001 11:49:59 -0400
Fernando Nasser <fnasser@redhat.com>
Re: Revised Patch to allow multiple table locks in "Unison"
Hiroshi Inoue wrote:
> 
> Is it paranoid to worry about the followings ?
> 
> 1) Concurrent 'lock table a, b;' and 'lock table b, a;'
>    could last forever in theory ?

You would need a very evil timeslice duration on a single processor, but
it
could happen on a dual processor.  However, the two processes would have
to
be synchronized in a very narrow window of instructions, the schedulers
in
both machines would have to be precisely synchronized and absolutely no
interruption (that is not common to both machines) could never occur.
Even a keyboard press will break the enchantment.

I guess it is what we call "unstable equilibrium", possible in theory
but
never happens in practice except for an infinitesimal amount of time.
It is trying to make an egg stand on one end or something like that
(without breaking the egg, of course :-) ). 



> 2) 'Lock table a,b' could hardly acquire the lock when
>    both the table a and b are very frequently accessed.
> 

Yes, multiple locks with the back off is less aggressive than obtaining
and holding the locks (with individual lock commands).



-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Mon, 30 Jul 2001 12:16:22 -0400
Fernando Nasser <fnasser@redhat.com>
Re: Revised Patch to allow multiple table locks in "Unison"
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Actually, with this new code, we could go back to locking in oid order,
> > which would eliminate the problem.
> 
> No it wouldn't.  If anything, locking in a *randomized* order would be
> the best bet.  But I have no confidence in this approach, anyway.

I am looking for a way to get this in there without munging the lock
code, which is already quite complex.  What about doing some sort of
small sleep after we reset back to the beginning of the table list.

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

Mon, 30 Jul 2001 12:20:58 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison"
> > The second LOCK statements cannot release the locks already held,
> > therefore this is a deadlock.
> 
> But that is a programmer's error.  He/she is acquiring the locks in
> reversed order.  The fact that the second lock request is in a multiple
> lock is not much different from doing it with a single lock like in:
> 
>         Process 1                       Process 2
> 
>         LOCK a;                         LOCK b;
>         ...                             ...
>         LOCK b;                         LOCK a;
> 
> I guess you've mentioned this scenario because of the undetected
> deadlock
> concerns, right?
> 

I guess so.  The above would issue a deadlock error message, while the
LOCK a,b would just spin around.

> >  While that's no worse than we had
> > before, I believe that your patch introduces a possibility of
> > undetected deadlock.  Consider this:
> > 
> >         Process 1                       Process 2
> > 
> >         LOCK a,b;                       LOCK b,a;
> > 
> > A possible interleaving of execution is: 1 acquires lock a, 2 acquires
> > b, 1 tries to acquire b and fails, 2 tries to acquire a and fails,
> > 1 releases a, 2 releases b, 1 acquires b, 2 acquires a, 1 tries to
> > acquire a and fails, etc etc.  It's implausible that this condition
> > would persist in perfect lockstep for very long on a uniprocessor
> > machine ... but not so implausible if you have dual CPUs, each running
> > one of the two processes at exactly the same speed.
> > 
> > I haven't quite managed to work out a full scenario, but I think it is
> > possible that the combination of these two effects could result in an
> > indefinite, never-detected deadlock --- without implausible assumptions
> > about process speed. 
> 
> 
> I believe the undetected deadlock is not possible.  To get the multiple
> lock statement involved in the deadlock scenario, at least one of the
> locks
> desired must have been acquired (in a separate statement) by the other
> process.
> 
> The key property of the algorithm that prevents the undetected deadlocks
> is
> that it waits on the last failed-to-acquire lock. 
> 
> As the algorithm waits on the last non-obtained lock and restarts
> from there (in a circular fashion), it will eventually reach the lock
> that
> the other process has and then stop for good (thus allowing the deadlock
> detection to see it).
> 
> Even if the algorithm started always from the first specified lock and
> then
> got in the lockstep mode you've described, the (potential) deadlock 
> would
> not be detected because it had not happened yet.  It will only happen
> when
> the 2nd situation you've described ceases to exist and the crossed locks
> are
> attempted.  But them the processes are really stopped and the deadlock
> can be detected. 

The unusual case here is that deadlock is not checked on request, but
only after waiting on the lock for a while.  This is because deadlock
detection is an expensive operation.   In fact, you don't want deadlock
detection in this case because LOCK a,b could be evaluated as a,b or b,a
and you don't want it to fail randomly with deadlock messages.

I certainly would like to find a solution to this that makes everyone
comfortable.

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

Mon, 30 Jul 2001 12:55:38 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison"
On Mon, 30 Jul 2001, Bruce Momjian wrote:

> > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > > Actually, with this new code, we could go back to locking in oid order,
> > > which would eliminate the problem.
> >
> > No it wouldn't.  If anything, locking in a *randomized* order would be
> > the best bet.  But I have no confidence in this approach, anyway.
>
> I am looking for a way to get this in there without munging the lock
> code, which is already quite complex.  What about doing some sort of
> small sleep after we reset back to the beginning of the table list.

It seems to me that we already have a small sleep in place. After all, in
order to acquite a lock, the shared memory area has to be accessed. So,
the contenders for a lock both have to go through a spin lock. So, if we
have the two "stuck" processes as in Tom's example, one will win at
acquiring the spin lock and the other will have to wait. So, they become
desynchronized, regardless of how many CPUs or what memory architecture
you have.

Neil

-- 
Neil Padgett
Red Hat Canada Ltd.                       E-Mail:  npadgett@redhat.com
2323 Yonge Street, Suite #300,
Toronto, ON  M4P 2C9



Mon, 30 Jul 2001 12:57:25 -0400 (EDT)
Neil Padgett <npadgett@redhat.com>
Re: Revised Patch to allow multiple table locks in "Unison"
> On Mon, 30 Jul 2001, Bruce Momjian wrote:
> 
> > > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > > > Actually, with this new code, we could go back to locking in oid order,
> > > > which would eliminate the problem.
> > >
> > > No it wouldn't.  If anything, locking in a *randomized* order would be
> > > the best bet.  But I have no confidence in this approach, anyway.
> >
> > I am looking for a way to get this in there without munging the lock
> > code, which is already quite complex.  What about doing some sort of
> > small sleep after we reset back to the beginning of the table list.
> 
> It seems to me that we already have a small sleep in place. After all, in
> order to acquite a lock, the shared memory area has to be accessed. So,
> the contenders for a lock both have to go through a spin lock. So, if we
> have the two "stuck" processes as in Tom's example, one will win at
> acquiring the spin lock and the other will have to wait. So, they become
> desynchronized, regardless of how many CPUs or what memory architecture
> you have.

I see your point now, that they can't synchronize because they have to
go through the same semaphore and therefore get out of sync.  Do they
get out of sync enough for one to get the lock while the other is not
holding it, or do the locks actually keep them in sync?  I don't know
the answer.

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

Mon, 30 Jul 2001 13:04:11 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison"
Bruce Momjian wrote:
> 
> The unusual case here is that deadlock is not checked on request, but
> only after waiting on the lock for a while.  This is because deadlock
> detection is an expensive operation.  

But that is what happens.  If one of the locks is not obtained, the
algorithm does wait on that lock (after releasing the other locks).
In the case of a deadlock (tom's scenario #1) it would wait forever,
but the deadlock detection will find it in there and break it.


> In fact, you don't want deadlock
> detection in this case because LOCK a,b could be evaluated as a,b or b,a
> and you don't want it to fail randomly with deadlock messages.
> 

It does not change the deadlock scenario at all.  It is still determined
by the resources in a previous (independent) LOCK statement and the ones
on
this LOCK statement (being it multiple or not) to be crossed. 

And deadlock failures will be intermittent anyway.  A potential deadlock
condition may or may not happen each time depending on the interleaving
of
execution of the two processes.


-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Mon, 30 Jul 2001 13:24:26 -0400
Fernando Nasser <fnasser@redhat.com>
Re: Revised Patch to allow multiple table locks in "Unison"
> Bruce Momjian wrote:
> > 
> > The unusual case here is that deadlock is not checked on request, but
> > only after waiting on the lock for a while.  This is because deadlock
> > detection is an expensive operation.  
> 
> But that is what happens.  If one of the locks is not obtained, the
> algorithm does wait on that lock (after releasing the other locks).
> In the case of a deadlock (tom's scenario #1) it would wait forever,
> but the deadlock detection will find it in there and break it.

OK, I thought you were talking about the LOCK a,b case, not the other
case where we had a previous LOCK statement.  Sorry.

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

Mon, 30 Jul 2001 13:30:29 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison"
Bruce Momjian wrote:
> 
> > It seems to me that we already have a small sleep in place. After all, in
> > order to acquite a lock, the shared memory area has to be accessed. So,
> > the contenders for a lock both have to go through a spin lock. So, if we
> > have the two "stuck" processes as in Tom's example, one will win at
> > acquiring the spin lock and the other will have to wait. So, they become
> > desynchronized, regardless of how many CPUs or what memory architecture
> > you have.
> 
> I see your point now, that they can't synchronize because they have to
> go through the same semaphore and therefore get out of sync.  Do they
> get out of sync enough for one to get the lock while the other is not
> holding it, or do the locks actually keep them in sync?  I don't know
> the answer.
> 

That is a good point.  With the current random sleeps it helps breaking
the
lockstep of the two processes, but when it is changed to a queue the
random
sleeps won't be there anymore.


-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Mon, 30 Jul 2001 14:09:41 -0400
Fernando Nasser <fnasser@redhat.com>
Re: Revised Patch to allow multiple table locks in "Unison"
> Bruce Momjian wrote:
> > 
> > > It seems to me that we already have a small sleep in place. After all, in
> > > order to acquite a lock, the shared memory area has to be accessed. So,
> > > the contenders for a lock both have to go through a spin lock. So, if we
> > > have the two "stuck" processes as in Tom's example, one will win at
> > > acquiring the spin lock and the other will have to wait. So, they become
> > > desynchronized, regardless of how many CPUs or what memory architecture
> > > you have.
> > 
> > I see your point now, that they can't synchronize because they have to
> > go through the same semaphore and therefore get out of sync.  Do they
> > get out of sync enough for one to get the lock while the other is not
> > holding it, or do the locks actually keep them in sync?  I don't know
> > the answer.
> > 
> 
> That is a good point.  With the current random sleeps it helps breaking
> the
> lockstep of the two processes, but when it is changed to a queue the
> random
> sleeps won't be there anymore.

Also most systems can't sleep less than one clock tick, 10ms, meaning
the sleeps aren't very random.

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

Mon, 30 Jul 2001 14:14:18 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison"
Fernando Nasser <fnasser@redhat.com> writes:
> But that is what happens.  If one of the locks is not obtained, the
> algorithm does wait on that lock (after releasing the other locks).
> In the case of a deadlock (tom's scenario #1) it would wait forever,
> but the deadlock detection will find it in there and break it.

I'm entirely unconvinced of that.  My example #2 shows that it's
possible for two multi-lock statements to bounce back and forth between
failed conditional-lock attempts, never doing a hard wait that would
allow the lock manager to detect deadlock.  That simple example requires
somewhat-implausible assumptions about relative process speed, but I
think that more complex cases might exhibit similar behavior that is
less dependent on precise timing.  (The fact that I have not come up with
one after a few minutes' thought can hardly be taken as proof that there
are no such cases.)

Basically, my objection here is that the proposed implementation tries
to avoid telling the lock manager what is going on.  Since the lock
manager has sole responsibility for detecting deadlock, it can't
possibly be a good idea not to give it complete information.

> And deadlock failures will be intermittent anyway.  A potential deadlock
> condition may or may not happen each time depending on the interleaving
> of execution of the two processes.

Of course.  The point is that we currently offer a guarantee that when a
deadlock does occur, it will be detected, reported, and recovered from
(by rolling back one or more of the conflicting transactions).  I am
afraid that the proposed multi-LOCK implementation will break that
guarantee.  I do not think the proposed feature is sufficiently valuable
to justify taking any such risk.

			regards, tom lane

Mon, 30 Jul 2001 14:22:30 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison" 
Neil Padgett <npadgett@redhat.com> writes:
> the contenders for a lock both have to go through a spin lock. So, if we
> have the two "stuck" processes as in Tom's example, one will win at
> acquiring the spin lock and the other will have to wait. So, they become
> desynchronized, regardless of how many CPUs or what memory architecture
> you have.

No, actually the existence of the lockmanager mutex (spinlock) makes the
scenario I described *more* plausible.  Note that the scenario involves
a strict alternation of lockmanager operations by the two processes:

>> A possible interleaving of execution is: 1 acquires lock a, 2 acquires
>> b, 1 tries to acquire b and fails, 2 tries to acquire a and fails,
>> 1 releases a, 2 releases b, 1 acquires b, 2 acquires a, 1 tries to
>> acquire a and fails, etc etc.  It's implausible that this condition
>> would persist in perfect lockstep for very long on a uniprocessor
>> machine ... but not so implausible if you have dual CPUs, each running
>> one of the two processes at exactly the same speed.

Each process will be acquiring the lockmanager spinlock, doing a little
computation, releasing the spinlock, doing a little more computation
(in the LOCK statement code, not in the lockmanager), and then trying to
acquire the spinlock again.  When process 1 has the spinlock, process 2
will be waiting to acquire it.  As soon as 1 releases the spinlock, 2
will successfully acquire it --- so, quite plausibly, 1 will complete
its outside-the-lock-manager operations and be back waiting for the
spinlock by the time 2 releases it.  Even if 1 is a little slower than
that, it will probably manage to come along and retake the spinlock
before 2 does.  So the existence of the spinlock actually smooths out
any irregularities in elapsed time and helps to ensure the two processes
stay in sync.

The pattern of alternating between hard and conditional locks won't help
any either.  If, say, 1 gets a little ahead and arrives at the first
part of its cycle ("acquire lock a") before 2 has released lock a, guess
what --- it blocks until it can get a.  I think that could help
stabilize the loop too.

Long-term persistence of this pattern is somewhat less plausible on a
uniprocessor, but given the way our spinlocks work I don't think it's
completely out of the question either.

			regards, tom lane

Mon, 30 Jul 2001 14:57:54 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison" 
Okay, I've developed a case where the proposed multi-LOCK implementation
will wait forever without detecting deadlock --- indeed, there is no
true deadlock, but it won't figure out how to get out of it.

The problem is that we have multiple types of locks, some of which
conflict and some of which don't.  It's sufficient to think about
"shared" (reader) and "exclusive" (writer) locks for this example.
Consider this scenario:

Process 1	Process 2	Process 3

SH LOCK A

		SH LOCK B

				EX LOCK B	3 now waits for 2

		EX LOCK A			2 now waits for 1

SH LOCK B

Since process 3 is waiting for ex lock on B, our normal behavior is to
queue up 1 behind 3 in the queue for B (if we let 1 go first, 3 could be
starved indefinitely by a succession of transactions that want to read
B).  In this situation, however, that would be a deadlock.  The deadlock
detection code recognizes this, and also recognizes that it can escape
from the deadlock by allowing 1 to go ahead of 3 --- so SH LOCK B is
granted to 1, and we can proceed.  Note that since the deadlock can only
be recognized by looking at locks and procs that are unrelated to the
queue for table B, it's extremely expensive to detect this case, and so
we don't try to do so during LockAcquire.  Only when the timeout expires
and we run the full deadlock-detection algorithm do we realize that we
have a problem and find a way out of it.

Now, let's extend this to a multi-LOCK scenario:

Process 1	Process 2	Process 3	Process 4	Process 5

SH LOCK A

		SH LOCK B			SH LOCK C

				EX LOCK B			EX LOCK C

		EX LOCK A			EX LOCK A

(at this point, 3 waits for 2, 5 waits for 4, 2 and 4 wait for 1)

MULTI SH LOCK B,C

Now, what will happen?  The multi lock code will do an unconditional
lock on B.  This will initially queue up behind 3.  After a deadlock
detection interval expires, the deadlock code will grant the requested
sh lock on B to 1 to escape otherwise-certain deadlock.  Now the multi-
lock code will do a conditional lock on C, which will fail (since the
lock manager will not run the deadlock detection code before failing the
conditional request).  The multi lock code then releases its shlock on B
and does an unconditional shlock on C.  One detection interval later,
it will have that, but its conditional lock on B will fail.  And away we
go.

The bottom line here is you cannot make this work correctly unless you
teach the lock manager about it.  Yes, it's not trivial.

			regards, tom lane

Mon, 30 Jul 2001 16:46:51 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison" 
Well, it seems to me that there are a number of places this can be taken
then:

1. Don't worry about these cases and apply the patch anyways.
	The patch will work correctly in most real world situations.

2. Add a counter / timer to detect livelock / alternating deadlock
conditions. If the counter / timer elapses, abort the offending
transaction (i.e. the one doing the multiple locking) and roll back.
	The patch then will always work correctly, AFAICT.
	
3. Go with the original patch I posted.
	Aside from not locking in user order (which is a debatable issue), this
patch seems correct. 

4. Go with the original patch but sans OID sort. In this case, LOCK a,b;
degrades into a short form for LOCK a; LOCk b; This is still useful for
Oracle compatibility.
	Satisfies the TODO list item.

5. Go with a major Lock manager overhaul. (which would be added to the
TODO list.) Defer this functionality until then.
	A lock manager overhaul will likely take a while so probably it won't
be done for some time. This means the multiple lock syntax will continue
to be missing from PostgreSQL, possibly for several years.

Personally, I think 1 is acceptable, and 2 is a better idea. 3/4 are
also
doable, but lose some of the advantages of the command. 5 is reasonable,
but disappointing, especially from a user standpoint.

Thoughts?

Neil

-- 
Neil Padgett
Red Hat Canada Ltd.                       E-Mail:  npadgett@redhat.com
2323 Yonge Street, Suite #300, 
Toronto, ON  M4P 2C9

Tue, 31 Jul 2001 14:56:51 -0400
Neil Padgett <npadgett@redhat.com>
Re: Revised Patch to allow multiple table locks in "Unison"
Neil Padgett <npadgett@redhat.com> writes:
> 4. Go with the original patch but sans OID sort. In this case, LOCK a,b;
> degrades into a short form for LOCK a; LOCk b; This is still useful for
> Oracle compatibility.
> 	Satisfies the TODO list item.

I like this one.  It's not clear to me that this TODO item is worth
even as much effort as we've put into the discussion already ;-),
let alone a major lockmanager overhaul.  And I definitely don't want a
version that "usually works" ... simple and reliable is beautiful.

			regards, tom lane

Tue, 31 Jul 2001 15:56:39 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison" 
> Well, it seems to me that there are a number of places this can be taken
> then:
> 
> 1. Don't worry about these cases and apply the patch anyways.
> 	The patch will work correctly in most real world situations.
> 
> 2. Add a counter / timer to detect livelock / alternating deadlock
> conditions. If the counter / timer elapses, abort the offending
> transaction (i.e. the one doing the multiple locking) and roll back.
> 	The patch then will always work correctly, AFAICT.
> 	
> 3. Go with the original patch I posted.
> 	Aside from not locking in user order (which is a debatable issue), this
> patch seems correct. 
> 
> 4. Go with the original patch but sans OID sort. In this case, LOCK a,b;
> degrades into a short form for LOCK a; LOCk b; This is still useful for
> Oracle compatibility.
> 	Satisfies the TODO list item.
> 
> 5. Go with a major Lock manager overhaul. (which would be added to the
> TODO list.) Defer this functionality until then.
> 	A lock manager overhaul will likely take a while so probably it won't
> be done for some time. This means the multiple lock syntax will continue
> to be missing from PostgreSQL, possibly for several years.
> 
> Personally, I think 1 is acceptable, and 2 is a better idea. 3/4 are
> also
> doable, but lose some of the advantages of the command. 5 is reasonable,
> but disappointing, especially from a user standpoint.

I have been thinking about this too.  I have two ideas.

One, we could have you sleep on the lock, and when you get it, release
it and then start acquiring the locks in the order specified again.  You
could lose the lock by the time you get back to the table you had a lock
on, but I think this reduces the chances of getting in a loop with
others.

Another idea is to change the lock code so instead of returning a lock
failure on first try, it goes to sleep for DEADLOCK seconds, and once it
wakes up, and determines there is no deadlock, returns a lock failure. 
That way, it can release all the locks and do a non-timeout lock on the
table that failed.  We would then need to release the lock and do the
steps outlined above.

One advantage of this last idea is that it would make multi-table lock
better in cases where there is more than one table that is high-use
because we are waiting a little to see if we get the lock before
failing.  The downside is that we hold the previously aquired locks
longer.  I think I can help you modify the lock manager to do the delay.

I know it is frustrating to develop a patch and then have to contort it
to meet everyones ideas, but in the long run, it makes for better code
and a more reliable database.

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

Tue, 31 Jul 2001 17:51:45 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison"
Bruce Momjian wrote:
> 
> > Well, it seems to me that there are a number of places this can be taken
> > then:
> >

Honestly, I don't understand what "Unison" means.
What spec is preferable for the command ?

regards,
Hiroshi Inoue

Wed, 01 Aug 2001 12:51:26 +0900
Hiroshi Inoue <Inoue@tpf.co.jp>
Re: Revised Patch to allow multiple table locks in "Unison"
Bruce Momjian wrote:

> I have been thinking about this too.  I have two ideas.
> 
> One, we could have you sleep on the lock, and when you get it, release
> it and then start acquiring the locks in the order specified again.  You
> could lose the lock by the time you get back to the table you had a lock
> on, but I think this reduces the chances of getting in a loop with
> others.
> 

I think this could work. But I worry it makes starvation even more
likely. This might be acceptable if a "passive" lock grabber is what we
want to have here, though.

One idea I've been floating is to allow some syntax whereby multiple
locks can be grabbed in either this manner, that is a passive manner, or
instead in an aggressive manner (i.e. grab locks in order, and hold them
-- in other words LOCK a,b; -> LOCK a; LOCK b;). This could be done by
means of some additional keywords, perhaps "WITH AGGRESSIVE" or "WITH
PASSIVE", or something to this effect. This shifts some of the burden to
the database programmer, with regards to trading off throughput for
fairness.

> Another idea is to change the lock code so instead of returning a lock
> failure on first try, it goes to sleep for DEADLOCK seconds, and once it
> wakes up, and determines there is no deadlock, returns a lock failure.
> That way, it can release all the locks and do a non-timeout lock on the
> table that failed.  We would then need to release the lock and do the
> steps outlined above.

This is interesting. I'd like to hear what other people think about
this.

> 
> One advantage of this last idea is that it would make multi-table lock
> better in cases where there is more than one table that is high-use
> because we are waiting a little to see if we get the lock before
> failing.  The downside is that we hold the previously aquired locks
> longer.  I think I can help you modify the lock manager to do the delay.

In other words it sounds like you are making a tradeoff for greater
throughput in exchange for possibly reduced concurrency. This can be a
design decision on our part, and might be a reasonable thing to do. How
hard do you think it will be to tune the DEADLOCK timer to a reasonable
value? Would it have to vary based on load? This could be as simple as
having the timeout could elapse early, if a certain number of lock
attempts are registered by the lock manager while the backend is
sleeping.

> I know it is frustrating to develop a patch and then have to contort it
> to meet everyones ideas, but in the long run, it makes for better code
> and a more reliable database.

I think I can agree that a reliable database with good code is what
everyone wants!

Neil

-- 
Neil Padgett
Red Hat Canada Ltd.                       E-Mail:  npadgett@redhat.com
2323 Yonge Street, Suite #300, 
Toronto, ON  M4P 2C9

Wed, 01 Aug 2001 15:59:12 -0400
Neil Padgett <npadgett@redhat.com>
Re: Revised Patch to allow multiple table locks in "Unison"
Neil Padgett <npadgett@redhat.com> writes:
> Bruce Momjian wrote:
>> One, we could have you sleep on the lock, and when you get it, release
>> it and then start acquiring the locks in the order specified again.

> I think this could work. But I worry it makes starvation even more
> likely.

I agree, it looks like a recipe for starvation.

>> Another idea is to change the lock code so instead of returning a lock
>> failure on first try, it goes to sleep for DEADLOCK seconds, and once it
>> wakes up, and determines there is no deadlock, returns a lock failure.
>> That way, it can release all the locks and do a non-timeout lock on the
>> table that failed.  We would then need to release the lock and do the
>> steps outlined above.

> This is interesting. I'd like to hear what other people think about
> this.

I doubt that this works --- it still has the same fundamental problem:
that you're not telling the lock manager what it needs to know to
fulfill its responsibility of detecting deadlock.  I believe I can still
produce a ping-pong undetected deadlock example with the above behavior;
it'd just take a few more processes.  The issue is that you are
expecting the lock manager to detect or not detect deadlock, when you
still have some lock requests up your sleeve that it's not seen yet.
As long as you can block before presenting them all, it can never work.

> In other words it sounds like you are making a tradeoff for greater
> throughput in exchange for possibly reduced concurrency. This can be a
> design decision on our part, and might be a reasonable thing to do. How
> hard do you think it will be to tune the DEADLOCK timer to a reasonable
> value? Would it have to vary based on load?

Right now the deadlock timeout is extremely noncritical; I doubt many
people actually bother to tune it.  I would rather not see us change
things in a way that makes that number become significant.

			regards, tom lane

Wed, 01 Aug 2001 16:23:11 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison" 
> > This is interesting. I'd like to hear what other people think about
> > this.
> 
> I doubt that this works --- it still has the same fundamental problem:
> that you're not telling the lock manager what it needs to know to
> fulfill its responsibility of detecting deadlock.  I believe I can still
> produce a ping-pong undetected deadlock example with the above behavior;
> it'd just take a few more processes.  The issue is that you are
> expecting the lock manager to detect or not detect deadlock, when you
> still have some lock requests up your sleeve that it's not seen yet.
> As long as you can block before presenting them all, it can never work.

I know there has been talk about having this done in the lock manager,
and I know it isn't worth the effort, but I am wondering how you would
do it even if you were doing in the lock manager with more information
available.

You could query all the locks at once but we really can already do that
now.  We could detect deadlock better.  Would that be the only
advantage?

Seems this whole idea maybe wasn't a good one.

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

Wed, 1 Aug 2001 21:45:15 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison"
Bruce Momjian <pgman@candle.pha.pa.us> writes:
>> it'd just take a few more processes.  The issue is that you are
>> expecting the lock manager to detect or not detect deadlock, when you
>> still have some lock requests up your sleeve that it's not seen yet.
>> As long as you can block before presenting them all, it can never work.

> I know there has been talk about having this done in the lock manager,
> and I know it isn't worth the effort, but I am wondering how you would
> do it even if you were doing in the lock manager with more information
> available.

I'd have to go back and study my 1980's-vintage operating system theory
textbooks before answering that ;-).  But acquisition of multiple locks
is a solved problem, AFAIR.

Likely we'd have to throw out the existing lockmanager datastructures
and start fresh, however --- they assume that a proc waits for only one
lock at a time.  It'd be a nontrivial bit of work.

			regards, tom lane

Thu, 02 Aug 2001 00:18:19 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison" 
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> >> it'd just take a few more processes.  The issue is that you are
> >> expecting the lock manager to detect or not detect deadlock, when you
> >> still have some lock requests up your sleeve that it's not seen yet.
> >> As long as you can block before presenting them all, it can never work.
> 
> > I know there has been talk about having this done in the lock manager,
> > and I know it isn't worth the effort, but I am wondering how you would
> > do it even if you were doing in the lock manager with more information
> > available.
> 
> I'd have to go back and study my 1980's-vintage operating system theory
> textbooks before answering that ;-).  But acquisition of multiple locks
> is a solved problem, AFAIR.
> 
> Likely we'd have to throw out the existing lockmanager datastructures
> and start fresh, however --- they assume that a proc waits for only one
> lock at a time.  It'd be a nontrivial bit of work.

Oh, OK.  Just checking.  It seems the starvation problem kept hitting us
as soon as we fixed the deadlock, cycling problem, and I was wondering
if there even was a solution.  My guess is that you would have to put
the multi-lock request in several lock queues and make sure they all got
done at some point.  A mess.

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

Thu, 2 Aug 2001 06:15:26 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison"
Bruce Momjian wrote:
> 
> Oh, OK.  Just checking.  It seems the starvation problem kept hitting us
> as soon as we fixed the deadlock, cycling problem, and I was wondering
> if there even was a solution.  My guess is that you would have to put
> the multi-lock request in several lock queues and make sure they all got
> done at some point.  A mess.
> 

What about having the syntax

LOCK a,b,c;

now just as a shorthand for 

LOCK a;
LOCK b;
LOCK c;

This would save typing and allow for Oracle compatibility.


If one day we get a multiple-lock facility inside the 
lock manager, we add the optional keyword "SIMULTANEOUSLY"
so that other mode is used instead.


One more reason for adding the "simple" version of the multiple
lock is that we may need it already.

I wonder how we handle

LOCK v;

where "v" is a view.  We should be locking all the base tables.

Suppose that the base tables for "v" are "a", "b" and "c".
In this case

LOCK v;

should be rewritten as

LOCK a,b,c;





-- 
Fernando Nasser
Red Hat - Toronto                       E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Thu, 02 Aug 2001 15:24:01 -0400
Fernando Nasser <fnasser@cygnus.com>
Re: Revised Patch to allow multiple table locks in "Unison"
Fernando Nasser <fnasser@cygnus.com> writes:
> What about having the syntax
> LOCK a,b,c;
> now just as a shorthand for 
> LOCK a;
> LOCK b;
> LOCK c;
> This would save typing and allow for Oracle compatibility.

This seems fine to me (and in fact I thought we'd already agreed to it).
Maybe some day we will get ambitious enough to make it do
parallel-locking, but for now we can get 80% of what we want with 0.8%
of the effort ;-)

> I wonder how we handle
> LOCK v;
> where "v" is a view.

regression=# create view v as select * from int4_tbl;
CREATE
regression=# lock table v;
ERROR:  LOCK TABLE: v is not a table

> We should be locking all the base tables.

I consider that debatable.  It hard-wires a rather constricted idea
of what the semantics of a view are.

			regards, tom lane

Thu, 02 Aug 2001 16:05:44 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison" 
Tom Lane wrote:
> 
> Fernando Nasser <fnasser@cygnus.com> writes:
> > What about having the syntax
> > LOCK a,b,c;
> > now just as a shorthand for
> > LOCK a;
> > LOCK b;
> > LOCK c;
> > This would save typing and allow for Oracle compatibility.
> 
> This seems fine to me (and in fact I thought we'd already agreed to it).

Here's the patch then. =)

Neil

-- 
Neil Padgett
Red Hat Canada Ltd.                       E-Mail:  npadgett@redhat.com
2323 Yonge Street, Suite #300, 
Toronto, ON  M4P 2C9

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/07/31 22:04:05
*************** 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/07/31 22:04:06
*************** _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/07/31 22:04:06
*************** _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/07/31 22:04:10
*************** 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/07/31 22:04:11
*************** 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/07/31 22:04:14
*************** 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);
  				}

Thu, 02 Aug 2001 16:11:51 -0400
Neil Padgett <npadgett@redhat.com>
Re: Revised Patch to allow multiple table locks in "Unison"
Tom Lane wrote:
> 
> Fernando Nasser <fnasser@cygnus.com> writes:
> > What about having the syntax
> > LOCK a,b,c;
> > now just as a shorthand for
> > LOCK a;
> > LOCK b;
> > LOCK c;
> > This would save typing and allow for Oracle compatibility.
> 
> This seems fine to me (and in fact I thought we'd already agreed to it).
> Maybe some day we will get ambitious enough to make it do
> parallel-locking, but for now we can get 80% of what we want with 0.8%
> of the effort ;-)
> 

Agreed.

> > I wonder how we handle
> > LOCK v;
> > where "v" is a view.
> 
> regression=# create view v as select * from int4_tbl;
> CREATE
> regression=# lock table v;
> ERROR:  LOCK TABLE: v is not a table
> 
> > We should be locking all the base tables.
> 
> I consider that debatable.  It hard-wires a rather constricted idea
> of what the semantics of a view are.
> 

I've only mentioned it because it is what Oracle does.  It says explicitly
(in their documentation) that if "table" is "LOCK VIEW table" is actually
a view, all base tables necessary to compute that view are locked.

I guess the principle (for Oracle folks) was that, for the user, there should
be no distinction between a real table and a view. Thus, it should not matter
for the user if the thing that is being locked is a real table or if it
is actually being implemented as a view.  Consider that it may have been 
a table one day, but the DBA changed it into a view.  So that SQL will
not work anymore and give the "ERROR:  LOCK TABLE: v is not a table" message.
This violates the Data Independence notion.



-- 
Fernando Nasser
Red Hat - Toronto                       E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Thu, 02 Aug 2001 16:19:36 -0400
Fernando Nasser <fnasser@cygnus.com>
Re: Revised Patch to allow multiple table locks in "Unison"
Fernando Nasser <fnasser@cygnus.com> writes:
> I guess the principle (for Oracle folks) was that, for the user, there should
> be no distinction between a real table and a view. Thus, it should not matter
> for the user if the thing that is being locked is a real table or if it
> is actually being implemented as a view.  Consider that it may have been 
> a table one day, but the DBA changed it into a view.  So that SQL will
> not work anymore and give the "ERROR:  LOCK TABLE: v is not a table" message.
> This violates the Data Independence notion.

I don't really buy this, because it makes life difficult for DBAs who
want to do creative things with views.  Update rules don't necessarily
touch exactly the same set of tables that are mentioned in the select
rule.  But that's the only set that a LOCK implementation might possibly
know about.

Consider: for the view as view (ie, select) there's no real need to do
locking at all.  The implicit read locks that will be grabbed as the
view is expanded will do fine.  For updates, the behavior can and should
be defined by the rewrite rules that the DBA supplies.  (Hmm, I'm not
sure that LOCK is one of the allowed query types in a rule --- if not,
it probably should be, so that the rule author can ensure the right
kinds of locks are grabbed in the right sequence.)

Another serious issue, which gets back to your original point, is that
we have no good idea what order to lock the base tables in.  If we had
a concurrent-lock implementation it wouldn't matter, but in the absence
of one I am not sure it's a good idea to put in a LOCK that is going to
lock base tables in some arbitrary order.

			regards, tom lane

Thu, 02 Aug 2001 16:58:50 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison" 
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.

			regards, tom lane

Thu, 02 Aug 2001 17:01:05 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison" 
> Fernando Nasser <fnasser@cygnus.com> writes:
> > What about having the syntax
> > LOCK a,b,c;
> > now just as a shorthand for 
> > LOCK a;
> > LOCK b;
> > LOCK c;
> > This would save typing and allow for Oracle compatibility.
> 
> This seems fine to me (and in fact I thought we'd already agreed to it).
> Maybe some day we will get ambitious enough to make it do
> parallel-locking, but for now we can get 80% of what we want with 0.8%
> of the effort ;-)

I think that was my point, that even in the lock manager, we would have
starvation problems and things would get very complicated.  In
hindsight, the idea of locking multiple tables in unison was just not
reasonable in PostgreSQL at this time.

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

Thu, 2 Aug 2001 17:32:20 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison"
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);
  				}

Thu, 02 Aug 2001 18:20:54 -0400
Neil Padgett <npadgett@redhat.com>
Re: Revised Patch to allow multiple table locks in "Unison"

Your patch has been added to the PostgreSQL unapplied patches list at:

	http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

> Tom Lane wrote:
> > 
> > Fernando Nasser <fnasser@cygnus.com> writes:
> > > What about having the syntax
> > > LOCK a,b,c;
> > > now just as a shorthand for
> > > LOCK a;
> > > LOCK b;
> > > LOCK c;
> > > This would save typing and allow for Oracle compatibility.
> > 
> > This seems fine to me (and in fact I thought we'd already agreed to it).
> 
> Here's the patch then. =)
> 
> Neil
> 
> -- 
> Neil Padgett
> Red Hat Canada Ltd.                       E-Mail:  npadgett@redhat.com
> 2323 Yonge Street, Suite #300, 
> Toronto, ON  M4P 2C9
> 
> 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/07/31 22:04:05
> *************** 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/07/31 22:04:06
> *************** _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/07/31 22:04:06
> *************** _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/07/31 22:04:10
> *************** 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/07/31 22:04:11
> *************** 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/07/31 22:04:14
> *************** 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 4: Don't 'kill -9' the postmaster
> 

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

Thu, 2 Aug 2001 18:23:03 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison"
Tom Lane wrote:
> 
> Fernando Nasser <fnasser@cygnus.com> writes:
> > I guess the principle (for Oracle folks) was that, for the user, there should
> > be no distinction between a real table and a view. Thus, it should not matter
> > for the user if the thing that is being locked is a real table or if it
> > is actually being implemented as a view.  Consider that it may have been
> > a table one day, but the DBA changed it into a view.  So that SQL will
> > not work anymore and give the "ERROR:  LOCK TABLE: v is not a table" message.
> > This violates the Data Independence notion.
> 
> I don't really buy this, because it makes life difficult for DBAs who
> want to do creative things with views.  Update rules don't necessarily
> touch exactly the same set of tables that are mentioned in the select
> rule.  But that's the only set that a LOCK implementation might possibly
> know about.
> 
> Consider: for the view as view (ie, select) there's no real need to do
> locking at all.  The implicit read locks that will be grabbed as the
> view is expanded will do fine.  For updates, the behavior can and should
> be defined by the rewrite rules that the DBA supplies.  (Hmm, I'm not
> sure that LOCK is one of the allowed query types in a rule --- if not,
> it probably should be, so that the rule author can ensure the right
> kinds of locks are grabbed in the right sequence.)
> 

These are good points.  I suppose Oracle needs this because they
have DBMS-implemented updatable views (not with rules as we do).

BTW, it seems we have a SQL non-conformance issue here: views that are
only projections+selections of a single base table are SQL-updatable.  
We should allow updates to those by rewriting them to refer to the base table.
And instead of just ignoring updates (unless we have rules in place) for
non-updatable views we should print some error like 
   "ERROR: attempt to modify non-updatable view".


> Another serious issue, which gets back to your original point, is that
> we have no good idea what order to lock the base tables in.  If we had
> a concurrent-lock implementation it wouldn't matter, but in the absence
> of one I am not sure it's a good idea to put in a LOCK that is going to
> lock base tables in some arbitrary order.
> 

This is true.  It should not be allowed (as it is not useful, as you've
pointed out) for non-updatable views.



-- 
Fernando Nasser
Red Hat - Toronto                       E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Thu, 02 Aug 2001 19:06:20 -0400
Fernando Nasser <fnasser@cygnus.com>
Re: Revised Patch to allow multiple table locks in "Unison"
Fernando Nasser <fnasser@cygnus.com> writes:
> BTW, it seems we have a SQL non-conformance issue here: views that are
> only projections+selections of a single base table are SQL-updatable.

Indeed.  In Postgres terms I think this means that if a CREATE VIEW
describes a view that meets the spec's constraints to be "updatable",
we should automatically create a default set of insert/update/delete
rules for it.  This is (or should be) on the TODO list.

			regards, tom lane

Thu, 02 Aug 2001 19:26:19 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison" 
Tom Lane wrote:
> 
> Fernando Nasser <fnasser@cygnus.com> writes:
> > BTW, it seems we have a SQL non-conformance issue here: views that are
> > only projections+selections of a single base table are SQL-updatable.
> 
> Indeed.  In Postgres terms I think this means that if a CREATE VIEW
> describes a view that meets the spec's constraints to be "updatable",
> we should automatically create a default set of insert/update/delete
> rules for it.  This is (or should be) on the TODO list.
> 

Agreed. 

We should also emit an error if someone tries to update a non-updatable view
(i.e., it is a view and there is no user defined rules for that update operation).
Silently ignoring the update scares me and I bet it is not what the standard
would tell us to do.  Any suggestion on how can we do this?  I thought of
adding default rules for those cases so they generate the error.  But we would
need an error() function or something to invoke from there. 

-- 
Fernando Nasser
Red Hat - Toronto                       E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Thu, 02 Aug 2001 19:58:25 -0400
Fernando Nasser <fnasser@cygnus.com>
Re: Revised Patch to allow multiple table locks in "Unison"
[ it's past time to move this thread over to pghackers ]

Fernando Nasser <fnasser@cygnus.com> writes:
> Tom Lane wrote:
>> Fernando Nasser <fnasser@cygnus.com> writes:
>>> BTW, it seems we have a SQL non-conformance issue here: views that are
>>> only projections+selections of a single base table are SQL-updatable.
>> 
>> Indeed.  In Postgres terms I think this means that if a CREATE VIEW
>> describes a view that meets the spec's constraints to be "updatable",
>> we should automatically create a default set of insert/update/delete
>> rules for it.  This is (or should be) on the TODO list.

> Agreed. 

> We should also emit an error if someone tries to update a
> non-updatable view (i.e., it is a view and there is no user defined
> rules for that update operation).  Silently ignoring the update scares
> me and I bet it is not what the standard would tell us to do.  Any
> suggestion on how can we do this?

It's already there as of 7.1:

regression=# create view v as select * from a;
CREATE
regression=# insert into v default values;
ERROR:  Cannot insert into a view without an appropriate rule
regression=#

The parts of the behavior that actually need some debate are what the
interaction should be between default rules and explicitly created rules
--- in particular, how not to break existing pg_dump scripts.  Here's
a very off-the-cuff suggestion that might or might not survive scrutiny:

1. Add an "is_default" boolean column to pg_rewrite.  This will always
be FALSE for entries made by explicit CREATE RULE commands, but will be
TRUE for entries created automatically when a CREATE VIEW is done for an
updatable view.

2. When a CREATE RULE is done, look to see if there is an is_default
rule for the same ev_class and ev_type (ie, same target table/view
and same action type).  If so, delete it.  This allows CREATE RULE
following CREATE VIEW to override the default rules.  A variant is to
delete *all* default rules for the target object regardless of action
type --- this might be safer, on the theory that if you have a
nondefault ON INSERT rule you likely don't want a default ON DELETE.

3. pg_dump would ignore (ie, not dump) is_default rules, knowing that
they'd get remade by CREATE VIEW.  This prevents default rules from
becoming "real" rules after a dump/reload cycle.

Comments?

			regards, tom lane

Thu, 02 Aug 2001 20:11:37 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Rules for updatable views (was Re: [PATCHES] Revised Patch to allow
	multiple table locks in "Unison")

Patch applied.  Thanks.  I had to edit it because the patch contents wrapped in
the email message.

> 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);
>   				}
> 

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

Sat, 4 Aug 2001 15:39:08 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison"
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

Sat, 04 Aug 2001 15:51:36 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison" 
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);
  				}
Sat, 4 Aug 2001 18:03:31 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison"
Tom Lane wrote:
> 
> 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.

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@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.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);
  				}

Tue, 07 Aug 2001 14:48:02 -0400
Neil Padgett <npadgett@redhat.com>
Re: Revised Patch to allow multiple table locks in "Unison"

Your patch has been added to the PostgreSQL unapplied patches list at:

	http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

> Tom Lane wrote:
> > 
> > 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.
> 
> 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@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.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@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

Wed, 8 Aug 2001 11:20:00 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison"

Patch applied.  Thanks.


> Tom Lane wrote:
> > 
> > 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.
> 
> 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@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.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@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

Fri, 10 Aug 2001 10:07:43 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Revised Patch to allow multiple table locks in "Unison"