On Tue, 2021-11-02 at 14:26 -0400, Stephen Frost wrote:
> Think you meant 'Stephen' there. ;)
Yes ;-)
> > The approach in the patch looks alright to me, but another one
> > could
> > be to build a SelectStmt when parsing CHECKPOINT. I think that'd
> > simplify the standard_ProcessUtility() changes.
Nathan, if I understand correctly, that would mean no CheckPointStmt at
all. So it would either lack the right command tag, or we would have to
hack it in somewhere. The utility changes in the existing patch are
fairly minor, so I'll stick with that approach unless I'm missing
something.
> For my 2c, at least, I'm not really partial to either approach,
> though
> I'd want to see what error messages end up looking like. Seems like
> we
> might want to exercise a bit more control than we'd be able to if we
> transformed it directly into a SelectStmt (that is, we might add a
> HINT:
> roles with execute rights on pg_checkpoint() can run this command, or
> something; maybe not too tho).
I changed the error message to:
ERROR: permission denied for command CHECKPOINT
HINT: The CHECKPOINT command requires the EXECUTE privilege
on the function pg_checkpoint().
New version attached.
Andres suggested that I also consider a new form of the GRANT clause
that works on the CHECKPOINT command directly. I looked into that
briefly, but in every other case it seems that GRANT works on an object
(like a function). It doesn't feel like grating on a command is the
right solution.
The approach of using a function's ACL to represent the ACL of a
higher-level command (as in this patch) does feel right to me. It feels
like something we might extend to similar situations in the future; and
even if we don't, it seems like a clean solution in isolation.
Regards,
Jeff Davis
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 4b49dff2ffc..3558044221a 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -25487,6 +25487,24 @@ LOG: Grand total: 1651920 bytes in 201 blocks; 622360 free (88 chunks); 1029560
</thead>
<tbody>
+ <row>
+ <entry role="func_table_entry"><para role="func_signature">
+ <indexterm>
+ <primary>pg_checkpoint</primary>
+ </indexterm>
+ <function>pg_checkpoint</function> ()
+ <returnvalue>void</returnvalue>
+ </para>
+ <para>
+ Request an immediate checkpoint. This function implements the <xref
+ linkend="sql-checkpoint"/> command, so the behavior is identical. This
+ function is restricted to superusers by default, but other users can
+ be granted EXECUTE to run the function. If a user has permission to
+ execute this function, they also have permission to issue a
+ <command>CHECKPOINT</command> command.
+ </para></entry>
+ </row>
+
<row>
<entry role="func_table_entry"><para role="func_signature">
<indexterm>
diff --git a/doc/src/sgml/ref/checkpoint.sgml b/doc/src/sgml/ref/checkpoint.sgml
index 2afee6d7b59..7c6bea05fa0 100644
--- a/doc/src/sgml/ref/checkpoint.sgml
+++ b/doc/src/sgml/ref/checkpoint.sgml
@@ -52,7 +52,11 @@ CHECKPOINT
</para>
<para>
- Only superusers can call <command>CHECKPOINT</command>.
+ The <command>CHECKPOINT</command> is equivalent to calling the function
+ <link linkend="functions-admin-backup-table">pg_checkpoint()</link>. By
+ default, only superusers can this function, but if other users are granted
+ privileges on it, they may also call the <command>CHECKPOINT</command>
+ command.
</para>
</refsect1>
diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c
index b98deb72ec6..c9e1df39c11 100644
--- a/src/backend/access/transam/xlogfuncs.c
+++ b/src/backend/access/transam/xlogfuncs.c
@@ -26,6 +26,7 @@
#include "funcapi.h"
#include "miscadmin.h"
#include "pgstat.h"
+#include "postmaster/bgwriter.h"
#include "replication/walreceiver.h"
#include "storage/fd.h"
#include "storage/ipc.h"
@@ -44,6 +45,18 @@
static StringInfo label_file;
static StringInfo tblspc_map_file;
+/*
+ * Implements the CHECKPOINT command. To allow non-superusers to perform the
+ * CHECKPOINT command, grant privileges on this function.
+ */
+Datum
+pg_checkpoint(PG_FUNCTION_ARGS)
+{
+ RequestCheckpoint(CHECKPOINT_IMMEDIATE | CHECKPOINT_WAIT |
+ (RecoveryInProgress() ? 0 : CHECKPOINT_FORCE));
+ PG_RETURN_VOID();
+}
+
/*
* pg_start_backup: set up for taking an on-line backup dump
*
diff --git a/src/backend/catalog/system_functions.sql b/src/backend/catalog/system_functions.sql
index 54c93b16c4c..4437eb3010b 100644
--- a/src/backend/catalog/system_functions.sql
+++ b/src/backend/catalog/system_functions.sql
@@ -603,6 +603,8 @@ AS 'unicode_is_normalized';
-- available to superuser / cluster owner, if they choose.
--
+REVOKE EXECUTE ON FUNCTION pg_checkpoint() FROM public;
+
REVOKE EXECUTE ON FUNCTION pg_start_backup(text, boolean, boolean) FROM public;
REVOKE EXECUTE ON FUNCTION pg_stop_backup() FROM public;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index bf085aa93b2..dcd822075f5 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -24,6 +24,7 @@
#include "catalog/catalog.h"
#include "catalog/index.h"
#include "catalog/namespace.h"
+#include "catalog/objectaccess.h"
#include "catalog/pg_inherits.h"
#include "catalog/toasting.h"
#include "commands/alter.h"
@@ -67,6 +68,7 @@
#include "tcop/pquery.h"
#include "tcop/utility.h"
#include "utils/acl.h"
+#include "utils/fmgroids.h"
#include "utils/guc.h"
#include "utils/lsyscache.h"
#include "utils/rel.h"
@@ -939,13 +941,31 @@ standard_ProcessUtility(PlannedStmt *pstmt,
break;
case T_CheckPointStmt:
- if (!superuser())
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- errmsg("must be superuser to do CHECKPOINT")));
+ {
+ /*
+ * Invoke pg_checkpoint(). Implementing the CHECKPOINT command
+ * with a function allows administrators to grant privileges
+ * on the CHECKPOINT command by granting privileges on the
+ * pg_checkpoint() function. It also calls the function
+ * execute hook, if present.
+ */
+ AclResult aclresult;
+ FmgrInfo flinfo;
+
+ aclresult = pg_proc_aclcheck(F_PG_CHECKPOINT, GetUserId(),
+ ACL_EXECUTE);
+ if (aclresult != ACLCHECK_OK)
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("permission denied for command CHECKPOINT"),
+ errhint("The CHECKPOINT command requires the EXECUTE privilege on the function pg_checkpoint().")));
+
+ InvokeFunctionExecuteHook(F_PG_CHECKPOINT);
- RequestCheckpoint(CHECKPOINT_IMMEDIATE | CHECKPOINT_WAIT |
- (RecoveryInProgress() ? 0 : CHECKPOINT_FORCE));
+ fmgr_info(F_PG_CHECKPOINT, &flinfo);
+
+ (void) FunctionCall0Coll(&flinfo, InvalidOid);
+ }
break;
case T_ReindexStmt:
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index 9faf017457a..35c399e77dd 100644
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -53,6 +53,6 @@
*/
/* yyyymmddN */
-#define CATALOG_VERSION_NO 202110272
+#define CATALOG_VERSION_NO 202111021
#endif
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index d068d6532ec..538efbc3642 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -6258,6 +6258,9 @@
proname => 'pg_terminate_backend', provolatile => 'v', prorettype => 'bool',
proargtypes => 'int4 int8', proargnames => '{pid,timeout}',
prosrc => 'pg_terminate_backend' },
+{ oid => '2137', descr => 'request a checkpoint',
+ proname => 'pg_checkpoint', provolatile => 'v', proparallel => 'r',
+ prorettype => 'void', proargtypes => '', prosrc => 'pg_checkpoint' },
{ oid => '2172', descr => 'prepare for taking an online backup',
proname => 'pg_start_backup', provolatile => 'v', proparallel => 'r',
prorettype => 'pg_lsn', proargtypes => 'text bool bool',
