On Tue, May 07, 2024 at 03:02:01PM -0400, Tom Lane wrote:
> Nathan Bossart <nathandboss...@gmail.com> writes:
>> On Tue, May 07, 2024 at 01:44:16PM -0400, Tom Lane wrote:
>>> +1 to include that, as it offers a defense if someone invokes this
>>> function directly. In HEAD we could then rip out the test in the
>>> view.
>
>> I apologize for belaboring this point, but I don't see how we would be
>> comfortable removing that check unless we are okay with other sessions'
>> temporary sequences appearing in the view, albeit with a NULL last_value.
>
> Oh! You're right, I'm wrong. I was looking at the CASE filter, which
> we could get rid of -- but the "WHERE NOT pg_is_other_temp_schema(N.oid)"
> part has to stay.
Okay, phew. We can still do something like v3-0002 for v18. I'll give
Michael a chance to comment on 0001 before committing/back-patching that
one.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
>From 2a37834699587eef18b50bf8d58723790bbcdde7 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nat...@postgresql.org>
Date: Tue, 30 Apr 2024 20:54:51 -0500
Subject: [PATCH v3 1/2] Fix pg_sequence_last_value() for non-permanent
sequences on standbys.
---
src/backend/commands/sequence.c | 22 ++++++++++++++++------
src/test/recovery/t/001_stream_rep.pl | 8 ++++++++
2 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index 46103561c3..9d7468d7bb 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -1780,8 +1780,8 @@ pg_sequence_last_value(PG_FUNCTION_ARGS)
Buffer buf;
HeapTupleData seqtuple;
Form_pg_sequence_data seq;
- bool is_called;
- int64 result;
+ bool is_called = false;
+ int64 result = 0;
/* open and lock sequence */
init_sequence(relid, &elm, &seqrel);
@@ -1792,12 +1792,22 @@ pg_sequence_last_value(PG_FUNCTION_ARGS)
errmsg("permission denied for sequence %s",
RelationGetRelationName(seqrel))));
- seq = read_seq_tuple(seqrel, &buf, &seqtuple);
+ /*
+ * For the benefit of the pg_sequences system view, we return NULL for
+ * temporary and unlogged sequences on standbys instead of throwing an
+ * error. We also always return NULL for other sessions' temporary
+ * sequences.
+ */
+ if ((RelationIsPermanent(seqrel) || !RecoveryInProgress()) &&
+ !RELATION_IS_OTHER_TEMP(seqrel))
+ {
+ seq = read_seq_tuple(seqrel, &buf, &seqtuple);
- is_called = seq->is_called;
- result = seq->last_value;
+ is_called = seq->is_called;
+ result = seq->last_value;
- UnlockReleaseBuffer(buf);
+ UnlockReleaseBuffer(buf);
+ }
sequence_close(seqrel, NoLock);
if (is_called)
diff --git a/src/test/recovery/t/001_stream_rep.pl b/src/test/recovery/t/001_stream_rep.pl
index 5311ade509..4c698b5ce1 100644
--- a/src/test/recovery/t/001_stream_rep.pl
+++ b/src/test/recovery/t/001_stream_rep.pl
@@ -95,6 +95,14 @@ $result = $node_standby_2->safe_psql('postgres', "SELECT * FROM seq1");
print "standby 2: $result\n";
is($result, qq(33|0|t), 'check streamed sequence content on standby 2');
+# Check pg_sequence_last_value() returns NULL for unlogged sequence on standby
+$node_primary->safe_psql('postgres',
+ "CREATE UNLOGGED SEQUENCE ulseq; SELECT nextval('ulseq')");
+$node_primary->wait_for_replay_catchup($node_standby_1);
+is($node_standby_1->safe_psql('postgres',
+ "SELECT pg_sequence_last_value('ulseq'::regclass) IS NULL"),
+ 't', 'pg_sequence_last_value() on unlogged sequence on standby 1');
+
# Check that only READ-only queries can run on standbys
is($node_standby_1->psql('postgres', 'INSERT INTO tab_int VALUES (1)'),
3, 'read-only queries on standby 1');
--
2.25.1
>From b96d1f21f6144640561360c84b361f569a2edc48 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nat...@postgresql.org>
Date: Tue, 7 May 2024 14:35:34 -0500
Subject: [PATCH v3 2/2] Simplify pg_sequences a bit.
XXX: NEEDS CATVERSION BUMP
---
src/backend/catalog/system_views.sql | 6 +-----
src/backend/commands/sequence.c | 15 +++++----------
src/test/regress/expected/rules.out | 5 +----
3 files changed, 7 insertions(+), 19 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 9d7468d7bb..f129375915 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -1786,19 +1786,14 @@ 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))));
-
/*
* For the benefit of the pg_sequences system view, we return NULL for
- * temporary and unlogged sequences on standbys instead of throwing an
- * error. We also always return NULL for other sessions' temporary
- * sequences.
+ * temporary and unlogged sequences on standbys as well as for sequences
+ * for which we lack USAGE or SELECT privileges. We also always return
+ * NULL for other sessions' temporary sequences.
*/
- if ((RelationIsPermanent(seqrel) || !RecoveryInProgress()) &&
+ if (pg_class_aclcheck(relid, GetUserId(), ACL_SELECT | ACL_USAGE) == ACLCHECK_OK &&
+ (RelationIsPermanent(seqrel) || !RecoveryInProgress()) &&
!RELATION_IS_OTHER_TEMP(seqrel))
{
seq = read_seq_tuple(seqrel, &buf, &seqtuple);
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
