On 2024/09/10 1:02, Robert Haas wrote:
On Mon, Sep 9, 2024 at 10:37 AM Fujii Masao <masao.fu...@oss.nttdata.com> wrote:
In v8.2, the RULE privilege for tables was removed, but for backward 
compatibility,
GRANT/REVOKE RULE, has_table_privilege(..., 'RULE') etc are still accepted,
though they don't perform any actions.

Do we still need to maintain this backward compatibility?
Could we consider removing the RULE privilege entirely?

8.2 is a long time ago. If it's really been dead since then, I think
we should remove it.

Ok, so, patch attached.

There was a test to check if has_table_privilege() accepted the keyword RULE.
The patch removed it since it's now unnecessary and would only waste cycles
testing that has_table_privilege() no longer accepts the keyword.

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
From 8e16ba2cfc988031d27a1a5ccbc71169e1956933 Mon Sep 17 00:00:00 2001
From: Fujii Masao <fu...@postgresql.org>
Date: Mon, 9 Sep 2024 23:33:55 +0900
Subject: [PATCH v1] Remove old RULE privilege completely.

The RULE privilege for tables was removed in v8.2, but for backward
compatibility, GRANT/REVOKE and privilege functions like
has_table_privilege continued to accept the RULE keyword without any effect.

After discussions on pgsql-hackers, it was agreed that this compatibility
is no longer needed. Since it's been long enough since the deprecation,
we've decided to fully remove support for RULE privilege,
so GRANT/REVOKE and privilege functions will no longer accept it.
---
 src/backend/catalog/aclchk.c             | 2 --
 src/backend/utils/adt/acl.c              | 6 ------
 src/test/regress/expected/privileges.out | 9 ---------
 src/test/regress/sql/privileges.sql      | 4 ----
 4 files changed, 21 deletions(-)

diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index a44ccee3b6..d2abc48fd8 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -2641,8 +2641,6 @@ string_to_privilege(const char *privname)
                return ACL_ALTER_SYSTEM;
        if (strcmp(privname, "maintain") == 0)
                return ACL_MAINTAIN;
-       if (strcmp(privname, "rule") == 0)
-               return 0;                               /* ignore old RULE 
privileges */
        ereport(ERROR,
                        (errcode(ERRCODE_SYNTAX_ERROR),
                         errmsg("unrecognized privilege type \"%s\"", 
privname)));
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index d7b39140b3..4ad222b8d1 100644
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -341,9 +341,6 @@ aclparse(const char *s, AclItem *aip, Node *escontext)
                        case ACL_MAINTAIN_CHR:
                                read = ACL_MAINTAIN;
                                break;
-                       case 'R':                       /* ignore old RULE 
privileges */
-                               read = 0;
-                               break;
                        default:
                                ereturn(escontext, NULL,
                                                
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
@@ -1639,7 +1636,6 @@ makeaclitem(PG_FUNCTION_ARGS)
                {"SET", ACL_SET},
                {"ALTER SYSTEM", ACL_ALTER_SYSTEM},
                {"MAINTAIN", ACL_MAINTAIN},
-               {"RULE", 0},                    /* ignore old RULE privileges */
                {NULL, 0}
        };
 
@@ -2063,8 +2059,6 @@ convert_table_priv_string(text *priv_type_text)
                {"TRIGGER WITH GRANT OPTION", 
ACL_GRANT_OPTION_FOR(ACL_TRIGGER)},
                {"MAINTAIN", ACL_MAINTAIN},
                {"MAINTAIN WITH GRANT OPTION", 
ACL_GRANT_OPTION_FOR(ACL_MAINTAIN)},
-               {"RULE", 0},                    /* ignore old RULE privileges */
-               {"RULE WITH GRANT OPTION", 0},
                {NULL, 0}
        };
 
diff --git a/src/test/regress/expected/privileges.out 
b/src/test/regress/expected/privileges.out
index fab0cc800f..430f097114 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -1422,15 +1422,6 @@ from (select oid from pg_roles where rolname = 
current_user) as t2;
  t
 (1 row)
 
--- 'rule' privilege no longer exists, but for backwards compatibility
--- has_table_privilege still recognizes the keyword and says FALSE
-select has_table_privilege(current_user,t1.oid,'rule')
-from (select oid from pg_class where relname = 'pg_authid') as t1;
- has_table_privilege 
----------------------
- f
-(1 row)
-
 select has_table_privilege(current_user,t1.oid,'references')
 from (select oid from pg_class where relname = 'pg_authid') as t1;
  has_table_privilege 
diff --git a/src/test/regress/sql/privileges.sql 
b/src/test/regress/sql/privileges.sql
index ae338e8cc8..e55b32f9d4 100644
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -1004,10 +1004,6 @@ from (select oid from pg_roles where rolname = 
current_user) as t2;
 select has_table_privilege(t2.oid,'pg_authid','delete')
 from (select oid from pg_roles where rolname = current_user) as t2;
 
--- 'rule' privilege no longer exists, but for backwards compatibility
--- has_table_privilege still recognizes the keyword and says FALSE
-select has_table_privilege(current_user,t1.oid,'rule')
-from (select oid from pg_class where relname = 'pg_authid') as t1;
 select has_table_privilege(current_user,t1.oid,'references')
 from (select oid from pg_class where relname = 'pg_authid') as t1;
 
-- 
2.45.2

Reply via email to