pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Nathan Bossart <nathandbossart@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@postgresql.org
Subject: Re: pg_sequence_last_value() for unlogged sequences on standbys
Date: Thu, 16 May 2024 20:33:35 -0500
Message-ID: <20240517013335.GA1743971@nathanxps13> (raw)
In-Reply-To: <20240513210107.GA684121@nathanxps13>
References: <2746846.1714771326@sss.pgh.pa.us>
	<ZjYDvGuHaf_Cx5Pb@paquier.xyz>
	<20240507171033.GA2524814@nathanxps13>
	<4179588.1715103856@sss.pgh.pa.us>
	<20240507184051.GA2600328@nathanxps13>
	<86044.1715108521@sss.pgh.pa.us>
	<20240507193942.GB2600328@nathanxps13>
	<Zjrc3e-3eg4i6WQV@paquier.xyz>
	<20240510210055.GA428795@nathanxps13>
	<20240513210107.GA684121@nathanxps13>

Here is a rebased version of 0002, which I intend to commit once v18
development begins.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

Attachments:

  [text/x-diff] v5-0001-Simplify-pg_sequences-a-bit.patch (3.2K, ../20240517013335.GA1743971@nathanxps13/2-v5-0001-Simplify-pg_sequences-a-bit.patch)
  download | inline diff:
From e9cba5e4303c7fa5ad2d7d5deb23fe0b1c740b09 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Tue, 7 May 2024 14:35:34 -0500
Subject: [PATCH v5 1/1] Simplify pg_sequences a bit.

XXX: NEEDS CATVERSION BUMP
---
 src/backend/catalog/system_views.sql |  6 +-----
 src/backend/commands/sequence.c      | 12 ++++--------
 src/test/regress/expected/rules.out  |  5 +----
 3 files changed, 6 insertions(+), 17 deletions(-)

diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 53047cab5f..b32e5c3170 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -176,11 +176,7 @@ CREATE VIEW pg_sequences AS
         S.seqincrement AS increment_by,
         S.seqcycle AS cycle,
         S.seqcache AS cache_size,
-        CASE
-            WHEN has_sequence_privilege(C.oid, 'SELECT,USAGE'::text)
-                THEN pg_sequence_last_value(C.oid)
-            ELSE NULL
-        END AS last_value
+        pg_sequence_last_value(C.oid) AS last_value
     FROM pg_sequence S 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)
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index 28f8522264..cd0e746577 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -1783,21 +1783,17 @@ pg_sequence_last_value(PG_FUNCTION_ARGS)
 	/* open and lock sequence */
 	init_sequence(relid, &elm, &seqrel);
 
-	if (pg_class_aclcheck(relid, GetUserId(), ACL_SELECT | ACL_USAGE) != ACLCHECK_OK)
-		ereport(ERROR,
-				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-				 errmsg("permission denied for sequence %s",
-						RelationGetRelationName(seqrel))));
-
 	/*
 	 * We return NULL for other sessions' temporary sequences.  The
 	 * pg_sequences system view already filters those out, but this offers a
 	 * defense against ERRORs in case someone invokes this function directly.
 	 *
 	 * Also, for the benefit of the pg_sequences view, we return NULL for
-	 * unlogged sequences on standbys instead of throwing an error.
+	 * unlogged sequences on standbys and for sequences for which we lack
+	 * USAGE or SELECT privileges instead of throwing an error.
 	 */
-	if (!RELATION_IS_OTHER_TEMP(seqrel) &&
+	if (pg_class_aclcheck(relid, GetUserId(), ACL_SELECT | ACL_USAGE) == ACLCHECK_OK &&
+		!RELATION_IS_OTHER_TEMP(seqrel) &&
 		(RelationIsPermanent(seqrel) || !RecoveryInProgress()))
 	{
 		Buffer		buf;
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index ef658ad740..04b3790bdd 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1699,10 +1699,7 @@ pg_sequences| SELECT n.nspname AS schemaname,
     s.seqincrement AS increment_by,
     s.seqcycle AS cycle,
     s.seqcache AS cache_size,
-        CASE
-            WHEN has_sequence_privilege(c.oid, 'SELECT,USAGE'::text) THEN pg_sequence_last_value((c.oid)::regclass)
-            ELSE NULL::bigint
-        END AS last_value
+    pg_sequence_last_value((c.oid)::regclass) AS last_value
    FROM ((pg_sequence s
      JOIN pg_class c ON ((c.oid = s.seqrelid)))
      LEFT JOIN pg_namespace n ON ((n.oid = c.relnamespace)))
-- 
2.25.1

view thread (19+ messages)  latest in thread

Message-ID: <20240517013335.GA1743971@nathanxps13>
Permalink:  ../20240517013335.GA1743971@nathanxps13/
Also on:    postgresql.org/message-id/20240517013335.GA1743971@nathanxps13

 ·  · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: nathandbossart@gmail.com, michael@paquier.xyz, tgl@sss.pgh.pa.us
  Subject: Re: pg_sequence_last_value() for unlogged sequences on standbys
  In-Reply-To: <20240517013335.GA1743971@nathanxps13>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

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