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

Bruce Momjian wrote:
> > I'm not sure offhand what keywords we'd want to use, but now is the time
> > to look at it, *before* it becomes set in stone that GRANT ON SEQUENCE
> > is just another spelling of GRANT ON TABLE.
>
> Sequences do not support INSERT, UPDATE, or DELETE, but we overload
> UPDATE to control nextval()/setval(), so I just allowed SELECT and
> UPDATE.  I am not sure it makes any sense to allow rules, references,
> and triggers on sequences.  However, using ALL or TABLE keywords you can
> define those permissions to a sequence.

Here is an updated patch.  The standard doesn't have GRANT VIEW so I
didn't implement that.

One tricky issue I realized is that we should dump out GRANT SEQUENCE,
if possible.  I have added code to check in pg_dump and use GRANT
SEQUENCE if only SELECT, UPDATE, or ALL are used.

--
  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    6 Jan 2006 20:33:45 -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 | 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 ] 
***************
*** 511,517 ****

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

     <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    6 Jan 2006 20:33:46 -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    6 Jan 2006 20:33:46 -0000
***************
*** 283,288 ****
--- 283,289 ----
      switch (stmt->objtype)
      {
          case ACL_OBJECT_RELATION:
+         case ACL_OBJECT_SEQUENCE:
              all_privileges = ACL_ALL_RIGHTS_RELATION;
              errormsg = _("invalid privilege type %s for table");
              break;
***************
*** 356,361 ****
--- 357,363 ----
      switch (istmt->objtype)
      {
          case ACL_OBJECT_RELATION:
+         case ACL_OBJECT_SEQUENCE:
              ExecGrant_Relation(istmt);
              break;
          case ACL_OBJECT_DATABASE:
***************
*** 395,400 ****
--- 397,403 ----
      switch (objtype)
      {
          case ACL_OBJECT_RELATION:
+         case ACL_OBJECT_SEQUENCE:
              foreach(cell, objnames)
              {
                  Oid            relOid;
***************
*** 577,582 ****
--- 580,599 ----
                       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_UPDATE))
+                 ereport(ERROR,
+                         (errcode(ERRCODE_INVALID_GRANT_OPERATION),
+                          errmsg("sequences only support SELECT 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    6 Jan 2006 20:33:47 -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/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    6 Jan 2006 20:33:49 -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    6 Jan 2006 20:33:50 -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    6 Jan 2006 20:33:51 -0000
***************
*** 395,404 ****
--- 395,420 ----
      /* Scan individual ACL items */
      for (i = 0; i < naclitems; i++)
      {
+         const char *outType = type;
+
          if (!parseAclItem(aclitems[i], type, name, remoteVersion,
                            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.
+          */
+         if (strcmp(outType, "SEQUENCE") == 0)
+         {
+             if (strcmp(privs->data, "SELECT") != 0 &&
+                 strcmp(privs->data, "UPDATE") != 0 &&
+                 strcmp(privs->data, "SELECT,UPDATE") != 0 &&
+                 strcmp(privs->data, "ALL") != 0)
+             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));
                  }
              }
--- 435,449 ----
                      : 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 ",
--- 460,466 ----
                  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 ",
--- 473,479 ----
                  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);
--- 496,503 ----
***************
*** 568,574 ****
      resetPQExpBuffer(privs);
      resetPQExpBuffer(privswgo);

!     if (strcmp(type, "TABLE") == 0)
      {
          CONVERT_PRIV('a', "INSERT");
          CONVERT_PRIV('r', "SELECT");
--- 582,588 ----
      resetPQExpBuffer(privs);
      resetPQExpBuffer(privswgo);

!     if (strcmp(type, "TABLE") == 0 || strcmp(type, "SEQUENCE") == 0)
      {
          CONVERT_PRIV('a', "INSERT");
          CONVERT_PRIV('r', "SELECT");
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    6 Jan 2006 20:33:56 -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    6 Jan 2006 20:33:56 -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 */