Re: [HACKERS] Inconsistent syntax in GRANT - Mailing list pgsql-patches

Tom Lane wrote:
> I wrote:
> > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> >> Does the standard require USAGE to support currval?
>
> > currval isn't in the standard (unless I missed something), so it has
> > nothing to say one way or the other on the point.
>
> Wait, I take that back.  Remember our previous discussions about this
> point: the spec's NEXT VALUE FOR construct is *not* equivalent to
> nextval, because they specify that the sequence advances just once per
> command even if the command says NEXT VALUE FOR in multiple places.
> This means that NEXT VALUE FOR is effectively both nextval and currval;
> the first one in a command does nextval and the rest do currval.
>
> Accordingly, I think it's reasonable to read the spec as saying that
> USAGE privilege encompasses both nextval and currval.

Here's a patch that more closely matches the ideas proposed.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
Index: doc/src/sgml/ref/grant.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/ref/grant.sgml,v
retrieving revision 1.50
diff -c -c -r1.50 grant.sgml
*** doc/src/sgml/ref/grant.sgml    20 Oct 2005 19:18:01 -0000    1.50
--- doc/src/sgml/ref/grant.sgml    7 Jan 2006 06:00:14 -0000
***************
*** 25,30 ****
--- 25,35 ----
      ON [ TABLE ] <replaceable class="PARAMETER">tablename</replaceable> [, ...]
      TO { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable
class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] [ WITH GRANT OPTION ] 

+ GRANT { { SELECT | USAGE | UPDATE }
+     [,...] | ALL [ PRIVILEGES ] }
+     ON SEQUENCE <replaceable class="PARAMETER">tablename</replaceable> [, ...]
+     TO { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable
class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] [ WITH GRANT OPTION ] 
+
  GRANT { { CREATE | TEMPORARY | TEMP } [,...] | ALL [ PRIVILEGES ] }
      ON DATABASE <replaceable>dbname</replaceable> [, ...]
      TO { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable
class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] [ WITH GRANT OPTION ] 
***************
*** 260,265 ****
--- 265,274 ----
         also met).  Essentially this allows the grantee to <quote>look up</>
         objects within the schema.
        </para>
+       <para>
+        For sequences, this privilege allows the use of the
+        <function>currval</function> and <function>nextval</function> functions.
+       </para>
       </listitem>
      </varlistentry>

***************
*** 511,517 ****

     <para>
      The <literal>RULE</literal> privilege, and privileges on
!     databases, tablespaces, schemas, languages, and sequences are
      <productname>PostgreSQL</productname> extensions.
     </para>
   </refsect1>
--- 520,526 ----

     <para>
      The <literal>RULE</literal> privilege, and privileges on
!     databases, tablespaces, schemas, and languages are
      <productname>PostgreSQL</productname> extensions.
     </para>
   </refsect1>
Index: doc/src/sgml/ref/revoke.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/ref/revoke.sgml,v
retrieving revision 1.35
diff -c -c -r1.35 revoke.sgml
*** doc/src/sgml/ref/revoke.sgml    20 Oct 2005 19:18:01 -0000    1.35
--- doc/src/sgml/ref/revoke.sgml    7 Jan 2006 06:00:14 -0000
***************
*** 28,33 ****
--- 28,40 ----
      [ CASCADE | RESTRICT ]

  REVOKE [ GRANT OPTION FOR ]
+     { { SELECT | UPDATE }
+     [,...] | ALL [ PRIVILEGES ] }
+     ON SEQUENCE <replaceable class="PARAMETER">tablename</replaceable> [, ...]
+     FROM { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable
class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] 
+     [ CASCADE | RESTRICT ]
+
+ REVOKE [ GRANT OPTION FOR ]
      { { CREATE | TEMPORARY | TEMP } [,...] | ALL [ PRIVILEGES ] }
      ON DATABASE <replaceable>dbname</replaceable> [, ...]
      FROM { <replaceable class="PARAMETER">username</replaceable> | GROUP <replaceable
class="PARAMETER">groupname</replaceable>| PUBLIC } [, ...] 
Index: src/backend/catalog/aclchk.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/catalog/aclchk.c,v
retrieving revision 1.123
diff -c -c -r1.123 aclchk.c
*** src/backend/catalog/aclchk.c    1 Dec 2005 02:03:00 -0000    1.123
--- src/backend/catalog/aclchk.c    7 Jan 2006 06:00:27 -0000
***************
*** 286,291 ****
--- 286,295 ----
              all_privileges = ACL_ALL_RIGHTS_RELATION;
              errormsg = _("invalid privilege type %s for table");
              break;
+         case ACL_OBJECT_SEQUENCE:
+             all_privileges = ACL_ALL_RIGHTS_SEQUENCE;
+             errormsg = _("invalid privilege type %s for sequence");
+             break;
          case ACL_OBJECT_DATABASE:
              all_privileges = ACL_ALL_RIGHTS_DATABASE;
              errormsg = _("invalid privilege type %s for database");
***************
*** 356,361 ****
--- 360,366 ----
      switch (istmt->objtype)
      {
          case ACL_OBJECT_RELATION:
+         case ACL_OBJECT_SEQUENCE:
              ExecGrant_Relation(istmt);
              break;
          case ACL_OBJECT_DATABASE:
***************
*** 395,400 ****
--- 400,406 ----
      switch (objtype)
      {
          case ACL_OBJECT_RELATION:
+         case ACL_OBJECT_SEQUENCE:
              foreach(cell, objnames)
              {
                  Oid            relOid;
***************
*** 577,582 ****
--- 583,602 ----
                       errmsg("\"%s\" is a composite type",
                              NameStr(pg_class_tuple->relname))));

+         if (istmt->objtype == ACL_OBJECT_SEQUENCE)
+         {
+             if (pg_class_tuple->relkind != RELKIND_SEQUENCE)
+                 ereport(ERROR,
+                         (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+                          errmsg("\"%s\" is not a sequence",
+                                 NameStr(pg_class_tuple->relname))));
+             if (istmt->privileges != ACL_ALL_RIGHTS_RELATION &&
+                 istmt->privileges & ~(ACL_SELECT | ACL_USAGE | ACL_UPDATE))
+                 ereport(ERROR,
+                         (errcode(ERRCODE_INVALID_GRANT_OPERATION),
+                          errmsg("sequences only support SELECT, USAGE, and UPDATE privileges")));
+         }
+
          /*
           * Get owner ID and working copy of existing ACL. If there's no ACL,
           * substitute the proper default.
Index: src/backend/catalog/pg_shdepend.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/catalog/pg_shdepend.c,v
retrieving revision 1.6
diff -c -c -r1.6 pg_shdepend.c
*** src/backend/catalog/pg_shdepend.c    1 Dec 2005 02:03:00 -0000    1.6
--- src/backend/catalog/pg_shdepend.c    7 Jan 2006 06:00:29 -0000
***************
*** 1133,1138 ****
--- 1133,1139 ----
                      switch (sdepForm->classid)
                      {
                          case RelationRelationId:
+                             /* could be a sequence */
                              istmt.objtype = ACL_OBJECT_RELATION;
                              break;
                          case DatabaseRelationId:
Index: src/backend/commands/sequence.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/sequence.c,v
retrieving revision 1.126
diff -c -c -r1.126 sequence.c
*** src/backend/commands/sequence.c    22 Nov 2005 18:17:09 -0000    1.126
--- src/backend/commands/sequence.c    7 Jan 2006 06:00:30 -0000
***************
*** 422,428 ****
      /* open and AccessShareLock sequence */
      init_sequence(relid, &elm, &seqrel);

!     if (pg_class_aclcheck(elm->relid, GetUserId(), ACL_UPDATE) != ACLCHECK_OK)
          ereport(ERROR,
                  (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
                   errmsg("permission denied for sequence %s",
--- 422,429 ----
      /* open and AccessShareLock sequence */
      init_sequence(relid, &elm, &seqrel);

!     if (pg_class_aclcheck(elm->relid, GetUserId(), ACL_USAGE) != ACLCHECK_OK &&
!         pg_class_aclcheck(elm->relid, GetUserId(), ACL_UPDATE) != ACLCHECK_OK)
          ereport(ERROR,
                  (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
                   errmsg("permission denied for sequence %s",
***************
*** 613,619 ****
      /* open and AccessShareLock sequence */
      init_sequence(relid, &elm, &seqrel);

!     if (pg_class_aclcheck(elm->relid, GetUserId(), ACL_SELECT) != ACLCHECK_OK)
          ereport(ERROR,
                  (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
                   errmsg("permission denied for sequence %s",
--- 614,621 ----
      /* open and AccessShareLock sequence */
      init_sequence(relid, &elm, &seqrel);

!     if (pg_class_aclcheck(elm->relid, GetUserId(), ACL_SELECT) != ACLCHECK_OK &&
!         pg_class_aclcheck(elm->relid, GetUserId(), ACL_USAGE) != ACLCHECK_OK)
          ereport(ERROR,
                  (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
                   errmsg("permission denied for sequence %s",
***************
*** 657,663 ****
      /* nextval() must have already been called for this sequence */
      Assert(last_used_seq->increment != 0);

!     if (pg_class_aclcheck(last_used_seq->relid, GetUserId(), ACL_SELECT) != ACLCHECK_OK)
          ereport(ERROR,
                  (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
                   errmsg("permission denied for sequence %s",
--- 659,666 ----
      /* nextval() must have already been called for this sequence */
      Assert(last_used_seq->increment != 0);

!     if (pg_class_aclcheck(last_used_seq->relid, GetUserId(), ACL_SELECT) != ACLCHECK_OK &&
!         pg_class_aclcheck(last_used_seq->relid, GetUserId(), ACL_USAGE) != ACLCHECK_OK)
          ereport(ERROR,
                  (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
                   errmsg("permission denied for sequence %s",
Index: src/backend/parser/gram.y
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/gram.y,v
retrieving revision 2.521
diff -c -c -r2.521 gram.y
*** src/backend/parser/gram.y    29 Dec 2005 04:53:18 -0000    2.521
--- src/backend/parser/gram.y    7 Jan 2006 06:00:36 -0000
***************
*** 3322,3327 ****
--- 3322,3334 ----
                      n->objs = $2;
                      $$ = n;
                  }
+             | SEQUENCE qualified_name_list
+                 {
+                     PrivTarget *n = makeNode(PrivTarget);
+                     n->objtype = ACL_OBJECT_SEQUENCE;
+                     n->objs = $2;
+                     $$ = n;
+                 }
              | FUNCTION function_with_argtypes_list
                  {
                      PrivTarget *n = makeNode(PrivTarget);
Index: src/backend/utils/adt/acl.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/acl.c,v
retrieving revision 1.129
diff -c -c -r1.129 acl.c
*** src/backend/utils/adt/acl.c    18 Nov 2005 02:38:23 -0000    1.129
--- src/backend/utils/adt/acl.c    7 Jan 2006 06:00:39 -0000
***************
*** 542,547 ****
--- 542,548 ----
      switch (objtype)
      {
          case ACL_OBJECT_RELATION:
+         case ACL_OBJECT_SEQUENCE:
              world_default = ACL_NO_RIGHTS;
              owner_default = ACL_ALL_RIGHTS_RELATION;
              break;
Index: src/bin/pg_dump/dumputils.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/pg_dump/dumputils.c,v
retrieving revision 1.23
diff -c -c -r1.23 dumputils.c
*** src/bin/pg_dump/dumputils.c    3 Dec 2005 21:06:18 -0000    1.23
--- src/bin/pg_dump/dumputils.c    7 Jan 2006 06:00:42 -0000
***************
*** 22,28 ****
  #define supports_grant_options(version) ((version) >= 70400)

  static bool parseAclItem(const char *item, const char *type, const char *name,
!              int remoteVersion,
               PQExpBuffer grantee, PQExpBuffer grantor,
               PQExpBuffer privs, PQExpBuffer privswgo);
  static char *copyAclUserName(PQExpBuffer output, char *input);
--- 22,28 ----
  #define supports_grant_options(version) ((version) >= 70400)

  static bool parseAclItem(const char *item, const char *type, const char *name,
!              int remoteVersion, bool *is_valid_for_sequence,
               PQExpBuffer grantee, PQExpBuffer grantor,
               PQExpBuffer privs, PQExpBuffer privswgo);
  static char *copyAclUserName(PQExpBuffer output, char *input);
***************
*** 395,404 ****
--- 395,417 ----
      /* Scan individual ACL items */
      for (i = 0; i < naclitems; i++)
      {
+         const char *outType = type;
+         bool    is_valid_for_sequence;
+
          if (!parseAclItem(aclitems[i], type, name, remoteVersion,
+                           &is_valid_for_sequence,
                            grantee, grantor, privs, privswgo))
              return false;

+         /*
+          *    For backward compatibility, non-SEQUENCE GRANT statements can
+          *    assign non-SELECT, non-UPDATE permissions.  We allow those
+          *    to be dumped by changing SEQUENCE to TABLE.  USAGE can be
+          *    assigned only to sequences.
+          */
+         if (strcmp(outType, "SEQUENCE") == 0 && !is_valid_for_sequence)
+             outType = "TABLE";
+
          if (grantor->len == 0 && owner)
              printfPQExpBuffer(grantor, "%s", owner);

***************
*** 419,433 ****
                      : strcmp(privs->data, "ALL") != 0)
                  {
                      appendPQExpBuffer(firstsql, "REVOKE ALL ON %s %s FROM %s;\n",
!                                       type, name,
                                        fmtId(grantee->data));
                      if (privs->len > 0)
                          appendPQExpBuffer(firstsql, "GRANT %s ON %s %s TO %s;\n",
!                                           privs->data, type, name,
                                            fmtId(grantee->data));
                      if (privswgo->len > 0)
                          appendPQExpBuffer(firstsql, "GRANT %s ON %s %s TO %s WITH GRANT OPTION;\n",
!                                           privswgo->data, type, name,
                                            fmtId(grantee->data));
                  }
              }
--- 432,446 ----
                      : strcmp(privs->data, "ALL") != 0)
                  {
                      appendPQExpBuffer(firstsql, "REVOKE ALL ON %s %s FROM %s;\n",
!                                       outType, name,
                                        fmtId(grantee->data));
                      if (privs->len > 0)
                          appendPQExpBuffer(firstsql, "GRANT %s ON %s %s TO %s;\n",
!                                           privs->data, outType, name,
                                            fmtId(grantee->data));
                      if (privswgo->len > 0)
                          appendPQExpBuffer(firstsql, "GRANT %s ON %s %s TO %s WITH GRANT OPTION;\n",
!                                           privswgo->data, outType, name,
                                            fmtId(grantee->data));
                  }
              }
***************
*** 444,450 ****
                  if (privs->len > 0)
                  {
                      appendPQExpBuffer(secondsql, "GRANT %s ON %s %s TO ",
!                                       privs->data, type, name);
                      if (grantee->len == 0)
                          appendPQExpBuffer(secondsql, "PUBLIC;\n");
                      else if (strncmp(grantee->data, "group ",
--- 457,463 ----
                  if (privs->len > 0)
                  {
                      appendPQExpBuffer(secondsql, "GRANT %s ON %s %s TO ",
!                                       privs->data, outType, name);
                      if (grantee->len == 0)
                          appendPQExpBuffer(secondsql, "PUBLIC;\n");
                      else if (strncmp(grantee->data, "group ",
***************
*** 457,463 ****
                  if (privswgo->len > 0)
                  {
                      appendPQExpBuffer(secondsql, "GRANT %s ON %s %s TO ",
!                                       privswgo->data, type, name);
                      if (grantee->len == 0)
                          appendPQExpBuffer(secondsql, "PUBLIC");
                      else if (strncmp(grantee->data, "group ",
--- 470,476 ----
                  if (privswgo->len > 0)
                  {
                      appendPQExpBuffer(secondsql, "GRANT %s ON %s %s TO ",
!                                       privswgo->data, outType, name);
                      if (grantee->len == 0)
                          appendPQExpBuffer(secondsql, "PUBLIC");
                      else if (strncmp(grantee->data, "group ",
***************
*** 480,489 ****
       * If we didn't find any owner privs, the owner must have revoked 'em all
       */
      if (!found_owner_privs && owner)
-     {
          appendPQExpBuffer(firstsql, "REVOKE ALL ON %s %s FROM %s;\n",
                            type, name, fmtId(owner));
-     }

      destroyPQExpBuffer(grantee);
      destroyPQExpBuffer(grantor);
--- 493,500 ----
***************
*** 517,523 ****
   */
  static bool
  parseAclItem(const char *item, const char *type, const char *name,
!              int remoteVersion,
               PQExpBuffer grantee, PQExpBuffer grantor,
               PQExpBuffer privs, PQExpBuffer privswgo)
  {
--- 528,534 ----
   */
  static bool
  parseAclItem(const char *item, const char *type, const char *name,
!              int remoteVersion, bool *is_valid_for_sequence,
               PQExpBuffer grantee, PQExpBuffer grantor,
               PQExpBuffer privs, PQExpBuffer privswgo)
  {
***************
*** 530,535 ****
--- 541,548 ----

      buf = strdup(item);

+     *is_valid_for_sequence = true;
+
      /* user or group name is string up to = */
      eqpos = copyAclUserName(grantee, buf);
      if (*eqpos != '=')
***************
*** 547,554 ****
--- 560,573 ----
      else
          resetPQExpBuffer(grantor);

+     if (strcmp(type, "SEQUENCE") == 0 &&
+         /* SELECT, USAGE, UPDATE, ALL */
+         strspn(eqpos + 1, "rUw*") != strlen(eqpos + 1))
+         *is_valid_for_sequence = false;
+
      /* privilege codes */
  #define CONVERT_PRIV(code, keywd) \
+ do { \
      if ((pos = strchr(eqpos + 1, code))) \
      { \
          if (*(pos + 1) == '*') \
***************
*** 563,578 ****
          } \
      } \
      else \
!         all_with_go = all_without_go = false

      resetPQExpBuffer(privs);
      resetPQExpBuffer(privswgo);

!     if (strcmp(type, "TABLE") == 0)
      {
          CONVERT_PRIV('a', "INSERT");
          CONVERT_PRIV('r', "SELECT");
          CONVERT_PRIV('R', "RULE");

          if (remoteVersion >= 70200)
          {
--- 582,600 ----
          } \
      } \
      else \
!         all_with_go = all_without_go = false; \
! } while (0)

      resetPQExpBuffer(privs);
      resetPQExpBuffer(privswgo);

!     if (strcmp(type, "TABLE") == 0 || strcmp(type, "SEQUENCE") == 0)
      {
          CONVERT_PRIV('a', "INSERT");
          CONVERT_PRIV('r', "SELECT");
          CONVERT_PRIV('R', "RULE");
+         if (strcmp(type, "SEQUENCE") == 0)
+             CONVERT_PRIV('U', "USAGE");

          if (remoteVersion >= 70200)
          {
Index: src/bin/pg_dump/pg_dump.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/pg_dump/pg_dump.c,v
retrieving revision 1.425
diff -c -c -r1.425 pg_dump.c
*** src/bin/pg_dump/pg_dump.c    6 Jan 2006 19:08:33 -0000    1.425
--- src/bin/pg_dump/pg_dump.c    7 Jan 2006 06:00:49 -0000
***************
*** 6776,6782 ****

          /* Handle the ACL here */
          namecopy = strdup(fmtId(tbinfo->dobj.name));
!         dumpACL(fout, tbinfo->dobj.catId, tbinfo->dobj.dumpId, "TABLE",
                  namecopy, tbinfo->dobj.name,
                  tbinfo->dobj.namespace->dobj.name, tbinfo->rolname,
                  tbinfo->relacl);
--- 6776,6784 ----

          /* Handle the ACL here */
          namecopy = strdup(fmtId(tbinfo->dobj.name));
!         dumpACL(fout, tbinfo->dobj.catId, tbinfo->dobj.dumpId,
!                 /* Issue GRANT SEQUENCE, if applicable */
!                 tbinfo->relkind != RELKIND_SEQUENCE ? "TABLE" : "SEQUENCE",
                  namecopy, tbinfo->dobj.name,
                  tbinfo->dobj.namespace->dobj.name, tbinfo->rolname,
                  tbinfo->relacl);
Index: src/include/nodes/parsenodes.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/nodes/parsenodes.h,v
retrieving revision 1.298
diff -c -c -r1.298 parsenodes.h
*** src/include/nodes/parsenodes.h    7 Dec 2005 15:20:55 -0000    1.298
--- src/include/nodes/parsenodes.h    7 Jan 2006 06:00:52 -0000
***************
*** 884,890 ****
   */
  typedef enum GrantObjectType
  {
!     ACL_OBJECT_RELATION,        /* table, view, sequence */
      ACL_OBJECT_DATABASE,        /* database */
      ACL_OBJECT_FUNCTION,        /* function */
      ACL_OBJECT_LANGUAGE,        /* procedural language */
--- 884,891 ----
   */
  typedef enum GrantObjectType
  {
!     ACL_OBJECT_RELATION,        /* table, view */
!     ACL_OBJECT_SEQUENCE,        /* sequence */
      ACL_OBJECT_DATABASE,        /* database */
      ACL_OBJECT_FUNCTION,        /* function */
      ACL_OBJECT_LANGUAGE,        /* procedural language */
Index: src/include/utils/acl.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/utils/acl.h,v
retrieving revision 1.91
diff -c -c -r1.91 acl.h
*** src/include/utils/acl.h    1 Dec 2005 02:03:01 -0000    1.91
--- src/include/utils/acl.h    7 Jan 2006 06:00:52 -0000
***************
*** 143,148 ****
--- 143,149 ----
   * Bitmasks defining "all rights" for each supported object type
   */
  #define ACL_ALL_RIGHTS_RELATION
(ACL_INSERT|ACL_SELECT|ACL_UPDATE|ACL_DELETE|ACL_RULE|ACL_REFERENCES|ACL_TRIGGER)
+ #define ACL_ALL_RIGHTS_SEQUENCE        (ACL_SELECT|ACL_UPDATE|ACL_USAGE)
  #define ACL_ALL_RIGHTS_DATABASE        (ACL_CREATE|ACL_CREATE_TEMP)
  #define ACL_ALL_RIGHTS_FUNCTION        (ACL_EXECUTE)
  #define ACL_ALL_RIGHTS_LANGUAGE        (ACL_USAGE)