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

Tom Lane wrote:
>
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > Patch applied.
>
> Idly looking this over again, I noticed a big OOPS:
>
> >> ! freeList(lockstmt->rellist);
>
> >> ! pfree(relname);
>
> >> ! pfree(relname);
>
> It is most definitely NOT the executor's business to release pieces of
> the querytree; this will certainly break plpgsql functions, for example,
> where the same querytree is executed repeatedly.

Thanks for having a look, Tom. I've taken you advice into account, and
I've reworked the patch.

A new patch is below.

Neil

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

Index: doc/src/sgml/ref/lock.sgml
===================================================================
RCS file:
/home/projects/pgsql/cvsroot/pgsql/doc/src/sgml/ref/lock.sgml,v
retrieving revision 1.26
diff -c -p -r1.26 lock.sgml
*** doc/src/sgml/ref/lock.sgml 2001/08/04 22:01:38 1.26
--- doc/src/sgml/ref/lock.sgml 2001/08/07 18:34:23
*************** Postgres documentation
*** 15,21 ****
LOCK
</refname>
<refpurpose>
! Explicitly lock a table inside a transaction
</refpurpose>
</refnamediv>
<refsynopsisdiv>
--- 15,21 ----
LOCK
</refname>
<refpurpose>
! Explicitly lock a table / tables inside a transaction
</refpurpose>
</refnamediv>
<refsynopsisdiv>
*************** Postgres documentation
*** 23,30 ****
<date>2001-07-09</date>
</refsynopsisdivinfo>
<synopsis>
! LOCK [ TABLE ] <replaceable class="PARAMETER">name</replaceable>
! LOCK [ TABLE ] <replaceable class="PARAMETER">name</replaceable> IN
<replaceable class="PARAMETER">lockmode</replaceable> MODE

where <replaceable class="PARAMETER">lockmode</replaceable> is one of:

--- 23,30 ----
<date>2001-07-09</date>
</refsynopsisdivinfo>
<synopsis>
! LOCK [ TABLE ] <replaceable class="PARAMETER">name</replaceable> [,
...]
! LOCK [ TABLE ] <replaceable class="PARAMETER">name</replaceable> [,
...] IN <replaceable class="PARAMETER">lockmode</replaceable> MODE

where <replaceable class="PARAMETER">lockmode</replaceable> is one of:

*************** ERROR <replaceable class="PARAMETER">nam
*** 373,378 ****
--- 373,379 ----
An example for this rule was given previously when discussing the
use of SHARE ROW EXCLUSIVE mode rather than SHARE mode.
</para>
+
</listitem>
</itemizedlist>

*************** ERROR <replaceable class="PARAMETER">nam
*** 383,388 ****
--- 384,395 ----
</para>
</note>

+ <para>
+ When locking multiple tables, the command LOCK a, b; is equivalent
to LOCK
+ a; LOCK b;. The tables are locked one-by-one in the order specified
in the
+ <command>LOCK</command> command.
+ </para>
+
<refsect2 id="R2-SQL-LOCK-3">
<refsect2info>
<date>1999-06-08</date>
*************** ERROR <replaceable class="PARAMETER">nam
*** 406,411 ****
--- 413,419 ----
<para>
<command>LOCK</command> works only inside transactions.
</para>
+
</refsect2>
</refsect1>

Index: src/backend/commands/command.c
===================================================================
RCS file:
/home/projects/pgsql/cvsroot/pgsql/src/backend/commands/command.c,v
retrieving revision 1.138
diff -c -p -r1.138 command.c
*** src/backend/commands/command.c 2001/08/04 22:01:38 1.138
--- src/backend/commands/command.c 2001/08/07 18:34:24
*************** needs_toast_table(Relation rel)
*** 1984,1991 ****
MAXALIGN(data_length);
return (tuple_length > TOAST_TUPLE_THRESHOLD);
}
!
!
/*
*
* LOCK TABLE
--- 1984,1990 ----
MAXALIGN(data_length);
return (tuple_length > TOAST_TUPLE_THRESHOLD);
}
!
/*
*
* LOCK TABLE
*************** needs_toast_table(Relation rel)
*** 1994,2019 ****
void
LockTableCommand(LockStmt *lockstmt)
{
! Relation rel;
! int aclresult;
!
! rel = heap_openr(lockstmt->relname, NoLock);
!
! if (rel->rd_rel->relkind != RELKIND_RELATION)
! elog(ERROR, "LOCK TABLE: %s is not a table", lockstmt->relname);
!
! if (lockstmt->mode == AccessShareLock)
! aclresult = pg_aclcheck(lockstmt->relname, GetUserId(), ACL_SELECT);
! else
! aclresult = pg_aclcheck(lockstmt->relname, GetUserId(),
! ACL_UPDATE | ACL_DELETE);
!
! if (aclresult != ACLCHECK_OK)
! elog(ERROR, "LOCK TABLE: permission denied");
!
! LockRelation(rel, lockstmt->mode);
!
! heap_close(rel, NoLock); /* close rel, keep lock */
}


--- 1993,2030 ----
void
LockTableCommand(LockStmt *lockstmt)
{
! List *p;
! Relation rel;
!
! /* Iterate over the list and open, lock, and close the relations
! one at a time
! */
!
! foreach(p, lockstmt->rellist)
! {
! char* relname = strVal(lfirst(p));
! int aclresult;
!
! rel = heap_openr(relname, NoLock);
!
! if (rel->rd_rel->relkind != RELKIND_RELATION)
! elog(ERROR, "LOCK TABLE: %s is not a table",
! relname);
!
! if (lockstmt->mode == AccessShareLock)
! aclresult = pg_aclcheck(relname, GetUserId(),
! ACL_SELECT);
! else
! aclresult = pg_aclcheck(relname, GetUserId(),
! ACL_UPDATE | ACL_DELETE);
!
! if (aclresult != ACLCHECK_OK)
! elog(ERROR, "LOCK TABLE: permission denied");
!
! LockRelation(rel, lockstmt->mode);
!
! heap_close(rel, NoLock); /* close rel, keep lock */
! }
}


Index: src/backend/nodes/copyfuncs.c
===================================================================
RCS file:
/home/projects/pgsql/cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v
retrieving revision 1.150
diff -c -p -r1.150 copyfuncs.c
*** src/backend/nodes/copyfuncs.c 2001/08/04 22:01:38 1.150
--- src/backend/nodes/copyfuncs.c 2001/08/07 18:34:24
*************** _copyLockStmt(LockStmt *from)
*** 2425,2432 ****
{
LockStmt *newnode = makeNode(LockStmt);

! if (from->relname)
! newnode->relname = pstrdup(from->relname);
newnode->mode = from->mode;

return newnode;
--- 2425,2432 ----
{
LockStmt *newnode = makeNode(LockStmt);

! Node_Copy(from, newnode, rellist);
!
newnode->mode = from->mode;

return newnode;
Index: src/backend/nodes/equalfuncs.c
===================================================================
RCS file:
/home/projects/pgsql/cvsroot/pgsql/src/backend/nodes/equalfuncs.c,v
retrieving revision 1.98
diff -c -p -r1.98 equalfuncs.c
*** src/backend/nodes/equalfuncs.c 2001/08/04 22:01:38 1.98
--- src/backend/nodes/equalfuncs.c 2001/08/07 18:34:24
*************** _equalDropUserStmt(DropUserStmt *a, Drop
*** 1283,1289 ****
static bool
_equalLockStmt(LockStmt *a, LockStmt *b)
{
! if (!equalstr(a->relname, b->relname))
return false;
if (a->mode != b->mode)
return false;
--- 1283,1289 ----
static bool
_equalLockStmt(LockStmt *a, LockStmt *b)
{
! if (!equal(a->rellist, b->rellist))
return false;
if (a->mode != b->mode)
return false;
Index: src/backend/parser/gram.y
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/parser/gram.y,v
retrieving revision 2.241
diff -c -p -r2.241 gram.y
*** src/backend/parser/gram.y 2001/08/06 05:42:48 2.241
--- src/backend/parser/gram.y 2001/08/07 18:34:30
*************** DeleteStmt: DELETE FROM relation_expr w
*** 3281,3291 ****
}
;

! LockStmt: LOCK_P opt_table relation_name opt_lock
{
LockStmt *n = makeNode(LockStmt);

! n->relname = $3;
n->mode = $4;
$$ = (Node *)n;
}
--- 3281,3291 ----
}
;

! LockStmt: LOCK_P opt_table relation_name_list opt_lock
{
LockStmt *n = makeNode(LockStmt);

! n->rellist = $3;
n->mode = $4;
$$ = (Node *)n;
}
Index: src/include/nodes/parsenodes.h
===================================================================
RCS file:
/home/projects/pgsql/cvsroot/pgsql/src/include/nodes/parsenodes.h,v
retrieving revision 1.138
diff -c -p -r1.138 parsenodes.h
*** src/include/nodes/parsenodes.h 2001/08/04 22:01:39 1.138
--- src/include/nodes/parsenodes.h 2001/08/07 18:34:30
*************** typedef struct VariableResetStmt
*** 760,766 ****
typedef struct LockStmt
{
NodeTag type;
! char *relname; /* relation to lock */
int mode; /* lock mode */
} LockStmt;

--- 760,766 ----
typedef struct LockStmt
{
NodeTag type;
! List *rellist; /* relations to lock */
int mode; /* lock mode */
} LockStmt;

Index: src/interfaces/ecpg/preproc/preproc.y
===================================================================
RCS file:
/home/projects/pgsql/cvsroot/pgsql/src/interfaces/ecpg/preproc/preproc.y,v
retrieving revision 1.148
diff -c -p -r1.148 preproc.y
*** src/interfaces/ecpg/preproc/preproc.y 2001/08/04 22:01:39 1.148
--- src/interfaces/ecpg/preproc/preproc.y 2001/08/07 18:34:32
*************** DeleteStmt: DELETE FROM relation_expr w
*** 2421,2427 ****
}
;

! LockStmt: LOCK_P opt_table relation_name opt_lock
{
$$ = cat_str(4, make_str("lock"), $2, $3, $4);
}
--- 2421,2427 ----
}
;

! LockStmt: LOCK_P opt_table relation_name_list opt_lock
{
$$ = cat_str(4, make_str("lock"), $2, $3, $4);
}