public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH 06/10] cirrus: build docs as a separate task..
24+ messages / 7 participants
[nested] [flat]

* [PATCH 06/10] cirrus: build docs as a separate task..
@ 2022-02-27 01:34  Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Justin Pryzby @ 2022-02-27 01:34 UTC (permalink / raw)

This will run the doc build if any docs have changed, even if Linux
fails, to allow catch doc build failures.

This automatically shows up as a separate "column" on cfbot.

Also, in the future, this will hopefully upload each patch's changed HTML docs
as an artifact, for easy review.

XXX: it ought to be possible to skip (re)building docs if they haven't
changed since the last build.  But that doesn't appear to be working.
https://cirrus-ci.com/task/4523764229603328
skip: "!changesInclude('.cirrus.yml', 'doc/**')"

ci-os-only: html
---
 .cirrus.yml | 48 ++++++++++++++++++++++++++++++++++--------------
 1 file changed, 34 insertions(+), 14 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index 3eda0268161..5cb18032779 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -736,20 +736,6 @@ task:
       make -s -j${BUILD_JOBS} clean
       time make -s -j${BUILD_JOBS} world-bin
 
-  ###
-  # Verify docs can be built
-  ###
-  # XXX: Only do this if there have been changes in doc/ since last build
-  always:
-    docs_build_script: |
-      time ./configure \
-        --cache gcc.cache \
-        CC="ccache gcc" \
-        CXX="ccache g++" \
-        CLANG="ccache clang"
-      make -s -j${BUILD_JOBS} clean
-      time make -s -j${BUILD_JOBS} -C doc
-
   ###
   # Verify headerscheck / cpluspluscheck succeed
   #
@@ -773,3 +759,37 @@ task:
 
   always:
     upload_caches: ccache
+
+
+###
+# Verify docs can be built
+# changesInclude() will skip this task if none of the commits since
+# CIRRUS_LAST_GREEN_CHANGE touched any relevant files. The comparison appears
+# to be like "git log a..b -- ./file", not "git diff a..b -- ./file"
+###
+
+task:
+  name: Documentation
+
+  env:
+    CPUS: 1
+    BUILD_JOBS: 1
+
+  only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*(docs|html).*'
+
+  container:
+    image: $CONTAINER_REPO/linux_debian_bullseye_ci:latest
+    cpu: $CPUS
+    memory: 2G
+
+  # Exercise HTML and other docs:
+  ninja_docs_build_script: |
+    mkdir build-ninja
+    cd build-ninja
+    time meson setup
+    time ninja docs
+
+  # Again, with autoconf
+  docs_build_script: |
+    time ./configure
+    make -s -j${BUILD_JOBS} -C doc
-- 
2.25.1


--sMkrXc3gAYLRVOjR
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0007-cirrus-warnings-use-a-single-common-always-block.patch"



^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: Granting SET and ALTER SYSTE privileges for GUCs
@ 2022-03-06 22:57  Tom Lane <[email protected]>
  0 siblings, 1 reply; 24+ messages in thread

From: Tom Lane @ 2022-03-06 22:57 UTC (permalink / raw)
  To: Mark Dilger <[email protected]>; +Cc: Joshua Brindle <[email protected]>; Andrew Dunstan <[email protected]>; Robert Haas <[email protected]>; Jeff Davis <[email protected]>; pgsql-hackers; Joe Conway <[email protected]>

Mark Dilger <[email protected]> writes:
>> On Mar 6, 2022, at 2:13 PM, Tom Lane <[email protected]> wrote:
>> ... Or, if that's our position, why are there
>> per-GUC changes at all, rather than just redefining what the
>> context values mean?  (That is, why not redefine USERSET and
>> SUSET as simply indicating the default ACL to be applied if there's
>> no entry in the catalog.)

> To my knowledge, there is no mechanism to revoke an implicit privilege.  You can revoke a privilege explicitly listed in an aclitem[], but only if the privilege is being tracked that way.

So?  What I'm suggesting is along the lines of

(1) pg_setting_acl starts out empty, or at least mostly empty (maybe
there are a few GUCs that need custom values).

(2) If there's a pg_setting_acl entry for a GUC that's to be set,
we apply it: either it grants the desired permission or it doesn't.

(3) If there's no entry, then for a USERSET GUC we assume that the
entry would be like "=s/postgres", while for any other context value
we assume the ACL grants nothing.

I don't think this is materially different from what we do with
permissions on (say) functions.  If you want to revoke the public
SET privilege on some USERSET variable, you instantiate the default
and then revoke.  You end up with an empty ACL stored in pg_setting_acl,
and voila.

It'd likely be necessary to refuse to record a grant/revoke on
an unknown GUC, since if we don't know the GUC then we can't know
what the relevant default ACL ought to be.  But I bet your existing
patch has some dubious behavior in that case too.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: Granting SET and ALTER SYSTE privileges for GUCs
@ 2022-03-06 23:06  Mark Dilger <[email protected]>
  parent: Tom Lane <[email protected]>
  0 siblings, 1 reply; 24+ messages in thread

From: Mark Dilger @ 2022-03-06 23:06 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Joshua Brindle <[email protected]>; Andrew Dunstan <[email protected]>; Robert Haas <[email protected]>; Jeff Davis <[email protected]>; pgsql-hackers; Joe Conway <[email protected]>



> On Mar 6, 2022, at 2:57 PM, Tom Lane <[email protected]> wrote:
> 
> I don't think this is materially different from what we do with
> permissions on (say) functions.  If you want to revoke the public
> SET privilege on some USERSET variable, you instantiate the default
> and then revoke.  You end up with an empty ACL stored in pg_setting_acl,
> and voila.

I assume you mean the implementation of REVOKE does this, not that the user needs to do both a grant and a revoke.

> It'd likely be necessary to refuse to record a grant/revoke on
> an unknown GUC, since if we don't know the GUC then we can't know
> what the relevant default ACL ought to be.  But I bet your existing
> patch has some dubious behavior in that case too.

The existing patch allows grants on unknown gucs, because it can't know what guc an upgrade script will introduce, and the grant statement may need to execute before the guc exists.  That opens a window for granting privileges on non-existent gucs.  That sounds bad, but I don't know of any actual harm that it does, beyond just being ugly.

With your proposal, it sounds like we could avoid that ugliness, so I'm inclined to at least try doing it as you propose.

Thanks for the suggestion!

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company









^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: Granting SET and ALTER SYSTE privileges for GUCs
@ 2022-03-06 23:27  Tom Lane <[email protected]>
  parent: Mark Dilger <[email protected]>
  0 siblings, 1 reply; 24+ messages in thread

From: Tom Lane @ 2022-03-06 23:27 UTC (permalink / raw)
  To: Mark Dilger <[email protected]>; +Cc: Joshua Brindle <[email protected]>; Andrew Dunstan <[email protected]>; Robert Haas <[email protected]>; Jeff Davis <[email protected]>; pgsql-hackers; Joe Conway <[email protected]>

Mark Dilger <[email protected]> writes:
> On Mar 6, 2022, at 2:57 PM, Tom Lane <[email protected]> wrote:
>> I don't think this is materially different from what we do with
>> permissions on (say) functions.  If you want to revoke the public
>> SET privilege on some USERSET variable, you instantiate the default
>> and then revoke.  You end up with an empty ACL stored in pg_setting_acl,
>> and voila.

> I assume you mean the implementation of REVOKE does this, not that the user needs to do both a grant and a revoke.

Right.  Again, look at what happens when you create a function and
then revoke its default PUBLIC EXECUTE permission.

>> It'd likely be necessary to refuse to record a grant/revoke on
>> an unknown GUC, since if we don't know the GUC then we can't know
>> what the relevant default ACL ought to be.  But I bet your existing
>> patch has some dubious behavior in that case too.

> The existing patch allows grants on unknown gucs, because it can't know what guc an upgrade script will introduce, and the grant statement may need to execute before the guc exists.

Yeah, that's the problematic case.  It might mostly work to assume that
an unknown GUC has an empty default ACL.  This could fail to retain the
default PUBLIC SET permission if it later turns out the GUC is USERSET
... but I suspect in most cases anybody who's messing with the permissions
would've started out by revoking that anyway.  We could make this
definitely work in pg_dump if we teach pg_dump to explicitly grant or
revoke the PUBLIC SET permission anytime it's emitting anything for
a GUC, even if it thinks that would be the default state anyway.
Extension scripts that want to modify permissions for their GUCs
could follow that same principle to be sure.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: Granting SET and ALTER SYSTE privileges for GUCs
@ 2022-03-15 20:59  Mark Dilger <[email protected]>
  parent: Tom Lane <[email protected]>
  0 siblings, 2 replies; 24+ messages in thread

From: Mark Dilger @ 2022-03-15 20:59 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Joshua Brindle <[email protected]>; Andrew Dunstan <[email protected]>; Robert Haas <[email protected]>; Jeff Davis <[email protected]>; pgsql-hackers; Joe Conway <[email protected]>



> On Mar 6, 2022, at 3:27 PM, Tom Lane <[email protected]> wrote:
> 
> Mark Dilger <[email protected]> writes:
>> The existing patch allows grants on unknown gucs, because it can't know what guc an upgrade script will introduce, and the grant statement may need to execute before the guc exists.
> 
> Yeah, that's the problematic case.  It might mostly work to assume that
> an unknown GUC has an empty default ACL.  This could fail to retain the
> default PUBLIC SET permission if it later turns out the GUC is USERSET

On further reflection, I concluded this isn't needed.  No current extension, whether in-core or third party, expects to be able to create a new GUC and then grant or revoke permissions on it.  They can already specify the guc context (PGC_USERS, etc).  Introducing a feature that depends on the dubious assumption that unrecognized GUCs will turn out to be USERSET doesn't seem warranted.

The patch attributes all grants of setting privileges to the bootstrap superuser.  Only superusers can grant or revoke privileges on settings, and all settings are implicitly owned by the bootstrap superuser because there is no explicit owner associated with settings.  Consequently, select_best_grantor(some_superuser, ..., BOOTSTRAP_SUPERUSERID, ...) always chooses the bootstrap superuser.  I don't see a problem with this, but wouldn't mind a second opinion.  Some people might find it surprising when viewing the pg_setting_acl.setacl field.




—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company





Attachments:

  [application/octet-stream] v10-0001-Allow-grant-and-revoke-of-privileges-on-settings.patch (134.1K, ../../[email protected]/2-v10-0001-Allow-grant-and-revoke-of-privileges-on-settings.patch)
  download | inline diff:
From e5d01d17ca0a38d8f06b3989323fb5105b64230d Mon Sep 17 00:00:00 2001
From: Mark Dilger <[email protected]>
Date: Tue, 8 Mar 2022 14:12:29 -0800
Subject: [PATCH v10] Allow grant and revoke of privileges on settings

Allow grant and revoke of privileges to set or alter system set
configuration variables.  Each (role,variable,privilege) triple can
be independently granted or revoked, so a user may be granted
privilege to SET but not to ALTER SYSTEM SET on a variable, or vice
versa.  Privilege to SET a userset variable is implicitly granted to
public, but may be revoked.
---
 doc/src/sgml/catalogs.sgml                  |  75 +++-
 doc/src/sgml/ddl.sgml                       |  42 ++-
 doc/src/sgml/func.sgml                      |  20 +-
 doc/src/sgml/ref/grant.sgml                 |   7 +
 doc/src/sgml/ref/revoke.sgml                |   7 +
 doc/src/sgml/ref/set.sgml                   |   5 +-
 src/backend/catalog/Makefile                |   3 +-
 src/backend/catalog/aclchk.c                | 262 ++++++++++++++
 src/backend/catalog/catalog.c               |   6 +
 src/backend/catalog/dependency.c            |   6 +
 src/backend/catalog/objectaddress.c         |  46 +++
 src/backend/catalog/pg_setting_acl.c        | 122 +++++++
 src/backend/catalog/system_views.sql        |  13 +
 src/backend/commands/alter.c                |   1 +
 src/backend/commands/dropcmds.c             |   7 +
 src/backend/commands/event_trigger.c        |   6 +
 src/backend/commands/seclabel.c             |   1 +
 src/backend/commands/tablecmds.c            |   1 +
 src/backend/parser/gram.y                   |  87 ++++-
 src/backend/utils/adt/acl.c                 | 291 ++++++++++++++++
 src/backend/utils/cache/lsyscache.c         |  20 ++
 src/backend/utils/cache/syscache.c          |  23 ++
 src/backend/utils/misc/guc.c                | 204 ++++++++++-
 src/bin/pg_dump/dumputils.c                 |   7 +-
 src/bin/pg_dump/pg_backup_archiver.c        |   2 +
 src/bin/pg_dump/pg_dump.c                   |   5 +-
 src/bin/pg_dump/pg_dumpall.c                |  61 ++++
 src/bin/psql/tab-complete.c                 | 185 +++++++++-
 src/include/catalog/dependency.h            |   1 +
 src/include/catalog/pg_default_acl.h        |   1 +
 src/include/catalog/pg_proc.dat             |  23 ++
 src/include/catalog/pg_setting_acl.h        |  63 ++++
 src/include/nodes/parsenodes.h              |   5 +-
 src/include/parser/kwlist.h                 |   1 +
 src/include/utils/acl.h                     |  11 +-
 src/include/utils/guc.h                     |   3 +
 src/include/utils/lsyscache.h               |   1 +
 src/include/utils/syscache.h                |   2 +
 src/test/modules/test_pg_dump/t/001_base.pl |  61 ++++
 src/test/regress/expected/guc_privs.out     | 367 ++++++++++++++++++++
 src/test/regress/expected/privileges.out    |  56 ++-
 src/test/regress/expected/rules.out         |  13 +
 src/test/regress/parallel_schedule          |   2 +-
 src/test/regress/sql/guc_privs.sql          | 165 +++++++++
 src/test/regress/sql/privileges.sql         |  44 ++-
 45 files changed, 2284 insertions(+), 50 deletions(-)
 create mode 100644 src/backend/catalog/pg_setting_acl.c
 create mode 100644 src/include/catalog/pg_setting_acl.h
 create mode 100644 src/test/regress/expected/guc_privs.out
 create mode 100644 src/test/regress/sql/guc_privs.sql

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 83987a9904..7b288cce30 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -105,6 +105,11 @@
       <entry>collations (locale information)</entry>
      </row>
 
+     <row>
+      <entry><link linkend="catalog-pg-setting-acl"><structname>pg_setting_acl</structname></link></entry>
+      <entry>configuration parameters which have privileges granted to roles</entry>
+     </row>
+
      <row>
       <entry><link linkend="catalog-pg-constraint"><structname>pg_constraint</structname></link></entry>
       <entry>check constraints, unique constraints, primary key constraints, foreign key constraints</entry>
@@ -2423,6 +2428,64 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</replaceable>:<replaceable>&l
   </para>
  </sect1>
 
+ <sect1 id="catalog-pg-setting-acl">
+  <title><structname>pg_setting_acl</structname></title>
+
+  <indexterm zone="catalog-pg-setting-acl">
+   <primary>pg_setting_acl</primary>
+  </indexterm>
+
+  <para>
+   The catalog <structname>pg_setting_acl</structname> records configuration
+   parameters which have had privileges to <literal>SET</literal> or
+   <literal>ALTER SYSTEM</literal> granted to one or more roles.
+  </para>
+
+  <para>
+   Unlike most system catalogs, <structname>pg_setting_acl</structname>
+   is shared across all databases of a cluster: there is only one
+   copy of <structname>pg_setting_acl</structname> per cluster, not
+   one per database.
+  </para>
+
+  <table>
+   <title><structname>pg_setting_acl</structname> Columns</title>
+   <tgroup cols="1">
+    <thead>
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       Column Type
+      </para>
+      <para>
+       Description
+      </para></entry>
+     </row>
+    </thead>
+
+    <tbody>
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>setting</structfield> <type>text</type>
+      </para>
+      <para>
+       The name of the configuration parameter for which privileges are granted.
+      </para></entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>setacl</structfield> <type>aclitem[]</type>
+      </para>
+      <para>
+       Access privileges; see <xref linkend="ddl-priv"/> for details
+      </para></entry>
+     </row>
+
+    </tbody>
+   </tgroup>
+  </table>
+ </sect1>
+
  <sect1 id="catalog-pg-constraint">
   <title><structname>pg_constraint</structname></title>
 
@@ -12540,11 +12603,13 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
     <term><literal>superuser</literal></term>
     <listitem>
      <para>
-      These settings can be set from <filename>postgresql.conf</filename>,
-      or within a session via the <command>SET</command> command; but only superusers
-      can change them via <command>SET</command>.  Changes in
-      <filename>postgresql.conf</filename> will affect existing sessions
-      only if no session-local value has been established with <command>SET</command>.
+      These settings can be set from <filename>postgresql.conf</filename>, or
+      within a session via the <command>SET</command> command; but only
+      superusers or users with <literal>SET VALUE</literal> privilege granted
+      on the setting can change them via <command>SET</command>.  Changes in
+      <filename>postgresql.conf</filename> will affect existing sessions only
+      if no session-local value has been established with
+      <command>SET</command>.
      </para>
     </listitem>
    </varlistentry>
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 166b7a352d..49a1f3796e 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -1691,7 +1691,8 @@ ALTER TABLE products RENAME TO items;
    <literal>INSERT</literal>, <literal>UPDATE</literal>, <literal>DELETE</literal>,
    <literal>TRUNCATE</literal>, <literal>REFERENCES</literal>, <literal>TRIGGER</literal>,
    <literal>CREATE</literal>, <literal>CONNECT</literal>, <literal>TEMPORARY</literal>,
-   <literal>EXECUTE</literal>, and <literal>USAGE</literal>.
+   <literal>EXECUTE</literal>, <literal>USAGE</literal>, <literal>SET VALUE</literal>
+   and <literal>ALTER SYSTEM</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.
@@ -1959,6 +1960,29 @@ REVOKE ALL ON accounts FROM PUBLIC;
       </para>
      </listitem>
     </varlistentry>
+
+    <varlistentry>
+     <term><literal>SET VALUE</literal></term>
+     <listitem>
+      <para>
+       Allows non-superuser roles to use the <command>SET</command> command to
+       change run-time configuration parameters.  By default,
+       <literal>superuser</literal> run-time configuration parameters may only
+       be set by superusers, and <literal>user</literal> run-time configuration
+       parameters can be set by any user.
+      </para>
+     </listitem>
+    </varlistentry>
+
+    <varlistentry>
+     <term><literal>ALTER SYSTEM</literal></term>
+     <listitem>
+      <para>
+       Allows non-superuser roles to use the <command>ALTER SYSTEM
+       SET</command> command to change server configuration parameters.
+      </para>
+     </listitem>
+    </varlistentry>
    </variablelist>
 
    The privileges required by other commands are listed on the
@@ -2097,6 +2121,16 @@ REVOKE ALL ON accounts FROM PUBLIC;
        <literal>TYPE</literal>
       </entry>
      </row>
+     <row>
+      <entry><literal>SET VALUE</literal></entry>
+      <entry><literal>s</literal></entry>
+      <entry>configuration parameter</entry>
+     </row>
+     <row>
+      <entry><literal>ALTER SYSTEM</literal></entry>
+      <entry><literal>A</literal></entry>
+      <entry>configuration parameter</entry>
+     </row>
      </tbody>
    </tgroup>
   </table>
@@ -2203,6 +2237,12 @@ REVOKE ALL ON accounts FROM PUBLIC;
       <entry><literal>U</literal></entry>
       <entry><literal>\dT+</literal></entry>
      </row>
+     <row>
+      <entry><literal>Configuration parameter</literal></entry>
+      <entry><literal>sA</literal></entry>
+      <entry>none</entry>
+      <entry><literal></literal></entry>
+     </row>
     </tbody>
    </tgroup>
   </table>
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 8a802fb225..36b33d8639 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -22691,8 +22691,7 @@ SELECT * FROM pg_ls_dir('.') WITH ORDINALITY AS t(ls,n);
    privilege is held with grant option. Also, multiple privilege types can be
    listed separated by commas, in which case the result will be true if any of
    the listed privileges is held. (Case of the privilege string is not
-   significant, and extra whitespace is allowed between but not within
-   privilege names.)
+   significant, and extra whitespace is allowed between privilege names.)
    Some examples:
 <programlisting>
 SELECT has_table_privilege('myschema.mytable', 'select');
@@ -22957,6 +22956,23 @@ SELECT has_function_privilege('joeuser', 'myfunc(int, text)', 'execute');
        </para></entry>
       </row>
 
+      <row>
+       <entry role="func_table_entry"><para role="func_signature">
+        <indexterm>
+         <primary>has_setting_privilege</primary>
+        </indexterm>
+        <function>has_setting_privilege</function> (
+          <optional> <parameter>user</parameter> <type>name</type> or <type>oid</type>, </optional>
+          <parameter>setting</parameter> <type>text</type> or <type>oid</type>,
+          <parameter>privilege</parameter> <type>text</type> )
+        <returnvalue>boolean</returnvalue>
+       </para>
+       <para>
+        Does user have privilege for setting?
+        Allowable privilege types are <literal>SET VALUE</literal> and <literal>ALTER SYSTEM</literal>.
+       </para></entry>
+      </row>
+
       <row>
        <entry role="func_table_entry"><para role="func_signature">
         <indexterm>
diff --git a/doc/src/sgml/ref/grant.sgml b/doc/src/sgml/ref/grant.sgml
index a897712de2..39c2ff1c81 100644
--- a/doc/src/sgml/ref/grant.sgml
+++ b/doc/src/sgml/ref/grant.sgml
@@ -92,6 +92,11 @@ GRANT { USAGE | ALL [ PRIVILEGES ] }
     TO <replaceable class="parameter">role_specification</replaceable> [, ...] [ WITH GRANT OPTION ]
     [ GRANTED BY <replaceable class="parameter">role_specification</replaceable> ]
 
+GRANT { { SET VALUE | ALTER SYSTEM } [, ... ] | ALL [ PRIVILEGES ] }
+    ON SETTING <replaceable class="parameter">configuration_parameter</replaceable> [, ...]
+    TO <replaceable class="parameter">role_specification</replaceable> [, ...] [ WITH GRANT OPTION ]
+    [ GRANTED BY <replaceable class="parameter">role_specification</replaceable> ]
+
 GRANT <replaceable class="parameter">role_name</replaceable> [, ...] TO <replaceable class="parameter">role_specification</replaceable> [, ...]
     [ WITH ADMIN OPTION ]
     [ GRANTED BY <replaceable class="parameter">role_specification</replaceable> ]
@@ -185,6 +190,8 @@ GRANT <replaceable class="parameter">role_name</replaceable> [, ...] TO <replace
      <term><literal>TEMPORARY</literal></term>
      <term><literal>EXECUTE</literal></term>
      <term><literal>USAGE</literal></term>
+     <term><literal>SET VALUE</literal></term>
+     <term><literal>ALTER SYSTEM</literal></term>
      <listitem>
       <para>
        Specific types of privileges, as defined in <xref linkend="ddl-priv"/>.
diff --git a/doc/src/sgml/ref/revoke.sgml b/doc/src/sgml/ref/revoke.sgml
index 3014c864ea..741ded8d1a 100644
--- a/doc/src/sgml/ref/revoke.sgml
+++ b/doc/src/sgml/ref/revoke.sgml
@@ -118,6 +118,13 @@ REVOKE [ GRANT OPTION FOR ]
     [ GRANTED BY <replaceable class="parameter">role_specification</replaceable> ]
     [ CASCADE | RESTRICT ]
 
+REVOKE [ GRANT OPTION FOR ]
+    { { SET VALUE | ALTER SYSTEM } [, ...] | ALL [ PRIVILEGES ] }
+    ON SETTING <replaceable class="parameter">configuration_parameter</replaceable> [, ...]
+    FROM <replaceable class="parameter">role_specification</replaceable> [, ...]
+    [ GRANTED BY <replaceable class="parameter">role_specification</replaceable> ]
+    [ CASCADE | RESTRICT ]
+
 REVOKE [ ADMIN OPTION FOR ]
     <replaceable class="parameter">role_name</replaceable> [, ...] FROM <replaceable class="parameter">role_specification</replaceable> [, ...]
     [ GRANTED BY <replaceable class="parameter">role_specification</replaceable> ]
diff --git a/doc/src/sgml/ref/set.sgml b/doc/src/sgml/ref/set.sgml
index 339ee9eec9..a08057d1d1 100644
--- a/doc/src/sgml/ref/set.sgml
+++ b/doc/src/sgml/ref/set.sgml
@@ -34,8 +34,9 @@ SET [ SESSION | LOCAL ] TIME ZONE { <replaceable class="parameter">timezone</rep
    parameters.  Many of the run-time parameters listed in
    <xref linkend="runtime-config"/> can be changed on-the-fly with
    <command>SET</command>.
-   (But some require superuser privileges to change, and others cannot
-   be changed after server or session start.)
+   (But some require either superuser privileges or granted <literal>SET
+   VALUE</literal> privileges to change, and others cannot be changed after
+   server or session start.)
    <command>SET</command> only affects the value used by the current
    session.
   </para>
diff --git a/src/backend/catalog/Makefile b/src/backend/catalog/Makefile
index eefebb7bb8..4b28216f89 100644
--- a/src/backend/catalog/Makefile
+++ b/src/backend/catalog/Makefile
@@ -28,6 +28,7 @@ OBJS = \
 	pg_cast.o \
 	pg_class.o \
 	pg_collation.o \
+	pg_setting_acl.o \
 	pg_constraint.o \
 	pg_conversion.o \
 	pg_db_role_setting.o \
@@ -54,7 +55,7 @@ include $(top_srcdir)/src/backend/common.mk
 # there are reputedly other, undocumented ordering dependencies.
 CATALOG_HEADERS := \
 	pg_proc.h pg_type.h pg_attribute.h pg_class.h \
-	pg_attrdef.h pg_constraint.h pg_inherits.h pg_index.h pg_operator.h \
+	pg_attrdef.h pg_setting_acl.h pg_constraint.h pg_inherits.h pg_index.h pg_operator.h \
 	pg_opfamily.h pg_opclass.h pg_am.h pg_amop.h pg_amproc.h \
 	pg_language.h pg_largeobject_metadata.h pg_largeobject.h pg_aggregate.h \
 	pg_statistic.h pg_statistic_ext.h pg_statistic_ext_data.h \
diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index 1dd03a8e51..83f066118a 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -49,6 +49,7 @@
 #include "catalog/pg_operator.h"
 #include "catalog/pg_opfamily.h"
 #include "catalog/pg_proc.h"
+#include "catalog/pg_setting_acl.h"
 #include "catalog/pg_statistic_ext.h"
 #include "catalog/pg_subscription.h"
 #include "catalog/pg_tablespace.h"
@@ -112,6 +113,7 @@ static void ExecGrant_Largeobject(InternalGrant *grantStmt);
 static void ExecGrant_Namespace(InternalGrant *grantStmt);
 static void ExecGrant_Tablespace(InternalGrant *grantStmt);
 static void ExecGrant_Type(InternalGrant *grantStmt);
+static void ExecGrant_Setting(InternalGrant *grantStmt);
 
 static void SetDefaultACLsInSchemas(InternalDefaultACL *iacls, List *nspnames);
 static void SetDefaultACL(InternalDefaultACL *iacls);
@@ -259,6 +261,9 @@ restrict_and_check_grant(bool is_grant, AclMode avail_goptions, bool all_privs,
 		case OBJECT_TYPE:
 			whole_mask = ACL_ALL_RIGHTS_TYPE;
 			break;
+		case OBJECT_SETTING:
+			whole_mask = ACL_ALL_RIGHTS_SETTING;
+			break;
 		default:
 			elog(ERROR, "unrecognized object type: %d", objtype);
 			/* not reached, but keep compiler quiet */
@@ -498,6 +503,10 @@ ExecuteGrantStmt(GrantStmt *stmt)
 			all_privileges = ACL_ALL_RIGHTS_FOREIGN_SERVER;
 			errormsg = gettext_noop("invalid privilege type %s for foreign server");
 			break;
+		case OBJECT_SETTING:
+			all_privileges = ACL_ALL_RIGHTS_SETTING;
+			errormsg = gettext_noop("invalid privilege type %s for setting");
+			break;
 		default:
 			elog(ERROR, "unrecognized GrantStmt.objtype: %d",
 				 (int) stmt->objtype);
@@ -600,6 +609,9 @@ ExecGrantStmt_oids(InternalGrant *istmt)
 		case OBJECT_TABLESPACE:
 			ExecGrant_Tablespace(istmt);
 			break;
+		case OBJECT_SETTING:
+			ExecGrant_Setting(istmt);
+			break;
 		default:
 			elog(ERROR, "unrecognized GrantStmt.objtype: %d",
 				 (int) istmt->objtype);
@@ -759,6 +771,38 @@ objectNamesToOids(ObjectType objtype, List *objnames)
 				objects = lappend_oid(objects, srvid);
 			}
 			break;
+		case OBJECT_SETTING:
+			foreach(cell, objnames)
+			{
+				char	   *setting = strVal(lfirst(cell));
+				Oid			settingid = get_setting_oid(setting, true);
+
+				if (!OidIsValid(settingid))
+				{
+					/*
+					 * Lookup the existing entry, or if necessary, add a new
+					 * entry for this parameter.  Entries only exist for
+					 * parameters which currently have, or previously have had,
+					 * privileges assigned.
+					 *
+					 * It is tempting to sanity-check the given configuration
+					 * parameter name against known guc names in the guc
+					 * tables, but for handling upgrades we need to accept
+					 * setting names that do not yet exist.
+					 */
+					settingid = SettingAclCreate(setting, true);
+
+					/*
+					 * Prevent error when processing duplicate objects, and
+					 * make this new entry visible to our later selves which
+					 * will need to update the Acl.
+					 */
+					CommandCounterIncrement();
+				}
+
+				objects = lappend_oid(objects, settingid);
+			}
+			break;
 		default:
 			elog(ERROR, "unrecognized GrantStmt.objtype: %d",
 				 (int) objtype);
@@ -1494,6 +1538,9 @@ RemoveRoleFromObjectACL(Oid roleid, Oid classid, Oid objid)
 			case ForeignDataWrapperRelationId:
 				istmt.objtype = OBJECT_FDW;
 				break;
+			case SettingAclRelationId:
+				istmt.objtype = OBJECT_SETTING;
+				break;
 			default:
 				elog(ERROR, "unexpected object class %u", classid);
 				break;
@@ -3225,6 +3272,139 @@ ExecGrant_Type(InternalGrant *istmt)
 	table_close(relation, RowExclusiveLock);
 }
 
+static void
+ExecGrant_Setting(InternalGrant *istmt)
+{
+	Relation	relation;
+	ListCell   *cell;
+
+	if (istmt->all_privs && istmt->privileges == ACL_NO_RIGHTS)
+		istmt->privileges = ACL_ALL_RIGHTS_SETTING;
+
+	relation = table_open(SettingAclRelationId, RowExclusiveLock);
+
+	foreach(cell, istmt->objects)
+	{
+		Oid			settingId = lfirst_oid(cell);
+		Form_pg_setting_acl pg_setting_acl_tuple;
+		Datum		aclDatum;
+		bool		isNull;
+		AclMode		avail_goptions;
+		AclMode		this_privileges;
+		Acl		   *old_acl;
+		Acl		   *new_acl;
+		Oid			grantorId;
+		HeapTuple	tuple;
+		HeapTuple	newtuple;
+		Datum		values[Natts_pg_setting_acl];
+		bool		nulls[Natts_pg_setting_acl];
+		bool		replaces[Natts_pg_setting_acl];
+		int			noldmembers;
+		int			nnewmembers;
+		Oid		   *oldmembers;
+		Oid		   *newmembers;
+
+		tuple = SearchSysCache1(SETTINGOID, ObjectIdGetDatum(settingId));
+		if (!HeapTupleIsValid(tuple))
+			elog(ERROR, "cache lookup failed for setting %u", settingId);
+
+		pg_setting_acl_tuple = (Form_pg_setting_acl) GETSTRUCT(tuple);
+
+		/*
+		 * Get owner ID and working copy of existing ACL. If there's no ACL,
+		 * substitute the proper default.
+		 */
+		aclDatum = SysCacheGetAttr(SETTINGNAME, tuple, Anum_pg_setting_acl_setacl,
+								   &isNull);
+
+		/*
+		 * If the acl is null, we need to create a more permissive default acl
+		 * for userset variables than for any others.
+		 */
+		if (isNull)
+		{
+			const char *setting;
+
+			setting = text_to_cstring(&pg_setting_acl_tuple->setting);
+			old_acl = aclsettingdefault(find_option_context(setting) == PGC_USERSET);
+			/* There are no old member roles according to the catalogs */
+			noldmembers = 0;
+			oldmembers = NULL;
+		}
+		else
+		{
+			old_acl = DatumGetAclPCopy(aclDatum);
+			/* Get the roles mentioned in the existing ACL */
+			noldmembers = aclmembers(old_acl, &oldmembers);
+		}
+
+		/* Determine ID to do the grant as, and available grant options */
+		select_best_grantor(GetUserId(), istmt->privileges,
+							old_acl, BOOTSTRAP_SUPERUSERID,
+							&grantorId, &avail_goptions);
+
+		/*
+		 * Restrict the privileges to what we can actually grant, and emit the
+		 * standards-mandated warning and error messages.
+		 */
+		this_privileges =
+			restrict_and_check_grant(istmt->is_grant, avail_goptions,
+									 istmt->all_privs, istmt->privileges,
+									 settingId, grantorId, OBJECT_SETTING,
+									 text_to_cstring(&pg_setting_acl_tuple->setting),
+									 0, NULL);
+
+		/*
+		 * Generate new ACL.
+		 */
+		new_acl = merge_acl_with_grant(old_acl, istmt->is_grant,
+									   istmt->grant_option, istmt->behavior,
+									   istmt->grantees, this_privileges,
+									   grantorId, InvalidOid);
+
+		/*
+		 * We need the members of both old and new ACLs so we can correct the
+		 * shared dependency information.
+		 */
+		nnewmembers = aclmembers(new_acl, &newmembers);
+
+		/* finished building new ACL value, now insert it */
+		MemSet(values, 0, sizeof(values));
+		MemSet(nulls, false, sizeof(nulls));
+		MemSet(replaces, false, sizeof(replaces));
+
+		replaces[Anum_pg_setting_acl_setacl - 1] = true;
+		values[Anum_pg_setting_acl_setacl - 1] = PointerGetDatum(new_acl);
+
+		newtuple = heap_modify_tuple(tuple, RelationGetDescr(relation), values,
+									 nulls, replaces);
+
+		CatalogTupleUpdate(relation, &newtuple->t_self, newtuple);
+
+		/* Update initial privileges for extensions */
+		recordExtensionInitPriv(settingId, SettingAclRelationId, 0, new_acl);
+
+		/* Update the shared dependency ACL info */
+		updateAclDependencies(SettingAclRelationId,
+							  pg_setting_acl_tuple->oid, 0,
+							  InvalidOid,
+							  noldmembers, oldmembers,
+							  nnewmembers, newmembers);
+
+		ReleaseSysCache(tuple);
+
+		pfree(new_acl);
+
+		/* Post alter hook called for grant and revoke */
+		InvokeObjectPostAlterHook(SettingAclRelationId, settingId, 0);
+
+		/* prevent error when processing duplicate objects */
+		CommandCounterIncrement();
+	}
+
+	table_close(relation, RowExclusiveLock);
+}
+
 
 static AclMode
 string_to_privilege(const char *privname)
@@ -3255,6 +3435,10 @@ string_to_privilege(const char *privname)
 		return ACL_CREATE_TEMP;
 	if (strcmp(privname, "connect") == 0)
 		return ACL_CONNECT;
+	if (strcmp(privname, "set value") == 0)
+		return ACL_SET_VALUE;
+	if (strcmp(privname, "alter system") == 0)
+		return ACL_ALTER_SYSTEM;
 	if (strcmp(privname, "rule") == 0)
 		return 0;				/* ignore old RULE privileges */
 	ereport(ERROR,
@@ -3292,6 +3476,10 @@ privilege_to_string(AclMode privilege)
 			return "TEMP";
 		case ACL_CONNECT:
 			return "CONNECT";
+		case ACL_SET_VALUE:
+			return "SET VALUE";
+		case ACL_ALTER_SYSTEM:
+			return "ALTER SYSTEM";
 		default:
 			elog(ERROR, "unrecognized privilege: %d", (int) privilege);
 	}
@@ -3328,6 +3516,13 @@ aclcheck_error(AclResult aclerr, ObjectType objtype,
 					case OBJECT_COLUMN:
 						msg = gettext_noop("permission denied for column %s");
 						break;
+					case OBJECT_SETTING:
+						/*
+						 * Quote the object name for backward compatibility
+						 * with behavior before SET was handled here.
+						 */
+						msg = gettext_noop("permission denied to set parameter \"%s\"");
+						break;
 					case OBJECT_CONVERSION:
 						msg = gettext_noop("permission denied for conversion %s");
 						break;
@@ -3564,6 +3759,7 @@ aclcheck_error(AclResult aclerr, ObjectType objtype,
 					case OBJECT_AMPROC:
 					case OBJECT_ATTRIBUTE:
 					case OBJECT_CAST:
+					case OBJECT_SETTING:
 					case OBJECT_DEFAULT:
 					case OBJECT_DEFACL:
 					case OBJECT_DOMCONSTRAINT:
@@ -4000,6 +4196,59 @@ pg_database_aclmask(Oid db_oid, Oid roleid,
 	return result;
 }
 
+/*
+ * Exported routine for examining a user's privileges for a configuration
+ * parameter (GUC)
+ */
+AclMode
+pg_setting_acl_aclmask(Oid config_oid, Oid roleid,
+						AclMode mask, AclMaskHow how)
+{
+	AclMode		result;
+	HeapTuple	tuple;
+	Datum		aclDatum;
+	bool		isNull;
+	Acl		   *acl;
+
+	/* Superusers bypass all permission checking. */
+	if (superuser_arg(roleid))
+		return mask;
+
+	/*
+	 * Get the setting's ACL from pg_setting_acl
+	 */
+	tuple = SearchSysCache1(SETTINGOID, ObjectIdGetDatum(config_oid));
+	if (!HeapTupleIsValid(tuple))
+		ereport(ERROR,
+				(errcode(ERRCODE_UNDEFINED_DATABASE),
+				 errmsg("setting with OID %u does not exist",
+						config_oid)));
+
+	aclDatum = SysCacheGetAttr(SETTINGOID, tuple, Anum_pg_setting_acl_setacl,
+							   &isNull);
+	if (isNull)
+	{
+		/* No ACL, so build default ACL */
+		acl = acldefault(OBJECT_SETTING, InvalidOid);
+		aclDatum = (Datum) 0;
+	}
+	else
+	{
+		/* detoast ACL if necessary */
+		acl = DatumGetAclP(aclDatum);
+	}
+
+	result = aclmask(acl, roleid, InvalidOid, mask, how);
+
+	/* if we have a detoasted copy, free it */
+	if (acl && (Pointer) acl != DatumGetPointer(aclDatum))
+		pfree(acl);
+
+	ReleaseSysCache(tuple);
+
+	return result;
+}
+
 /*
  * Exported routine for examining a user's privileges for a function
  */
@@ -4713,6 +4962,19 @@ pg_database_aclcheck(Oid db_oid, Oid roleid, AclMode mode)
 		return ACLCHECK_NO_PRIV;
 }
 
+/*
+ * Exported routine for checking a user's access privileges to a configuration
+ * parameter
+ */
+AclResult
+pg_setting_acl_aclcheck(Oid config_oid, Oid roleid, AclMode mode)
+{
+	if (pg_setting_acl_aclmask(config_oid, roleid, mode, ACLMASK_ANY) != 0)
+		return ACLCHECK_OK;
+	else
+		return ACLCHECK_NO_PRIV;
+}
+
 /*
  * Exported routine for checking a user's access privileges to a function
  */
diff --git a/src/backend/catalog/catalog.c b/src/backend/catalog/catalog.c
index dfd5fb669e..78868e7052 100644
--- a/src/backend/catalog/catalog.c
+++ b/src/backend/catalog/catalog.c
@@ -34,6 +34,7 @@
 #include "catalog/pg_largeobject.h"
 #include "catalog/pg_namespace.h"
 #include "catalog/pg_replication_origin.h"
+#include "catalog/pg_setting_acl.h"
 #include "catalog/pg_shdepend.h"
 #include "catalog/pg_shdescription.h"
 #include "catalog/pg_shseclabel.h"
@@ -246,6 +247,7 @@ IsSharedRelation(Oid relationId)
 	/* These are the shared catalogs (look for BKI_SHARED_RELATION) */
 	if (relationId == AuthIdRelationId ||
 		relationId == AuthMemRelationId ||
+		relationId == SettingAclRelationId ||
 		relationId == DatabaseRelationId ||
 		relationId == SharedDescriptionRelationId ||
 		relationId == SharedDependRelationId ||
@@ -260,6 +262,8 @@ IsSharedRelation(Oid relationId)
 		relationId == AuthIdOidIndexId ||
 		relationId == AuthMemRoleMemIndexId ||
 		relationId == AuthMemMemRoleIndexId ||
+		relationId == SettingAclSettingIndexId ||
+		relationId == SettingAclOidIndexId ||
 		relationId == DatabaseNameIndexId ||
 		relationId == DatabaseOidIndexId ||
 		relationId == SharedDescriptionObjIndexId ||
@@ -277,6 +281,8 @@ IsSharedRelation(Oid relationId)
 	/* These are their toast tables and toast indexes */
 	if (relationId == PgAuthidToastTable ||
 		relationId == PgAuthidToastIndex ||
+		relationId == PgSettingAclToastTable ||
+		relationId == PgSettingAclToastIndex ||
 		relationId == PgDatabaseToastTable ||
 		relationId == PgDatabaseToastIndex ||
 		relationId == PgDbRoleSettingToastTable ||
diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index ab9e42d7d1..5d2504463d 100644
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -52,6 +52,7 @@
 #include "catalog/pg_publication_namespace.h"
 #include "catalog/pg_publication_rel.h"
 #include "catalog/pg_rewrite.h"
+#include "catalog/pg_setting_acl.h"
 #include "catalog/pg_statistic_ext.h"
 #include "catalog/pg_subscription.h"
 #include "catalog/pg_tablespace.h"
@@ -150,6 +151,7 @@ static const Oid object_classes[] = {
 	TypeRelationId,				/* OCLASS_TYPE */
 	CastRelationId,				/* OCLASS_CAST */
 	CollationRelationId,		/* OCLASS_COLLATION */
+	SettingAclRelationId,		/* OCLASS_SETTING */
 	ConstraintRelationId,		/* OCLASS_CONSTRAINT */
 	ConversionRelationId,		/* OCLASS_CONVERSION */
 	AttrDefaultRelationId,		/* OCLASS_DEFAULT */
@@ -1504,6 +1506,7 @@ doDeletion(const ObjectAddress *object, int flags)
 			/*
 			 * These global object types are not supported here.
 			 */
+		case OCLASS_SETTING:
 		case OCLASS_ROLE:
 		case OCLASS_DATABASE:
 		case OCLASS_TBLSPACE:
@@ -2778,6 +2781,9 @@ getObjectClass(const ObjectAddress *object)
 		case CollationRelationId:
 			return OCLASS_COLLATION;
 
+		case SettingAclRelationId:
+			return OCLASS_SETTING;
+
 		case ConstraintRelationId:
 			return OCLASS_CONSTRAINT;
 
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index f30c742d48..1ef833a0e3 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -51,6 +51,7 @@
 #include "catalog/pg_publication_namespace.h"
 #include "catalog/pg_publication_rel.h"
 #include "catalog/pg_rewrite.h"
+#include "catalog/pg_setting_acl.h"
 #include "catalog/pg_statistic_ext.h"
 #include "catalog/pg_subscription.h"
 #include "catalog/pg_tablespace.h"
@@ -2309,6 +2310,7 @@ pg_get_object_address(PG_FUNCTION_ARGS)
 		case OBJECT_COLUMN:
 		case OBJECT_ATTRIBUTE:
 		case OBJECT_COLLATION:
+		case OBJECT_SETTING:
 		case OBJECT_CONVERSION:
 		case OBJECT_STATISTIC_EXT:
 		case OBJECT_TSPARSER:
@@ -3510,6 +3512,22 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok)
 				break;
 			}
 
+		case OCLASS_SETTING:
+			{
+				char	   *setting;
+
+				setting = get_setting_name(object->objectId);
+				if (!setting)
+				{
+					if (!missing_ok)
+						elog(ERROR, "cache lookup failed for setting %u",
+							 object->objectId);
+					break;
+				}
+				appendStringInfo(&buffer, _("setting %s"), setting);
+				break;
+			}
+
 		case OCLASS_SCHEMA:
 			{
 				char	   *nspname;
@@ -4473,6 +4491,10 @@ getObjectTypeDescription(const ObjectAddress *object, bool missing_ok)
 			appendStringInfoString(&buffer, "collation");
 			break;
 
+		case OCLASS_SETTING:
+			appendStringInfoString(&buffer, "setting");
+			break;
+
 		case OCLASS_CONSTRAINT:
 			getConstraintTypeDescription(&buffer, object->objectId,
 										 missing_ok);
@@ -4977,6 +4999,30 @@ getObjectIdentityParts(const ObjectAddress *object,
 				break;
 			}
 
+		case OCLASS_SETTING:
+			{
+				HeapTuple	configTup;
+				Form_pg_setting_acl configForm;
+				char	   *namestr;
+
+				configTup = SearchSysCache1(SETTINGOID,
+											ObjectIdGetDatum(object->objectId));
+				if (!HeapTupleIsValid(configTup))
+				{
+					if (!missing_ok)
+						elog(ERROR, "cache lookup failed for setting %u",
+							 object->objectId);
+					break;
+				}
+				configForm = (Form_pg_setting_acl) GETSTRUCT(configTup);
+				namestr = text_to_cstring(&configForm->setting);
+				appendStringInfoString(&buffer, namestr);
+				if (objname)
+					*objname = list_make1(namestr);
+				ReleaseSysCache(configTup);
+				break;
+			}
+
 		case OCLASS_CONVERSION:
 			{
 				HeapTuple	conTup;
diff --git a/src/backend/catalog/pg_setting_acl.c b/src/backend/catalog/pg_setting_acl.c
new file mode 100644
index 0000000000..3c6594da03
--- /dev/null
+++ b/src/backend/catalog/pg_setting_acl.c
@@ -0,0 +1,122 @@
+/*-------------------------------------------------------------------------
+ *
+ * pg_setting_acl.c
+ *	  routines to support manipulation of the pg_setting_acl relation
+ *
+ * Portions Copyright (c) 1996-2021, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ *
+ * IDENTIFICATION
+ *	  src/backend/catalog/pg_setting_acl.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "postgres.h"
+
+#include "access/genam.h"
+#include "access/heapam.h"
+#include "access/htup_details.h"
+#include "access/sysattr.h"
+#include "access/table.h"
+#include "access/tableam.h"
+#include "catalog/catalog.h"
+#include "catalog/dependency.h"
+#include "catalog/indexing.h"
+#include "catalog/objectaccess.h"
+#include "catalog/pg_namespace.h"
+#include "catalog/pg_setting_acl.h"
+#include "mb/pg_wchar.h"
+#include "utils/builtins.h"
+#include "utils/fmgroids.h"
+#include "utils/guc.h"
+#include "utils/pg_locale.h"
+#include "utils/rel.h"
+#include "utils/syscache.h"
+
+
+/*
+ * SettingAclCreate
+ *
+ * Add a new tuple to pg_setting_acl.
+ *
+ * setting: the setting name to create.
+ * if_not_exists: if true, don't fail on duplicate name, but rather return
+ * the existing entry's Oid.
+ */
+Oid
+SettingAclCreate(const char *setting, bool if_not_exists)
+{
+	Relation	rel;
+	TupleDesc	tupDesc;
+	HeapTuple	tuple;
+	Datum		values[Natts_pg_setting_acl];
+	bool		nulls[Natts_pg_setting_acl];
+	Oid			settingId;
+	const char *canonical;
+
+	/*
+	 * Check whether the setting (by the given name or alias)
+	 * already exists.
+	 */
+	settingId = get_setting_oid(setting, true);
+	if (OidIsValid(settingId))
+	{
+		if (if_not_exists)
+			return settingId;
+		ereport(ERROR,
+				(errcode(ERRCODE_DUPLICATE_OBJECT),
+				 errmsg("setting \"%s\" already exists",
+						setting)));
+	}
+
+	/*
+	 * We must not require the setting to be in the list of existent GUCs,
+	 * as we may be called at different points during upgrades or the
+	 * installation or removal of extensions.  Instead, perform a basic sanity
+	 * check of the setting, and translate old forms of known names to their
+	 * canonical forms.
+	 *
+	 * If you deprecate a configuration name in favor of a new spelling, be
+	 * sure to consider whether to also upgrade pg_setting_acl entries.
+	 */
+	if (!valid_variable_name(setting, NULL))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_NAME),
+				 errmsg("invalid setting name \"%s\"",
+						setting)));
+	canonical = GetConfigOptionCanonicalName(setting);
+	if (!canonical)
+		canonical = setting;
+
+	/*
+	 * Create and insert a new record, starting with a blank Acl.
+	 *
+	 * We don't take a strong enough lock to prevent concurrent insertions,
+	 * relying instead on the unique index.
+	 */
+	rel = table_open(SettingAclRelationId, RowExclusiveLock);
+	tupDesc = RelationGetDescr(rel);
+	MemSet(values, 0, sizeof(values));
+	MemSet(nulls, false, sizeof(nulls));
+	values[Anum_pg_setting_acl_setting - 1] =
+		DirectFunctionCall1(textin, CStringGetDatum(canonical));
+	settingId = GetNewOidWithIndex(rel,
+									   SettingAclOidIndexId,
+									   Anum_pg_setting_acl_oid);
+	values[Anum_pg_setting_acl_oid - 1] = ObjectIdGetDatum(settingId);
+	nulls[Anum_pg_setting_acl_setacl - 1] = true;
+	tuple = heap_form_tuple(tupDesc, values, nulls);
+	CatalogTupleInsert(rel, tuple);
+
+	/* Post creation hook for new setting */
+	InvokeObjectPostCreateHook(SettingAclRelationId, settingId, 0);
+
+	/*
+	 * Close pg_setting_acl, but keep lock till commit.
+	 */
+	heap_freetuple(tuple);
+	table_close(rel, NoLock);
+
+	return settingId;
+}
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 40b7bca5a9..92b226b8d2 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -595,6 +595,19 @@ CREATE RULE pg_settings_n AS
 
 GRANT SELECT, UPDATE ON pg_settings TO PUBLIC;
 
+CREATE VIEW pg_setting_privileges AS
+	SELECT grantor.rolname AS grantor,
+		   grantee.rolname AS grantee,
+		   set_acl.setting AS setting,
+		   acl.privilege_type AS privilege_type,
+		   acl.is_grantable
+		FROM pg_catalog.pg_setting_acl set_acl,
+		LATERAL (SELECT * FROM aclexplode(set_acl.setacl)) acl
+		LEFT JOIN pg_catalog.pg_authid grantee ON acl.grantee = grantee.oid
+		LEFT JOIN pg_catalog.pg_authid grantor ON acl.grantor = grantor.oid;
+
+GRANT SELECT ON pg_setting_privileges TO PUBLIC;
+
 CREATE VIEW pg_file_settings AS
    SELECT * FROM pg_show_all_file_settings() AS A;
 
diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c
index 1f64c8aa51..3dd0c5cadb 100644
--- a/src/backend/commands/alter.c
+++ b/src/backend/commands/alter.c
@@ -639,6 +639,7 @@ AlterObjectNamespace_oid(Oid classId, Oid objid, Oid nspOid,
 			break;
 
 		case OCLASS_CAST:
+		case OCLASS_SETTING:
 		case OCLASS_CONSTRAINT:
 		case OCLASS_DEFAULT:
 		case OCLASS_LANGUAGE:
diff --git a/src/backend/commands/dropcmds.c b/src/backend/commands/dropcmds.c
index c9b5732448..00398727ed 100644
--- a/src/backend/commands/dropcmds.c
+++ b/src/backend/commands/dropcmds.c
@@ -276,6 +276,13 @@ does_not_exist_skipping(ObjectType objtype, Node *object)
 				name = NameListToString(castNode(List, object));
 			}
 			break;
+		case OBJECT_SETTING:
+			if (!schema_does_not_exist_skipping(castNode(List, object), &msg, &name))
+			{
+				msg = gettext_noop("setting \"%s\" does not exist, skipping");
+				name = NameListToString(castNode(List, object));
+			}
+			break;
 		case OBJECT_CONVERSION:
 			if (!schema_does_not_exist_skipping(castNode(List, object), &msg, &name))
 			{
diff --git a/src/backend/commands/event_trigger.c b/src/backend/commands/event_trigger.c
index 3c3fc2515b..f500e8c40d 100644
--- a/src/backend/commands/event_trigger.c
+++ b/src/backend/commands/event_trigger.c
@@ -937,6 +937,7 @@ EventTriggerSupportsObjectType(ObjectType obtype)
 {
 	switch (obtype)
 	{
+		case OBJECT_SETTING:
 		case OBJECT_DATABASE:
 		case OBJECT_TABLESPACE:
 		case OBJECT_ROLE:
@@ -1012,6 +1013,7 @@ EventTriggerSupportsObjectClass(ObjectClass objclass)
 {
 	switch (objclass)
 	{
+		case OCLASS_SETTING:
 		case OCLASS_DATABASE:
 		case OCLASS_TBLSPACE:
 		case OCLASS_ROLE:
@@ -2022,6 +2024,8 @@ stringify_grant_objtype(ObjectType objtype)
 	{
 		case OBJECT_COLUMN:
 			return "COLUMN";
+		case OBJECT_SETTING:
+			return "SETTING";
 		case OBJECT_TABLE:
 			return "TABLE";
 		case OBJECT_SEQUENCE:
@@ -2105,6 +2109,8 @@ stringify_adefprivs_objtype(ObjectType objtype)
 	{
 		case OBJECT_COLUMN:
 			return "COLUMNS";
+		case OBJECT_SETTING:
+			return "SETTINGS";
 		case OBJECT_TABLE:
 			return "TABLES";
 		case OBJECT_SEQUENCE:
diff --git a/src/backend/commands/seclabel.c b/src/backend/commands/seclabel.c
index 7a62d547e2..7b05eb0110 100644
--- a/src/backend/commands/seclabel.c
+++ b/src/backend/commands/seclabel.c
@@ -67,6 +67,7 @@ SecLabelSupportsObjectType(ObjectType objtype)
 		case OBJECT_ATTRIBUTE:
 		case OBJECT_CAST:
 		case OBJECT_COLLATION:
+		case OBJECT_SETTING:
 		case OBJECT_CONVERSION:
 		case OBJECT_DEFAULT:
 		case OBJECT_DEFACL:
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index dc5872f988..11671390a5 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -12620,6 +12620,7 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
 			case OCLASS_TYPE:
 			case OCLASS_CAST:
 			case OCLASS_COLLATION:
+			case OCLASS_SETTING:
 			case OCLASS_CONVERSION:
 			case OCLASS_LANGUAGE:
 			case OCLASS_LARGEOBJECT:
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index a03b33b53b..172d9e31e3 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -363,8 +363,8 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 %type <str>		foreign_server_version opt_foreign_server_version
 %type <str>		opt_in_database
 
-%type <str>		OptSchemaName
-%type <list>	OptSchemaEltList
+%type <str>		OptSchemaName setting_name
+%type <list>	OptSchemaEltList setting_target
 
 %type <chr>		am_type
 
@@ -728,7 +728,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 	ROUTINE ROUTINES ROW ROWS RULE
 
 	SAVEPOINT SCHEMA SCHEMAS SCROLL SEARCH SECOND_P SECURITY SELECT SEQUENCE SEQUENCES
-	SERIALIZABLE SERVER SESSION SESSION_USER SET SETS SETOF SHARE SHOW
+	SERIALIZABLE SERVER SESSION SESSION_USER SET SETS SETOF SETTING SHARE SHOW
 	SIMILAR SIMPLE SKIP SMALLINT SNAPSHOT SOME SQL_P STABLE STANDALONE_P
 	START STATEMENT STATISTICS STDIN STDOUT STORAGE STORED STRICT_P STRIP_P
 	SUBSCRIPTION SUBSTRING SUPPORT SYMMETRIC SYSID SYSTEM_P
@@ -6981,6 +6981,20 @@ GrantStmt:	GRANT privileges ON privilege_target TO grantee_list
 					n->grantor = $8;
 					$$ = (Node*)n;
 				}
+			| GRANT privileges ON SETTING setting_target TO grantee_list
+			opt_grant_grant_option opt_granted_by
+				{
+					GrantStmt *n = makeNode(GrantStmt);
+					n->is_grant = true;
+					n->privileges = $2;
+					n->targtype = ACL_TARGET_OBJECT;
+					n->objtype = OBJECT_SETTING;
+					n->objects = $5;
+					n->grantees = $7;
+					n->grant_option = $8;
+					n->grantor = $9;
+					$$ = (Node*)n;
+				}
 		;
 
 RevokeStmt:
@@ -7014,6 +7028,36 @@ RevokeStmt:
 					n->behavior = $11;
 					$$ = (Node *)n;
 				}
+			| REVOKE privileges ON SETTING setting_target FROM grantee_list
+			opt_granted_by opt_drop_behavior
+				{
+					GrantStmt *n = makeNode(GrantStmt);
+					n->is_grant = false;
+					n->grant_option = false;
+					n->privileges = $2;
+					n->targtype = ACL_TARGET_OBJECT;
+					n->objtype = OBJECT_SETTING;
+					n->objects = $5;
+					n->grantees = $7;
+					n->grantor = $8;
+					n->behavior = $9;
+					$$ = (Node *)n;
+				}
+			| REVOKE GRANT OPTION FOR privileges ON SETTING setting_target
+			FROM grantee_list opt_granted_by opt_drop_behavior
+				{
+					GrantStmt *n = makeNode(GrantStmt);
+					n->is_grant = false;
+					n->grant_option = true;
+					n->privileges = $5;
+					n->targtype = ACL_TARGET_OBJECT;
+					n->objtype = OBJECT_SETTING;
+					n->objects = $8;
+					n->grantees = $10;
+					n->grantor = $11;
+					n->behavior = $12;
+					$$ = (Node *)n;
+				}
 		;
 
 
@@ -7073,6 +7117,20 @@ privilege:	SELECT opt_column_list
 				n->cols = $2;
 				$$ = n;
 			}
+		| SET VALUE_P
+			{
+				AccessPriv *n = makeNode(AccessPriv);
+				n->priv_name = pstrdup("set value");
+				n->cols = NULL;
+				$$ = n;
+			}
+		| ALTER SYSTEM_P
+			{
+				AccessPriv *n = makeNode(AccessPriv);
+				n->priv_name = pstrdup("alter system");
+				n->cols = NULL;
+				$$ = n;
+			}
 		| ColId opt_column_list
 			{
 				AccessPriv *n = makeNode(AccessPriv);
@@ -7082,6 +7140,27 @@ privilege:	SELECT opt_column_list
 			}
 		;
 
+setting_target:
+			setting_name
+				{
+					$$ = list_make1(makeString($1));
+				}
+			| setting_target ',' setting_name
+				{
+					$$ = lappend($1, makeString($3));
+				}
+		;
+
+setting_name:
+			ColId
+				{
+					$$ = $1;
+				}
+			| setting_name '.' ColId
+				{
+					$$ = psprintf("%s.%s", $1, $3);
+				}
+		;
 
 /* Don't bother trying to fold the first two rules into one using
  * opt_table.  You're going to get conflicts.
@@ -15912,6 +15991,7 @@ unreserved_keyword:
 			| SESSION
 			| SET
 			| SETS
+			| SETTING
 			| SHARE
 			| SHOW
 			| SIMPLE
@@ -16499,6 +16579,7 @@ bare_label_keyword:
 			| SET
 			| SETOF
 			| SETS
+			| SETTING
 			| SHARE
 			| SHOW
 			| SIMILAR
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index 0a16f8156c..fa15f3155c 100644
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -23,6 +23,7 @@
 #include "catalog/pg_authid.h"
 #include "catalog/pg_class.h"
 #include "catalog/pg_database.h"
+#include "catalog/pg_setting_acl.h"
 #include "catalog/pg_type.h"
 #include "commands/dbcommands.h"
 #include "commands/proclang.h"
@@ -36,6 +37,7 @@
 #include "utils/array.h"
 #include "utils/builtins.h"
 #include "utils/catcache.h"
+#include "utils/guc.h"
 #include "utils/inval.h"
 #include "utils/lsyscache.h"
 #include "utils/memutils.h"
@@ -109,6 +111,7 @@ static Oid	convert_tablespace_name(text *tablespacename);
 static AclMode convert_tablespace_priv_string(text *priv_type_text);
 static Oid	convert_type_name(text *typename);
 static AclMode convert_type_priv_string(text *priv_type_text);
+static AclMode convert_setting_priv_string(text *priv_setting_text);
 static AclMode convert_role_priv_string(text *priv_type_text);
 static AclResult pg_role_aclcheck(Oid role_oid, Oid roleid, AclMode mode);
 
@@ -306,6 +309,12 @@ aclparse(const char *s, AclItem *aip)
 			case ACL_CONNECT_CHR:
 				read = ACL_CONNECT;
 				break;
+			case ACL_SET_VALUE_CHR:
+				read = ACL_SET_VALUE;
+				break;
+			case ACL_ALTER_SYSTEM_CHR:
+				read = ACL_ALTER_SYSTEM;
+				break;
 			case 'R':			/* ignore old RULE privileges */
 				read = 0;
 				break;
@@ -794,6 +803,10 @@ acldefault(ObjectType objtype, Oid ownerId)
 			world_default = ACL_USAGE;
 			owner_default = ACL_ALL_RIGHTS_TYPE;
 			break;
+		case OBJECT_SETTING:
+			world_default = ACL_NO_RIGHTS;
+			owner_default = ACL_ALL_RIGHTS_SETTING;
+			break;
 		default:
 			elog(ERROR, "unrecognized objtype: %d", (int) objtype);
 			world_default = ACL_NO_RIGHTS;	/* keep compiler quiet */
@@ -838,6 +851,41 @@ acldefault(ObjectType objtype, Oid ownerId)
 	return acl;
 }
 
+/*
+ * aclsettingdefault()
+ *
+ * Creates and returns an ACL describing the default access permissions for a
+ * setting, taking into account whether it is a PGC_USERSET GUC setting.
+ */
+Acl *
+aclsettingdefault(bool is_userset)
+{
+	Acl		   *acl;
+
+	/*
+	 * Treat all settings as belonging to the bootstrap user.  This works
+	 * better than passing InvalidOid, as we want the bootstrap user to retain
+	 * privileges even after running a REVOKE .. FROM PUBLIC on a default ACL.
+	 */
+	acl = acldefault(OBJECT_SETTING, BOOTSTRAP_SUPERUSERID);
+
+	/*
+	 * Special case for USERSET gucs.  By default, public can SET
+	 */
+	if (is_userset)
+	{
+		AclItem item;
+
+		item.ai_grantee = ACL_ID_PUBLIC;
+		item.ai_grantor = BOOTSTRAP_SUPERUSERID;
+		item.ai_privs = ACL_SET_VALUE;
+
+		acl = aclupdate(acl, &item, ACL_MODECHG_ADD, BOOTSTRAP_SUPERUSERID,
+						DROP_RESTRICT);
+	}
+
+	return acl;
+}
 
 /*
  * SQL-accessible version of acldefault().  Hackish mapping from "char" type to
@@ -895,6 +943,17 @@ acldefault_sql(PG_FUNCTION_ARGS)
 	PG_RETURN_ACL_P(acldefault(objtype, owner));
 }
 
+/*
+ * SQL-accessible version of aclsettingdefault().
+ */
+Datum
+aclsettingdefault_sql(PG_FUNCTION_ARGS)
+{
+	const char *setting = text_to_cstring(PG_GETARG_TEXT_P(0));
+
+	PG_RETURN_ACL_P(aclsettingdefault(find_option_context(setting) ==
+									  PGC_USERSET));
+}
 
 /*
  * Update an ACL array to add or remove specified privileges.
@@ -1602,6 +1661,10 @@ convert_priv_string(text *priv_type_text)
 		return ACL_CREATE_TEMP;
 	if (pg_strcasecmp(priv_type, "CONNECT") == 0)
 		return ACL_CONNECT;
+	if (pg_strcasecmp(priv_type, "SET VALUE") == 0)
+		return ACL_SET_VALUE;
+	if (pg_strcasecmp(priv_type, "ALTER SYSTEM") == 0)
+		return ACL_ALTER_SYSTEM;
 	if (pg_strcasecmp(priv_type, "RULE") == 0)
 		return 0;				/* ignore old RULE privileges */
 
@@ -1698,6 +1761,10 @@ convert_aclright_to_string(int aclright)
 			return "TEMPORARY";
 		case ACL_CONNECT:
 			return "CONNECT";
+		case ACL_SET_VALUE:
+			return "SET VALUE";
+		case ACL_ALTER_SYSTEM:
+			return "ALTER SYSTEM";
 		default:
 			elog(ERROR, "unrecognized aclright: %d", aclright);
 			return NULL;
@@ -4429,6 +4496,193 @@ convert_type_priv_string(text *priv_type_text)
 	return convert_any_priv_string(priv_type_text, type_priv_map);
 }
 
+/*
+ * has_setting_privilege variants
+ *		These are all named "has_setting_privilege" at the SQL level.
+ *		They take various combinations of setting name, setting OID,
+ *		user name, user OID, or implicit user = current_user.
+ *
+ *		The result is a boolean value: true if user has the indicated
+ *		privilege, false if not, or NULL if object doesn't exist.
+ */
+
+/*
+ * has_setting_privilege_name_name
+ *		Check user privileges on a setting given
+ *		name username, text setting, and text priv name.
+ */
+Datum
+has_setting_privilege_name_name(PG_FUNCTION_ARGS)
+{
+	Name		username = PG_GETARG_NAME(0);
+	text	   *setting = PG_GETARG_TEXT_PP(1);
+	text	   *priv_setting_text = PG_GETARG_TEXT_PP(2);
+	Oid			roleid;
+	Oid			settingoid;
+	AclMode		mode;
+	AclResult	aclresult;
+
+	roleid = get_role_oid_or_public(NameStr(*username));
+
+	settingoid = get_setting_oid(text_to_cstring(setting), true);
+	if (!OidIsValid(settingoid))
+		PG_RETURN_NULL();
+	mode = convert_setting_priv_string(priv_setting_text);
+
+	aclresult = pg_setting_acl_aclcheck(settingoid, roleid, mode);
+
+	PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
+}
+
+/*
+ * has_setting_privilege_name
+ *		Check user privileges on a setting given
+ *		text setting and text priv name.
+ *		current_user is assumed
+ */
+Datum
+has_setting_privilege_name(PG_FUNCTION_ARGS)
+{
+	text	   *setting = PG_GETARG_TEXT_PP(0);
+	text	   *priv_setting_text = PG_GETARG_TEXT_PP(1);
+	Oid			roleid;
+	Oid			settingoid;
+	AclMode		mode;
+	AclResult	aclresult;
+
+	roleid = GetUserId();
+	settingoid = get_setting_oid(text_to_cstring(setting), true);
+	if (!OidIsValid(settingoid))
+		PG_RETURN_NULL();
+	mode = convert_setting_priv_string(priv_setting_text);
+
+	aclresult = pg_setting_acl_aclcheck(settingoid, roleid, mode);
+
+	PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
+}
+
+/*
+ * has_setting_privilege_name_id
+ *		Check user privileges on a setting given
+ *		name usename, setting oid, and text priv name.
+ */
+Datum
+has_setting_privilege_name_id(PG_FUNCTION_ARGS)
+{
+	Name		username = PG_GETARG_NAME(0);
+	Oid			settingoid = PG_GETARG_OID(1);
+	text	   *priv_setting_text = PG_GETARG_TEXT_PP(2);
+	Oid			roleid;
+	AclMode		mode;
+	AclResult	aclresult;
+
+	roleid = get_role_oid_or_public(NameStr(*username));
+	mode = convert_setting_priv_string(priv_setting_text);
+
+	if (!SearchSysCacheExists1(SETTINGOID, ObjectIdGetDatum(settingoid)))
+		PG_RETURN_NULL();
+
+	aclresult = pg_setting_acl_aclcheck(settingoid, roleid, mode);
+
+	PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
+}
+
+/*
+ * has_setting_privilege_id
+ *		Check user privileges on a setting given
+ *		setting oid, and text priv name.
+ *		current_user is assumed
+ */
+Datum
+has_setting_privilege_id(PG_FUNCTION_ARGS)
+{
+	Oid			settingoid = PG_GETARG_OID(0);
+	text	   *priv_setting_text = PG_GETARG_TEXT_PP(1);
+	Oid			roleid;
+	AclMode		mode;
+	AclResult	aclresult;
+
+	roleid = GetUserId();
+	mode = convert_setting_priv_string(priv_setting_text);
+
+	if (!SearchSysCacheExists1(SETTINGOID, ObjectIdGetDatum(settingoid)))
+		PG_RETURN_NULL();
+
+	aclresult = pg_setting_acl_aclcheck(settingoid, roleid, mode);
+
+	PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
+}
+
+/*
+ * has_setting_privilege_id_name
+ *		Check user privileges on a setting given
+ *		roleid, text setting, and text priv name.
+ */
+Datum
+has_setting_privilege_id_name(PG_FUNCTION_ARGS)
+{
+	Oid			roleid = PG_GETARG_OID(0);
+	text	   *setting = PG_GETARG_TEXT_PP(1);
+	text	   *priv_setting_text = PG_GETARG_TEXT_PP(2);
+	Oid			settingoid;
+	AclMode		mode;
+	AclResult	aclresult;
+
+	settingoid = get_setting_oid(text_to_cstring(setting), true);
+	if (!OidIsValid(settingoid))
+		PG_RETURN_NULL();
+	mode = convert_setting_priv_string(priv_setting_text);
+
+	aclresult = pg_setting_acl_aclcheck(settingoid, roleid, mode);
+
+	PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
+}
+
+/*
+ * has_setting_privilege_id_id
+ *		Check user privileges on a setting given
+ *		roleid, setting oid, and text priv name.
+ */
+Datum
+has_setting_privilege_id_id(PG_FUNCTION_ARGS)
+{
+	Oid			roleid = PG_GETARG_OID(0);
+	Oid			settingoid = PG_GETARG_OID(1);
+	text	   *priv_setting_text = PG_GETARG_TEXT_PP(2);
+	AclMode		mode;
+	AclResult	aclresult;
+
+	mode = convert_setting_priv_string(priv_setting_text);
+
+	if (!SearchSysCacheExists1(SETTINGOID, ObjectIdGetDatum(settingoid)))
+		PG_RETURN_NULL();
+
+	aclresult = pg_setting_acl_aclcheck(settingoid, roleid, mode);
+
+	PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
+}
+
+/*
+ *		Support routines for has_setting_privilege family.
+ */
+
+/*
+ * convert_setting_priv_string
+ *		Convert text string to AclMode value.
+ */
+static AclMode
+convert_setting_priv_string(text *priv_setting_text)
+{
+	static const priv_map setting_priv_map[] = {
+		{"SET VALUE", ACL_SET_VALUE},
+		{"SET VALUE WITH GRANT OPTION", ACL_GRANT_OPTION_FOR(ACL_SET_VALUE)},
+		{"ALTER SYSTEM", ACL_ALTER_SYSTEM},
+		{"ALTER SYSTEM WITH GRANT OPTION", ACL_GRANT_OPTION_FOR(ACL_ALTER_SYSTEM)},
+		{NULL, 0}
+	};
+
+	return convert_any_priv_string(priv_setting_text, setting_priv_map);
+}
 
 /*
  * pg_has_role variants
@@ -4670,6 +4924,43 @@ initialize_acl(void)
 	}
 }
 
+/*
+ * get_setting_oid - Given a configuration parameter name, look up the
+ * configuration parameter's OID.  Note that names which are aliases for
+ * a canonical name will be translated automatically and the OID found.
+ *
+ * If missing_ok is false, throw an error if the configuration parameter name
+ * is not found.
+ *
+ * Returns the Oid of the configuration parameter.
+ */
+Oid
+get_setting_oid(const char *setting, bool missing_ok)
+{
+	Oid			oid;
+
+	/* Check for the variable by the name we were given */
+	oid = GetSysCacheOid1(SETTINGNAME, Anum_pg_setting_acl_oid,
+						  PointerGetDatum(cstring_to_text(setting)));
+	if (!OidIsValid(oid))
+	{
+		const char *canonical;
+
+		/* Check if the variable has a different canonical spelling */
+		canonical = GetConfigOptionCanonicalName(setting);
+		if (canonical != NULL)
+			oid = GetSysCacheOid1(SETTINGNAME, Anum_pg_setting_acl_oid,
+								  PointerGetDatum(cstring_to_text(canonical)));
+
+		if (!OidIsValid(oid) && !missing_ok)
+			ereport(ERROR,
+					(errcode(ERRCODE_UNDEFINED_OBJECT),
+					 errmsg("setting \"%s\" does not exist", setting)));
+	}
+
+	return oid;
+}
+
 /*
  * RoleMembershipCacheCallback
  *		Syscache inval callback function
diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c
index feef999863..f4e806fe8e 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -32,6 +32,7 @@
 #include "catalog/pg_operator.h"
 #include "catalog/pg_proc.h"
 #include "catalog/pg_range.h"
+#include "catalog/pg_setting_acl.h"
 #include "catalog/pg_statistic.h"
 #include "catalog/pg_transform.h"
 #include "catalog/pg_type.h"
@@ -3304,6 +3305,25 @@ free_attstatsslot(AttStatsSlot *sslot)
 		pfree(sslot->numbers_arr);
 }
 
+char *
+get_setting_name(Oid configid)
+{
+	HeapTuple	tp;
+
+	tp = SearchSysCache1(SETTINGOID, ObjectIdGetDatum(configid));
+	if (HeapTupleIsValid(tp))
+	{
+		Form_pg_setting_acl configtup = (Form_pg_setting_acl) GETSTRUCT(tp);
+		char	   *result;
+
+		result = pstrdup(text_to_cstring(&configtup->setting));
+		ReleaseSysCache(tp);
+		return result;
+	}
+	else
+		return NULL;
+}
+
 /*				---------- PG_NAMESPACE CACHE ----------				 */
 
 /*
diff --git a/src/backend/utils/cache/syscache.c b/src/backend/utils/cache/syscache.c
index f4e7819f1e..cbcbf02839 100644
--- a/src/backend/utils/cache/syscache.c
+++ b/src/backend/utils/cache/syscache.c
@@ -57,6 +57,7 @@
 #include "catalog/pg_rewrite.h"
 #include "catalog/pg_seclabel.h"
 #include "catalog/pg_sequence.h"
+#include "catalog/pg_setting_acl.h"
 #include "catalog/pg_shdepend.h"
 #include "catalog/pg_shdescription.h"
 #include "catalog/pg_shseclabel.h"
@@ -762,6 +763,28 @@ static const struct cachedesc cacheinfo[] = {
 		},
 		32
 	},
+	{SettingAclRelationId,		/* SETTINGNAME */
+		SettingAclSettingIndexId,
+		1,
+		{
+			Anum_pg_setting_acl_setting,
+			0,
+			0,
+			0
+		},
+		4
+	},
+	{SettingAclRelationId,		/* SETTINGOID */
+		SettingAclOidIndexId,
+		1,
+		{
+			Anum_pg_setting_acl_oid,
+			0,
+			0,
+			0
+		},
+		4
+	},
 	{StatisticExtDataRelationId,	/* STATEXTDATASTXOID */
 		StatisticExtDataStxoidInhIndexId,
 		2,
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 6d11f9c71b..e64d0b2564 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -43,7 +43,9 @@
 #include "access/xlog_internal.h"
 #include "access/xlogrecovery.h"
 #include "catalog/namespace.h"
+#include "catalog/objectaccess.h"
 #include "catalog/pg_authid.h"
+#include "catalog/pg_setting_acl.h"
 #include "catalog/storage.h"
 #include "commands/async.h"
 #include "commands/prepare.h"
@@ -5053,6 +5055,10 @@ static struct config_enum ConfigureNamesEnum[] =
  * the following mappings to any unrecognized name.  Note that an old name
  * should be mapped to a new one only if the new variable has very similar
  * semantics to the old.
+ *
+ * If you deprecate a name in favor of a new spelling, be sure to consider what
+ * upgrade support will be needed, if any, for existing pg_setting_acl
+ * entries.
  */
 static const char *const map_old_guc_names[] = {
 	"sort_mem", "work_mem",
@@ -5452,25 +5458,29 @@ add_guc_variable(struct config_generic *var, int elevel)
 }
 
 /*
- * Decide whether a proposed custom variable name is allowed.
+ * Decide whether a proposed variable name is allowed.
  *
- * It must be two or more identifiers separated by dots, where the rules
- * for what is an identifier agree with scan.l.  (If you change this rule,
- * adjust the errdetail in find_option().)
+ * It must be one or more identifiers separated by zero or more dots, where the
+ * rules for what is an identifier agree with scan.l.  (If you change this
+ * rule, adjust the errdetail in find_option().)
+ *
+ * partcnt: returns by reference the number of dot separated identifiers.
  */
-static bool
-valid_custom_variable_name(const char *name)
+bool
+valid_variable_name(const char *name, int *partcnt)
 {
-	bool		saw_sep = false;
+	int			parts = 1;
 	bool		name_start = true;
 
+	if (partcnt)
+		*partcnt = -1;
 	for (const char *p = name; *p; p++)
 	{
 		if (*p == GUC_QUALIFIER_SEPARATOR)
 		{
 			if (name_start)
 				return false;	/* empty name component */
-			saw_sep = true;
+			parts++;
 			name_start = true;
 		}
 		else if (strchr("ABCDEFGHIJKLMNOPQRSTUVWXYZ"
@@ -5487,8 +5497,25 @@ valid_custom_variable_name(const char *name)
 	}
 	if (name_start)
 		return false;			/* empty name component */
-	/* OK if we found at least one separator */
-	return saw_sep;
+	if (partcnt)
+		*partcnt = parts;
+	return true;
+}
+
+
+/*
+ * Decide whether a proposed custom variable name is allowed.
+ *
+ * It must be two or more identifiers separated by dots, where the rules
+ * for what is an identifier agree with scan.l.  (If you change this rule,
+ * adjust the errdetail in find_option().)
+ */
+static bool
+valid_custom_variable_name(const char *name)
+{
+	int			partcnt;
+
+	return (valid_variable_name(name, &partcnt) && partcnt > 1);
 }
 
 /*
@@ -7542,6 +7569,24 @@ set_config_option(const char *name, const char *value,
 		case PGC_SUSET:
 			if (context == PGC_USERSET || context == PGC_BACKEND)
 			{
+				/*
+				 * Check whether the current user has granted privilege to set
+				 * this GUC.
+				 */
+				Oid			settingid = get_setting_oid(name, true);
+
+				if (OidIsValid(settingid))
+				{
+					AclResult	aclresult;
+
+					aclresult = pg_setting_acl_aclcheck(settingid, GetUserId(),
+														ACL_SET_VALUE);
+
+					if (aclresult == ACLCHECK_OK)
+						break;		/* okay */
+				}
+
+				/* No granted privilege */
 				ereport(elevel,
 						(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 						 errmsg("permission denied to set parameter \"%s\"",
@@ -7550,7 +7595,35 @@ set_config_option(const char *name, const char *value,
 			}
 			break;
 		case PGC_USERSET:
-			/* always okay */
+			if (context == PGC_USERSET)
+			{
+				/*
+				 * If this GUC setting has an entry in pg_setting_acl, then the
+				 * current user must have privileges per the acl to set this guc.
+				 * Otherwise, all users are implicitly allowed to set it.
+				 */
+				Oid			settingid = get_setting_oid(name, true);
+
+				if (OidIsValid(settingid))
+				{
+					AclResult	aclresult;
+
+					aclresult = pg_setting_acl_aclcheck(settingid, GetUserId(),
+														ACL_SET_VALUE);
+
+					if (aclresult == ACLCHECK_OK)
+						break;		/* okay */
+
+					/* No granted privilege */
+					ereport(elevel,
+							(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+							 errmsg("permission denied to set parameter \"%s\"",
+									name)));
+					return 0;
+				}
+
+				/* No pg_setting_acl entry, okay */
+			}
 			break;
 	}
 
@@ -8140,6 +8213,23 @@ set_config_option(const char *name, const char *value,
 }
 
 
+/*
+ * Get the context required to set the variable, or -1 if the given name cannot
+ * be found.
+ */
+GucContext
+find_option_context(const char *name)
+{
+	struct config_generic *conf;
+
+	conf = find_option(name, false, false, ERROR);
+	if (conf)
+		return conf->context;
+
+	return -1;	 /* Not reached */
+}
+
+
 /*
  * Set the fields for source file and line number the setting came from.
  */
@@ -8584,6 +8674,7 @@ AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt)
 {
 	char	   *name;
 	char	   *value;
+	Oid			settingId = InvalidOid;
 	bool		resetall = false;
 	ConfigVariable *head = NULL;
 	ConfigVariable *tail = NULL;
@@ -8591,16 +8682,31 @@ AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt)
 	char		AutoConfFileName[MAXPGPATH];
 	char		AutoConfTmpFileName[MAXPGPATH];
 
-	if (!superuser())
-		ereport(ERROR,
-				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-				 errmsg("must be superuser to execute ALTER SYSTEM command")));
-
 	/*
 	 * Extract statement arguments
 	 */
 	name = altersysstmt->setstmt->name;
 
+	/*
+	 * Check permission to run ALTER SYSTEM on the target variable registered.
+	 */
+	if (!superuser())
+	{
+		AclResult	aclresult;
+
+		settingId = get_setting_oid(name, true);
+		if (!OidIsValid(settingId))
+			ereport(ERROR,
+					(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+					 errmsg("permission denied to set parameter \"%s\"",
+							name)));
+
+		aclresult = pg_setting_acl_aclcheck(settingId, GetUserId(),
+											 ACL_ALTER_SYSTEM);
+		if (aclresult != ACLCHECK_OK)
+			aclcheck_error(aclresult, OBJECT_SETTING, name);
+	}
+
 	switch (altersysstmt->setstmt->kind)
 	{
 		case VAR_SET_VALUE:
@@ -8733,6 +8839,21 @@ AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt)
 		replace_auto_config_value(&head, &tail, name, value);
 	}
 
+	/*
+	 * Invoke the post-alter hook for setting this GUC variable.  Guc variables
+	 * do not always have corresponding entries in pg_setting_acl; the hooks
+	 * must therefore be prepared to receive settingId = InvalidOid.  We also
+	 * abuse the notion of subId to pass the kind of alteration (set vs. alter
+	 * system), and auxiliaryId to pass the VariableSetKind.
+	 *
+	 * We do this here rather than at the end, because ALTER SYSTEM is not
+	 * transactional.  If the hook aborts our transaction, it will be cleaner
+	 * to do so before we touch any files.
+	 */
+	InvokeObjectPostAlterHookArg(SettingAclRelationId, settingId,
+								 ACL_ALTER_SYSTEM, altersysstmt->setstmt->kind,
+								 false);
+
 	/*
 	 * To ensure crash safety, first write the new file data to a temp file,
 	 * then atomically rename it into place.
@@ -8793,6 +8914,9 @@ void
 ExecSetVariableStmt(VariableSetStmt *stmt, bool isTopLevel)
 {
 	GucAction	action = stmt->is_local ? GUC_ACTION_LOCAL : GUC_ACTION_SET;
+	GucContext	context;
+	AclResult	aclresult;
+	Oid			settingId;
 
 	/*
 	 * Workers synchronize these parameters at the start of the parallel
@@ -8803,6 +8927,24 @@ ExecSetVariableStmt(VariableSetStmt *stmt, bool isTopLevel)
 				(errcode(ERRCODE_INVALID_TRANSACTION_STATE),
 				 errmsg("cannot set parameters during a parallel operation")));
 
+	/* Get the Oid of this setting, or InvalidOid if none. */
+	settingId = get_setting_oid(stmt->name, true);
+
+	/*
+	 * Superusers and users who have been granted SET privilege can set with
+	 * PGC_SUSET context.  All others have only PGC_USERSET.
+	 */
+	context = PGC_USERSET;
+	if (superuser())
+		context = PGC_SUSET;
+	else if (OidIsValid(settingId))
+	{
+		aclresult = pg_setting_acl_aclcheck(settingId, GetUserId(),
+											ACL_SET_VALUE);
+		if (aclresult == ACLCHECK_OK)
+			context = PGC_SUSET;
+	}
+
 	switch (stmt->kind)
 	{
 		case VAR_SET_VALUE:
@@ -8811,7 +8953,7 @@ ExecSetVariableStmt(VariableSetStmt *stmt, bool isTopLevel)
 				WarnNoTransactionBlock(isTopLevel, "SET LOCAL");
 			(void) set_config_option(stmt->name,
 									 ExtractSetVariableArgs(stmt),
-									 (superuser() ? PGC_SUSET : PGC_USERSET),
+									 context,
 									 PGC_S_SESSION,
 									 action, true, 0, false);
 			break;
@@ -8896,7 +9038,7 @@ ExecSetVariableStmt(VariableSetStmt *stmt, bool isTopLevel)
 
 			(void) set_config_option(stmt->name,
 									 NULL,
-									 (superuser() ? PGC_SUSET : PGC_USERSET),
+									 context,
 									 PGC_S_SESSION,
 									 action, true, 0, false);
 			break;
@@ -8904,6 +9046,16 @@ ExecSetVariableStmt(VariableSetStmt *stmt, bool isTopLevel)
 			ResetAllOptions();
 			break;
 	}
+
+	/*
+	 * Invoke the post-alter hook for setting this GUC variable.  Guc variables
+	 * do not always have corresponding entries in pg_setting_acl; the hooks
+	 * must therefore be prepared to receive settingId = InvalidOid.  We also
+	 * abuse the notion of subId to pass the kind of alteration (set vs. alter
+	 * system), and auxiliaryId to pass the VariableSetKind.
+	 */
+	InvokeObjectPostAlterHookArg(SettingAclRelationId, settingId,
+								 ACL_SET_VALUE, stmt->kind, false);
 }
 
 /*
@@ -9664,6 +9816,22 @@ get_explain_guc_options(int *num)
 	return result;
 }
 
+/*
+ * Return GUC variable canonical name, or NULL if no variable by the given
+ * name or alias exists.
+ */
+const char *
+GetConfigOptionCanonicalName(const char *alias)
+{
+	struct config_generic *record;
+
+	record = find_option(alias, false, true, LOG);
+	if (record == NULL)
+		return NULL;
+
+	return record->name;
+}
+
 /*
  * Return GUC variable value by name; optionally return canonical form of
  * name.  If the GUC is unset, then throw an error unless missing_ok is true,
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index 6086d57cf3..a84efd79e5 100644
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -37,7 +37,7 @@ static void AddAcl(PQExpBuffer aclbuf, const char *keyword,
  *	nspname: the namespace the object is in (NULL if none); not pre-quoted
  *	type: the object type (as seen in GRANT command: must be one of
  *		TABLE, SEQUENCE, FUNCTION, PROCEDURE, LANGUAGE, SCHEMA, DATABASE, TABLESPACE,
- *		FOREIGN DATA WRAPPER, SERVER, or LARGE OBJECT)
+ *		FOREIGN DATA WRAPPER, SERVER, SETTING or LARGE OBJECT)
  *	acls: the ACL string fetched from the database
  *	baseacls: the initial ACL string for this object
  *	owner: username of object owner (will be passed through fmtId); can be
@@ -501,6 +501,11 @@ do { \
 		CONVERT_PRIV('U', "USAGE");
 	else if (strcmp(type, "FOREIGN TABLE") == 0)
 		CONVERT_PRIV('r', "SELECT");
+	else if (strcmp(type, "SETTING") == 0)
+	{
+		CONVERT_PRIV('s', "SET VALUE");
+		CONVERT_PRIV('A', "ALTER SYSTEM");
+	}
 	else if (strcmp(type, "LARGE OBJECT") == 0)
 	{
 		CONVERT_PRIV('r', "SELECT");
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index d41a99d6ea..bb0badca65 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -3435,6 +3435,7 @@ _getObjectDescription(PQExpBuffer buf, TocEntry *te)
 		strcmp(type, "SCHEMA") == 0 ||
 		strcmp(type, "EVENT TRIGGER") == 0 ||
 		strcmp(type, "FOREIGN DATA WRAPPER") == 0 ||
+		strcmp(type, "SETTING") == 0 ||
 		strcmp(type, "SERVER") == 0 ||
 		strcmp(type, "PUBLICATION") == 0 ||
 		strcmp(type, "SUBSCRIPTION") == 0 ||
@@ -3618,6 +3619,7 @@ _printTocEntry(ArchiveHandle *AH, TocEntry *te, bool isData)
 			strcmp(te->desc, "TEXT SEARCH DICTIONARY") == 0 ||
 			strcmp(te->desc, "TEXT SEARCH CONFIGURATION") == 0 ||
 			strcmp(te->desc, "FOREIGN DATA WRAPPER") == 0 ||
+			strcmp(te->desc, "SETTING") == 0 ||
 			strcmp(te->desc, "SERVER") == 0 ||
 			strcmp(te->desc, "STATISTICS") == 0 ||
 			strcmp(te->desc, "PUBLICATION") == 0 ||
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index e69dcf8a48..dea1b89ff8 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -14166,6 +14166,9 @@ dumpDefaultACL(Archive *fout, const DefaultACLInfo *daclinfo)
 		case DEFACLOBJ_NAMESPACE:
 			type = "SCHEMAS";
 			break;
+		case DEFACLOBJ_SETTING:
+			type = "SETTINGS";
+			break;
 		default:
 			/* shouldn't get here */
 			fatal("unrecognized object type in default privileges: %d",
@@ -14209,7 +14212,7 @@ dumpDefaultACL(Archive *fout, const DefaultACLInfo *daclinfo)
  *		or InvalidDumpId if there is no need for a second dependency.
  * 'type' must be one of
  *		TABLE, SEQUENCE, FUNCTION, LANGUAGE, SCHEMA, DATABASE, TABLESPACE,
- *		FOREIGN DATA WRAPPER, SERVER, or LARGE OBJECT.
+ *		FOREIGN DATA WRAPPER, SERVER, SETTING or LARGE OBJECT.
  * 'name' is the formatted name of the object.  Must be quoted etc. already.
  * 'subname' is the formatted name of the sub-object, if any.  Must be quoted.
  *		(Currently we assume that subname is only provided for table columns.)
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 9c9f7c6d63..ecb6a45a5f 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -36,6 +36,7 @@ static void help(void);
 static void dropRoles(PGconn *conn);
 static void dumpRoles(PGconn *conn);
 static void dumpRoleMembership(PGconn *conn);
+static void dumpRoleGUCPrivs(PGconn *conn);
 static void dropTablespaces(PGconn *conn);
 static void dumpTablespaces(PGconn *conn);
 static void dropDBs(PGconn *conn);
@@ -585,6 +586,10 @@ main(int argc, char *argv[])
 
 			/* Dump role memberships */
 			dumpRoleMembership(conn);
+
+			/* Dump role guc privileges */
+			if (server_version >= 150000)
+				dumpRoleGUCPrivs(conn);
 		}
 
 		/* Dump tablespaces */
@@ -1024,6 +1029,62 @@ dropTablespaces(PGconn *conn)
 	fprintf(OPF, "\n\n");
 }
 
+/*
+ * Dump role configuration parameter privileges.  This code is used for 15.0
+ * and later servers.
+ *
+ * Note: we expect dumpRoles already created all the roles, but there are
+ * no per-role configuration parameter privileges yet..
+ */
+static void
+dumpRoleGUCPrivs(PGconn *conn)
+{
+	PGresult   *res;
+	int			i;
+
+	/*
+	 * Get all settings which have non-default acls defined.
+	 */
+	res = executeQuery(conn, "SELECT setting, "
+					   "pg_catalog.pg_get_userbyid(10) AS setowner, "
+					   "setacl, aclsettingdefault(setting) AS acldefault "
+					   "FROM pg_catalog.pg_setting_acl "
+					   "ORDER BY oid");
+
+	fprintf(OPF, "--\n-- Role privileges on configuration parameters\n--\n\n");
+
+	for (i = 0; i < PQntuples(res); i++)
+	{
+		PQExpBuffer buf = createPQExpBuffer();
+		char	   *setting = PQgetvalue(res, i, 0);
+		char	   *setowner = PQgetvalue(res, i, 1);
+		char	   *setacl = PQgetvalue(res, i, 2);
+		char	   *acldefault = PQgetvalue(res, i, 3);
+		char	   *fsetting;
+
+		/* needed for buildACLCommands() */
+		fsetting = pg_strdup(fmtId(setting));
+
+		if (!buildACLCommands(fsetting, NULL, NULL, "SETTING",
+							  setacl, acldefault,
+							  setowner, "", server_version, buf))
+		{
+			pg_log_error("could not parse ACL list (%s) for tablespace \"%s\"",
+						 setacl, setting);
+			PQfinish(conn);
+			exit_nicely(1);
+		}
+
+		fprintf(OPF, "%s", buf->data);
+
+		free(fsetting);
+		destroyPQExpBuffer(buf);
+	}
+
+	PQclear(res);
+	fprintf(OPF, "\n\n");
+}
+
 /*
  * Dump tablespaces.
  */
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 6957567264..c259dcf79f 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -3672,7 +3672,11 @@ psql_completion(const char *text, int start, int end)
  * ALTER DEFAULT PRIVILEGES, so use TailMatches
  */
 	/* Complete GRANT/REVOKE with a list of roles and privileges */
-	else if (TailMatches("GRANT|REVOKE"))
+	else if (TailMatches("REVOKE", "GRANT"))
+		COMPLETE_WITH("OPTION FOR");
+	else if (TailMatches("REVOKE", "GRANT", "OPTION"))
+		COMPLETE_WITH("FOR");
+	else if (TailMatches("REVOKE"))
 	{
 		/*
 		 * With ALTER DEFAULT PRIVILEGES, restrict completion to grantable
@@ -3684,8 +3688,11 @@ psql_completion(const char *text, int start, int end)
 						  "EXECUTE", "USAGE", "ALL");
 		else
 			COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_roles,
+									 "ALTER",
 									 "SELECT",
+									 "SET",
 									 "INSERT",
+									 "GRANT",
 									 "UPDATE",
 									 "DELETE",
 									 "TRUNCATE",
@@ -3698,12 +3705,146 @@ psql_completion(const char *text, int start, int end)
 									 "USAGE",
 									 "ALL");
 	}
+	else if (TailMatches("GRANT") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR"))
+	{
+		/*
+		 * With ALTER DEFAULT PRIVILEGES, restrict completion to grantable
+		 * privileges (can't grant roles)
+		 */
+		if (HeadMatches("ALTER", "DEFAULT", "PRIVILEGES"))
+			COMPLETE_WITH("SELECT", "INSERT", "UPDATE",
+						  "DELETE", "TRUNCATE", "REFERENCES", "TRIGGER",
+						  "EXECUTE", "USAGE", "ALL");
+		else
+			COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_roles,
+									 "ALTER",
+									 "SELECT",
+									 "SET",
+									 "INSERT",
+									 "UPDATE",
+									 "DELETE",
+									 "TRUNCATE",
+									 "REFERENCES",
+									 "TRIGGER",
+									 "CREATE",
+									 "CONNECT",
+									 "TEMPORARY",
+									 "EXECUTE",
+									 "USAGE",
+									 "ALL");
+	}
+
+	else if (TailMatches("GRANT|REVOKE", "SET") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "SET"))
+		COMPLETE_WITH("VALUE");
+	else if (TailMatches("GRANT|REVOKE", "ALTER") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALTER"))
+		COMPLETE_WITH("SYSTEM");
+
+	else if (TailMatches("GRANT|REVOKE", "SET", "VALUE") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "SET", "VALUE"))
+		COMPLETE_WITH(",",
+					  "ON SETTING");
+	else if (TailMatches("GRANT|REVOKE", "ALTER", "SYSTEM") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALTER", "SYSTEM"))
+		COMPLETE_WITH(",",
+					  "ON SETTING");
+
+	else if (TailMatches("GRANT|REVOKE", "SET", "VALUE,") ||
+			 TailMatches("GRANT|REVOKE", "SET", "VALUE", ",") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "SET", "VALUE,") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "SET", "VALUE", ","))
+		COMPLETE_WITH("ALTER SYSTEM ON SETTING");
+	else if (TailMatches("GRANT|REVOKE", "ALTER", "SYSTEM,") ||
+			 TailMatches("GRANT|REVOKE", "ALTER", "SYSTEM", ",") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALTER", "SYSTEM,") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALTER", "SYSTEM", ","))
+		COMPLETE_WITH("SET VALUE ON SETTING");
+
+	else if (TailMatches("GRANT|REVOKE", "SET", "VALUE,", "ALTER") ||
+			 TailMatches("GRANT|REVOKE", "SET", "VALUE", ",", "ALTER") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "SET", "VALUE,", "ALTER") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "SET", "VALUE", ",", "ALTER"))
+		COMPLETE_WITH("SYSTEM ON SETTING");
+	else if (TailMatches("GRANT|REVOKE", "ALTER", "SYSTEM,", "SET") ||
+			 TailMatches("GRANT|REVOKE", "ALTER", "SYSTEM", ",", "SET") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALTER", "SYSTEM,", "SET") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALTER", "SYSTEM", ",", "SET"))
+		COMPLETE_WITH("VALUE ON SETTING");
+
+	else if (TailMatches("GRANT|REVOKE", "SET", "VALUE,", "ALTER", "SYSTEM") ||
+			 TailMatches("GRANT|REVOKE", "SET", "VALUE", ",", "ALTER", "SYSTEM") ||
+			 TailMatches("GRANT|REVOKE", "ALTER", "SYSTEM,", "SET", "VALUE") ||
+			 TailMatches("GRANT|REVOKE", "ALTER", "SYSTEM", ",", "SET", "VALUE"))
+		COMPLETE_WITH("ON SETTING");
+
+	else if (TailMatches("GRANT|REVOKE", "SET", "VALUE", "ON") ||
+			 TailMatches("GRANT|REVOKE", "ALTER", "SYSTEM", "ON") ||
+			 TailMatches("GRANT|REVOKE", "SET", "VALUE,", "ALTER", "SYSTEM", "ON") ||
+			 TailMatches("GRANT|REVOKE", "SET", "VALUE", ",", "ALTER", "SYSTEM", "ON") ||
+			 TailMatches("GRANT|REVOKE", "ALTER", "SYSTEM,", "SET", "VALUE", "ON") ||
+			 TailMatches("GRANT|REVOKE", "ALTER", "SYSTEM", ",", "SET", "VALUE", "ON") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "SET", "VALUE", "ON") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALTER", "SYSTEM", "ON") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "SET", "VALUE,", "ALTER", "SYSTEM", "ON") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "SET", "VALUE", ",", "ALTER", "SYSTEM", "ON") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALTER", "SYSTEM,", "SET", "VALUE", "ON") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALTER", "SYSTEM", ",", "SET", "VALUE", "ON"))
+		COMPLETE_WITH("SETTING");
+
+	else if (TailMatches("GRANT|REVOKE", "SET", "VALUE", "ON", "SETTING") ||
+			 TailMatches("GRANT|REVOKE", "ALTER", "SYSTEM", "ON", "SETTING") ||
+			 TailMatches("GRANT|REVOKE", "SET", "VALUE,", "ALTER", "SYSTEM", "ON", "SETTING") ||
+			 TailMatches("GRANT|REVOKE", "SET", "VALUE", ",", "ALTER", "SYSTEM", "ON", "SETTING") ||
+			 TailMatches("GRANT|REVOKE", "ALTER", "SYSTEM,", "SET", "VALUE", "ON", "SETTING") ||
+			 TailMatches("GRANT|REVOKE", "ALTER", "SYSTEM", ",", "SET", "VALUE", "ON", "SETTING") ||
+			 TailMatches("GRANT|REVOKE", "ALL", "ON", "SETTING") ||
+			 TailMatches("GRANT|REVOKE", "ALL", "PRIVILEGES", "ON", "SETTING") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "SET", "VALUE", "ON", "SETTING") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALTER", "SYSTEM", "ON", "SETTING") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "SET", "VALUE,", "ALTER", "SYSTEM", "ON", "SETTING") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "SET", "VALUE", ",", "ALTER", "SYSTEM", "ON", "SETTING") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALTER", "SYSTEM,", "SET", "VALUE", "ON", "SETTING") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALTER", "SYSTEM", ",", "SET", "VALUE", "ON", "SETTING") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALL", "ON", "SETTING") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALL", "PRIVILEGES", "ON", "SETTING"))
+		COMPLETE_WITH_QUERY(Query_for_list_of_set_vars);
+
+	else if (TailMatches("GRANT", "SET", "VALUE", "ON", "SETTING", MatchAny) ||
+			 TailMatches("GRANT", "ALTER", "SYSTEM", "ON", "SETTING", MatchAny) ||
+			 TailMatches("GRANT", "SET", "VALUE,", "ALTER", "SYSTEM", "ON", "SETTING", MatchAny) ||
+			 TailMatches("GRANT", "SET", "VALUE", ",", "ALTER", "SYSTEM", "ON", "SETTING", MatchAny) ||
+			 TailMatches("GRANT", "ALTER", "SYSTEM,", "SET", "VALUE", "ON", "SETTING", MatchAny) ||
+			 TailMatches("GRANT", "ALTER", "SYSTEM", ",", "SET", "VALUE", "ON", "SETTING", MatchAny) ||
+			 TailMatches("GRANT", "ALL", "ON", "SETTING", MatchAny) ||
+			 TailMatches("GRANT", "ALL", "PRIVILEGES", "ON", "SETTING", MatchAny))
+		COMPLETE_WITH("TO");
+
+	else if (TailMatches("REVOKE", "SET", "VALUE", "ON", "SETTING", MatchAny) ||
+			 TailMatches("REVOKE", "ALTER", "SYSTEM", "ON", "SETTING", MatchAny) ||
+			 TailMatches("REVOKE", "SET", "VALUE,", "ALTER", "SYSTEM", "ON", "SETTING", MatchAny) ||
+			 TailMatches("REVOKE", "SET", "VALUE", ",", "ALTER", "SYSTEM", "ON", "SETTING", MatchAny) ||
+			 TailMatches("REVOKE", "ALL", "ON", "SETTING", MatchAny) ||
+			 TailMatches("REVOKE", "ALL", "PRIVILEGES", "ON", "SETTING", MatchAny) ||
+			 TailMatches("REVOKE", "ALTER", "SYSTEM,", "SET", "VALUE", "ON", "SETTING", MatchAny) ||
+			 TailMatches("REVOKE", "ALTER", "SYSTEM", ",", "SET", "VALUE", "ON", "SETTING", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "SET", "VALUE", "ON", "SETTING", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALTER", "SYSTEM", "ON", "SETTING", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "SET", "VALUE,", "ALTER", "SYSTEM", "ON", "SETTING", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "SET", "VALUE", ",", "ALTER", "SYSTEM", "ON", "SETTING", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALTER", "SYSTEM,", "SET", "VALUE", "ON", "SETTING", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALTER", "SYSTEM", ",", "SET", "VALUE", "ON", "SETTING", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALL", "ON", "SETTING", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALL", "PRIVILEGES", "ON", "SETTING", MatchAny))
+		COMPLETE_WITH("FROM");
 
 	/*
 	 * Complete GRANT/REVOKE <privilege> with "ON", GRANT/REVOKE <role> with
 	 * TO/FROM
 	 */
-	else if (TailMatches("GRANT|REVOKE", MatchAny))
+	else if (TailMatches("GRANT|REVOKE", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny))
 	{
 		if (TailMatches("SELECT|INSERT|UPDATE|DELETE|TRUNCATE|REFERENCES|TRIGGER|CREATE|CONNECT|TEMPORARY|TEMP|EXECUTE|USAGE|ALL"))
 			COMPLETE_WITH("ON");
@@ -3720,7 +3861,8 @@ psql_completion(const char *text, int start, int end)
 	 * here will only work if the privilege list contains exactly one
 	 * privilege.
 	 */
-	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON"))
+	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON"))
 	{
 		/*
 		 * With ALTER DEFAULT PRIVILEGES, restrict completion to the kinds of
@@ -3750,13 +3892,15 @@ psql_completion(const char *text, int start, int end)
 											"TABLESPACE",
 											"TYPE");
 	}
-	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON", "ALL"))
+	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON", "ALL") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "ALL"))
 		COMPLETE_WITH("FUNCTIONS IN SCHEMA",
 					  "PROCEDURES IN SCHEMA",
 					  "ROUTINES IN SCHEMA",
 					  "SEQUENCES IN SCHEMA",
 					  "TABLES IN SCHEMA");
-	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON", "FOREIGN"))
+	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON", "FOREIGN") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "FOREIGN"))
 		COMPLETE_WITH("DATA WRAPPER", "SERVER");
 
 	/*
@@ -3765,7 +3909,8 @@ psql_completion(const char *text, int start, int end)
 	 *
 	 * Complete "GRANT/REVOKE * ON *" with "TO/FROM".
 	 */
-	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON", MatchAny))
+	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", MatchAny))
 	{
 		if (TailMatches("DATABASE"))
 			COMPLETE_WITH_QUERY(Query_for_list_of_databases);
@@ -3803,6 +3948,25 @@ psql_completion(const char *text, int start, int end)
 			 (HeadMatches("REVOKE") && TailMatches("FROM")))
 		COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_roles,
 								 Keywords_for_list_of_grant_roles);
+	else if (HeadMatches("GRANT") && TailMatches("TO", MatchAny))
+		COMPLETE_WITH("WITH ADMIN OPTION",
+					  "WITH GRANT OPTION",
+					  "GRANTED BY");
+	else if (HeadMatches("GRANT") && TailMatches("TO", MatchAny, "WITH"))
+		COMPLETE_WITH("ADMIN OPTION",
+					  "GRANT OPTION");
+	else if (HeadMatches("GRANT") && TailMatches("TO", MatchAny, "WITH", "ADMIN"))
+		COMPLETE_WITH("OPTION");
+	else if ((HeadMatches("GRANT") && TailMatches("TO", MatchAny, "WITH", "ADMIN", "OPTION")) ||
+			 (HeadMatches("GRANT") && TailMatches("TO", MatchAny, "WITH", "GRANT", "OPTION")))
+		COMPLETE_WITH("GRANTED BY");
+	else if ((HeadMatches("GRANT") && TailMatches("TO", MatchAny, "WITH", "ADMIN", "OPTION", "GRANTED")) ||
+			 (HeadMatches("GRANT") && TailMatches("TO", MatchAny, "WITH", "GRANT", "OPTION", "GRANTED")))
+		COMPLETE_WITH("BY");
+	else if ((HeadMatches("GRANT") && TailMatches("TO", MatchAny, "WITH", "ADMIN", "OPTION", "GRANTED", "BY")) ||
+			 (HeadMatches("GRANT") && TailMatches("TO", MatchAny, "WITH", "GRANT", "OPTION", "GRANTED", "BY")))
+		COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_roles,
+								 Keywords_for_list_of_grant_roles);
 	/* Complete "ALTER DEFAULT PRIVILEGES ... GRANT/REVOKE ... TO/FROM */
 	else if (HeadMatches("ALTER", "DEFAULT", "PRIVILEGES") && TailMatches("TO|FROM"))
 		COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_roles,
@@ -3814,7 +3978,8 @@ psql_completion(const char *text, int start, int end)
 		COMPLETE_WITH("FROM");
 
 	/* Complete "GRANT/REVOKE * ON ALL * IN SCHEMA *" with TO/FROM */
-	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON", "ALL", MatchAny, "IN", "SCHEMA", MatchAny))
+	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON", "ALL", MatchAny, "IN", "SCHEMA", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "ALL", MatchAny, "IN", "SCHEMA", MatchAny))
 	{
 		if (TailMatches("GRANT", MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny))
 			COMPLETE_WITH("TO");
@@ -3823,7 +3988,8 @@ psql_completion(const char *text, int start, int end)
 	}
 
 	/* Complete "GRANT/REVOKE * ON FOREIGN DATA WRAPPER *" with TO/FROM */
-	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON", "FOREIGN", "DATA", "WRAPPER", MatchAny))
+	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON", "FOREIGN", "DATA", "WRAPPER", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "FOREIGN", "DATA", "WRAPPER", MatchAny))
 	{
 		if (TailMatches("GRANT", MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny))
 			COMPLETE_WITH("TO");
@@ -3832,7 +3998,8 @@ psql_completion(const char *text, int start, int end)
 	}
 
 	/* Complete "GRANT/REVOKE * ON FOREIGN SERVER *" with TO/FROM */
-	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON", "FOREIGN", "SERVER", MatchAny))
+	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON", "FOREIGN", "SERVER", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "FOREIGN", "SERVER", MatchAny))
 	{
 		if (TailMatches("GRANT", MatchAny, MatchAny, MatchAny, MatchAny, MatchAny))
 			COMPLETE_WITH("TO");
diff --git a/src/include/catalog/dependency.h b/src/include/catalog/dependency.h
index 344482ec87..a0c3966da9 100644
--- a/src/include/catalog/dependency.h
+++ b/src/include/catalog/dependency.h
@@ -92,6 +92,7 @@ typedef enum ObjectClass
 	OCLASS_TYPE,				/* pg_type */
 	OCLASS_CAST,				/* pg_cast */
 	OCLASS_COLLATION,			/* pg_collation */
+	OCLASS_SETTING,				/* pg_setting_acl */
 	OCLASS_CONSTRAINT,			/* pg_constraint */
 	OCLASS_CONVERSION,			/* pg_conversion */
 	OCLASS_DEFAULT,				/* pg_attrdef */
diff --git a/src/include/catalog/pg_default_acl.h b/src/include/catalog/pg_default_acl.h
index 2a79155636..12ec9b0a31 100644
--- a/src/include/catalog/pg_default_acl.h
+++ b/src/include/catalog/pg_default_acl.h
@@ -66,6 +66,7 @@ DECLARE_UNIQUE_INDEX_PKEY(pg_default_acl_oid_index, 828, DefaultAclOidIndexId, o
 #define DEFACLOBJ_FUNCTION		'f' /* function */
 #define DEFACLOBJ_TYPE			'T' /* type */
 #define DEFACLOBJ_NAMESPACE		'n' /* namespace */
+#define DEFACLOBJ_SETTING	'c' /* configuration parameter */
 
 #endif							/* EXPOSE_TO_CLIENT_CODE */
 
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index d8e8715ed1..9c6cf7c558 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -2085,6 +2085,10 @@
   descr => 'show hardwired default privileges, primarily for use by the information schema',
   proname => 'acldefault', prorettype => '_aclitem', proargtypes => 'char oid',
   prosrc => 'acldefault_sql' },
+{ oid => '9806',
+  descr => 'show hardwired default privileges on settings',
+  proname => 'aclsettingdefault', prorettype => '_aclitem', proargtypes => 'text',
+  prosrc => 'aclsettingdefault_sql' },
 { oid => '1689',
   descr => 'convert ACL item array to table, primarily for use by information schema',
   proname => 'aclexplode', prorows => '10', proretset => 't',
@@ -7207,6 +7211,25 @@
   proname => 'has_type_privilege', provolatile => 's', prorettype => 'bool',
   proargtypes => 'oid text', prosrc => 'has_type_privilege_id' },
 
+{ oid => '8050', descr => 'user privilege on setting by username, setting name',
+  proname => 'has_setting_privilege', provolatile => 's', prorettype => 'bool',
+  proargtypes => 'name text text', prosrc => 'has_setting_privilege_name_name' },
+{ oid => '8051', descr => 'user privilege on setting by username, setting oid',
+  proname => 'has_setting_privilege', provolatile => 's', prorettype => 'bool',
+  proargtypes => 'name oid text', prosrc => 'has_setting_privilege_name_id' },
+{ oid => '8052', descr => 'user privilege on setting by user oid, setting name',
+  proname => 'has_setting_privilege', provolatile => 's', prorettype => 'bool',
+  proargtypes => 'oid text text', prosrc => 'has_setting_privilege_id_name' },
+{ oid => '8053', descr => 'user privilege on setting by user oid, setting oid',
+  proname => 'has_setting_privilege', provolatile => 's', prorettype => 'bool',
+  proargtypes => 'oid oid text', prosrc => 'has_setting_privilege_id_id' },
+{ oid => '8054', descr => 'current user privilege on setting by setting name',
+  proname => 'has_setting_privilege', provolatile => 's', prorettype => 'bool',
+  proargtypes => 'text text', prosrc => 'has_setting_privilege_name' },
+{ oid => '8055', descr => 'current user privilege on setting by setting oid',
+  proname => 'has_setting_privilege', provolatile => 's', prorettype => 'bool',
+  proargtypes => 'oid text', prosrc => 'has_setting_privilege_id' },
+
 { oid => '2705', descr => 'user privilege on role by username, role name',
   proname => 'pg_has_role', provolatile => 's', prorettype => 'bool',
   proargtypes => 'name name text', prosrc => 'pg_has_role_name_name' },
diff --git a/src/include/catalog/pg_setting_acl.h b/src/include/catalog/pg_setting_acl.h
new file mode 100644
index 0000000000..b7dee55e5c
--- /dev/null
+++ b/src/include/catalog/pg_setting_acl.h
@@ -0,0 +1,63 @@
+/*-------------------------------------------------------------------------
+ *
+ * pg_setting_acl.h
+ *	  definition of the "configuration parameter" system catalog
+ *	  (pg_setting_acl).
+ *
+ *
+ * Portions Copyright (c) 1996-2021, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/include/catalog/pg_setting_acl.h
+ *
+ * NOTES
+ *	  The Catalog.pm module reads this file and derives schema
+ *	  information.
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef PG_SETTING_ACL_H
+#define PG_SETTING_ACL_H
+
+#include "catalog/genbki.h"
+#include "catalog/pg_setting_acl_d.h"
+
+/* ----------------
+ *		pg_setting_acl definition.  cpp turns this into
+ *		typedef struct FormData_pg_setting_acl
+ * ----------------
+ */
+CATALOG(pg_setting_acl,8924,SettingAclRelationId) BKI_SHARED_RELATION
+{
+	Oid			oid;			/* oid */
+	/*
+
+	 * Variable-length fields start here, but we allow direct access to
+	 * setting.
+	 */
+	text		setting BKI_FORCE_NOT_NULL;
+
+#ifdef CATALOG_VARLEN
+	/* Access privileges */
+	aclitem		setacl[1] BKI_DEFAULT(_null_);
+#endif
+} FormData_pg_setting_acl;
+
+
+/* ----------------
+ *		Form_pg_setting_acl corresponds to a pointer to a tuple with
+ *		the format of pg_setting_acl relation.
+ * ----------------
+ */
+typedef FormData_pg_setting_acl *Form_pg_setting_acl;
+
+DECLARE_TOAST(pg_setting_acl, 8925, 8926);
+#define PgSettingAclToastTable 8925
+#define PgSettingAclToastIndex 8926
+
+DECLARE_UNIQUE_INDEX(pg_setting_acl_setting_index, 8927, SettingAclSettingIndexId, on pg_setting_acl using btree(setting text_ops));
+DECLARE_UNIQUE_INDEX_PKEY(pg_setting_acl_oid_index, 8928, SettingAclOidIndexId, on pg_setting_acl using btree(oid oid_ops));
+
+extern Oid	SettingAclCreate(const char *setting, bool if_not_exists);
+
+#endif							/* PG_SETTING_ACL_H */
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 1617702d9d..5f5e4ae8e1 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -92,7 +92,9 @@ typedef uint32 AclMode;			/* a bitmask of privilege bits */
 #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_SET_VALUE	(1<<12) /* for configuration parameters */
+#define ACL_ALTER_SYSTEM (1<<13) /* for configuration parameters */
+#define N_ACL_RIGHTS	14		/* 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
@@ -1795,6 +1797,7 @@ typedef enum ObjectType
 	OBJECT_CAST,
 	OBJECT_COLUMN,
 	OBJECT_COLLATION,
+	OBJECT_SETTING,
 	OBJECT_CONVERSION,
 	OBJECT_DATABASE,
 	OBJECT_DEFAULT,
diff --git a/src/include/parser/kwlist.h b/src/include/parser/kwlist.h
index bcef7eed2f..168aaafb93 100644
--- a/src/include/parser/kwlist.h
+++ b/src/include/parser/kwlist.h
@@ -380,6 +380,7 @@ PG_KEYWORD("session_user", SESSION_USER, RESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("set", SET, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("setof", SETOF, COL_NAME_KEYWORD, BARE_LABEL)
 PG_KEYWORD("sets", SETS, UNRESERVED_KEYWORD, BARE_LABEL)
+PG_KEYWORD("setting", SETTING, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("share", SHARE, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("show", SHOW, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("similar", SIMILAR, TYPE_FUNC_NAME_KEYWORD, BARE_LABEL)
diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h
index 1ce4c5556e..b50ef98c69 100644
--- a/src/include/utils/acl.h
+++ b/src/include/utils/acl.h
@@ -146,9 +146,11 @@ typedef struct ArrayType Acl;
 #define ACL_CREATE_CHR			'C'
 #define ACL_CREATE_TEMP_CHR		'T'
 #define ACL_CONNECT_CHR			'c'
+#define ACL_SET_VALUE_CHR		's'
+#define ACL_ALTER_SYSTEM_CHR	'A'
 
 /* string holding all privilege code chars, in order by bitmask position */
-#define ACL_ALL_RIGHTS_STR	"arwdDxtXUCTc"
+#define ACL_ALL_RIGHTS_STR	"arwdDxtXUCTcsA"
 
 /*
  * Bitmasks defining "all rights" for each supported object type
@@ -165,6 +167,7 @@ typedef struct ArrayType Acl;
 #define ACL_ALL_RIGHTS_SCHEMA		(ACL_USAGE|ACL_CREATE)
 #define ACL_ALL_RIGHTS_TABLESPACE	(ACL_CREATE)
 #define ACL_ALL_RIGHTS_TYPE			(ACL_USAGE)
+#define ACL_ALL_RIGHTS_SETTING		(ACL_SET_VALUE|ACL_ALTER_SYSTEM)
 
 /* operation codes for pg_*_aclmask */
 typedef enum
@@ -186,6 +189,7 @@ typedef enum
  * routines used internally
  */
 extern Acl *acldefault(ObjectType objtype, Oid ownerId);
+extern Acl *aclsettingdefault(bool is_userset);
 extern Acl *get_user_default_acl(ObjectType objtype, Oid ownerId,
 								 Oid nsp_oid);
 extern void recordDependencyOnNewAcl(Oid classId, Oid objectId, int32 objsubId,
@@ -223,6 +227,8 @@ extern void select_best_grantor(Oid roleId, AclMode privileges,
 
 extern void initialize_acl(void);
 
+extern Oid	get_setting_oid(const char *setting, bool missing_ok);
+
 /*
  * prototypes for functions in aclchk.c
  */
@@ -243,6 +249,8 @@ extern AclMode pg_class_aclmask_ext(Oid table_oid, Oid roleid,
 									bool *is_missing);
 extern AclMode pg_database_aclmask(Oid db_oid, Oid roleid,
 								   AclMode mask, AclMaskHow how);
+extern AclMode pg_setting_acl_aclmask(Oid config_oid, Oid roleid,
+								   AclMode mask, AclMaskHow how);
 extern AclMode pg_proc_aclmask(Oid proc_oid, Oid roleid,
 							   AclMode mask, AclMaskHow how);
 extern AclMode pg_language_aclmask(Oid lang_oid, Oid roleid,
@@ -271,6 +279,7 @@ extern AclResult pg_class_aclcheck(Oid table_oid, Oid roleid, AclMode mode);
 extern AclResult pg_class_aclcheck_ext(Oid table_oid, Oid roleid,
 									   AclMode mode, bool *is_missing);
 extern AclResult pg_database_aclcheck(Oid db_oid, Oid roleid, AclMode mode);
+extern AclResult pg_setting_acl_aclcheck(Oid config_oid, Oid roleid, AclMode mode);
 extern AclResult pg_proc_aclcheck(Oid proc_oid, Oid roleid, AclMode mode);
 extern AclResult pg_language_aclcheck(Oid lang_oid, Oid roleid, AclMode mode);
 extern AclResult pg_largeobject_aclcheck_snapshot(Oid lang_oid, Oid roleid,
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index ea774968f0..e3baa87535 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -381,7 +381,10 @@ extern int	set_config_option(const char *name, const char *value,
 							  GucContext context, GucSource source,
 							  GucAction action, bool changeVal, int elevel,
 							  bool is_reload);
+extern GucContext find_option_context(const char *name);
 extern void AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt);
+extern bool valid_variable_name(const char *name, int *partcnt);
+extern const char *GetConfigOptionCanonicalName(const char *alias);
 extern char *GetConfigOptionByName(const char *name, const char **varname,
 								   bool missing_ok);
 extern void GetConfigOptionByNum(int varnum, const char **values, bool *noshow);
diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h
index b8dd27d4a9..7a8ba82770 100644
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -188,6 +188,7 @@ extern int32 get_attavgwidth(Oid relid, AttrNumber attnum);
 extern bool get_attstatsslot(AttStatsSlot *sslot, HeapTuple statstuple,
 							 int reqkind, Oid reqop, int flags);
 extern void free_attstatsslot(AttStatsSlot *sslot);
+extern char *get_setting_name(Oid configid);
 extern char *get_namespace_name(Oid nspid);
 extern char *get_namespace_name_or_temp(Oid nspid);
 extern Oid	get_range_subtype(Oid rangeOid);
diff --git a/src/include/utils/syscache.h b/src/include/utils/syscache.h
index 9c1a76e8bb..5961921462 100644
--- a/src/include/utils/syscache.h
+++ b/src/include/utils/syscache.h
@@ -89,6 +89,8 @@ enum SysCacheIdentifier
 	REPLORIGNAME,
 	RULERELNAME,
 	SEQRELID,
+	SETTINGNAME,
+	SETTINGOID,
 	STATEXTDATASTXOID,
 	STATEXTNAMENSP,
 	STATEXTOID,
diff --git a/src/test/modules/test_pg_dump/t/001_base.pl b/src/test/modules/test_pg_dump/t/001_base.pl
index c73bd37835..832804b394 100644
--- a/src/test/modules/test_pg_dump/t/001_base.pl
+++ b/src/test/modules/test_pg_dump/t/001_base.pl
@@ -317,6 +317,66 @@ my %tests = (
 		like         => { pg_dumpall_globals => 1, },
 	},
 
+	'GRANT ALTER SYSTEM ON SETTING full_page_writes TO regress_dump_test_role' => {
+		create_order => 2,
+		create_sql   =>
+			'GRANT ALTER SYSTEM ON SETTING full_page_writes TO regress_dump_test_role;',
+		regexp       =>
+
+			qr/^GRANT ALTER SYSTEM ON SETTING full_page_writes TO regress_dump_test_role;/m,
+		like         => { pg_dumpall_globals => 1, },
+	},
+
+	'GRANT ALL ON SETTING fsync TO regress_dump_test_role WITH GRANT OPTION' => {
+		create_order => 2,
+		create_sql =>
+			'GRANT SET VALUE, ALTER SYSTEM ON SETTING fsync TO regress_dump_test_role WITH GRANT OPTION;',
+		regexp =>
+			# "set value" plus "alter system" is "all" privileges on settings
+			qr/^GRANT ALL ON SETTING fsync TO regress_dump_test_role WITH GRANT OPTION;/m,
+		like => { pg_dumpall_globals => 1, },
+	},
+
+	'GRANT ALTER SYSTEM ON SETTING zero_damaged_pages" TO regress_dump_test_role' => {
+		create_order => 2,
+		create_sql =>
+			# configuration parameters get cased folded
+			'GRANT ALTER SYSTEM ON SETTING ZERO_DAMAGED_PAGES TO regress_dump_test_role;',
+		regexp =>
+			qr/^GRANT ALTER SYSTEM ON SETTING zero_damaged_pages TO regress_dump_test_role;/m,
+		like => { pg_dumpall_globals => 1, },
+	},
+
+	'GRANT ALL ON SETTING ignore_checksum_failure TO regress_dump_test_role' => {
+		create_order => 2,
+		create_sql =>
+			# GRANTED BY CURRENT_ROLE is allowed for SQL compatibility, but is ignored
+			'GRANT ALTER SYSTEM, SET VALUE ON SETTING ignore_checksum_failure TO regress_dump_test_role GRANTED BY CURRENT_ROLE;',
+		regexp =>
+			qr/^GRANT ALL ON SETTING ignore_checksum_failure TO regress_dump_test_role/m,
+		like => { pg_dumpall_globals => 1, },
+	},
+
+	'REVOKE SET VALUE ON SETTING work_mem FROM PUBLIC' => {
+		create_order => 2,
+		create_sql =>
+			'REVOKE ALL PRIVILEGES ON SETTING work_mem FROM PUBLIC;',
+		regexp =>
+			# Pubilc only has "set value" by default, so revoking "all privileges" is simplified to revoking "set value"
+			qr/^REVOKE SET VALUE ON SETTING work_mem FROM PUBLIC;/m,
+		like => { pg_dumpall_globals => 1, },
+	},
+
+	'GRANT ALL ON SETTING "DateStyle" TO regress_dump_test_role' => {
+		create_order => 2,
+		create_sql =>
+			'GRANT ALL ON SETTING DateStyle TO regress_dump_test_role WITH GRANT OPTION; REVOKE GRANT OPTION FOR ALL ON SETTING DateStyle FROM regress_dump_test_role;',
+		regexp =>
+			# The revoke simplifies the ultimate grant so as to not include "with grant option"
+			qr/^GRANT ALL ON SETTING "DateStyle" TO regress_dump_test_role;/m,
+		like => { pg_dumpall_globals => 1, },
+	},
+
 	'CREATE SCHEMA public' => {
 		regexp => qr/^CREATE SCHEMA public;/m,
 		like   => {
@@ -791,6 +851,7 @@ foreach my $run (sort keys %pgdump_runs)
 			if (!ok($output_file =~ $tests{$test}->{regexp},
 					"$run: should dump $test"))
 			{
+				die "\n\n--\n\n$output_file\n\n--\n\n";
 				diag("Review $run results in $tempdir");
 			}
 		}
diff --git a/src/test/regress/expected/guc_privs.out b/src/test/regress/expected/guc_privs.out
new file mode 100644
index 0000000000..3d99cc028c
--- /dev/null
+++ b/src/test/regress/expected/guc_privs.out
@@ -0,0 +1,367 @@
+-- Test superuser
+-- Superuser DBA
+CREATE ROLE regress_admin SUPERUSER;
+-- Perform operations as user 'regress_admin' --
+SET SESSION AUTHORIZATION regress_admin;
+-- PGC_BACKEND
+SET ignore_system_indexes = OFF;  -- fail, cannot be set after connection start
+ERROR:  parameter "ignore_system_indexes" cannot be set after connection start
+RESET ignore_system_indexes;  -- fail, cannot be set after connection start
+ERROR:  parameter "ignore_system_indexes" cannot be set after connection start
+ALTER SYSTEM SET ignore_system_indexes = OFF;  -- ok
+ALTER SYSTEM RESET ignore_system_indexes;  -- ok
+-- PGC_INTERNAL
+SET block_size = 50;  -- fail, cannot be changed
+ERROR:  parameter "block_size" cannot be changed
+RESET block_size;  -- fail, cannot be changed
+ERROR:  parameter "block_size" cannot be changed
+ALTER SYSTEM SET block_size = 50;  -- fail, cannot be changed
+ERROR:  parameter "block_size" cannot be changed
+ALTER SYSTEM RESET block_size;  -- fail, cannot be changed
+ERROR:  parameter "block_size" cannot be changed
+-- PGC_POSTMASTER
+SET autovacuum_freeze_max_age = 1000050000;  -- fail, requires restart
+ERROR:  parameter "autovacuum_freeze_max_age" cannot be changed without restarting the server
+RESET autovacuum_freeze_max_age;  -- fail, requires restart
+ERROR:  parameter "autovacuum_freeze_max_age" cannot be changed without restarting the server
+ALTER SYSTEM SET autovacuum_freeze_max_age = 1000050000;  -- ok
+ALTER SYSTEM RESET autovacuum_freeze_max_age;  -- ok
+ALTER SYSTEM SET config_file = '/usr/local/data/postgresql.conf';  -- fail, cannot be changed
+ERROR:  parameter "config_file" cannot be changed
+ALTER SYSTEM RESET config_file;  -- fail, cannot be changed
+ERROR:  parameter "config_file" cannot be changed
+-- PGC_SIGHUP
+SET autovacuum = OFF;  -- fail, requires reload
+ERROR:  parameter "autovacuum" cannot be changed now
+RESET autovacuum;  -- fail, requires reload
+ERROR:  parameter "autovacuum" cannot be changed now
+ALTER SYSTEM SET autovacuum = OFF;  -- ok
+ALTER SYSTEM RESET autovacuum;  -- ok
+-- PGC_SUSET
+SET lc_messages = 'C';  -- ok
+RESET lc_messages;  -- ok
+ALTER SYSTEM SET lc_messages = 'C';  -- ok
+ALTER SYSTEM RESET lc_messages;  -- ok
+-- PGC_SU_BACKEND
+SET jit_debugging_support = OFF;  -- fail, cannot be set after connection start
+ERROR:  parameter "jit_debugging_support" cannot be set after connection start
+RESET jit_debugging_support;  -- fail, cannot be set after connection start
+ERROR:  parameter "jit_debugging_support" cannot be set after connection start
+ALTER SYSTEM SET jit_debugging_support = OFF;  -- ok
+ALTER SYSTEM RESET jit_debugging_support;  -- ok
+-- PGC_USERSET
+SET DateStyle = 'ISO, MDY';  -- ok
+RESET DateStyle;  -- ok
+ALTER SYSTEM SET DateStyle = 'ISO, MDY';  -- ok
+ALTER SYSTEM RESET DateStyle;  -- ok
+ALTER SYSTEM SET ssl_renegotiation_limit = 0;  -- fail, cannot be changed
+ERROR:  parameter "ssl_renegotiation_limit" cannot be changed
+ALTER SYSTEM RESET ssl_renegotiation_limit;  -- fail, cannot be changed
+ERROR:  parameter "ssl_renegotiation_limit" cannot be changed
+-- Finished testing superuser
+RESET statement_timeout;
+-- Create non-superuser with privileges to configure host resource usage
+CREATE ROLE regress_host_resource_admin NOSUPERUSER;
+-- Check the new role does not yet have privileges on settings
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'SET VALUE, ALTER SYSTEM');
+ has_setting_privilege 
+-----------------------
+ t
+(1 row)
+
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'SET VALUE');
+ has_setting_privilege 
+-----------------------
+ t
+(1 row)
+
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'ALTER SYSTEM');
+ has_setting_privilege 
+-----------------------
+ f
+(1 row)
+
+-- Check inappropriate and nonsense privilege types
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'SELECT, UPDATE, CREATE');
+ERROR:  unrecognized privilege type: "SELECT"
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'USAGE');
+ERROR:  unrecognized privilege type: "USAGE"
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'WHATEVER');
+ERROR:  unrecognized privilege type: "WHATEVER"
+-- Revoke, grant, and revoke again a SUSET setting not yet granted
+SELECT has_setting_privilege('regress_host_resource_admin', 'zero_damaged_pages', 'SET VALUE');
+ has_setting_privilege 
+-----------------------
+ 
+(1 row)
+
+SELECT has_setting_privilege('regress_host_resource_admin', 'zero_damaged_pages', 'ALTER SYSTEM');
+ has_setting_privilege 
+-----------------------
+ 
+(1 row)
+
+REVOKE SET VALUE ON SETTING zero_damaged_pages FROM regress_host_resource_admin;
+SELECT has_setting_privilege('regress_host_resource_admin', 'zero_damaged_pages', 'SET VALUE');
+ has_setting_privilege 
+-----------------------
+ f
+(1 row)
+
+SELECT has_setting_privilege('regress_host_resource_admin', 'zero_damaged_pages', 'ALTER SYSTEM');
+ has_setting_privilege 
+-----------------------
+ f
+(1 row)
+
+GRANT SET VALUE ON SETTING zero_damaged_pages TO regress_host_resource_admin;
+SELECT has_setting_privilege('regress_host_resource_admin', 'zero_damaged_pages', 'SET VALUE');
+ has_setting_privilege 
+-----------------------
+ t
+(1 row)
+
+SELECT has_setting_privilege('regress_host_resource_admin', 'zero_damaged_pages', 'ALTER SYSTEM');
+ has_setting_privilege 
+-----------------------
+ f
+(1 row)
+
+REVOKE SET VALUE ON SETTING zero_damaged_pages FROM regress_host_resource_admin;
+SELECT has_setting_privilege('regress_host_resource_admin', 'zero_damaged_pages', 'SET VALUE');
+ has_setting_privilege 
+-----------------------
+ f
+(1 row)
+
+SELECT has_setting_privilege('regress_host_resource_admin', 'zero_damaged_pages', 'ALTER SYSTEM');
+ has_setting_privilege 
+-----------------------
+ f
+(1 row)
+
+-- Revoke, grant, and revoke again a USERSET setting not yet granted
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'SET VALUE');
+ has_setting_privilege 
+-----------------------
+ t
+(1 row)
+
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'ALTER SYSTEM');
+ has_setting_privilege 
+-----------------------
+ f
+(1 row)
+
+REVOKE SET VALUE ON SETTING work_mem FROM regress_host_resource_admin;
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'SET VALUE');
+ has_setting_privilege 
+-----------------------
+ t
+(1 row)
+
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'ALTER SYSTEM');
+ has_setting_privilege 
+-----------------------
+ f
+(1 row)
+
+GRANT SET VALUE ON SETTING work_mem TO regress_host_resource_admin;
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'SET VALUE');
+ has_setting_privilege 
+-----------------------
+ t
+(1 row)
+
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'ALTER SYSTEM');
+ has_setting_privilege 
+-----------------------
+ f
+(1 row)
+
+REVOKE SET VALUE ON SETTING work_mem FROM regress_host_resource_admin;
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'SET VALUE');
+ has_setting_privilege 
+-----------------------
+ t
+(1 row)
+
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'ALTER SYSTEM');
+ has_setting_privilege 
+-----------------------
+ f
+(1 row)
+
+-- Grant privileges on settings to the new non-superuser role
+GRANT SET VALUE, ALTER SYSTEM ON SETTING
+	autovacuum_work_mem, hash_mem_multiplier, logical_decoding_work_mem,
+	maintenance_work_mem, max_stack_depth, min_dynamic_shared_memory,
+	shared_buffers, temp_buffers, temp_file_limit, work_mem
+TO regress_host_resource_admin;
+-- Check the new role now has privilges on settings
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'SET VALUE, ALTER SYSTEM');
+ has_setting_privilege 
+-----------------------
+ t
+(1 row)
+
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'SET VALUE');
+ has_setting_privilege 
+-----------------------
+ t
+(1 row)
+
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'ALTER SYSTEM');
+ has_setting_privilege 
+-----------------------
+ t
+(1 row)
+
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'SET VALUE WITH GRANT OPTION, ALTER SYSTEM WITH GRANT OPTION');
+ has_setting_privilege 
+-----------------------
+ f
+(1 row)
+
+-- Check again the inappropriate and nonsense privilege types.  The prior similar check
+-- was performed before any entry for work_mem existed.
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'SELECT, UPDATE, CREATE');
+ERROR:  unrecognized privilege type: "SELECT"
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'USAGE');
+ERROR:  unrecognized privilege type: "USAGE"
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'WHATEVER');
+ERROR:  unrecognized privilege type: "WHATEVER"
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'WHATEVER WITH GRANT OPTION');
+ERROR:  unrecognized privilege type: "WHATEVER WITH GRANT OPTION"
+-- Check other function signatures
+SELECT has_setting_privilege('regress_host_resource_admin',
+							 (SELECT oid FROM pg_catalog.pg_setting_acl WHERE setting = 'max_stack_depth'),
+							 'SET VALUE');
+ has_setting_privilege 
+-----------------------
+ t
+(1 row)
+
+SELECT has_setting_privilege((SELECT oid FROM pg_catalog.pg_authid WHERE rolname = 'regress_host_resource_admin'),
+							 'max_stack_depth',
+							 'SET VALUE');
+ has_setting_privilege 
+-----------------------
+ t
+(1 row)
+
+SELECT has_setting_privilege((SELECT oid FROM pg_catalog.pg_authid WHERE rolname = 'regress_host_resource_admin'),
+							 (SELECT oid FROM pg_catalog.pg_setting_acl WHERE setting = 'max_stack_depth'),
+							 'SET VALUE');
+ has_setting_privilege 
+-----------------------
+ t
+(1 row)
+
+SELECT has_setting_privilege('hash_mem_multiplier', 'set value');
+ has_setting_privilege 
+-----------------------
+ t
+(1 row)
+
+SELECT has_setting_privilege((SELECT oid FROM pg_catalog.pg_setting_acl WHERE setting = 'work_mem'),
+							 'alter system with grant option');
+ has_setting_privilege 
+-----------------------
+ t
+(1 row)
+
+-- Perform all operations as user 'regress_host_resource_admin' --
+SET SESSION AUTHORIZATION regress_host_resource_admin;
+ALTER SYSTEM SET autovacuum_work_mem = 32;  -- ok, privileges have been granted
+ALTER SYSTEM SET ignore_system_indexes = OFF;  -- fail, insufficient privileges
+ERROR:  permission denied to set parameter "ignore_system_indexes"
+ALTER SYSTEM RESET autovacuum_multixact_freeze_max_age;  -- fail, insufficient privileges
+ERROR:  permission denied to set parameter "autovacuum_multixact_freeze_max_age"
+SET jit_provider = 'llvmjit';  -- fail, insufficient privileges
+ERROR:  parameter "jit_provider" cannot be changed without restarting the server
+ALTER SYSTEM SET shared_buffers = 50;  -- ok
+ALTER SYSTEM RESET shared_buffers;  -- ok
+SET autovacuum_work_mem = 50;  -- cannot be changed now
+ERROR:  parameter "autovacuum_work_mem" cannot be changed now
+ALTER SYSTEM RESET temp_file_limit;  -- ok
+SET TimeZone = 'Europe/Helsinki';  -- ok
+ALTER SYSTEM RESET autovacuum_work_mem;  -- ok, privileges have been granted
+RESET TimeZone;  -- ok
+SET SESSION AUTHORIZATION regress_admin;
+DROP ROLE regress_host_resource_admin;  -- fail, privileges remain
+ERROR:  role "regress_host_resource_admin" cannot be dropped because some objects depend on it
+DETAIL:  privileges for setting work_mem
+privileges for setting maintenance_work_mem
+privileges for setting autovacuum_work_mem
+privileges for setting hash_mem_multiplier
+privileges for setting logical_decoding_work_mem
+privileges for setting max_stack_depth
+privileges for setting min_dynamic_shared_memory
+privileges for setting shared_buffers
+privileges for setting temp_buffers
+privileges for setting temp_file_limit
+-- Use "revoke" to remove the privileges and allow the role to be dropped
+REVOKE SET VALUE, ALTER SYSTEM ON SETTING
+	autovacuum_work_mem, hash_mem_multiplier, logical_decoding_work_mem,
+	maintenance_work_mem, max_stack_depth, min_dynamic_shared_memory,
+	shared_buffers, temp_buffers, temp_file_limit, work_mem
+FROM regress_host_resource_admin;
+DROP ROLE regress_host_resource_admin;  -- ok
+-- Try that again, but use "drop owned by" instead of "revoke"
+CREATE ROLE regress_host_resource_admin NOSUPERUSER;
+SET SESSION AUTHORIZATION regress_host_resource_admin;
+ALTER SYSTEM SET autovacuum_work_mem = 32;  -- fail, privileges not yet granted
+ERROR:  permission denied to set parameter "autovacuum_work_mem"
+SET SESSION AUTHORIZATION regress_admin;
+GRANT SET VALUE, ALTER SYSTEM ON SETTING
+	autovacuum_work_mem, hash_mem_multiplier, logical_decoding_work_mem,
+	maintenance_work_mem, max_stack_depth, min_dynamic_shared_memory,
+	shared_buffers, temp_buffers, temp_file_limit, work_mem
+TO regress_host_resource_admin;
+DROP ROLE regress_host_resource_admin;  -- fail, privileges remain
+ERROR:  role "regress_host_resource_admin" cannot be dropped because some objects depend on it
+DETAIL:  privileges for setting work_mem
+privileges for setting maintenance_work_mem
+privileges for setting autovacuum_work_mem
+privileges for setting hash_mem_multiplier
+privileges for setting logical_decoding_work_mem
+privileges for setting max_stack_depth
+privileges for setting min_dynamic_shared_memory
+privileges for setting shared_buffers
+privileges for setting temp_buffers
+privileges for setting temp_file_limit
+DROP OWNED BY regress_host_resource_admin RESTRICT; -- cascade should not be needed
+SET SESSION AUTHORIZATION regress_host_resource_admin;
+ALTER SYSTEM SET autovacuum_work_mem = 32;  -- fail, "drop owned" has dropped privileges
+ERROR:  permission denied to set parameter "autovacuum_work_mem"
+SET SESSION AUTHORIZATION regress_admin;
+DROP ROLE regress_host_resource_admin;  -- ok
+-- Try that again, but use "reassign owned by" this time
+CREATE ROLE regress_host_resource_admin NOSUPERUSER;
+CREATE ROLE regress_host_resource_newadmin NOSUPERUSER;
+GRANT SET VALUE, ALTER SYSTEM ON SETTING
+	autovacuum_work_mem, hash_mem_multiplier, logical_decoding_work_mem,
+	maintenance_work_mem, max_stack_depth, min_dynamic_shared_memory,
+	shared_buffers, temp_buffers, temp_file_limit, work_mem
+TO regress_host_resource_admin;
+REASSIGN OWNED BY regress_host_resource_admin TO regress_host_resource_newadmin;
+SET SESSION AUTHORIZATION regress_host_resource_admin;
+ALTER SYSTEM SET autovacuum_work_mem = 32;  -- ok, "reassign owned" did not change privileges
+SET SESSION AUTHORIZATION regress_admin;
+DROP ROLE regress_host_resource_admin;  -- fail, privileges remain
+ERROR:  role "regress_host_resource_admin" cannot be dropped because some objects depend on it
+DETAIL:  privileges for setting work_mem
+privileges for setting maintenance_work_mem
+privileges for setting autovacuum_work_mem
+privileges for setting hash_mem_multiplier
+privileges for setting logical_decoding_work_mem
+privileges for setting max_stack_depth
+privileges for setting min_dynamic_shared_memory
+privileges for setting shared_buffers
+privileges for setting temp_buffers
+privileges for setting temp_file_limit
+DROP ROLE regress_host_resource_newadmin;  -- ok, nothing was transferred
+-- Use "drop owned by" so we can drop the role
+DROP OWNED BY regress_host_resource_admin;  -- ok
+DROP ROLE regress_host_resource_admin;  -- ok
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index 291e21d7a6..3158903bda 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -54,6 +54,36 @@ REVOKE pg_read_all_settings FROM regress_priv_user8;
 DROP USER regress_priv_user10;
 DROP USER regress_priv_user9;
 DROP USER regress_priv_user8;
+GRANT SET VALUE ON SETTING enable_memoize TO regress_priv_user6;
+GRANT SET VALUE ON SETTING enable_nestloop TO regress_priv_user6;
+SET ROLE regress_priv_user6;
+SET enable_memoize TO false;
+SET enable_nestloop TO false;
+RESET enable_memoize;
+RESET enable_nestloop;
+RESET ROLE;
+GRANT ALTER SYSTEM ON SETTING enable_seqscan TO regress_priv_user7; -- ok
+GRANT ALTER SYSTEM ON SETTING sort_mem TO regress_priv_user7; -- old name for "work_mem"
+GRANT ALTER SYSTEM ON SETTING maintenance_work_mem TO regress_priv_user7; -- ok
+GRANT ALTER SYSTEM ON SETTING no_such_param TO regress_priv_user7; -- ok
+ERROR:  unrecognized configuration parameter "no_such_param"
+GRANT ALTER SYSTEM ON SETTING no_such_extension.no_such_param TO regress_priv_user7; -- ok
+ERROR:  unrecognized configuration parameter "no_such_extension.no_such_param"
+GRANT ALTER SYSTEM ON SETTING no_such_extension.no_such_param.longer.than.maximum.namedata.length TO regress_priv_user7; -- ok
+ERROR:  unrecognized configuration parameter "no_such_extension.no_such_param.longer.than.maximum.namedata.length"
+GRANT ALTER SYSTEM ON SETTING "" TO regress_priv_user7; -- bad name
+ERROR:  zero-length delimited identifier at or near """"
+LINE 1: GRANT ALTER SYSTEM ON SETTING "" TO regress_priv_user7;
+                                      ^
+GRANT ALTER SYSTEM ON SETTING " " TO regress_priv_user7; -- bad name
+ERROR:  invalid setting name " "
+GRANT ALTER SYSTEM ON SETTING " foo " TO regress_priv_user7; -- bad name
+ERROR:  invalid setting name " foo "
+GRANT SELECT ON public.persons2 TO regress_priv_user7;
+SET ROLE regress_priv_user7;
+ALTER SYSTEM SET enable_seqscan = OFF;
+ALTER SYSTEM RESET enable_seqscan;
+RESET ROLE;
 CREATE GROUP regress_priv_group1;
 CREATE GROUP regress_priv_group2 WITH USER regress_priv_user1, regress_priv_user2;
 ALTER GROUP regress_priv_group1 ADD USER regress_priv_user4;
@@ -2355,10 +2385,32 @@ DROP USER regress_priv_user2;
 DROP USER regress_priv_user3;
 DROP USER regress_priv_user4;
 DROP USER regress_priv_user5;
-DROP USER regress_priv_user6;
-DROP USER regress_priv_user7;
+DROP USER regress_priv_user6; -- privileges remain
+ERROR:  role "regress_priv_user6" cannot be dropped because some objects depend on it
+DETAIL:  privileges for setting enable_memoize
+privileges for setting enable_nestloop
+DROP USER regress_priv_user7; -- privileges remain
+ERROR:  role "regress_priv_user7" cannot be dropped because some objects depend on it
+DETAIL:  privileges for table persons2
+privileges for setting enable_seqscan
+privileges for setting work_mem
+privileges for setting maintenance_work_mem
 DROP USER regress_priv_user8; -- does not exist
 ERROR:  role "regress_priv_user8" does not exist
+REVOKE SELECT ON public.persons2 FROM regress_priv_user7;
+REVOKE ALTER SYSTEM ON SETTING enable_seqscan FROM regress_priv_user7; -- ok
+REVOKE ALTER SYSTEM ON SETTING work_mem FROM regress_priv_user7; -- ok, use new name
+REVOKE ALTER SYSTEM ON SETTING vacuum_mem FROM regress_priv_user7; -- ok, use old name
+REVOKE ALTER SYSTEM ON SETTING no_such_param FROM regress_priv_user7; -- ok
+ERROR:  unrecognized configuration parameter "no_such_param"
+REVOKE ALTER SYSTEM ON SETTING no_such_extension.no_such_param FROM regress_priv_user7; -- ok
+ERROR:  unrecognized configuration parameter "no_such_extension.no_such_param"
+REVOKE ALTER SYSTEM ON SETTING no_such_extension.no_such_param.longer.than.maximum.namedata.length FROM regress_priv_user7; -- ok
+ERROR:  unrecognized configuration parameter "no_such_extension.no_such_param.longer.than.maximum.namedata.length"
+DROP USER regress_priv_user7; -- ok
+REVOKE SET VALUE ON SETTING enable_memoize FROM regress_priv_user6;
+REVOKE SET VALUE ON SETTING enable_nestloop FROM regress_priv_user6;
+DROP USER regress_priv_user6; -- ok
 -- permissions with LOCK TABLE
 CREATE USER regress_locktable_user;
 CREATE TABLE lock_table (a int);
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index ac468568a1..0d0e3ff410 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1686,6 +1686,19 @@ pg_sequences| SELECT n.nspname AS schemaname,
      JOIN pg_class c ON ((c.oid = s.seqrelid)))
      LEFT JOIN pg_namespace n ON ((n.oid = c.relnamespace)))
   WHERE ((NOT pg_is_other_temp_schema(n.oid)) AND (c.relkind = 'S'::"char"));
+pg_setting_privileges| SELECT grantor.rolname AS grantor,
+    grantee.rolname AS grantee,
+    set_acl.setting,
+    acl.privilege_type,
+    acl.is_grantable
+   FROM pg_setting_acl set_acl,
+    ((LATERAL ( SELECT aclexplode.grantor,
+            aclexplode.grantee,
+            aclexplode.privilege_type,
+            aclexplode.is_grantable
+           FROM aclexplode(set_acl.setacl) aclexplode(grantor, grantee, privilege_type, is_grantable)) acl
+     LEFT JOIN pg_authid grantee ON ((acl.grantee = grantee.oid)))
+     LEFT JOIN pg_authid grantor ON ((acl.grantor = grantor.oid)));
 pg_settings| SELECT a.name,
     a.setting,
     a.unit,
diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule
index 6d8f524ae9..4dfa0642c0 100644
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -86,7 +86,7 @@ test: brin_bloom brin_multi
 # psql depends on create_am
 # amutils depends on geometry, create_index_spgist, hash_index, brin
 # ----------
-test: create_table_like alter_generic alter_operator misc async dbsize misc_functions sysviews tsrf tid tidscan tidrangescan collate.icu.utf8 incremental_sort create_role
+test: create_table_like alter_generic alter_operator misc async dbsize misc_functions sysviews tsrf tid tidscan tidrangescan collate.icu.utf8 incremental_sort create_role guc_privs
 
 # collate.*.utf8 tests cannot be run in parallel with each other
 test: rules psql psql_crosstab amutils stats_ext collate.linux.utf8
diff --git a/src/test/regress/sql/guc_privs.sql b/src/test/regress/sql/guc_privs.sql
new file mode 100644
index 0000000000..5914ae69de
--- /dev/null
+++ b/src/test/regress/sql/guc_privs.sql
@@ -0,0 +1,165 @@
+-- Test superuser
+-- Superuser DBA
+CREATE ROLE regress_admin SUPERUSER;
+-- Perform operations as user 'regress_admin' --
+SET SESSION AUTHORIZATION regress_admin;
+-- PGC_BACKEND
+SET ignore_system_indexes = OFF;  -- fail, cannot be set after connection start
+RESET ignore_system_indexes;  -- fail, cannot be set after connection start
+ALTER SYSTEM SET ignore_system_indexes = OFF;  -- ok
+ALTER SYSTEM RESET ignore_system_indexes;  -- ok
+-- PGC_INTERNAL
+SET block_size = 50;  -- fail, cannot be changed
+RESET block_size;  -- fail, cannot be changed
+ALTER SYSTEM SET block_size = 50;  -- fail, cannot be changed
+ALTER SYSTEM RESET block_size;  -- fail, cannot be changed
+-- PGC_POSTMASTER
+SET autovacuum_freeze_max_age = 1000050000;  -- fail, requires restart
+RESET autovacuum_freeze_max_age;  -- fail, requires restart
+ALTER SYSTEM SET autovacuum_freeze_max_age = 1000050000;  -- ok
+ALTER SYSTEM RESET autovacuum_freeze_max_age;  -- ok
+ALTER SYSTEM SET config_file = '/usr/local/data/postgresql.conf';  -- fail, cannot be changed
+ALTER SYSTEM RESET config_file;  -- fail, cannot be changed
+-- PGC_SIGHUP
+SET autovacuum = OFF;  -- fail, requires reload
+RESET autovacuum;  -- fail, requires reload
+ALTER SYSTEM SET autovacuum = OFF;  -- ok
+ALTER SYSTEM RESET autovacuum;  -- ok
+-- PGC_SUSET
+SET lc_messages = 'C';  -- ok
+RESET lc_messages;  -- ok
+ALTER SYSTEM SET lc_messages = 'C';  -- ok
+ALTER SYSTEM RESET lc_messages;  -- ok
+-- PGC_SU_BACKEND
+SET jit_debugging_support = OFF;  -- fail, cannot be set after connection start
+RESET jit_debugging_support;  -- fail, cannot be set after connection start
+ALTER SYSTEM SET jit_debugging_support = OFF;  -- ok
+ALTER SYSTEM RESET jit_debugging_support;  -- ok
+-- PGC_USERSET
+SET DateStyle = 'ISO, MDY';  -- ok
+RESET DateStyle;  -- ok
+ALTER SYSTEM SET DateStyle = 'ISO, MDY';  -- ok
+ALTER SYSTEM RESET DateStyle;  -- ok
+ALTER SYSTEM SET ssl_renegotiation_limit = 0;  -- fail, cannot be changed
+ALTER SYSTEM RESET ssl_renegotiation_limit;  -- fail, cannot be changed
+-- Finished testing superuser
+RESET statement_timeout;
+-- Create non-superuser with privileges to configure host resource usage
+CREATE ROLE regress_host_resource_admin NOSUPERUSER;
+-- Check the new role does not yet have privileges on settings
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'SET VALUE, ALTER SYSTEM');
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'SET VALUE');
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'ALTER SYSTEM');
+-- Check inappropriate and nonsense privilege types
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'SELECT, UPDATE, CREATE');
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'USAGE');
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'WHATEVER');
+-- Revoke, grant, and revoke again a SUSET setting not yet granted
+SELECT has_setting_privilege('regress_host_resource_admin', 'zero_damaged_pages', 'SET VALUE');
+SELECT has_setting_privilege('regress_host_resource_admin', 'zero_damaged_pages', 'ALTER SYSTEM');
+REVOKE SET VALUE ON SETTING zero_damaged_pages FROM regress_host_resource_admin;
+SELECT has_setting_privilege('regress_host_resource_admin', 'zero_damaged_pages', 'SET VALUE');
+SELECT has_setting_privilege('regress_host_resource_admin', 'zero_damaged_pages', 'ALTER SYSTEM');
+GRANT SET VALUE ON SETTING zero_damaged_pages TO regress_host_resource_admin;
+SELECT has_setting_privilege('regress_host_resource_admin', 'zero_damaged_pages', 'SET VALUE');
+SELECT has_setting_privilege('regress_host_resource_admin', 'zero_damaged_pages', 'ALTER SYSTEM');
+REVOKE SET VALUE ON SETTING zero_damaged_pages FROM regress_host_resource_admin;
+SELECT has_setting_privilege('regress_host_resource_admin', 'zero_damaged_pages', 'SET VALUE');
+SELECT has_setting_privilege('regress_host_resource_admin', 'zero_damaged_pages', 'ALTER SYSTEM');
+-- Revoke, grant, and revoke again a USERSET setting not yet granted
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'SET VALUE');
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'ALTER SYSTEM');
+REVOKE SET VALUE ON SETTING work_mem FROM regress_host_resource_admin;
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'SET VALUE');
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'ALTER SYSTEM');
+GRANT SET VALUE ON SETTING work_mem TO regress_host_resource_admin;
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'SET VALUE');
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'ALTER SYSTEM');
+REVOKE SET VALUE ON SETTING work_mem FROM regress_host_resource_admin;
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'SET VALUE');
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'ALTER SYSTEM');
+-- Grant privileges on settings to the new non-superuser role
+GRANT SET VALUE, ALTER SYSTEM ON SETTING
+	autovacuum_work_mem, hash_mem_multiplier, logical_decoding_work_mem,
+	maintenance_work_mem, max_stack_depth, min_dynamic_shared_memory,
+	shared_buffers, temp_buffers, temp_file_limit, work_mem
+TO regress_host_resource_admin;
+-- Check the new role now has privilges on settings
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'SET VALUE, ALTER SYSTEM');
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'SET VALUE');
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'ALTER SYSTEM');
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'SET VALUE WITH GRANT OPTION, ALTER SYSTEM WITH GRANT OPTION');
+-- Check again the inappropriate and nonsense privilege types.  The prior similar check
+-- was performed before any entry for work_mem existed.
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'SELECT, UPDATE, CREATE');
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'USAGE');
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'WHATEVER');
+SELECT has_setting_privilege('regress_host_resource_admin', 'work_mem', 'WHATEVER WITH GRANT OPTION');
+-- Check other function signatures
+SELECT has_setting_privilege('regress_host_resource_admin',
+							 (SELECT oid FROM pg_catalog.pg_setting_acl WHERE setting = 'max_stack_depth'),
+							 'SET VALUE');
+SELECT has_setting_privilege((SELECT oid FROM pg_catalog.pg_authid WHERE rolname = 'regress_host_resource_admin'),
+							 'max_stack_depth',
+							 'SET VALUE');
+SELECT has_setting_privilege((SELECT oid FROM pg_catalog.pg_authid WHERE rolname = 'regress_host_resource_admin'),
+							 (SELECT oid FROM pg_catalog.pg_setting_acl WHERE setting = 'max_stack_depth'),
+							 'SET VALUE');
+SELECT has_setting_privilege('hash_mem_multiplier', 'set value');
+SELECT has_setting_privilege((SELECT oid FROM pg_catalog.pg_setting_acl WHERE setting = 'work_mem'),
+							 'alter system with grant option');
+-- Perform all operations as user 'regress_host_resource_admin' --
+SET SESSION AUTHORIZATION regress_host_resource_admin;
+ALTER SYSTEM SET autovacuum_work_mem = 32;  -- ok, privileges have been granted
+ALTER SYSTEM SET ignore_system_indexes = OFF;  -- fail, insufficient privileges
+ALTER SYSTEM RESET autovacuum_multixact_freeze_max_age;  -- fail, insufficient privileges
+SET jit_provider = 'llvmjit';  -- fail, insufficient privileges
+ALTER SYSTEM SET shared_buffers = 50;  -- ok
+ALTER SYSTEM RESET shared_buffers;  -- ok
+SET autovacuum_work_mem = 50;  -- cannot be changed now
+ALTER SYSTEM RESET temp_file_limit;  -- ok
+SET TimeZone = 'Europe/Helsinki';  -- ok
+ALTER SYSTEM RESET autovacuum_work_mem;  -- ok, privileges have been granted
+RESET TimeZone;  -- ok
+SET SESSION AUTHORIZATION regress_admin;
+DROP ROLE regress_host_resource_admin;  -- fail, privileges remain
+-- Use "revoke" to remove the privileges and allow the role to be dropped
+REVOKE SET VALUE, ALTER SYSTEM ON SETTING
+	autovacuum_work_mem, hash_mem_multiplier, logical_decoding_work_mem,
+	maintenance_work_mem, max_stack_depth, min_dynamic_shared_memory,
+	shared_buffers, temp_buffers, temp_file_limit, work_mem
+FROM regress_host_resource_admin;
+DROP ROLE regress_host_resource_admin;  -- ok
+-- Try that again, but use "drop owned by" instead of "revoke"
+CREATE ROLE regress_host_resource_admin NOSUPERUSER;
+SET SESSION AUTHORIZATION regress_host_resource_admin;
+ALTER SYSTEM SET autovacuum_work_mem = 32;  -- fail, privileges not yet granted
+SET SESSION AUTHORIZATION regress_admin;
+GRANT SET VALUE, ALTER SYSTEM ON SETTING
+	autovacuum_work_mem, hash_mem_multiplier, logical_decoding_work_mem,
+	maintenance_work_mem, max_stack_depth, min_dynamic_shared_memory,
+	shared_buffers, temp_buffers, temp_file_limit, work_mem
+TO regress_host_resource_admin;
+DROP ROLE regress_host_resource_admin;  -- fail, privileges remain
+DROP OWNED BY regress_host_resource_admin RESTRICT; -- cascade should not be needed
+SET SESSION AUTHORIZATION regress_host_resource_admin;
+ALTER SYSTEM SET autovacuum_work_mem = 32;  -- fail, "drop owned" has dropped privileges
+SET SESSION AUTHORIZATION regress_admin;
+DROP ROLE regress_host_resource_admin;  -- ok
+-- Try that again, but use "reassign owned by" this time
+CREATE ROLE regress_host_resource_admin NOSUPERUSER;
+CREATE ROLE regress_host_resource_newadmin NOSUPERUSER;
+GRANT SET VALUE, ALTER SYSTEM ON SETTING
+	autovacuum_work_mem, hash_mem_multiplier, logical_decoding_work_mem,
+	maintenance_work_mem, max_stack_depth, min_dynamic_shared_memory,
+	shared_buffers, temp_buffers, temp_file_limit, work_mem
+TO regress_host_resource_admin;
+REASSIGN OWNED BY regress_host_resource_admin TO regress_host_resource_newadmin;
+SET SESSION AUTHORIZATION regress_host_resource_admin;
+ALTER SYSTEM SET autovacuum_work_mem = 32;  -- ok, "reassign owned" did not change privileges
+SET SESSION AUTHORIZATION regress_admin;
+DROP ROLE regress_host_resource_admin;  -- fail, privileges remain
+DROP ROLE regress_host_resource_newadmin;  -- ok, nothing was transferred
+-- Use "drop owned by" so we can drop the role
+DROP OWNED BY regress_host_resource_admin;  -- ok
+DROP ROLE regress_host_resource_admin;  -- ok
diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
index c8c545b64c..9f1f436b33 100644
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -66,6 +66,33 @@ DROP USER regress_priv_user10;
 DROP USER regress_priv_user9;
 DROP USER regress_priv_user8;
 
+GRANT SET VALUE ON SETTING enable_memoize TO regress_priv_user6;
+GRANT SET VALUE ON SETTING enable_nestloop TO regress_priv_user6;
+
+SET ROLE regress_priv_user6;
+SET enable_memoize TO false;
+SET enable_nestloop TO false;
+RESET enable_memoize;
+RESET enable_nestloop;
+RESET ROLE;
+
+GRANT ALTER SYSTEM ON SETTING enable_seqscan TO regress_priv_user7; -- ok
+GRANT ALTER SYSTEM ON SETTING sort_mem TO regress_priv_user7; -- old name for "work_mem"
+GRANT ALTER SYSTEM ON SETTING maintenance_work_mem TO regress_priv_user7; -- ok
+GRANT ALTER SYSTEM ON SETTING no_such_param TO regress_priv_user7; -- ok
+GRANT ALTER SYSTEM ON SETTING no_such_extension.no_such_param TO regress_priv_user7; -- ok
+GRANT ALTER SYSTEM ON SETTING no_such_extension.no_such_param.longer.than.maximum.namedata.length TO regress_priv_user7; -- ok
+GRANT ALTER SYSTEM ON SETTING "" TO regress_priv_user7; -- bad name
+GRANT ALTER SYSTEM ON SETTING " " TO regress_priv_user7; -- bad name
+GRANT ALTER SYSTEM ON SETTING " foo " TO regress_priv_user7; -- bad name
+
+GRANT SELECT ON public.persons2 TO regress_priv_user7;
+
+SET ROLE regress_priv_user7;
+ALTER SYSTEM SET enable_seqscan = OFF;
+ALTER SYSTEM RESET enable_seqscan;
+RESET ROLE;
+
 CREATE GROUP regress_priv_group1;
 CREATE GROUP regress_priv_group2 WITH USER regress_priv_user1, regress_priv_user2;
 
@@ -1426,10 +1453,23 @@ DROP USER regress_priv_user2;
 DROP USER regress_priv_user3;
 DROP USER regress_priv_user4;
 DROP USER regress_priv_user5;
-DROP USER regress_priv_user6;
-DROP USER regress_priv_user7;
+DROP USER regress_priv_user6; -- privileges remain
+DROP USER regress_priv_user7; -- privileges remain
 DROP USER regress_priv_user8; -- does not exist
 
+REVOKE SELECT ON public.persons2 FROM regress_priv_user7;
+REVOKE ALTER SYSTEM ON SETTING enable_seqscan FROM regress_priv_user7; -- ok
+REVOKE ALTER SYSTEM ON SETTING work_mem FROM regress_priv_user7; -- ok, use new name
+REVOKE ALTER SYSTEM ON SETTING vacuum_mem FROM regress_priv_user7; -- ok, use old name
+REVOKE ALTER SYSTEM ON SETTING no_such_param FROM regress_priv_user7; -- ok
+REVOKE ALTER SYSTEM ON SETTING no_such_extension.no_such_param FROM regress_priv_user7; -- ok
+REVOKE ALTER SYSTEM ON SETTING no_such_extension.no_such_param.longer.than.maximum.namedata.length FROM regress_priv_user7; -- ok
+DROP USER regress_priv_user7; -- ok
+
+REVOKE SET VALUE ON SETTING enable_memoize FROM regress_priv_user6;
+REVOKE SET VALUE ON SETTING enable_nestloop FROM regress_priv_user6;
+
+DROP USER regress_priv_user6; -- ok
 
 -- permissions with LOCK TABLE
 CREATE USER regress_locktable_user;
-- 
2.35.1



^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: Granting SET and ALTER SYSTE privileges for GUCs
@ 2022-03-16 11:43  Andrew Dunstan <[email protected]>
  parent: Mark Dilger <[email protected]>
  1 sibling, 0 replies; 24+ messages in thread

From: Andrew Dunstan @ 2022-03-16 11:43 UTC (permalink / raw)
  To: Mark Dilger <[email protected]>; Tom Lane <[email protected]>; +Cc: Joshua Brindle <[email protected]>; Robert Haas <[email protected]>; Jeff Davis <[email protected]>; pgsql-hackers; Joe Conway <[email protected]>


On 3/15/22 16:59, Mark Dilger wrote:
>> On Mar 6, 2022, at 3:27 PM, Tom Lane <[email protected]> wrote:
>>
>> Mark Dilger <[email protected]> writes:
>>> The existing patch allows grants on unknown gucs, because it can't know what guc an upgrade script will introduce, and the grant statement may need to execute before the guc exists.
>> Yeah, that's the problematic case.  It might mostly work to assume that
>> an unknown GUC has an empty default ACL.  This could fail to retain the
>> default PUBLIC SET permission if it later turns out the GUC is USERSET
> On further reflection, I concluded this isn't needed.  No current extension, whether in-core or third party, expects to be able to create a new GUC and then grant or revoke permissions on it.  They can already specify the guc context (PGC_USERS, etc).  Introducing a feature that depends on the dubious assumption that unrecognized GUCs will turn out to be USERSET doesn't seem warranted.

Agreed.


>
> The patch attributes all grants of setting privileges to the bootstrap superuser.  Only superusers can grant or revoke privileges on settings, and all settings are implicitly owned by the bootstrap superuser because there is no explicit owner associated with settings.  Consequently, select_best_grantor(some_superuser, ..., BOOTSTRAP_SUPERUSERID, ...) always chooses the bootstrap superuser.  I don't see a problem with this, but wouldn't mind a second opinion.  Some people might find it surprising when viewing the pg_setting_acl.setacl field.


I think it's OK as long as we document it. An alternative might be to
invent a pseudo-superuser called, say, 'postgres_system', but that seems
like overkill to solve what is in effect a cosmetic problem.

Generally I think this is now in fairly good shape, I've played with it
and it seems to do what I expect in every case, and the things I found
surprising are gone.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com







^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: Granting SET and ALTER SYSTE privileges for GUCs
@ 2022-03-16 18:47  Tom Lane <[email protected]>
  parent: Mark Dilger <[email protected]>
  1 sibling, 4 replies; 24+ messages in thread

From: Tom Lane @ 2022-03-16 18:47 UTC (permalink / raw)
  To: Andrew Dunstan <[email protected]>; +Cc: Mark Dilger <[email protected]>; Joshua Brindle <[email protected]>; Robert Haas <[email protected]>; Jeff Davis <[email protected]>; pgsql-hackers; Joe Conway <[email protected]>

Andrew Dunstan <[email protected]> writes:
> Generally I think this is now in fairly good shape, I've played with it
> and it seems to do what I expect in every case, and the things I found
> surprising are gone.

Stepping back a bit ... do we really want to institutionalize the
term "setting" for GUC variables?  I realize that the view pg_settings
exists, but the documentation generally prefers the term "configuration
parameters".  Where config.sgml uses "setting" as a noun, it's usually
talking about a specific concrete value for a parameter, and you can
argue that the view's name comports with that meaning.  But you can't
GRANT a parameter's current value.

I don't have a better name to offer offhand --- I surely am not proposing
that we change the syntax to be "GRANT ... ON CONFIGURATION PARAMETER x",
because that's way too wordy.  But now is the time to bikeshed if we're
gonna bikeshed, or else admit that we're not interested in precise
vocabulary.

I'm also fairly allergic to the way that this patch has decided to assign
multi-word names to privilege types (ie SET VALUE, ALTER SYSTEM).  There
is no existing precedent for that, and I think it's going to break
client-side code that we don't need to break.  It's not coincidental that
this forces weird changes in rules about whitespace in the has_privilege
functions, for example; and if you think that isn't going to cause
problems I think you are wrong.  Perhaps we could just use "SET" and
"ALTER", or "SET" and "SYSTEM"?

I also agree with the upthread criticism that this is abusing the
ObjectPostAlterHook API unreasonably much.  In particular, it's
trying to use pg_setting_acl OIDs as identities for GUCs, which
they are not, first because they're unstable and second because
most GUCs won't even have an entry there.  I therefore judge the
hook calls added to ExecSetVariableStmt and AlterSystemSetConfigFile
to be 100% useless, in fact probably counterproductive because they
introduce a boatload of worries about whether the right things happen
if the hook errors out or does something guc.c isn't expecting.

I suggest that what might be saner is to consider that the "objects"
that the hook calls are concerned with are the pg_setting_acl entries,
not the underlying GUCs, and thus that the hooks need be invoked only
when creating, destroying or altering those entries.  If we do have
a need for a hook editorializing on GUC value settings, that would
have to be an independent API --- but I have heard no calls for
the ability to have such a hook, and I don't think that this patch
is the place to introduce one.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: Granting SET and ALTER SYSTE privileges for GUCs
@ 2022-03-16 18:59  Mark Dilger <[email protected]>
  parent: Tom Lane <[email protected]>
  3 siblings, 2 replies; 24+ messages in thread

From: Mark Dilger @ 2022-03-16 18:59 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Joshua Brindle <[email protected]>; Robert Haas <[email protected]>; Jeff Davis <[email protected]>; pgsql-hackers; Joe Conway <[email protected]>



> On Mar 16, 2022, at 11:47 AM, Tom Lane <[email protected]> wrote:
> 
> Stepping back a bit ... do we really want to institutionalize the
> term "setting" for GUC variables?  I realize that the view pg_settings
> exists, but the documentation generally prefers the term "configuration
> parameters".  Where config.sgml uses "setting" as a noun, it's usually
> talking about a specific concrete value for a parameter, and you can
> argue that the view's name comports with that meaning.  But you can't
> GRANT a parameter's current value.
> 
> I don't have a better name to offer offhand --- I surely am not proposing
> that we change the syntax to be "GRANT ... ON CONFIGURATION PARAMETER x",
> because that's way too wordy.  But now is the time to bikeshed if we're
> gonna bikeshed, or else admit that we're not interested in precise
> vocabulary.

Informally, we often use "GUC" on this list, but that isn't used formally, leaving "configuration parameter" and "setting" as the two obvious choices.  I preferred "configuration parameter" originally and was argued out of it.  My take on "setting" was also that it more naturally refers to the choice of setting, not the thing being set, such that "work_mem = 8192" means the configuration parameter "work_mem" has the setting "8192".  I'm happy to change the patch along these lines, but I vaguely recall it being Robert who liked the "setting" language.  Robert?  (Sorry if I misremember that...)

> I'm also fairly allergic to the way that this patch has decided to assign
> multi-word names to privilege types (ie SET VALUE, ALTER SYSTEM).  There
> is no existing precedent for that, and I think it's going to break
> client-side code that we don't need to break.  It's not coincidental that
> this forces weird changes in rules about whitespace in the has_privilege
> functions, for example; and if you think that isn't going to cause
> problems I think you are wrong.  Perhaps we could just use "SET" and
> "ALTER", or "SET" and "SYSTEM"?

I chose those particular multi-word names to avoid needing to promote any keywords.  That's been a while ago, and I don't recall exactly where the shift-reduce or reduce-reduce errors were coming from.  I'll play with it to see what I can find.

> I also agree with the upthread criticism that this is abusing the
> ObjectPostAlterHook API unreasonably much.  In particular, it's
> trying to use pg_setting_acl OIDs as identities for GUCs, which
> they are not, first because they're unstable and second because
> most GUCs won't even have an entry there.  I therefore judge the
> hook calls added to ExecSetVariableStmt and AlterSystemSetConfigFile
> to be 100% useless, in fact probably counterproductive because they
> introduce a boatload of worries about whether the right things happen
> if the hook errors out or does something guc.c isn't expecting.

I think Joshua was planning to use these hooks for security purposes.  The hooks are supposed to check whether the Oid is valid, and if not, still be able to make choices based on the other information.  Joshua, any comment on this?

> I suggest that what might be saner is to consider that the "objects"
> that the hook calls are concerned with are the pg_setting_acl entries,
> not the underlying GUCs, and thus that the hooks need be invoked only
> when creating, destroying or altering those entries.

That's certainly a thing we could do, but I got the impression that Joshua wanted to hook into SET, RESET, and ALTER SYSTEM SET, not merely into GRANT and REVOKE.

>  If we do have
> a need for a hook editorializing on GUC value settings, that would
> have to be an independent API --- but I have heard no calls for
> the ability to have such a hook, and I don't think that this patch
> is the place to introduce one.

Well, the call for it was in this thread, but I'm ok with yanking out that part of the patch if it seems half-baked.

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company









^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: Granting SET and ALTER SYSTE privileges for GUCs
@ 2022-03-16 19:06  Tom Lane <[email protected]>
  parent: Mark Dilger <[email protected]>
  1 sibling, 1 reply; 24+ messages in thread

From: Tom Lane @ 2022-03-16 19:06 UTC (permalink / raw)
  To: Mark Dilger <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Joshua Brindle <[email protected]>; Robert Haas <[email protected]>; Jeff Davis <[email protected]>; pgsql-hackers; Joe Conway <[email protected]>

Mark Dilger <[email protected]> writes:
> On Mar 16, 2022, at 11:47 AM, Tom Lane <[email protected]> wrote:
>> ... I therefore judge the
>> hook calls added to ExecSetVariableStmt and AlterSystemSetConfigFile
>> to be 100% useless, in fact probably counterproductive because they
>> introduce a boatload of worries about whether the right things happen
>> if the hook errors out or does something guc.c isn't expecting.

> I think Joshua was planning to use these hooks for security purposes.  The hooks are supposed to check whether the Oid is valid, and if not, still be able to make choices based on the other information.  Joshua, any comment on this?

It's going to be hard to do anything useful in a hook that (a) does
not know which GUC is being assigned to and (b) cannot do catalog
accesses for fear that we're not inside a transaction.  (b), in
particular, seems like a rather thorough API break; up to now
ObjectPostAlter hooks could assume that catalog accesses are OK.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: Granting SET and ALTER SYSTE privileges for GUCs
@ 2022-03-16 19:36  Joshua Brindle <[email protected]>
  parent: Tom Lane <[email protected]>
  0 siblings, 1 reply; 24+ messages in thread

From: Joshua Brindle @ 2022-03-16 19:36 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Mark Dilger <[email protected]>; Andrew Dunstan <[email protected]>; Robert Haas <[email protected]>; Jeff Davis <[email protected]>; pgsql-hackers; Joe Conway <[email protected]>

On Wed, Mar 16, 2022 at 3:06 PM Tom Lane <[email protected]> wrote:
>
> Mark Dilger <[email protected]> writes:
> > On Mar 16, 2022, at 11:47 AM, Tom Lane <[email protected]> wrote:
> >> ... I therefore judge the
> >> hook calls added to ExecSetVariableStmt and AlterSystemSetConfigFile
> >> to be 100% useless, in fact probably counterproductive because they
> >> introduce a boatload of worries about whether the right things happen
> >> if the hook errors out or does something guc.c isn't expecting.
>
> > I think Joshua was planning to use these hooks for security purposes.  The hooks are supposed to check whether the Oid is valid, and if not, still be able to make choices based on the other information.  Joshua, any comment on this?
>
> It's going to be hard to do anything useful in a hook that (a) does
> not know which GUC is being assigned to and (b) cannot do catalog
> accesses for fear that we're not inside a transaction.  (b), in
> particular, seems like a rather thorough API break; up to now
> ObjectPostAlter hooks could assume that catalog accesses are OK.
>

Can you elaborate on this point? This is perhaps an area where I don't
know the rules of the road, to test this hook I modified the set_user
extension, which normally uses the parse tree to figure out if someone
is trying to SET log_statement or ALTER SYSTEM log_statement and
replaced it with:

                case OAT_POST_ALTER:
                if (classId == SettingAclRelationId)
                {
                        ObjectAddress   object;
                        object.classId = SettingAclRelationId;
                        object.objectId = objectId;
                        object.objectSubId = 0;
                        if (strcmp(getObjectIdentity(&object),
"log_statement") == 0)
                        {
                                ereport(ERROR,
                                (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
                                 errmsg("Setting %s blocked",
getObjectIdentity(&object))));
                        }
                  }

Is that inherently unsafe?






^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: Granting SET and ALTER SYSTE privileges for GUCs
@ 2022-03-16 19:58  Tom Lane <[email protected]>
  parent: Joshua Brindle <[email protected]>
  0 siblings, 1 reply; 24+ messages in thread

From: Tom Lane @ 2022-03-16 19:58 UTC (permalink / raw)
  To: Joshua Brindle <[email protected]>; +Cc: Mark Dilger <[email protected]>; Andrew Dunstan <[email protected]>; Robert Haas <[email protected]>; Jeff Davis <[email protected]>; pgsql-hackers; Joe Conway <[email protected]>

Joshua Brindle <[email protected]> writes:
> On Wed, Mar 16, 2022 at 3:06 PM Tom Lane <[email protected]> wrote:
>> It's going to be hard to do anything useful in a hook that (a) does
>> not know which GUC is being assigned to and (b) cannot do catalog
>> accesses for fear that we're not inside a transaction.  (b), in
>> particular, seems like a rather thorough API break; up to now
>> ObjectPostAlter hooks could assume that catalog accesses are OK.

> Can you elaborate on this point?

Hmm, I glossed over too many details there perhaps.  I was thinking
about the restrictions on GUC check_hooks, which can be run outside
a transaction altogether.  But that's not quite relevant here.
ExecSetVariableStmt can assume it's inside a transaction, but what
it *can't* assume is that we've set a transaction snapshot as yet
(cf. PlannedStmtRequiresSnapshot).  If we call an ObjectPostAlter hook
there, and it does a catalog access, that's going to break things for
modifications of GUCs that are supposed to be modifiable without
freezing the transaction snapshot.  So the net result is the same:
catalog access not okay, at least not in general.

Between that and the fact that an OID-based API is largely useless here,
I don't think it's sane to try to use the existing ObjectPostAlter API.
Maybe there is a case for inventing a separate hook API that could pass
the GUC name as a string, instead.  I remain of the opinion that this
patch should not concern itself with that, though.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: Granting SET and ALTER SYSTE privileges for GUCs
@ 2022-03-16 20:07  Andrew Dunstan <[email protected]>
  parent: Tom Lane <[email protected]>
  3 siblings, 1 reply; 24+ messages in thread

From: Andrew Dunstan @ 2022-03-16 20:07 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Mark Dilger <[email protected]>; Joshua Brindle <[email protected]>; Robert Haas <[email protected]>; Jeff Davis <[email protected]>; pgsql-hackers; Joe Conway <[email protected]>


On 3/16/22 14:47, Tom Lane wrote:
> Andrew Dunstan <[email protected]> writes:
>> Generally I think this is now in fairly good shape, I've played with it
>> and it seems to do what I expect in every case, and the things I found
>> surprising are gone.
> Stepping back a bit ... do we really want to institutionalize the
> term "setting" for GUC variables?  I realize that the view pg_settings
> exists, but the documentation generally prefers the term "configuration
> parameters".  Where config.sgml uses "setting" as a noun, it's usually
> talking about a specific concrete value for a parameter, and you can
> argue that the view's name comports with that meaning.  But you can't
> GRANT a parameter's current value.
>
> I don't have a better name to offer offhand --- I surely am not proposing
> that we change the syntax to be "GRANT ... ON CONFIGURATION PARAMETER x",
> because that's way too wordy.  But now is the time to bikeshed if we're
> gonna bikeshed, or else admit that we're not interested in precise
> vocabulary.
>
> I'm also fairly allergic to the way that this patch has decided to assign
> multi-word names to privilege types (ie SET VALUE, ALTER SYSTEM).  There
> is no existing precedent for that, and I think it's going to break
> client-side code that we don't need to break.  It's not coincidental that
> this forces weird changes in rules about whitespace in the has_privilege
> functions, for example; and if you think that isn't going to cause
> problems I think you are wrong.  Perhaps we could just use "SET" and
> "ALTER", or "SET" and "SYSTEM"?


That's going to look weird, ISTM. This is less clear about what it's
granting.

     GRANT ALTER ON SOMETHING shared_buffers TO myuser;

If you don't like that maybe ALTER_SYSTEM and SET_VALUE would work,
although mostly we have avoided things like that.

How about MODIFY instead of SET VALUE and CONFIGURE instead of ALTER SYSTEM?

Personally I don't have problem with the use of SETTING. I think the
meaning is pretty plain in context and unlikely to produce any confusion.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com







^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: Granting SET and ALTER SYSTE privileges for GUCs
@ 2022-03-16 20:53  Tom Lane <[email protected]>
  parent: Andrew Dunstan <[email protected]>
  0 siblings, 1 reply; 24+ messages in thread

From: Tom Lane @ 2022-03-16 20:53 UTC (permalink / raw)
  To: Andrew Dunstan <[email protected]>; +Cc: Mark Dilger <[email protected]>; Joshua Brindle <[email protected]>; Robert Haas <[email protected]>; Jeff Davis <[email protected]>; pgsql-hackers; Joe Conway <[email protected]>

Andrew Dunstan <[email protected]> writes:
> On 3/16/22 14:47, Tom Lane wrote:
>> I'm also fairly allergic to the way that this patch has decided to assign
>> multi-word names to privilege types (ie SET VALUE, ALTER SYSTEM).  There
>> is no existing precedent for that, and I think it's going to break
>> client-side code that we don't need to break.  It's not coincidental that
>> this forces weird changes in rules about whitespace in the has_privilege
>> functions, for example; and if you think that isn't going to cause
>> problems I think you are wrong.  Perhaps we could just use "SET" and
>> "ALTER", or "SET" and "SYSTEM"?

> That's going to look weird, ISTM. This is less clear about what it's
> granting.
>      GRANT ALTER ON SOMETHING shared_buffers TO myuser;

True.  I think "GRANT SET" is clear enough, and it fits with the custom of
using the name of the SQL statement that the privilege allows you to
invoke.  (I gather from Mark's comments that Bison gave him problems with
that, but maybe that can be dealt with.)  But I concede that "ALTER" by
itself is pretty vague.

> If you don't like that maybe ALTER_SYSTEM and SET_VALUE would work,
> although mostly we have avoided things like that.
> How about MODIFY instead of SET VALUE and CONFIGURE instead of ALTER SYSTEM?

I thought about ALTER_SYSTEM too.  It's not great but maybe the best we
can do.  Not sure that CONFIGURE is better.

> Personally I don't have problem with the use of SETTING. I think the
> meaning is pretty plain in context and unlikely to produce any confusion.

I'm just unhappy about the disconnect with the documentation.  I wonder
if we could get away with s/configuration parameter/setting/g in the docs.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: Granting SET and ALTER SYSTE privileges for GUCs
@ 2022-03-16 21:09  Andrew Dunstan <[email protected]>
  parent: Tom Lane <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Andrew Dunstan @ 2022-03-16 21:09 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Mark Dilger <[email protected]>; Joshua Brindle <[email protected]>; Robert Haas <[email protected]>; Jeff Davis <[email protected]>; pgsql-hackers; Joe Conway <[email protected]>


On 3/16/22 16:53, Tom Lane wrote:

>> Personally I don't have problem with the use of SETTING. I think the
>> meaning is pretty plain in context and unlikely to produce any confusion.
> I'm just unhappy about the disconnect with the documentation.  I wonder
> if we could get away with s/configuration parameter/setting/g in the docs.
>
> 			


I don't think we need to fix that here though. If you can live with
SETTING for now I will undertake to fix the docs post-feature-freeze if
necessary.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com







^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: Granting SET and ALTER SYSTE privileges for GUCs
@ 2022-03-17 13:24  Joshua Brindle <[email protected]>
  parent: Tom Lane <[email protected]>
  0 siblings, 1 reply; 24+ messages in thread

From: Joshua Brindle @ 2022-03-17 13:24 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Mark Dilger <[email protected]>; Andrew Dunstan <[email protected]>; Robert Haas <[email protected]>; Jeff Davis <[email protected]>; pgsql-hackers; Joe Conway <[email protected]>

<snip>

> I remain of the opinion that this
> patch should not concern itself with that, though.

So you are saying that people can add new object types to PG with DAC
permissions and not concern themselves with MAC capable hooks? Is that
an official PG community stance?






^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: Granting SET and ALTER SYSTE privileges for GUCs
@ 2022-03-17 13:52  Peter Eisentraut <[email protected]>
  parent: Mark Dilger <[email protected]>
  1 sibling, 0 replies; 24+ messages in thread

From: Peter Eisentraut @ 2022-03-17 13:52 UTC (permalink / raw)
  To: Mark Dilger <[email protected]>; Tom Lane <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Joshua Brindle <[email protected]>; Robert Haas <[email protected]>; Jeff Davis <[email protected]>; pgsql-hackers; Joe Conway <[email protected]>

On 16.03.22 19:59, Mark Dilger wrote:
> Informally, we often use "GUC" on this list, but that isn't used formally, leaving "configuration parameter" and "setting" as the two obvious choices.  I preferred "configuration parameter" originally and was argued out of it.  My take on "setting" was also that it more naturally refers to the choice of setting, not the thing being set, such that "work_mem = 8192" means the configuration parameter "work_mem" has the setting "8192".

"The current setting of the work_mem parameter is 8192."

I think something based on "parameter" is good.  We also use that 
language in the protocol (e.g., ParameterStatus).






^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: Granting SET and ALTER SYSTE privileges for GUCs
@ 2022-03-17 13:54  Peter Eisentraut <[email protected]>
  parent: Tom Lane <[email protected]>
  3 siblings, 0 replies; 24+ messages in thread

From: Peter Eisentraut @ 2022-03-17 13:54 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; Andrew Dunstan <[email protected]>; +Cc: Mark Dilger <[email protected]>; Joshua Brindle <[email protected]>; Robert Haas <[email protected]>; Jeff Davis <[email protected]>; pgsql-hackers; Joe Conway <[email protected]>

On 16.03.22 19:47, Tom Lane wrote:
> I'm also fairly allergic to the way that this patch has decided to assign
> multi-word names to privilege types (ie SET VALUE, ALTER SYSTEM).  There
> is no existing precedent for that, and I think it's going to break
> client-side code that we don't need to break.  It's not coincidental that
> this forces weird changes in rules about whitespace in the has_privilege
> functions, for example; and if you think that isn't going to cause
> problems I think you are wrong.  Perhaps we could just use "SET" and
> "ALTER", or "SET" and "SYSTEM"?

I think Oracle and MS SQL Server have many multi-word privilege names. 
So users are quite used to that.  And if we want to add more complex 
privileges, we might run out of sensible single words eventually.  So I 
would not exclude this approach.






^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: Granting SET and ALTER SYSTE privileges for GUCs
@ 2022-03-17 16:04  Robert Haas <[email protected]>
  parent: Joshua Brindle <[email protected]>
  0 siblings, 2 replies; 24+ messages in thread

From: Robert Haas @ 2022-03-17 16:04 UTC (permalink / raw)
  To: Joshua Brindle <[email protected]>; +Cc: Tom Lane <[email protected]>; Mark Dilger <[email protected]>; Andrew Dunstan <[email protected]>; Jeff Davis <[email protected]>; pgsql-hackers; Joe Conway <[email protected]>

On Thu, Mar 17, 2022 at 9:25 AM Joshua Brindle
<[email protected]> wrote:
> <snip>
>
> > I remain of the opinion that this
> > patch should not concern itself with that, though.
>
> So you are saying that people can add new object types to PG with DAC
> permissions and not concern themselves with MAC capable hooks? Is that
> an official PG community stance?

I don't know that the community has an official position on that
topic, but I do not think it's reasonable to expect everyone who
tinkers with MAC permissions to try to make a corresponding equivalent
for DAC. The number of people using PostgreSQL with DAC is relatively
small, and the topic is extremely complicated, and a lot of hackers
don't really understand it well enough to be sure that whatever they
might do is right. I think it's reasonable to expect people who
understand DAC and care about it to put some energy into the topic,
and not just in terms of telling other people how they have to write
their patches.

I *don't* think it's appropriate for a patch that touches MAC to
deliberately sabotage the existing support we have for DAC or to just
ignore it where the right thing to do is obvious. But maintaining a
million lines of code is a lot of work, and I can't think of any
reason why the burden of maintaining relatively little-used features
should fall entirely on people who don't care about them.

-- 
Robert Haas
EDB: http://www.enterprisedb.com






^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: Granting SET and ALTER SYSTE privileges for GUCs
@ 2022-03-17 16:18  Robert Haas <[email protected]>
  parent: Tom Lane <[email protected]>
  3 siblings, 0 replies; 24+ messages in thread

From: Robert Haas @ 2022-03-17 16:18 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Mark Dilger <[email protected]>; Joshua Brindle <[email protected]>; Jeff Davis <[email protected]>; pgsql-hackers; Joe Conway <[email protected]>

On Wed, Mar 16, 2022 at 2:47 PM Tom Lane <[email protected]> wrote:
> Stepping back a bit ... do we really want to institutionalize the
> term "setting" for GUC variables?  I realize that the view pg_settings
> exists, but the documentation generally prefers the term "configuration
> parameters".  Where config.sgml uses "setting" as a noun, it's usually
> talking about a specific concrete value for a parameter, and you can
> argue that the view's name comports with that meaning.  But you can't
> GRANT a parameter's current value.

I agree that the lack of a good user-friendly term for GUCs is a real
problem. Here at EDB I've observed even relatively non-technical
people using that term, which appears nowhere in the documentation and
is utterly unintelligible to a typical end-user. Somebody gets on the
phone and tells the customer that they need to set a GUC and the
customer is like "what's a guck?" except that they probably don't
actually ask that question but are just confused and fail to
understand that a postgresql.conf change is being proposed. I hate it.
It sucks.

I have sort of been trying to promote the use of the word "setting"
and use it in my own writing, especially to end-users. That is
definitely more intelligible to random users, but it's admittedly also
awkward. "Set a setting" just sounds redundant. But "set a
configuration variable" sounds wordy, so I don't know.

-- 
Robert Haas
EDB: http://www.enterprisedb.com






^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: Granting SET and ALTER SYSTE privileges for GUCs
@ 2022-03-17 16:19  Mark Dilger <[email protected]>
  parent: Robert Haas <[email protected]>
  1 sibling, 0 replies; 24+ messages in thread

From: Mark Dilger @ 2022-03-17 16:19 UTC (permalink / raw)
  To: Robert Haas <[email protected]>; Joshua Brindle <[email protected]>; +Cc: Tom Lane <[email protected]>; Andrew Dunstan <[email protected]>; Jeff Davis <[email protected]>; pgsql-hackers; Joe Conway <[email protected]>



> On Mar 17, 2022, at 9:04 AM, Robert Haas <[email protected]> wrote:
> 
> not just in terms of telling other people how they have to write
> their patches.

...

>  the burden of maintaining relatively little-used features
> should fall entirely on people who don't care about them.

Joshua helped test the DAC portion of this patch, and answered a number of my questions on the topic, including in-person back in December.  I take your point, Robert, on the general principle, but the archives should reflect that Joshua did contribute to this specific patch.

Joshua, should we drop the DAC portion for postgres 15 and revisit the issue for postgres 16?  I think it's getting late in the development cycle to attempt what Tom described upthread, and I'd hate to see the rest of this patch punted.

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company









^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: Granting SET and ALTER SYSTE privileges for GUCs
@ 2022-03-17 16:29  Joshua Brindle <[email protected]>
  parent: Robert Haas <[email protected]>
  1 sibling, 2 replies; 24+ messages in thread

From: Joshua Brindle @ 2022-03-17 16:29 UTC (permalink / raw)
  To: Robert Haas <[email protected]>; +Cc: Tom Lane <[email protected]>; Mark Dilger <[email protected]>; Andrew Dunstan <[email protected]>; Jeff Davis <[email protected]>; pgsql-hackers; Joe Conway <[email protected]>

On Thu, Mar 17, 2022 at 12:04 PM Robert Haas <[email protected]> wrote:
>
> On Thu, Mar 17, 2022 at 9:25 AM Joshua Brindle
> <[email protected]> wrote:
> > <snip>
> >
> > > I remain of the opinion that this
> > > patch should not concern itself with that, though.
> >
> > So you are saying that people can add new object types to PG with DAC
> > permissions and not concern themselves with MAC capable hooks? Is that
> > an official PG community stance?
>
> I don't know that the community has an official position on that
> topic, but I do not think it's reasonable to expect everyone who
> tinkers with MAC permissions to try to make a corresponding equivalent
> for DAC. The number of people using PostgreSQL with DAC is relatively
> small, and the topic is extremely complicated, and a lot of hackers
> don't really understand it well enough to be sure that whatever they
> might do is right. I think it's reasonable to expect people who
> understand DAC and care about it to put some energy into the topic,
> and not just in terms of telling other people how they have to write
> their patches.
>
> I *don't* think it's appropriate for a patch that touches MAC to
> deliberately sabotage the existing support we have for DAC or to just
> ignore it where the right thing to do is obvious. But maintaining a
> million lines of code is a lot of work, and I can't think of any
> reason why the burden of maintaining relatively little-used features
> should fall entirely on people who don't care about them.

I agree with this view, outside of the mixup between MAC and DAC (DAC
is in-core, MAC is via hooks)

So there should be some committers or contributors that keep an eye
out and make sure new objects have appropriate hooks, and since an Oid
based hook for GUCs is inappropriate, I hope this patch can be rolled
into the contribution from Mark.

The attached patch adds a set of hooks for strings, and changes the
GUC patch from Mark to use it, I tested with this code:

void set_user_object_access_str(ObjectAccessType access, Oid classId,
const char *objName, int subId, void *arg)
{
if (next_object_access_hook_str)
{
(*next_object_access_hook_str)(access, classId, objName, subId, arg);
}
switch (access)
{
case OAT_POST_ALTER:
if (classId == SettingAclRelationId)
{
Oid userid = GetUserId();
bool is_superuser = superuser_arg(userid);
char *username = GETUSERNAMEFROMID(GetUserId());
const char *setting = "setting value";
const char *altering = "altering system";
const char *unknown = "unknown";
const char *action;
if (subId && ACL_SET_VALUE)
action = setting;
else if (subId && ACL_ALTER_SYSTEM)
action = altering;
else
action = unknown;

elog(WARNING, "%s is %s %s (%d)", username, action, objName, is_superuser);
if (strcmp(objName, "log_statement") == 0)
{
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("Setting %s blocked", objName)));
}
}
default:
break;
}
}

static void
set_user_object_access (ObjectAccessType access, Oid classId, Oid
objectId, int subId, void *arg)
{
if (next_object_access_hook)
{
(*next_object_access_hook)(access, classId, objectId, subId, arg);
}
switch (access)
{
case OAT_FUNCTION_EXECUTE:
{
/* Update the `set_config` Oid cache if necessary. */
set_user_cache_proc(InvalidOid);

/* Now see if this function is blocked */
set_user_block_set_config(objectId);
break;
}

/* fallthrough */

case OAT_POST_CREATE:
{
if (classId == ProcedureRelationId)
{
set_user_cache_proc(objectId);
}
break;
}
default:
break;
}
}


Attachments:

  [application/octet-stream] 0001-Add-String-object-access-hooks.patch (8.9K, ../../CAGB+Vh58bbGZy1Pf4FHctqw-eSjQM_1wv66wX_WzJ1VaF7brEQ@mail.gmail.com/2-0001-Add-String-object-access-hooks.patch)
  download | inline diff:
From 737d105298bbf0632700869b8b30601516ffd163 Mon Sep 17 00:00:00 2001
From: Joshua Brindle <[email protected]>
Date: Thu, 17 Mar 2022 09:24:57 -0700
Subject: [PATCH] Add String object access hooks

The first user of these will be the GUC access controls

Signed-off-by: Joshua Brindle <[email protected]>
---
 src/backend/catalog/objectaccess.c | 128 +++++++++++++++++++++++++++++
 src/backend/utils/misc/guc.c       |   4 +-
 src/include/catalog/objectaccess.h |  70 +++++++++++++++-
 3 files changed, 199 insertions(+), 3 deletions(-)

diff --git a/src/backend/catalog/objectaccess.c b/src/backend/catalog/objectaccess.c
index 549fac4539..72ad6e9a90 100644
--- a/src/backend/catalog/objectaccess.c
+++ b/src/backend/catalog/objectaccess.c
@@ -20,6 +20,8 @@
  * and logging plugins.
  */
 object_access_hook_type object_access_hook = NULL;
+object_access_hook_type_str object_access_hook_str = NULL;
+
 
 /*
  * RunObjectPostCreateHook
@@ -143,3 +145,129 @@ RunFunctionExecuteHook(Oid objectId)
 						   ProcedureRelationId, objectId, 0,
 						   NULL);
 }
+
+/* String versions */
+
+
+/*
+ * RunObjectPostCreateHook
+ *
+ * It is entrypoint of OAT_POST_CREATE event
+ */
+void
+RunObjectPostCreateHookStr(Oid classId, const char *objectName, int subId,
+						bool is_internal)
+{
+	ObjectAccessPostCreate pc_arg;
+
+	/* caller should check, but just in case... */
+	Assert(object_access_hook_str != NULL);
+
+	memset(&pc_arg, 0, sizeof(ObjectAccessPostCreate));
+	pc_arg.is_internal = is_internal;
+
+	(*object_access_hook_str) (OAT_POST_CREATE,
+						   classId, objectName, subId,
+						   (void *) &pc_arg);
+}
+
+/*
+ * RunObjectDropHook
+ *
+ * It is entrypoint of OAT_DROP event
+ */
+void
+RunObjectDropHookStr(Oid classId, const char *objectName, int subId,
+				  int dropflags)
+{
+	ObjectAccessDrop drop_arg;
+
+	/* caller should check, but just in case... */
+	Assert(object_access_hook_str != NULL);
+
+	memset(&drop_arg, 0, sizeof(ObjectAccessDrop));
+	drop_arg.dropflags = dropflags;
+
+	(*object_access_hook_str) (OAT_DROP,
+						   classId, objectName, subId,
+						   (void *) &drop_arg);
+}
+
+/*
+ * RunObjectTruncateHook
+ *
+ * It is the entrypoint of OAT_TRUNCATE event
+ */
+void
+RunObjectTruncateHookStr(const char *objectName)
+{
+	/* caller should check, but just in case... */
+	Assert(object_access_hook_str != NULL);
+
+	(*object_access_hook_str) (OAT_TRUNCATE,
+						   RelationRelationId, objectName, 0,
+						   NULL);
+}
+
+/*
+ * RunObjectPostAlterHook
+ *
+ * It is entrypoint of OAT_POST_ALTER event
+ */
+void
+RunObjectPostAlterHookStr(Oid classId, const char *objectName, int subId,
+					   Oid auxiliaryId, bool is_internal)
+{
+	ObjectAccessPostAlter pa_arg;
+
+	/* caller should check, but just in case... */
+	Assert(object_access_hook_str != NULL);
+
+	memset(&pa_arg, 0, sizeof(ObjectAccessPostAlter));
+	pa_arg.auxiliary_id = auxiliaryId;
+	pa_arg.is_internal = is_internal;
+
+	(*object_access_hook_str) (OAT_POST_ALTER,
+						   classId, objectName, subId,
+						   (void *) &pa_arg);
+}
+
+/*
+ * RunNamespaceSearchHook
+ *
+ * It is entrypoint of OAT_NAMESPACE_SEARCH event
+ */
+bool
+RunNamespaceSearchHookStr(const char *objectName, bool ereport_on_violation)
+{
+	ObjectAccessNamespaceSearch ns_arg;
+
+	/* caller should check, but just in case... */
+	Assert(object_access_hook_str != NULL);
+
+	memset(&ns_arg, 0, sizeof(ObjectAccessNamespaceSearch));
+	ns_arg.ereport_on_violation = ereport_on_violation;
+	ns_arg.result = true;
+
+	(*object_access_hook_str) (OAT_NAMESPACE_SEARCH,
+						   NamespaceRelationId, objectName, 0,
+						   (void *) &ns_arg);
+
+	return ns_arg.result;
+}
+
+/*
+ * RunFunctionExecuteHook
+ *
+ * It is entrypoint of OAT_FUNCTION_EXECUTE event
+ */
+void
+RunFunctionExecuteHookStr(const char *objectName)
+{
+	/* caller should check, but just in case... */
+	Assert(object_access_hook_str != NULL);
+
+	(*object_access_hook_str) (OAT_FUNCTION_EXECUTE,
+						   ProcedureRelationId, objectName, 0,
+						   NULL);
+}
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 683031db9a..8edee70394 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -8853,7 +8853,7 @@ AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt)
 	 * transactional.  If the hook aborts our transaction, it will be cleaner
 	 * to do so before we touch any files.
 	 */
-	InvokeObjectPostAlterHookArg(SettingAclRelationId, settingId,
+	InvokeObjectPostAlterHookArgStr(SettingAclRelationId, name,
 								 ACL_ALTER_SYSTEM, altersysstmt->setstmt->kind,
 								 false);
 
@@ -9057,7 +9057,7 @@ ExecSetVariableStmt(VariableSetStmt *stmt, bool isTopLevel)
 	 * abuse the notion of subId to pass the kind of alteration (set vs. alter
 	 * system), and auxiliaryId to pass the VariableSetKind.
 	 */
-	InvokeObjectPostAlterHookArg(SettingAclRelationId, settingId,
+	InvokeObjectPostAlterHookArgStr(SettingAclRelationId, stmt->name,
 								 ACL_SET_VALUE, stmt->kind, false);
 }
 
diff --git a/src/include/catalog/objectaccess.h b/src/include/catalog/objectaccess.h
index 508dfd0a6b..4c1211600e 100644
--- a/src/include/catalog/objectaccess.h
+++ b/src/include/catalog/objectaccess.h
@@ -121,15 +121,23 @@ typedef struct
 	bool		result;
 } ObjectAccessNamespaceSearch;
 
-/* Plugin provides a hook function matching this signature. */
+/* Plugin provides a hook function matching one or both of these signatures. */
 typedef void (*object_access_hook_type) (ObjectAccessType access,
 										 Oid classId,
 										 Oid objectId,
 										 int subId,
 										 void *arg);
 
+typedef void (*object_access_hook_type_str) (ObjectAccessType access,
+										 Oid classId,
+										 const char *objectStr,
+										 int subId,
+										 void *arg);
+
 /* Plugin sets this variable to a suitable hook function. */
 extern PGDLLIMPORT object_access_hook_type object_access_hook;
+extern PGDLLIMPORT object_access_hook_type_str object_access_hook_str;
+
 
 /* Core code uses these functions to call the hook (see macros below). */
 extern void RunObjectPostCreateHook(Oid classId, Oid objectId, int subId,
@@ -142,6 +150,18 @@ extern void RunObjectPostAlterHook(Oid classId, Oid objectId, int subId,
 extern bool RunNamespaceSearchHook(Oid objectId, bool ereport_on_violation);
 extern void RunFunctionExecuteHook(Oid objectId);
 
+/* String versions */
+extern void RunObjectPostCreateHookStr(Oid classId, const char *objectStr, int subId,
+									bool is_internal);
+extern void RunObjectDropHookStr(Oid classId, const char *objectStr, int subId,
+							  int dropflags);
+extern void RunObjectTruncateHookStr(const char *objectStr);
+extern void RunObjectPostAlterHookStr(Oid classId, const char *objectStr, int subId,
+								   Oid auxiliaryId, bool is_internal);
+extern bool RunNamespaceSearchHookStr(const char *objectStr, bool ereport_on_violation);
+extern void RunFunctionExecuteHookStr(const char *objectStr);
+
+
 /*
  * The following macros are wrappers around the functions above; these should
  * normally be used to invoke the hook in lieu of calling the above functions
@@ -194,4 +214,52 @@ extern void RunFunctionExecuteHook(Oid objectId);
 			RunFunctionExecuteHook(objectId);	\
 	} while(0)
 
+
+#define InvokeObjectPostCreateHookStr(classId,objectName,subId)			\
+	InvokeObjectPostCreateHookArgStr((classId),(objectName),(subId),false)
+#define InvokeObjectPostCreateHookArgStr(classId,objectName,subId,is_internal) \
+	do {															\
+		if (object_access_hook_str)										\
+			RunObjectPostCreateHookStr((classId),(objectName),(subId),	\
+									(is_internal));					\
+	} while(0)
+
+#define InvokeObjectDropHookStr(classId,objectId,subId)				\
+	InvokeObjectDropHookArgStr((classId),(objectName),(subId),0)
+#define InvokeObjectDropHookArgStr(classId,objectName,subId,dropflags)	\
+	do {															\
+		if (object_access_hook_str)										\
+			RunObjectDropHookStr((classId),(objectName),(subId),			\
+							  (dropflags));							\
+	} while(0)
+
+#define InvokeObjectTruncateHookStr(objectName)							\
+	do {															\
+		if (object_access_hook_str)										\
+			RunObjectTruncateHookStr(objectName);						\
+	} while(0)
+
+#define InvokeObjectPostAlterHookStr(className,objectName,subId)			\
+	InvokeObjectPostAlterHookArgStr((classId),(objectName),(subId),		\
+								 InvalidOid,false)
+#define InvokeObjectPostAlterHookArgStr(classId,objectName,subId,		\
+									 auxiliaryId,is_internal)		\
+	do {															\
+		if (object_access_hook_str)										\
+			RunObjectPostAlterHookStr((classId),(objectName),(subId),	\
+								   (auxiliaryId),(is_internal));	\
+	} while(0)
+
+#define InvokeNamespaceSearchHookStr(objectName, ereport_on_violation)	\
+	(!object_access_hook_str										\
+	 ? true															\
+	 : RunNamespaceSearchHookStr((objectName), (ereport_on_violation)))
+
+#define InvokeFunctionExecuteHookStr(objectName)		\
+	do {										\
+		if (object_access_hook_str)					\
+			RunFunctionExecuteHookStr(objectName);	\
+	} while(0)
+
+
 #endif							/* OBJECTACCESS_H */
-- 
2.35.1



^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: Granting SET and ALTER SYSTE privileges for GUCs
@ 2022-03-17 16:34  Robert Haas <[email protected]>
  parent: Joshua Brindle <[email protected]>
  1 sibling, 0 replies; 24+ messages in thread

From: Robert Haas @ 2022-03-17 16:34 UTC (permalink / raw)
  To: Joshua Brindle <[email protected]>; +Cc: Tom Lane <[email protected]>; Mark Dilger <[email protected]>; Andrew Dunstan <[email protected]>; Jeff Davis <[email protected]>; pgsql-hackers; Joe Conway <[email protected]>

On Thu, Mar 17, 2022 at 12:30 PM Joshua Brindle
<[email protected]> wrote:
> I agree with this view, outside of the mixup between MAC and DAC (DAC
> is in-core, MAC is via hooks)

An excellent point! Exactly why we need expert-level help with this stuff! :-)

> So there should be some committers or contributors that keep an eye
> out and make sure new objects have appropriate hooks, and since an Oid
> based hook for GUCs is inappropriate, I hope this patch can be rolled
> into the contribution from Mark.

I'm not going to take a position on this patch right this minute, but
I appreciate you providing it.

-- 
Robert Haas
EDB: http://www.enterprisedb.com






^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: Granting SET and ALTER SYSTE privileges for GUCs
@ 2022-03-17 16:36  Mark Dilger <[email protected]>
  parent: Joshua Brindle <[email protected]>
  1 sibling, 1 reply; 24+ messages in thread

From: Mark Dilger @ 2022-03-17 16:36 UTC (permalink / raw)
  To: Joshua Brindle <[email protected]>; +Cc: Robert Haas <[email protected]>; Tom Lane <[email protected]>; Andrew Dunstan <[email protected]>; Jeff Davis <[email protected]>; pgsql-hackers; Joe Conway <[email protected]>



> On Mar 17, 2022, at 9:29 AM, Joshua Brindle <[email protected]> wrote:
> 
> I hope this patch can be rolled
> into the contribution from Mark.

Working on it.... Thanks for the patch!

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company









^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: Granting SET and ALTER SYSTE privileges for GUCs
@ 2022-03-17 18:19  Joshua Brindle <[email protected]>
  parent: Mark Dilger <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Joshua Brindle @ 2022-03-17 18:19 UTC (permalink / raw)
  To: Mark Dilger <[email protected]>; +Cc: Robert Haas <[email protected]>; Tom Lane <[email protected]>; Andrew Dunstan <[email protected]>; Jeff Davis <[email protected]>; pgsql-hackers; Joe Conway <[email protected]>

On Thu, Mar 17, 2022 at 12:36 PM Mark Dilger
<[email protected]> wrote:
> > On Mar 17, 2022, at 9:29 AM, Joshua Brindle <[email protected]> wrote:
> >
> > I hope this patch can be rolled
> > into the contribution from Mark.
>
> Working on it.... Thanks for the patch!

Great, thanks.

I missed one objectId reference (InvokeObjectDropHookStr), fixed
version attached.


Attachments:

  [application/octet-stream] v2-0001-Add-String-object-access-hooks.patch (8.9K, ../../CAGB+Vh7UpK_RDZtQngKRfBxqHh+N7Ok4KxnZ4JiY3KQd3FQFbA@mail.gmail.com/2-v2-0001-Add-String-object-access-hooks.patch)
  download | inline diff:
From dd0ed26c6eb8ff454ce5faf0cc743b3a0d287c3c Mon Sep 17 00:00:00 2001
From: Joshua Brindle <[email protected]>
Date: Thu, 17 Mar 2022 09:24:57 -0700
Subject: [PATCH] Add String object access hooks

The first user of these will be the GUC access controls

Signed-off-by: Joshua Brindle <[email protected]>
---
 src/backend/catalog/objectaccess.c | 128 +++++++++++++++++++++++++++++
 src/backend/utils/misc/guc.c       |   4 +-
 src/include/catalog/objectaccess.h |  70 +++++++++++++++-
 3 files changed, 199 insertions(+), 3 deletions(-)

diff --git a/src/backend/catalog/objectaccess.c b/src/backend/catalog/objectaccess.c
index 549fac4539..72ad6e9a90 100644
--- a/src/backend/catalog/objectaccess.c
+++ b/src/backend/catalog/objectaccess.c
@@ -20,6 +20,8 @@
  * and logging plugins.
  */
 object_access_hook_type object_access_hook = NULL;
+object_access_hook_type_str object_access_hook_str = NULL;
+
 
 /*
  * RunObjectPostCreateHook
@@ -143,3 +145,129 @@ RunFunctionExecuteHook(Oid objectId)
 						   ProcedureRelationId, objectId, 0,
 						   NULL);
 }
+
+/* String versions */
+
+
+/*
+ * RunObjectPostCreateHook
+ *
+ * It is entrypoint of OAT_POST_CREATE event
+ */
+void
+RunObjectPostCreateHookStr(Oid classId, const char *objectName, int subId,
+						bool is_internal)
+{
+	ObjectAccessPostCreate pc_arg;
+
+	/* caller should check, but just in case... */
+	Assert(object_access_hook_str != NULL);
+
+	memset(&pc_arg, 0, sizeof(ObjectAccessPostCreate));
+	pc_arg.is_internal = is_internal;
+
+	(*object_access_hook_str) (OAT_POST_CREATE,
+						   classId, objectName, subId,
+						   (void *) &pc_arg);
+}
+
+/*
+ * RunObjectDropHook
+ *
+ * It is entrypoint of OAT_DROP event
+ */
+void
+RunObjectDropHookStr(Oid classId, const char *objectName, int subId,
+				  int dropflags)
+{
+	ObjectAccessDrop drop_arg;
+
+	/* caller should check, but just in case... */
+	Assert(object_access_hook_str != NULL);
+
+	memset(&drop_arg, 0, sizeof(ObjectAccessDrop));
+	drop_arg.dropflags = dropflags;
+
+	(*object_access_hook_str) (OAT_DROP,
+						   classId, objectName, subId,
+						   (void *) &drop_arg);
+}
+
+/*
+ * RunObjectTruncateHook
+ *
+ * It is the entrypoint of OAT_TRUNCATE event
+ */
+void
+RunObjectTruncateHookStr(const char *objectName)
+{
+	/* caller should check, but just in case... */
+	Assert(object_access_hook_str != NULL);
+
+	(*object_access_hook_str) (OAT_TRUNCATE,
+						   RelationRelationId, objectName, 0,
+						   NULL);
+}
+
+/*
+ * RunObjectPostAlterHook
+ *
+ * It is entrypoint of OAT_POST_ALTER event
+ */
+void
+RunObjectPostAlterHookStr(Oid classId, const char *objectName, int subId,
+					   Oid auxiliaryId, bool is_internal)
+{
+	ObjectAccessPostAlter pa_arg;
+
+	/* caller should check, but just in case... */
+	Assert(object_access_hook_str != NULL);
+
+	memset(&pa_arg, 0, sizeof(ObjectAccessPostAlter));
+	pa_arg.auxiliary_id = auxiliaryId;
+	pa_arg.is_internal = is_internal;
+
+	(*object_access_hook_str) (OAT_POST_ALTER,
+						   classId, objectName, subId,
+						   (void *) &pa_arg);
+}
+
+/*
+ * RunNamespaceSearchHook
+ *
+ * It is entrypoint of OAT_NAMESPACE_SEARCH event
+ */
+bool
+RunNamespaceSearchHookStr(const char *objectName, bool ereport_on_violation)
+{
+	ObjectAccessNamespaceSearch ns_arg;
+
+	/* caller should check, but just in case... */
+	Assert(object_access_hook_str != NULL);
+
+	memset(&ns_arg, 0, sizeof(ObjectAccessNamespaceSearch));
+	ns_arg.ereport_on_violation = ereport_on_violation;
+	ns_arg.result = true;
+
+	(*object_access_hook_str) (OAT_NAMESPACE_SEARCH,
+						   NamespaceRelationId, objectName, 0,
+						   (void *) &ns_arg);
+
+	return ns_arg.result;
+}
+
+/*
+ * RunFunctionExecuteHook
+ *
+ * It is entrypoint of OAT_FUNCTION_EXECUTE event
+ */
+void
+RunFunctionExecuteHookStr(const char *objectName)
+{
+	/* caller should check, but just in case... */
+	Assert(object_access_hook_str != NULL);
+
+	(*object_access_hook_str) (OAT_FUNCTION_EXECUTE,
+						   ProcedureRelationId, objectName, 0,
+						   NULL);
+}
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 683031db9a..8edee70394 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -8853,7 +8853,7 @@ AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt)
 	 * transactional.  If the hook aborts our transaction, it will be cleaner
 	 * to do so before we touch any files.
 	 */
-	InvokeObjectPostAlterHookArg(SettingAclRelationId, settingId,
+	InvokeObjectPostAlterHookArgStr(SettingAclRelationId, name,
 								 ACL_ALTER_SYSTEM, altersysstmt->setstmt->kind,
 								 false);
 
@@ -9057,7 +9057,7 @@ ExecSetVariableStmt(VariableSetStmt *stmt, bool isTopLevel)
 	 * abuse the notion of subId to pass the kind of alteration (set vs. alter
 	 * system), and auxiliaryId to pass the VariableSetKind.
 	 */
-	InvokeObjectPostAlterHookArg(SettingAclRelationId, settingId,
+	InvokeObjectPostAlterHookArgStr(SettingAclRelationId, stmt->name,
 								 ACL_SET_VALUE, stmt->kind, false);
 }
 
diff --git a/src/include/catalog/objectaccess.h b/src/include/catalog/objectaccess.h
index 508dfd0a6b..4d54ae2a7d 100644
--- a/src/include/catalog/objectaccess.h
+++ b/src/include/catalog/objectaccess.h
@@ -121,15 +121,23 @@ typedef struct
 	bool		result;
 } ObjectAccessNamespaceSearch;
 
-/* Plugin provides a hook function matching this signature. */
+/* Plugin provides a hook function matching one or both of these signatures. */
 typedef void (*object_access_hook_type) (ObjectAccessType access,
 										 Oid classId,
 										 Oid objectId,
 										 int subId,
 										 void *arg);
 
+typedef void (*object_access_hook_type_str) (ObjectAccessType access,
+										 Oid classId,
+										 const char *objectStr,
+										 int subId,
+										 void *arg);
+
 /* Plugin sets this variable to a suitable hook function. */
 extern PGDLLIMPORT object_access_hook_type object_access_hook;
+extern PGDLLIMPORT object_access_hook_type_str object_access_hook_str;
+
 
 /* Core code uses these functions to call the hook (see macros below). */
 extern void RunObjectPostCreateHook(Oid classId, Oid objectId, int subId,
@@ -142,6 +150,18 @@ extern void RunObjectPostAlterHook(Oid classId, Oid objectId, int subId,
 extern bool RunNamespaceSearchHook(Oid objectId, bool ereport_on_violation);
 extern void RunFunctionExecuteHook(Oid objectId);
 
+/* String versions */
+extern void RunObjectPostCreateHookStr(Oid classId, const char *objectStr, int subId,
+									bool is_internal);
+extern void RunObjectDropHookStr(Oid classId, const char *objectStr, int subId,
+							  int dropflags);
+extern void RunObjectTruncateHookStr(const char *objectStr);
+extern void RunObjectPostAlterHookStr(Oid classId, const char *objectStr, int subId,
+								   Oid auxiliaryId, bool is_internal);
+extern bool RunNamespaceSearchHookStr(const char *objectStr, bool ereport_on_violation);
+extern void RunFunctionExecuteHookStr(const char *objectStr);
+
+
 /*
  * The following macros are wrappers around the functions above; these should
  * normally be used to invoke the hook in lieu of calling the above functions
@@ -194,4 +214,52 @@ extern void RunFunctionExecuteHook(Oid objectId);
 			RunFunctionExecuteHook(objectId);	\
 	} while(0)
 
+
+#define InvokeObjectPostCreateHookStr(classId,objectName,subId)			\
+	InvokeObjectPostCreateHookArgStr((classId),(objectName),(subId),false)
+#define InvokeObjectPostCreateHookArgStr(classId,objectName,subId,is_internal) \
+	do {															\
+		if (object_access_hook_str)										\
+			RunObjectPostCreateHookStr((classId),(objectName),(subId),	\
+									(is_internal));					\
+	} while(0)
+
+#define InvokeObjectDropHookStr(classId,objectName,subId)				\
+	InvokeObjectDropHookArgStr((classId),(objectName),(subId),0)
+#define InvokeObjectDropHookArgStr(classId,objectName,subId,dropflags)	\
+	do {															\
+		if (object_access_hook_str)										\
+			RunObjectDropHookStr((classId),(objectName),(subId),			\
+							  (dropflags));							\
+	} while(0)
+
+#define InvokeObjectTruncateHookStr(objectName)							\
+	do {															\
+		if (object_access_hook_str)										\
+			RunObjectTruncateHookStr(objectName);						\
+	} while(0)
+
+#define InvokeObjectPostAlterHookStr(className,objectName,subId)			\
+	InvokeObjectPostAlterHookArgStr((classId),(objectName),(subId),		\
+								 InvalidOid,false)
+#define InvokeObjectPostAlterHookArgStr(classId,objectName,subId,		\
+									 auxiliaryId,is_internal)		\
+	do {															\
+		if (object_access_hook_str)										\
+			RunObjectPostAlterHookStr((classId),(objectName),(subId),	\
+								   (auxiliaryId),(is_internal));	\
+	} while(0)
+
+#define InvokeNamespaceSearchHookStr(objectName, ereport_on_violation)	\
+	(!object_access_hook_str										\
+	 ? true															\
+	 : RunNamespaceSearchHookStr((objectName), (ereport_on_violation)))
+
+#define InvokeFunctionExecuteHookStr(objectName)		\
+	do {										\
+		if (object_access_hook_str)					\
+			RunFunctionExecuteHookStr(objectName);	\
+	} while(0)
+
+
 #endif							/* OBJECTACCESS_H */
-- 
2.35.1



^ permalink  raw  reply  [nested|flat] 24+ messages in thread


end of thread, other threads:[~2022-03-17 18:19 UTC | newest]

Thread overview: 24+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-02-27 01:34 [PATCH 06/10] cirrus: build docs as a separate task.. Justin Pryzby <[email protected]>
2022-03-06 22:57 Re: Granting SET and ALTER SYSTE privileges for GUCs Tom Lane <[email protected]>
2022-03-06 23:06 ` Re: Granting SET and ALTER SYSTE privileges for GUCs Mark Dilger <[email protected]>
2022-03-06 23:27   ` Re: Granting SET and ALTER SYSTE privileges for GUCs Tom Lane <[email protected]>
2022-03-15 20:59     ` Re: Granting SET and ALTER SYSTE privileges for GUCs Mark Dilger <[email protected]>
2022-03-16 11:43       ` Re: Granting SET and ALTER SYSTE privileges for GUCs Andrew Dunstan <[email protected]>
2022-03-16 18:47       ` Re: Granting SET and ALTER SYSTE privileges for GUCs Tom Lane <[email protected]>
2022-03-16 18:59         ` Re: Granting SET and ALTER SYSTE privileges for GUCs Mark Dilger <[email protected]>
2022-03-16 19:06           ` Re: Granting SET and ALTER SYSTE privileges for GUCs Tom Lane <[email protected]>
2022-03-16 19:36             ` Re: Granting SET and ALTER SYSTE privileges for GUCs Joshua Brindle <[email protected]>
2022-03-16 19:58               ` Re: Granting SET and ALTER SYSTE privileges for GUCs Tom Lane <[email protected]>
2022-03-17 13:24                 ` Re: Granting SET and ALTER SYSTE privileges for GUCs Joshua Brindle <[email protected]>
2022-03-17 16:04                   ` Re: Granting SET and ALTER SYSTE privileges for GUCs Robert Haas <[email protected]>
2022-03-17 16:19                     ` Re: Granting SET and ALTER SYSTE privileges for GUCs Mark Dilger <[email protected]>
2022-03-17 16:29                     ` Re: Granting SET and ALTER SYSTE privileges for GUCs Joshua Brindle <[email protected]>
2022-03-17 16:34                       ` Re: Granting SET and ALTER SYSTE privileges for GUCs Robert Haas <[email protected]>
2022-03-17 16:36                       ` Re: Granting SET and ALTER SYSTE privileges for GUCs Mark Dilger <[email protected]>
2022-03-17 18:19                         ` Re: Granting SET and ALTER SYSTE privileges for GUCs Joshua Brindle <[email protected]>
2022-03-17 13:52           ` Re: Granting SET and ALTER SYSTE privileges for GUCs Peter Eisentraut <[email protected]>
2022-03-16 20:07         ` Re: Granting SET and ALTER SYSTE privileges for GUCs Andrew Dunstan <[email protected]>
2022-03-16 20:53           ` Re: Granting SET and ALTER SYSTE privileges for GUCs Tom Lane <[email protected]>
2022-03-16 21:09             ` Re: Granting SET and ALTER SYSTE privileges for GUCs Andrew Dunstan <[email protected]>
2022-03-17 13:54         ` Re: Granting SET and ALTER SYSTE privileges for GUCs Peter Eisentraut <[email protected]>
2022-03-17 16:18         ` Re: Granting SET and ALTER SYSTE privileges for GUCs Robert Haas <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox