agora inbox for pgsql-hackers@postgresql.orghelp / color / mirror / Atom feed
[PATCH v1 1/2] add grantable privileges for cluster, refresh matview, and reindex 2+ messages / 2 participants [nested] [flat]
* [PATCH v1 1/2] add grantable privileges for cluster, refresh matview, and reindex @ 2022-12-07 19:20 Nathan Bossart <nathandbossart@gmail.com> 0 siblings, 0 replies; 2+ messages in thread From: Nathan Bossart @ 2022-12-07 19:20 UTC (permalink / raw) --- doc/src/sgml/ddl.sgml | 62 ++++++++++-- doc/src/sgml/func.sgml | 5 +- .../sgml/ref/alter_default_privileges.sgml | 4 +- doc/src/sgml/ref/cluster.sgml | 6 +- doc/src/sgml/ref/grant.sgml | 5 +- .../sgml/ref/refresh_materialized_view.sgml | 3 +- doc/src/sgml/ref/reindex.sgml | 6 +- doc/src/sgml/ref/revoke.sgml | 2 +- src/backend/catalog/aclchk.c | 12 +++ src/backend/commands/cluster.c | 19 ++-- src/backend/commands/indexcmds.c | 31 +++--- src/backend/commands/matview.c | 3 +- src/backend/commands/tablecmds.c | 17 ++-- src/backend/utils/adt/acl.c | 24 +++++ src/bin/pg_dump/dumputils.c | 3 + src/bin/pg_dump/t/002_pg_dump.pl | 2 +- src/bin/psql/tab-complete.c | 4 +- src/include/commands/tablecmds.h | 4 +- src/include/nodes/parsenodes.h | 5 +- src/include/utils/acl.h | 7 +- src/test/regress/expected/create_index.out | 4 +- src/test/regress/expected/dependency.out | 22 ++--- src/test/regress/expected/privileges.out | 96 ++++++++++++++----- src/test/regress/expected/rowsecurity.out | 34 +++---- src/test/regress/sql/dependency.sql | 2 +- src/test/regress/sql/privileges.sql | 61 ++++++++++++ 26 files changed, 333 insertions(+), 110 deletions(-) diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml index 38618de01c..eec49debfe 100644 --- a/doc/src/sgml/ddl.sgml +++ b/doc/src/sgml/ddl.sgml @@ -1692,8 +1692,9 @@ ALTER TABLE products RENAME TO items; <literal>TRUNCATE</literal>, <literal>REFERENCES</literal>, <literal>TRIGGER</literal>, <literal>CREATE</literal>, <literal>CONNECT</literal>, <literal>TEMPORARY</literal>, <literal>EXECUTE</literal>, <literal>USAGE</literal>, <literal>SET</literal>, - <literal>ALTER SYSTEM</literal>, <literal>VACUUM</literal>, and - <literal>ANALYZE</literal>. + <literal>ALTER SYSTEM</literal>, <literal>VACUUM</literal>, + <literal>ANALYZE</literal>, <literal>CLUSTER</literal>, + <literal>REFRESH</literal>, and <literal>REINDEX</literal>. The privileges applicable to a particular object vary depending on the object's type (table, function, etc.). More detail about the meanings of these privileges appears below. @@ -2001,6 +2002,34 @@ REVOKE ALL ON accounts FROM PUBLIC; </para> </listitem> </varlistentry> + + <varlistentry> + <term><literal>CLUSTER</literal></term> + <listitem> + <para> + Allows <command>CLUSTER</command> on a relation. + </para> + </listitem> + </varlistentry> + + <varlistentry> + <term><literal>REFRESH</literal></term> + <listitem> + <para> + Allows <command>REFRESH MATERIALIZED VIEW</command> on a materialized + view. + </para> + </listitem> + </varlistentry> + + <varlistentry> + <term><literal>REINDEX</literal</term> + <listitem> + <para> + Allows <command>REINDEX</command> on a relation and its indexes. + </para> + </listitem> + </varlistentry> </variablelist> The privileges required by other commands are listed on the @@ -2160,6 +2189,21 @@ REVOKE ALL ON accounts FROM PUBLIC; <entry><literal>z</literal></entry> <entry><literal>TABLE</literal></entry> </row> + <row> + <entry><literal>CLUSTER</literal></entry> + <entry><literal>S</literal></entry> + <entry><literal>TABLE</literal></entry> + </row> + <row> + <entry><literal>REFRESH</literal></entry> + <entry><literal>f</literal></entry> + <entry><literal>TABLE</literal></entry> + </row> + <row> + <entry><literal>REINDEX</literal></entry> + <entry><literal>n</literal></entry> + <entry><literal>TABLE</literal></entry> + </row> </tbody> </tgroup> </table> @@ -2250,7 +2294,7 @@ REVOKE ALL ON accounts FROM PUBLIC; </row> <row> <entry><literal>TABLE</literal> (and table-like objects)</entry> - <entry><literal>arwdDxtvz</literal></entry> + <entry><literal>arwdDxtvzSfn</literal></entry> <entry>none</entry> <entry><literal>\dp</literal></entry> </row> @@ -2308,12 +2352,12 @@ GRANT SELECT (col1), UPDATE (col1) ON mytable TO miriam_rw; would show: <programlisting> => \dp mytable - Access privileges - Schema | Name | Type | Access privileges | Column privileges | Policies ---------+---------+-------+-------------------------+-----------------------+---------- - public | mytable | table | miriam=arwdDxtvz/miriam+| col1: +| - | | | =r/miriam +| miriam_rw=rw/miriam | - | | | admin=arw/miriam | | + Access privileges + Schema | Name | Type | Access privileges | Column privileges | Policies +--------+---------+-------+----------------------------+-----------------------+---------- + public | mytable | table | miriam=arwdDxtvzSfn/miriam+| col1: +| + | | | =r/miriam +| miriam_rw=rw/miriam | + | | | admin=arw/miriam | | (1 row) </programlisting> </para> diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index e57ffce971..06d54e47ba 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -22995,8 +22995,9 @@ SELECT has_function_privilege('joeuser', 'myfunc(int, text)', 'execute'); are <literal>SELECT</literal>, <literal>INSERT</literal>, <literal>UPDATE</literal>, <literal>DELETE</literal>, <literal>TRUNCATE</literal>, <literal>REFERENCES</literal>, - <literal>TRIGGER</literal>, <literal>VACUUM</literal> and - <literal>ANALYZE</literal>. + <literal>TRIGGER</literal>, <literal>VACUUM</literal>, + <literal>ANALYZE</literal>, <literal>CLUSTER</literal>, + <literal>REFRESH</literal>, and <literal>REINDEX</literal>. </para></entry> </row> diff --git a/doc/src/sgml/ref/alter_default_privileges.sgml b/doc/src/sgml/ref/alter_default_privileges.sgml index 0da295daff..9153359edc 100644 --- a/doc/src/sgml/ref/alter_default_privileges.sgml +++ b/doc/src/sgml/ref/alter_default_privileges.sgml @@ -28,7 +28,7 @@ ALTER DEFAULT PRIVILEGES <phrase>where <replaceable class="parameter">abbreviated_grant_or_revoke</replaceable> is one of:</phrase> -GRANT { { SELECT | INSERT | UPDATE | DELETE | TRUNCATE | REFERENCES | TRIGGER | VACUUM | ANALYZE } +GRANT { { SELECT | INSERT | UPDATE | DELETE | TRUNCATE | REFERENCES | TRIGGER | VACUUM | ANALYZE | CLUSTER | REFRESH | REINDEX } [, ...] | ALL [ PRIVILEGES ] } ON TABLES TO { [ GROUP ] <replaceable class="parameter">role_name</replaceable> | PUBLIC } [, ...] [ WITH GRANT OPTION ] @@ -51,7 +51,7 @@ GRANT { USAGE | CREATE | ALL [ PRIVILEGES ] } TO { [ GROUP ] <replaceable class="parameter">role_name</replaceable> | PUBLIC } [, ...] [ WITH GRANT OPTION ] REVOKE [ GRANT OPTION FOR ] - { { SELECT | INSERT | UPDATE | DELETE | TRUNCATE | REFERENCES | TRIGGER | VACUUM | ANALYZE } + { { SELECT | INSERT | UPDATE | DELETE | TRUNCATE | REFERENCES | TRIGGER | VACUUM | ANALYZE | CLUSTER | REFRESH | REINDEX } [, ...] | ALL [ PRIVILEGES ] } ON TABLES FROM { [ GROUP ] <replaceable class="parameter">role_name</replaceable> | PUBLIC } [, ...] diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml index c37f4236f1..24eccab251 100644 --- a/doc/src/sgml/ref/cluster.sgml +++ b/doc/src/sgml/ref/cluster.sgml @@ -69,9 +69,9 @@ CLUSTER [VERBOSE] <para> <command>CLUSTER</command> without any parameter reclusters all the previously-clustered tables in the current database that the calling user - owns, or all such tables if called by a superuser. This - form of <command>CLUSTER</command> cannot be executed inside a transaction - block. + owns or has the <literal>CLUSTER</literal> privilege for, or all such tables + if called by a superuser. This form of <command>CLUSTER</command> cannot be + executed inside a transaction block. </para> <para> diff --git a/doc/src/sgml/ref/grant.sgml b/doc/src/sgml/ref/grant.sgml index c3c585be7e..fff8cffb07 100644 --- a/doc/src/sgml/ref/grant.sgml +++ b/doc/src/sgml/ref/grant.sgml @@ -21,7 +21,7 @@ PostgreSQL documentation <refsynopsisdiv> <synopsis> -GRANT { { SELECT | INSERT | UPDATE | DELETE | TRUNCATE | REFERENCES | TRIGGER | VACUUM | ANALYZE } +GRANT { { SELECT | INSERT | UPDATE | DELETE | TRUNCATE | REFERENCES | TRIGGER | VACUUM | ANALYZE | CLUSTER | REFRESH | REINDEX } [, ...] | ALL [ PRIVILEGES ] } ON { [ TABLE ] <replaceable class="parameter">table_name</replaceable> [, ...] | ALL TABLES IN SCHEMA <replaceable class="parameter">schema_name</replaceable> [, ...] } @@ -195,6 +195,9 @@ GRANT <replaceable class="parameter">role_name</replaceable> [, ...] TO <replace <term><literal>ALTER SYSTEM</literal></term> <term><literal>VACUUM</literal></term> <term><literal>ANALYZE</literal></term> + <term><literal>CLUSTER</literal></term> + <term><literal>REFRESH</literal></term> + <term><literal>REINDEX</literal></term> <listitem> <para> Specific types of privileges, as defined in <xref linkend="ddl-priv"/>. diff --git a/doc/src/sgml/ref/refresh_materialized_view.sgml b/doc/src/sgml/ref/refresh_materialized_view.sgml index 675d6090f3..90b3de5274 100644 --- a/doc/src/sgml/ref/refresh_materialized_view.sgml +++ b/doc/src/sgml/ref/refresh_materialized_view.sgml @@ -32,7 +32,8 @@ REFRESH MATERIALIZED VIEW [ CONCURRENTLY ] <replaceable class="parameter">name</ <para> <command>REFRESH MATERIALIZED VIEW</command> completely replaces the contents of a materialized view. To execute this command you must be the - owner of the materialized view. The old contents are discarded. If + owner of the materialized view or have the <literal>REFRESH</literal> + privilege on the materialized view. The old contents are discarded. If <literal>WITH DATA</literal> is specified (or defaults) the backing query is executed to provide the new data, and the materialized view is left in a scannable state. If <literal>WITH NO DATA</literal> is specified no new diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml index fcbda88149..560e536b3c 100644 --- a/doc/src/sgml/ref/reindex.sgml +++ b/doc/src/sgml/ref/reindex.sgml @@ -293,14 +293,16 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { DA <para> Reindexing a single index or table requires being the owner of that - index or table. Reindexing a schema or database requires being the + index or table or having the <literal>REINDEX</literal> privilege on the + table. Reindexing a schema or database requires being the owner of that schema or database. Note specifically that it's thus possible for non-superusers to rebuild indexes of tables owned by other users. However, as a special exception, when <command>REINDEX DATABASE</command>, <command>REINDEX SCHEMA</command> or <command>REINDEX SYSTEM</command> is issued by a non-superuser, indexes on shared catalogs will be skipped unless the user owns the - catalog (which typically won't be the case). Of course, superusers + catalog (which typically won't be the case) or has the + <literal>REINDEX</literal> privilege on the catalog. Of course, superusers can always reindex anything. </para> diff --git a/doc/src/sgml/ref/revoke.sgml b/doc/src/sgml/ref/revoke.sgml index e28d192fd3..7da9ece998 100644 --- a/doc/src/sgml/ref/revoke.sgml +++ b/doc/src/sgml/ref/revoke.sgml @@ -22,7 +22,7 @@ PostgreSQL documentation <refsynopsisdiv> <synopsis> REVOKE [ GRANT OPTION FOR ] - { { SELECT | INSERT | UPDATE | DELETE | TRUNCATE | REFERENCES | TRIGGER | VACUUM | ANALYZE } + { { SELECT | INSERT | UPDATE | DELETE | TRUNCATE | REFERENCES | TRIGGER | VACUUM | ANALYZE | CLUSTER | REFRESH | REINDEX } [, ...] | ALL [ PRIVILEGES ] } ON { [ TABLE ] <replaceable class="parameter">table_name</replaceable> [, ...] | ALL TABLES IN SCHEMA <replaceable>schema_name</replaceable> [, ...] } diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c index bd967eaa78..b0b44b3fb2 100644 --- a/src/backend/catalog/aclchk.c +++ b/src/backend/catalog/aclchk.c @@ -3424,6 +3424,12 @@ string_to_privilege(const char *privname) return ACL_VACUUM; if (strcmp(privname, "analyze") == 0) return ACL_ANALYZE; + if (strcmp(privname, "cluster") == 0) + return ACL_CLUSTER; + if (strcmp(privname, "refresh") == 0) + return ACL_REFRESH; + if (strcmp(privname, "reindex") == 0) + return ACL_REINDEX; if (strcmp(privname, "rule") == 0) return 0; /* ignore old RULE privileges */ ereport(ERROR, @@ -3469,6 +3475,12 @@ privilege_to_string(AclMode privilege) return "VACUUM"; case ACL_ANALYZE: return "ANALYZE"; + case ACL_CLUSTER: + return "CLUSTER"; + case ACL_REFRESH: + return "REFRESH"; + case ACL_REINDEX: + return "REINDEX"; default: elog(ERROR, "unrecognized privilege: %d", (int) privilege); } diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 07e091bb87..c39a9cd221 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -138,6 +138,7 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) { /* This is the single-relation case. */ Oid tableOid; + AclMode acl = ACL_CLUSTER; /* * Find, lock, and check permissions on the table. We obtain @@ -147,7 +148,8 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) tableOid = RangeVarGetRelidExtended(stmt->relation, AccessExclusiveLock, 0, - RangeVarCallbackOwnsTable, NULL); + RangeVarCallbackForTablePrivs, + &acl); rel = table_open(tableOid, NoLock); /* @@ -364,8 +366,9 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params) */ if (recheck) { - /* Check that the user still owns the relation */ - if (!object_ownercheck(RelationRelationId, tableOid, save_userid)) + /* Check that the user still has privileges for the relation */ + if (!object_ownercheck(RelationRelationId, tableOid, save_userid) && + pg_class_aclcheck(tableOid, save_userid, ACL_CLUSTER) != ACLCHECK_OK) { relation_close(OldHeap, AccessExclusiveLock); goto out; @@ -1612,7 +1615,7 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap, /* - * Get a list of tables that the current user owns and + * Get a list of tables that the current user has privileges on and * have indisclustered set. Return the list in a List * of RelToCluster * (stored in the specified memory context), each one giving the tableOid * and the indexOid on which the table is already clustered. @@ -1629,8 +1632,8 @@ get_tables_to_cluster(MemoryContext cluster_context) List *rtcs = NIL; /* - * Get all indexes that have indisclustered set and are owned by - * appropriate user. + * Get all indexes that have indisclustered set and that the current user + * has the appropriate privileges for. */ indRelation = table_open(IndexRelationId, AccessShareLock); ScanKeyInit(&entry, @@ -1644,7 +1647,8 @@ get_tables_to_cluster(MemoryContext cluster_context) index = (Form_pg_index) GETSTRUCT(indexTuple); - if (!object_ownercheck(RelationRelationId, index->indrelid, GetUserId())) + if (!object_ownercheck(RelationRelationId, index->indrelid, GetUserId()) && + pg_class_aclcheck(index->indrelid, GetUserId(), ACL_CLUSTER) != ACLCHECK_OK) continue; /* Use a permanent memory context for the result list */ @@ -1694,6 +1698,7 @@ get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid indexOid) /* Silently skip partitions which the user has no access to. */ if (!object_ownercheck(RelationRelationId, relid, GetUserId()) && + pg_class_aclcheck(relid, GetUserId(), ACL_CLUSTER) != ACLCHECK_OK && (!object_ownercheck(DatabaseRelationId, MyDatabaseId, GetUserId()) || IsSharedRelation(relid))) continue; diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index b5b860c3ab..e30e2ef844 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -2754,6 +2754,7 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation, char relkind; struct ReindexIndexCallbackState *state = arg; LOCKMODE table_lockmode; + Oid table_oid; /* * Lock level here should match table lock in reindex_index() for @@ -2793,14 +2794,17 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation, errmsg("\"%s\" is not an index", relation->relname))); /* Check permissions */ - if (!object_ownercheck(RelationRelationId, relId, GetUserId())) - aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_INDEX, relation->relname); + table_oid = IndexGetRelation(relId, true); + if (!object_ownercheck(RelationRelationId, relId, GetUserId()) && + OidIsValid(table_oid) && + pg_class_aclcheck(table_oid, GetUserId(), ACL_REINDEX) != ACLCHECK_OK) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("permission denied for \"%s\"", relation->relname))); /* Lock heap before index to avoid deadlock. */ if (relId != oldRelId) { - Oid table_oid = IndexGetRelation(relId, true); - /* * If the OID isn't valid, it means the index was concurrently * dropped, which is not a problem for us; just return normally. @@ -2822,6 +2826,7 @@ ReindexTable(RangeVar *relation, ReindexParams *params, bool isTopLevel) { Oid heapOid; bool result; + AclMode acl = ACL_REINDEX; /* * The lock level used here should match reindex_relation(). @@ -2835,7 +2840,7 @@ ReindexTable(RangeVar *relation, ReindexParams *params, bool isTopLevel) (params->options & REINDEXOPT_CONCURRENTLY) != 0 ? ShareUpdateExclusiveLock : ShareLock, 0, - RangeVarCallbackOwnsTable, NULL); + RangeVarCallbackForTablePrivs, &acl); if (get_rel_relkind(heapOid) == RELKIND_PARTITIONED_TABLE) ReindexPartitions(heapOid, params, isTopLevel); @@ -3001,15 +3006,17 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, continue; /* - * The table can be reindexed if the user is superuser, the table - * owner, or the database/schema owner (but in the latter case, only - * if it's not a shared relation). object_ownercheck includes the - * superuser case, and depending on objectKind we already know that - * the user has permission to run REINDEX on this database or schema - * per the permission checks at the beginning of this routine. + * The table can be reindexed if the user has been granted REINDEX on + * the table or the user is a superuser, the table owner, or the + * database/schema owner (but in the latter case, only if it's not a + * shared relation). object_ownercheck includes the superuser case, + * and depending on objectKind we already know that the user has + * permission to run REINDEX on this database or schema per the + * permission checks at the beginning of this routine. */ if (classtuple->relisshared && - !object_ownercheck(RelationRelationId, relid, GetUserId())) + !object_ownercheck(RelationRelationId, relid, GetUserId()) && + pg_class_aclcheck(relid, GetUserId(), ACL_REINDEX) != ACLCHECK_OK) continue; /* diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c index 9ac0383459..c3d1d3d321 100644 --- a/src/backend/commands/matview.c +++ b/src/backend/commands/matview.c @@ -155,6 +155,7 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString, int save_sec_context; int save_nestlevel; ObjectAddress address; + AclMode acl = ACL_REFRESH; /* Determine strength of lock needed. */ concurrent = stmt->concurrent; @@ -165,7 +166,7 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString, */ matviewOid = RangeVarGetRelidExtended(stmt->relation, lockmode, 0, - RangeVarCallbackOwnsTable, NULL); + RangeVarCallbackForTablePrivs, &acl); matviewRel = table_open(matviewOid, NoLock); relowner = matviewRel->rd_rel->relowner; diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index ee88e87d76..bcc11b8017 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -16930,13 +16930,13 @@ AtEOSubXact_on_commit_actions(bool isCommit, SubTransactionId mySubid, * This is intended as a callback for RangeVarGetRelidExtended(). It allows * the relation to be locked only if (1) it's a plain or partitioned table, * materialized view, or TOAST table and (2) the current user is the owner (or - * the superuser). This meets the permission-checking needs of CLUSTER, - * REINDEX TABLE, and REFRESH MATERIALIZED VIEW; we expose it here so that it - * can be used by all. + * the superuser) or has been granted any of the privileges specified in *acl. + * This meets the permission-checking needs of CLUSTER, REINDEX TABLE, and + * REFRESH MATERIALIZED VIEW; we expose it here so that it can be used by all. */ void -RangeVarCallbackOwnsTable(const RangeVar *relation, - Oid relId, Oid oldRelId, void *arg) +RangeVarCallbackForTablePrivs(const RangeVar *relation, + Oid relId, Oid oldRelId, void *acl) { char relkind; @@ -16959,8 +16959,11 @@ RangeVarCallbackOwnsTable(const RangeVar *relation, errmsg("\"%s\" is not a table or materialized view", relation->relname))); /* Check permissions */ - if (!object_ownercheck(RelationRelationId, relId, GetUserId())) - aclcheck_error(ACLCHECK_NOT_OWNER, get_relkind_objtype(get_rel_relkind(relId)), relation->relname); + if (!object_ownercheck(RelationRelationId, relId, GetUserId()) && + pg_class_aclcheck(relId, GetUserId(), *((AclMode *) acl)) != ACLCHECK_OK) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("permission denied for \"%s\"", relation->relname))); } /* diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c index ed1b6a41cf..022de60f93 100644 --- a/src/backend/utils/adt/acl.c +++ b/src/backend/utils/adt/acl.c @@ -327,6 +327,15 @@ aclparse(const char *s, AclItem *aip) case ACL_ANALYZE_CHR: read = ACL_ANALYZE; break; + case ACL_CLUSTER_CHR: + read = ACL_CLUSTER; + break; + case ACL_REFRESH_CHR: + read = ACL_REFRESH; + break; + case ACL_REINDEX_CHR: + read = ACL_REINDEX; + break; case 'R': /* ignore old RULE privileges */ read = 0; break; @@ -1603,6 +1612,9 @@ makeaclitem(PG_FUNCTION_ARGS) {"ALTER SYSTEM", ACL_ALTER_SYSTEM}, {"VACUUM", ACL_VACUUM}, {"ANALYZE", ACL_ANALYZE}, + {"CLUSTER", ACL_CLUSTER}, + {"REFRESH", ACL_REFRESH}, + {"REINDEX", ACL_REINDEX}, {"RULE", 0}, /* ignore old RULE privileges */ {NULL, 0} }; @@ -1715,6 +1727,12 @@ convert_aclright_to_string(int aclright) return "VACUUM"; case ACL_ANALYZE: return "ANALYZE"; + case ACL_CLUSTER: + return "CLUSTER"; + case ACL_REFRESH: + return "REFRESH"; + case ACL_REINDEX: + return "REINDEX"; default: elog(ERROR, "unrecognized aclright: %d", aclright); return NULL; @@ -2028,6 +2046,12 @@ convert_table_priv_string(text *priv_type_text) {"VACUUM WITH GRANT OPTION", ACL_GRANT_OPTION_FOR(ACL_VACUUM)}, {"ANALYZE", ACL_ANALYZE}, {"ANALYZE WITH GRANT OPTION", ACL_GRANT_OPTION_FOR(ACL_ANALYZE)}, + {"CLUSTER", ACL_CLUSTER}, + {"CLUSTER WITH GRANT OPTION", ACL_GRANT_OPTION_FOR(ACL_CLUSTER)}, + {"REFRESH", ACL_REFRESH}, + {"REFRESH WITH GRANT OPTION", ACL_GRANT_OPTION_FOR(ACL_REFRESH)}, + {"REINDEX", ACL_REINDEX}, + {"REINDEX WITH GRANT OPTION", ACL_GRANT_OPTION_FOR(ACL_REINDEX)}, {"RULE", 0}, /* ignore old RULE privileges */ {"RULE WITH GRANT OPTION", 0}, {NULL, 0} diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c index 9311417f18..f6f94913f4 100644 --- a/src/bin/pg_dump/dumputils.c +++ b/src/bin/pg_dump/dumputils.c @@ -459,6 +459,9 @@ do { \ CONVERT_PRIV('D', "TRUNCATE"); CONVERT_PRIV('v', "VACUUM"); CONVERT_PRIV('z', "ANALYZE"); + CONVERT_PRIV('S', "CLUSTER"); + CONVERT_PRIV('f', "REFRESH"); + CONVERT_PRIV('n', "REINDEX"); } } diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl index 6656222363..85e4654922 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -618,7 +618,7 @@ my %tests = ( \QREVOKE ALL ON TABLES FROM regress_dump_test_role;\E\n \QALTER DEFAULT PRIVILEGES \E \QFOR ROLE regress_dump_test_role \E - \QGRANT INSERT,REFERENCES,DELETE,TRIGGER,TRUNCATE,VACUUM,ANALYZE,UPDATE ON TABLES TO regress_dump_test_role;\E + \QGRANT INSERT,REFERENCES,DELETE,TRIGGER,TRUNCATE,VACUUM,ANALYZE,CLUSTER,REFRESH,REINDEX,UPDATE ON TABLES TO regress_dump_test_role;\E /xm, like => { %full_runs, section_post_data => 1, }, unlike => { no_privs => 1, }, diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 89e7317c23..a096e92b0d 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -1147,7 +1147,7 @@ static const SchemaQuery Query_for_trigger_of_table = { #define Privilege_options_of_grant_and_revoke \ "SELECT", "INSERT", "UPDATE", "DELETE", "TRUNCATE", "REFERENCES", "TRIGGER", \ "CREATE", "CONNECT", "TEMPORARY", "EXECUTE", "USAGE", "SET", "ALTER SYSTEM", \ -"VACUUM", "ANALYZE", "ALL" +"VACUUM", "ANALYZE", "CLUSTER", "REFRESH", "REINDEX", "ALL" /* * These object types were introduced later than our support cutoff of @@ -3783,7 +3783,7 @@ psql_completion(const char *text, int start, int end) COMPLETE_WITH("SELECT", "INSERT", "UPDATE", "DELETE", "TRUNCATE", "REFERENCES", "TRIGGER", "CREATE", "EXECUTE", "USAGE", "VACUUM", "ANALYZE", - "ALL"); + "CLUSTER", "REFRESH", "REINDEX", "ALL"); else if (TailMatches("GRANT")) COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_roles, Privilege_options_of_grant_and_revoke); diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h index 03f14d6be1..88fafb8798 100644 --- a/src/include/commands/tablecmds.h +++ b/src/include/commands/tablecmds.h @@ -95,8 +95,8 @@ extern void AtEOSubXact_on_commit_actions(bool isCommit, SubTransactionId mySubid, SubTransactionId parentSubid); -extern void RangeVarCallbackOwnsTable(const RangeVar *relation, - Oid relId, Oid oldRelId, void *arg); +extern void RangeVarCallbackForTablePrivs(const RangeVar *relation, + Oid relId, Oid oldRelId, void *acl); extern void RangeVarCallbackOwnsRelation(const RangeVar *relation, Oid relId, Oid oldRelId, void *arg); diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 6a6d3293e4..52fa7b2d04 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -97,7 +97,10 @@ typedef uint64 AclMode; /* a bitmask of privilege bits */ #define ACL_ALTER_SYSTEM (1<<13) /* for configuration parameters */ #define ACL_VACUUM (1<<14) /* for relations */ #define ACL_ANALYZE (1<<15) /* for relations */ -#define N_ACL_RIGHTS 16 /* 1 plus the last 1<<x */ +#define ACL_CLUSTER (1<<16) /* for relations */ +#define ACL_REFRESH (1<<17) /* for relations */ +#define ACL_REINDEX (1<<18) /* for relations */ +#define N_ACL_RIGHTS 19 /* 1 plus the last 1<<x */ #define ACL_NO_RIGHTS 0 /* Currently, SELECT ... FOR [KEY] UPDATE/SHARE requires UPDATE privileges */ #define ACL_SELECT_FOR_UPDATE ACL_UPDATE diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h index e566ff0c73..64a50a3d9e 100644 --- a/src/include/utils/acl.h +++ b/src/include/utils/acl.h @@ -150,15 +150,18 @@ typedef struct ArrayType Acl; #define ACL_ALTER_SYSTEM_CHR 'A' #define ACL_VACUUM_CHR 'v' #define ACL_ANALYZE_CHR 'z' +#define ACL_CLUSTER_CHR 'S' +#define ACL_REFRESH_CHR 'f' +#define ACL_REINDEX_CHR 'n' /* string holding all privilege code chars, in order by bitmask position */ -#define ACL_ALL_RIGHTS_STR "arwdDxtXUCTcsAvz" +#define ACL_ALL_RIGHTS_STR "arwdDxtXUCTcsAvzSfn" /* * Bitmasks defining "all rights" for each supported object type */ #define ACL_ALL_RIGHTS_COLUMN (ACL_INSERT|ACL_SELECT|ACL_UPDATE|ACL_REFERENCES) -#define ACL_ALL_RIGHTS_RELATION (ACL_INSERT|ACL_SELECT|ACL_UPDATE|ACL_DELETE|ACL_TRUNCATE|ACL_REFERENCES|ACL_TRIGGER|ACL_VACUUM|ACL_ANALYZE) +#define ACL_ALL_RIGHTS_RELATION (ACL_INSERT|ACL_SELECT|ACL_UPDATE|ACL_DELETE|ACL_TRUNCATE|ACL_REFERENCES|ACL_TRIGGER|ACL_VACUUM|ACL_ANALYZE|ACL_CLUSTER|ACL_REFRESH|ACL_REINDEX) #define ACL_ALL_RIGHTS_SEQUENCE (ACL_USAGE|ACL_SELECT|ACL_UPDATE) #define ACL_ALL_RIGHTS_DATABASE (ACL_CREATE|ACL_CREATE_TEMP|ACL_CONNECT) #define ACL_ALL_RIGHTS_FDW (ACL_USAGE) diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index 6cd57e3eaa..a6f6c9fbef 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -2825,9 +2825,9 @@ RESET ROLE; GRANT USAGE ON SCHEMA pg_toast TO regress_reindexuser; SET SESSION ROLE regress_reindexuser; REINDEX TABLE pg_toast.pg_toast_1260; -ERROR: must be owner of table pg_toast_1260 +ERROR: permission denied for "pg_toast_1260" REINDEX INDEX pg_toast.pg_toast_1260_index; -ERROR: must be owner of index pg_toast_1260_index +ERROR: permission denied for "pg_toast_1260_index" -- Clean up RESET ROLE; REVOKE USAGE ON SCHEMA pg_toast FROM regress_reindexuser; diff --git a/src/test/regress/expected/dependency.out b/src/test/regress/expected/dependency.out index 81d8376509..5dca0dff7c 100644 --- a/src/test/regress/expected/dependency.out +++ b/src/test/regress/expected/dependency.out @@ -19,7 +19,7 @@ DETAIL: privileges for table deptest REVOKE SELECT ON deptest FROM GROUP regress_dep_group; DROP GROUP regress_dep_group; -- can't drop the user if we revoke the privileges partially -REVOKE SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES, VACUUM, ANALYZE ON deptest FROM regress_dep_user; +REVOKE SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES, VACUUM, ANALYZE, CLUSTER, REFRESH, REINDEX ON deptest FROM regress_dep_user; DROP USER regress_dep_user; ERROR: role "regress_dep_user" cannot be dropped because some objects depend on it DETAIL: privileges for table deptest @@ -63,21 +63,21 @@ CREATE TABLE deptest (a serial primary key, b text); GRANT ALL ON deptest1 TO regress_dep_user2; RESET SESSION AUTHORIZATION; \z deptest1 - Access privileges - Schema | Name | Type | Access privileges | Column privileges | Policies ---------+----------+-------+--------------------------------------------------------+-------------------+---------- - public | deptest1 | table | regress_dep_user0=arwdDxtvz/regress_dep_user0 +| | - | | | regress_dep_user1=a*r*w*d*D*x*t*v*z*/regress_dep_user0+| | - | | | regress_dep_user2=arwdDxtvz/regress_dep_user1 | | + Access privileges + Schema | Name | Type | Access privileges | Column privileges | Policies +--------+----------+-------+--------------------------------------------------------------+-------------------+---------- + public | deptest1 | table | regress_dep_user0=arwdDxtvzSfn/regress_dep_user0 +| | + | | | regress_dep_user1=a*r*w*d*D*x*t*v*z*S*f*n*/regress_dep_user0+| | + | | | regress_dep_user2=arwdDxtvzSfn/regress_dep_user1 | | (1 row) DROP OWNED BY regress_dep_user1; -- all grants revoked \z deptest1 - Access privileges - Schema | Name | Type | Access privileges | Column privileges | Policies ---------+----------+-------+-----------------------------------------------+-------------------+---------- - public | deptest1 | table | regress_dep_user0=arwdDxtvz/regress_dep_user0 | | + Access privileges + Schema | Name | Type | Access privileges | Column privileges | Policies +--------+----------+-------+--------------------------------------------------+-------------------+---------- + public | deptest1 | table | regress_dep_user0=arwdDxtvzSfn/regress_dep_user0 | | (1 row) -- table was dropped diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out index 7933314fd3..1f7050438f 100644 --- a/src/test/regress/expected/privileges.out +++ b/src/test/regress/expected/privileges.out @@ -2570,39 +2570,39 @@ grant select on dep_priv_test to regress_priv_user4 with grant option; set session role regress_priv_user4; grant select on dep_priv_test to regress_priv_user5; \dp dep_priv_test - Access privileges - Schema | Name | Type | Access privileges | Column privileges | Policies ---------+---------------+-------+-------------------------------------------------+-------------------+---------- - public | dep_priv_test | table | regress_priv_user1=arwdDxtvz/regress_priv_user1+| | - | | | regress_priv_user2=r*/regress_priv_user1 +| | - | | | regress_priv_user3=r*/regress_priv_user1 +| | - | | | regress_priv_user4=r*/regress_priv_user2 +| | - | | | regress_priv_user4=r*/regress_priv_user3 +| | - | | | regress_priv_user5=r/regress_priv_user4 | | + Access privileges + Schema | Name | Type | Access privileges | Column privileges | Policies +--------+---------------+-------+----------------------------------------------------+-------------------+---------- + public | dep_priv_test | table | regress_priv_user1=arwdDxtvzSfn/regress_priv_user1+| | + | | | regress_priv_user2=r*/regress_priv_user1 +| | + | | | regress_priv_user3=r*/regress_priv_user1 +| | + | | | regress_priv_user4=r*/regress_priv_user2 +| | + | | | regress_priv_user4=r*/regress_priv_user3 +| | + | | | regress_priv_user5=r/regress_priv_user4 | | (1 row) set session role regress_priv_user2; revoke select on dep_priv_test from regress_priv_user4 cascade; \dp dep_priv_test - Access privileges - Schema | Name | Type | Access privileges | Column privileges | Policies ---------+---------------+-------+-------------------------------------------------+-------------------+---------- - public | dep_priv_test | table | regress_priv_user1=arwdDxtvz/regress_priv_user1+| | - | | | regress_priv_user2=r*/regress_priv_user1 +| | - | | | regress_priv_user3=r*/regress_priv_user1 +| | - | | | regress_priv_user4=r*/regress_priv_user3 +| | - | | | regress_priv_user5=r/regress_priv_user4 | | + Access privileges + Schema | Name | Type | Access privileges | Column privileges | Policies +--------+---------------+-------+----------------------------------------------------+-------------------+---------- + public | dep_priv_test | table | regress_priv_user1=arwdDxtvzSfn/regress_priv_user1+| | + | | | regress_priv_user2=r*/regress_priv_user1 +| | + | | | regress_priv_user3=r*/regress_priv_user1 +| | + | | | regress_priv_user4=r*/regress_priv_user3 +| | + | | | regress_priv_user5=r/regress_priv_user4 | | (1 row) set session role regress_priv_user3; revoke select on dep_priv_test from regress_priv_user4 cascade; \dp dep_priv_test - Access privileges - Schema | Name | Type | Access privileges | Column privileges | Policies ---------+---------------+-------+-------------------------------------------------+-------------------+---------- - public | dep_priv_test | table | regress_priv_user1=arwdDxtvz/regress_priv_user1+| | - | | | regress_priv_user2=r*/regress_priv_user1 +| | - | | | regress_priv_user3=r*/regress_priv_user1 | | + Access privileges + Schema | Name | Type | Access privileges | Column privileges | Policies +--------+---------------+-------+----------------------------------------------------+-------------------+---------- + public | dep_priv_test | table | regress_priv_user1=arwdDxtvzSfn/regress_priv_user1+| | + | | | regress_priv_user2=r*/regress_priv_user1 +| | + | | | regress_priv_user3=r*/regress_priv_user1 | | (1 row) set session role regress_priv_user1; @@ -2914,3 +2914,53 @@ DROP ROLE regress_both; DROP ROLE regress_only_vacuum_all; DROP ROLE regress_only_analyze_all; DROP ROLE regress_both_all; +-- CLUSTER +CREATE ROLE regress_no_cluster; +CREATE ROLE regress_cluster; +CREATE TABLE cluster_test (a INT); +CREATE INDEX ON cluster_test (a); +GRANT CLUSTER ON cluster_test TO regress_cluster; +SET ROLE regress_no_cluster; +CLUSTER cluster_test USING cluster_test_a_idx; +ERROR: permission denied for "cluster_test" +RESET ROLE; +SET ROLE regress_cluster; +CLUSTER cluster_test USING cluster_test_a_idx; +RESET ROLE; +DROP TABLE cluster_test; +DROP ROLE regress_no_cluster; +DROP ROLE regress_cluster; +-- REFRESH MATERIALIZED VIEW +CREATE ROLE regress_no_refresh; +CREATE ROLE regress_refresh; +CREATE MATERIALIZED VIEW refresh_test AS SELECT 1; +GRANT REFRESH ON refresh_test TO regress_refresh; +SET ROLE regress_no_refresh; +REFRESH MATERIALIZED VIEW refresh_test; +ERROR: permission denied for "refresh_test" +RESET ROLE; +SET ROLE regress_refresh; +REFRESH MATERIALIZED VIEW refresh_test; +RESET ROLE; +DROP MATERIALIZED VIEW refresh_test; +DROP ROLE regress_no_refresh; +DROP ROLE regress_refresh; +-- REINDEX +CREATE ROLE regress_no_reindex; +CREATE ROLE regress_reindex; +CREATE TABLE reindex_test (a INT); +CREATE INDEX ON reindex_test (a); +GRANT REINDEX ON reindex_test TO regress_reindex; +SET ROLE regress_no_reindex; +REINDEX TABLE reindex_test; +ERROR: permission denied for "reindex_test" +REINDEX INDEX reindex_test_a_idx; +ERROR: permission denied for "reindex_test_a_idx" +RESET ROLE; +SET ROLE regress_reindex; +REINDEX TABLE reindex_test; +REINDEX INDEX reindex_test_a_idx; +RESET ROLE; +DROP TABLE reindex_test; +DROP ROLE regress_no_reindex; +DROP ROLE regress_reindex; diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out index 31509a0a6f..797da365b8 100644 --- a/src/test/regress/expected/rowsecurity.out +++ b/src/test/regress/expected/rowsecurity.out @@ -93,23 +93,23 @@ CREATE POLICY p2r ON document AS RESTRICTIVE TO regress_rls_dave CREATE POLICY p1r ON document AS RESTRICTIVE TO regress_rls_dave USING (cid <> 44); \dp - Access privileges - Schema | Name | Type | Access privileges | Column privileges | Policies ---------------------+----------+-------+-----------------------------------------------+-------------------+-------------------------------------------- - regress_rls_schema | category | table | regress_rls_alice=arwdDxtvz/regress_rls_alice+| | - | | | =arwdDxtvz/regress_rls_alice | | - regress_rls_schema | document | table | regress_rls_alice=arwdDxtvz/regress_rls_alice+| | p1: + - | | | =arwdDxtvz/regress_rls_alice | | (u): (dlevel <= ( SELECT uaccount.seclv + - | | | | | FROM uaccount + - | | | | | WHERE (uaccount.pguser = CURRENT_USER)))+ - | | | | | p2r (RESTRICTIVE): + - | | | | | (u): ((cid <> 44) AND (cid < 50)) + - | | | | | to: regress_rls_dave + - | | | | | p1r (RESTRICTIVE): + - | | | | | (u): (cid <> 44) + - | | | | | to: regress_rls_dave - regress_rls_schema | uaccount | table | regress_rls_alice=arwdDxtvz/regress_rls_alice+| | - | | | =r/regress_rls_alice | | + Access privileges + Schema | Name | Type | Access privileges | Column privileges | Policies +--------------------+----------+-------+--------------------------------------------------+-------------------+-------------------------------------------- + regress_rls_schema | category | table | regress_rls_alice=arwdDxtvzSfn/regress_rls_alice+| | + | | | =arwdDxtvzSfn/regress_rls_alice | | + regress_rls_schema | document | table | regress_rls_alice=arwdDxtvzSfn/regress_rls_alice+| | p1: + + | | | =arwdDxtvzSfn/regress_rls_alice | | (u): (dlevel <= ( SELECT uaccount.seclv + + | | | | | FROM uaccount + + | | | | | WHERE (uaccount.pguser = CURRENT_USER)))+ + | | | | | p2r (RESTRICTIVE): + + | | | | | (u): ((cid <> 44) AND (cid < 50)) + + | | | | | to: regress_rls_dave + + | | | | | p1r (RESTRICTIVE): + + | | | | | (u): (cid <> 44) + + | | | | | to: regress_rls_dave + regress_rls_schema | uaccount | table | regress_rls_alice=arwdDxtvzSfn/regress_rls_alice+| | + | | | =r/regress_rls_alice | | (3 rows) \d document diff --git a/src/test/regress/sql/dependency.sql b/src/test/regress/sql/dependency.sql index 99b905a938..7c5457db9e 100644 --- a/src/test/regress/sql/dependency.sql +++ b/src/test/regress/sql/dependency.sql @@ -21,7 +21,7 @@ REVOKE SELECT ON deptest FROM GROUP regress_dep_group; DROP GROUP regress_dep_group; -- can't drop the user if we revoke the privileges partially -REVOKE SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES, VACUUM, ANALYZE ON deptest FROM regress_dep_user; +REVOKE SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES, VACUUM, ANALYZE, CLUSTER, REFRESH, REINDEX ON deptest FROM regress_dep_user; DROP USER regress_dep_user; -- now we are OK to drop him diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql index 1bcaaba4eb..98eb7f05be 100644 --- a/src/test/regress/sql/privileges.sql +++ b/src/test/regress/sql/privileges.sql @@ -1916,3 +1916,64 @@ DROP ROLE regress_both; DROP ROLE regress_only_vacuum_all; DROP ROLE regress_only_analyze_all; DROP ROLE regress_both_all; + +-- CLUSTER +CREATE ROLE regress_no_cluster; +CREATE ROLE regress_cluster; + +CREATE TABLE cluster_test (a INT); +CREATE INDEX ON cluster_test (a); +GRANT CLUSTER ON cluster_test TO regress_cluster; + +SET ROLE regress_no_cluster; +CLUSTER cluster_test USING cluster_test_a_idx; +RESET ROLE; + +SET ROLE regress_cluster; +CLUSTER cluster_test USING cluster_test_a_idx; +RESET ROLE; + +DROP TABLE cluster_test; +DROP ROLE regress_no_cluster; +DROP ROLE regress_cluster; + +-- REFRESH MATERIALIZED VIEW +CREATE ROLE regress_no_refresh; +CREATE ROLE regress_refresh; + +CREATE MATERIALIZED VIEW refresh_test AS SELECT 1; +GRANT REFRESH ON refresh_test TO regress_refresh; + +SET ROLE regress_no_refresh; +REFRESH MATERIALIZED VIEW refresh_test; +RESET ROLE; + +SET ROLE regress_refresh; +REFRESH MATERIALIZED VIEW refresh_test; +RESET ROLE; + +DROP MATERIALIZED VIEW refresh_test; +DROP ROLE regress_no_refresh; +DROP ROLE regress_refresh; + +-- REINDEX +CREATE ROLE regress_no_reindex; +CREATE ROLE regress_reindex; + +CREATE TABLE reindex_test (a INT); +CREATE INDEX ON reindex_test (a); +GRANT REINDEX ON reindex_test TO regress_reindex; + +SET ROLE regress_no_reindex; +REINDEX TABLE reindex_test; +REINDEX INDEX reindex_test_a_idx; +RESET ROLE; + +SET ROLE regress_reindex; +REINDEX TABLE reindex_test; +REINDEX INDEX reindex_test_a_idx; +RESET ROLE; + +DROP TABLE reindex_test; +DROP ROLE regress_no_reindex; +DROP ROLE regress_reindex; -- 2.25.1 --zYM0uCDKw75PZbzx Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v1-0002-add-predefined-roles-for-cluster-refresh-matview-.patch" ^ permalink raw reply [nested|flat] 2+ messages in thread
* [PATCH] REPACK CONCURRENTLY: Initialize the range table more honestly @ 2026-07-03 10:47 Álvaro Herrera <alvherre@kurilemu.de> 0 siblings, 0 replies; 2+ messages in thread From: Álvaro Herrera @ 2026-07-03 10:47 UTC (permalink / raw) We were skipping a bunch of things that are mostly unnecessary for REPACK. However, one thing that seems would be better to pass closer to truth, is the updatedCols bitmapset in the range table entry for the repacked table. Cons up an RTE and install it into the EState. This only has an effect on btree indexes, because certain operations are optimized in the case of unchanged columns; and even then, correctnesss is not being compromised. The values we pass after this commit are not fully trustworthy either, because we simply say "all columns were updated" for all insert/updates, regardless of whether their values were actually modified or not. However, this way we err to the side of caution rather than to the opposite direction as we were originally doing. This could be refined in the future, but there's a trade-off: determining whether the column was in fact updated could be expensive. Author: Antonin Houska <ah@cybertec.at> Reviewed-by: Ewan Young <kdbase.hack@gmail.com> Backpatch-through: 19 Discussion: https://postgr.es/m/18222.1782126731@localhost --- src/backend/commands/repack.c | 57 +++++++++++++++++++++++++++++++++-- 1 file changed, 55 insertions(+), 2 deletions(-) diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c index 83a49afe7e1..faa07d1a118 100644 --- a/src/backend/commands/repack.c +++ b/src/backend/commands/repack.c @@ -63,6 +63,7 @@ #include "libpq/pqmq.h" #include "miscadmin.h" #include "optimizer/optimizer.h" +#include "parser/parse_relation.h" #include "pgstat.h" #include "replication/logicalrelation.h" #include "storage/bufmgr.h" @@ -3019,8 +3020,60 @@ initialize_change_context(ChangeContext *chgcxt, /* Only initialize fields needed by ExecInsertIndexTuples(). */ chgcxt->cc_estate = CreateExecutorState(); - chgcxt->cc_rri = (ResultRelInfo *) palloc(sizeof(ResultRelInfo)); - InitResultRelInfo(chgcxt->cc_rri, relation, 0, 0, 0); + /* + * Set up a range table for the executor, containing our repacked table as + * its only member. + */ + { + RangeTblEntry *rte; + TupleDesc desc = RelationGetDescr(relation); + List *perminfos = NIL; + Bitmapset *updatedCols = NULL; + RTEPermissionInfo *perminfo; + + /* + * For our use, the RTE only needs to have perminfoindex initialized, + * but there's no reason to not set the fields whose values we have at + * hand. + */ + rte = makeNode(RangeTblEntry); + rte->rtekind = RTE_RELATION; + rte->relid = RelationGetRelid(relation); + rte->relkind = RelationGetForm(relation)->relkind; + /* Create the RTEPermissionInfo instance (and set ->perminfoindex). */ + addRTEPermissionInfo(&perminfos, rte); + + /* + * Initialize updatedCols to show that all columns are updated. This + * is of course not necessarily true, and we cannot know this early; + * but this is only used by ExecInsertIndexTuples to flag index + * updates with no logical value changes, so if it's wrong, nothing + * terribly bad happens. We may want to improve this someday though. + * + * Don't claim that dropped columns are changed though. + */ + for (int i = 0; i < desc->natts; i++) + { + CompactAttribute *attr = TupleDescCompactAttr(desc, i); + + if (attr->attisdropped) + continue; + updatedCols = bms_add_member(updatedCols, + i + 1 - FirstLowInvalidHeapAttributeNumber); + } + + /* install updatedCols in the right place */ + perminfo = getRTEPermissionInfo(perminfos, rte); + perminfo->updatedCols = updatedCols; + + /* finally we can initialize the range table proper */ + ExecInitRangeTable(chgcxt->cc_estate, list_make1(rte), perminfos, + bms_make_singleton(1)); + } + + /* Set up our ResultRelInfo to use for index updates */ + chgcxt->cc_rri = makeNode(ResultRelInfo); + InitResultRelInfo(chgcxt->cc_rri, relation, 1, NULL, 0); ExecOpenIndices(chgcxt->cc_rri, false); /* -- 2.47.3 --pi6rmivyoaehyqn6-- ^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2026-07-03 10:47 UTC | newest] Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2022-12-07 19:20 [PATCH v1 1/2] add grantable privileges for cluster, refresh matview, and reindex Nathan Bossart <nathandbossart@gmail.com> 2026-07-03 10:47 [PATCH] REPACK CONCURRENTLY: Initialize the range table more honestly Álvaro Herrera <alvherre@kurilemu.de>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox