Re: Revised Patch to allow multiple table locks in "Unison" - Mailing list pgsql-patches

From Bruce Momjian
Subject Re: Revised Patch to allow multiple table locks in "Unison"
Date
Msg-id 200108041939.f74Jd8D16889@candle.pha.pa.us
Whole thread Raw
In response to Re: Revised Patch to allow multiple table locks in "Unison"  (Neil Padgett <npadgett@redhat.com>)
Responses Re: Revised Patch to allow multiple table locks in "Unison"  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-patches
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

pgsql-patches by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: Full Text Indexing Patch
Next
From: Bruce Momjian
Date:
Subject: Re: [HACKERS] Small patch for Hurd