Thread: Revised Patch to allow multiple table locks in "Unison"
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);
}
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 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
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
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
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
> 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
> 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
> > 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
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
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
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
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
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
> 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
> 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
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
> > 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
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
> 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
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
> 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
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
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
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
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
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
> 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
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
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
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
> > 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
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
> 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
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
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
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);
}
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
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
> 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
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
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
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
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);
}
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
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
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
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
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); }
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);
}
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
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