[PATCH] unalias of ACL_SELECT_FOR_UPDATE - Mailing list pgsql-hackers

Currently, the ACL_SELECT_FOR_UPDATE privilege is defined as an alias
of ACL_UPDATE as follows:

  at src/include/nodes/parsenodes.h:
           :
  /* Currently, SELECT ... FOR UPDATE/FOR SHARE requires UPDATE privileges */
  #define ACL_SELECT_FOR_UPDATE   ACL_UPDATE
           :

It is unconfortable for us because SE-PostgreSQL have two individual
permissions for updates (db_table:{update}) and explicit table locks
(db_table:{lock}), but it unables to discriminate whether the given
relation is actually used for UPDATE or SELECT FOR UPDATE.
So, I would like to define the bit as (1<<12) which is not in use.

I think we have two options to solve the matter.

The one is to check ACL_SELECT_FOR_UPDATE by UPDATE privilege, as
the attached patch doing. It implicitly expands the users privilege
when he has ACL_UPDATE, so it enables GRANT UPDATE to cover both of
ACL_UPDATE and ACL_SELECT_FOR_UPDATE as the current implementation
doing.

The other is to add a new privilege for explicit table locks, such
as something like LOCK privilege. It is a straightforward approach,
but we need to have a user visible changes.

Which is more preferable design?

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>
*** base/src/include/nodes/parsenodes.h    2009-04-09 00:13:21.000000000 +0900
--- base_ex/src/include/nodes/parsenodes.h    2009-04-09 00:41:07.000000000 +0900
*************** typedef uint32 AclMode;            /* a bitmask o
*** 71,80 ****
  #define ACL_CREATE        (1<<9)    /* for namespaces and databases */
  #define ACL_CREATE_TEMP (1<<10) /* for databases */
  #define ACL_CONNECT        (1<<11) /* for databases */
- #define N_ACL_RIGHTS    12        /* 1 plus the last 1<<x */
- #define ACL_NO_RIGHTS    0
  /* Currently, SELECT ... FOR UPDATE/FOR SHARE requires UPDATE privileges */
! #define ACL_SELECT_FOR_UPDATE    ACL_UPDATE


  /*****************************************************************************
--- 71,80 ----
  #define ACL_CREATE        (1<<9)    /* for namespaces and databases */
  #define ACL_CREATE_TEMP (1<<10) /* for databases */
  #define ACL_CONNECT        (1<<11) /* for databases */
  /* Currently, SELECT ... FOR UPDATE/FOR SHARE requires UPDATE privileges */
! #define ACL_SELECT_FOR_UPDATE    (1<<12)
! #define N_ACL_RIGHTS    13        /* 1 plus the last 1<<x */
! #define ACL_NO_RIGHTS    0


  /*****************************************************************************
*** base/src/backend/utils/adt/acl.c    2009-02-09 13:08:34.000000000 +0900
--- base_ex/src/backend/utils/adt/acl.c    2009-04-06 11:49:32.000000000 +0900
*************** aclmask(const Acl *acl, Oid roleid, Oid
*** 1141,1151 ****
      for (i = 0; i < num; i++)
      {
          AclItem    *aidata = &aidat[i];

          if (aidata->ai_grantee == ACL_ID_PUBLIC ||
              aidata->ai_grantee == roleid)
          {
!             result |= aidata->ai_privs & mask;
              if ((how == ACLMASK_ALL) ? (result == mask) : (result != 0))
                  return result;
          }
--- 1141,1156 ----
      for (i = 0; i < num; i++)
      {
          AclItem    *aidata = &aidat[i];
+         AclMode        aiprivs = aidata->ai_privs;
+
+         /* fixup ACL_SELECT_FOR_UPDATE */
+         if (aiprivs & ACL_UPDATE)
+             aiprivs |= ACL_SELECT_FOR_UPDATE;

          if (aidata->ai_grantee == ACL_ID_PUBLIC ||
              aidata->ai_grantee == roleid)
          {
!             result |= aiprivs & mask;
              if ((how == ACLMASK_ALL) ? (result == mask) : (result != 0))
                  return result;
          }
*************** aclmask(const Acl *acl, Oid roleid, Oid
*** 1162,1167 ****
--- 1167,1177 ----
      for (i = 0; i < num; i++)
      {
          AclItem    *aidata = &aidat[i];
+         AclMode        aiprivs = aidata->ai_privs;
+
+         /* fixup ACL_SELECT_FOR_UPDATE */
+         if (aiprivs & ACL_UPDATE)
+             aiprivs |= ACL_SELECT_FOR_UPDATE;

          if (aidata->ai_grantee == ACL_ID_PUBLIC ||
              aidata->ai_grantee == roleid)
*************** aclmask(const Acl *acl, Oid roleid, Oid
*** 1170,1176 ****
          if ((aidata->ai_privs & remaining) &&
              has_privs_of_role(roleid, aidata->ai_grantee))
          {
!             result |= aidata->ai_privs & mask;
              if ((how == ACLMASK_ALL) ? (result == mask) : (result != 0))
                  return result;
              remaining = mask & ~result;
--- 1180,1186 ----
          if ((aidata->ai_privs & remaining) &&
              has_privs_of_role(roleid, aidata->ai_grantee))
          {
!             result |= aiprivs & mask;
              if ((how == ACLMASK_ALL) ? (result == mask) : (result != 0))
                  return result;
              remaining = mask & ~result;