On Wed, May 08, 2024 at 11:01:01AM +0900, Michael Paquier wrote:
> On Tue, May 07, 2024 at 02:39:42PM -0500, Nathan Bossart wrote:
>> 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.
>
> What you are doing in 0001, and 0002 for v18 sounds fine to me.
Great. Rather than commit this on a Friday afternoon, I'll just post what
I have staged for commit early next week.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
>From 19d9a1dd88385664e6991121e4751aba85a45639 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nat...@postgresql.org>
Date: Fri, 10 May 2024 15:55:24 -0500
Subject: [PATCH v4 1/1] Fix pg_sequence_last_value() for unlogged sequences on
standbys.
Presently, when this function is called for an unlogged sequence on
a standby server, it will error out with a message like
ERROR: could not open file "base/5/16388": No such file or directory
Since the pg_sequences system view uses pg_sequence_last_value(),
it can error similarly. To fix, modify the function to return NULL
for unlogged sequences on standby servers. Since this bug is
present on all versions since v15, this approach is preferable to
making the ERROR nicer because we need to repair the pg_sequences
view without modifying its definition on released versions. For
consistency, this commit also modifies the function to return NULL
for other sessions' temporary sequences. The pg_sequences view
already appropriately filters out such sequences, so there's no bug
there, but we might as well offer some defense in case someone
invokes this function directly.
Unlogged sequences were first introduced in v15, but temporary
sequences are much older, so while the fix for unlogged sequences
is only back-patched to v15, the temporary sequence portion is
back-patched to all supported versions.
We could also remove the privilege check in the pg_sequences view
definition in v18 if we modify this function to return NULL for
sequences for which the current user lacks privileges, but that is
left as a future exercise for when v18 development begins.
Reviewed-by: Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/20240501005730.GA594666%40nathanxps13
Backpatch-through: 12
---
doc/src/sgml/system-views.sgml | 34 +++++++++++++++++++++++----
src/backend/commands/sequence.c | 31 +++++++++++++++++-------
src/test/recovery/t/001_stream_rep.pl | 8 +++++++
3 files changed, 60 insertions(+), 13 deletions(-)
diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml
index a54f4a4743..9842ee276e 100644
--- a/doc/src/sgml/system-views.sgml
+++ b/doc/src/sgml/system-views.sgml
@@ -3091,15 +3091,41 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts
ppx
<para>
The last sequence value written to disk. If caching is used,
this value can be greater than the last value handed out from the
- sequence. Null if the sequence has not been read from yet. Also, if
- the current user does not have <literal>USAGE</literal>
- or <literal>SELECT</literal> privilege on the sequence, the value is
- null.
+ sequence.
</para></entry>
</row>
</tbody>
</tgroup>
</table>
+
+ <para>
+ The <structfield>last_value</structfield> column will read as null if any of
+ the following are true:
+ <itemizedlist>
+ <listitem>
+ <para>
+ The sequence has not been read from yet.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ The current user does not have <literal>USAGE</literal> or
+ <literal>SELECT</literal> privilege on the sequence.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ The sequence is a temporary sequence created by another session.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ The sequence is unlogged and the server is a standby.
+ </para>
+ </listitem>
+ </itemizedlist>
+ </para>
+
</sect1>
<sect1 id="view-pg-settings">
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index 46103561c3..28f8522264 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -1777,11 +1777,8 @@ pg_sequence_last_value(PG_FUNCTION_ARGS)
Oid relid = PG_GETARG_OID(0);
SeqTable elm;
Relation seqrel;
- 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 +1789,28 @@ pg_sequence_last_value(PG_FUNCTION_ARGS)
errmsg("permission denied for sequence %s",
RelationGetRelationName(seqrel))));
- seq = read_seq_tuple(seqrel, &buf, &seqtuple);
+ /*
+ * 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.
+ */
+ if (!RELATION_IS_OTHER_TEMP(seqrel) &&
+ (RelationIsPermanent(seqrel) || !RecoveryInProgress()))
+ {
+ Buffer buf;
+ HeapTupleData seqtuple;
+ Form_pg_sequence_data seq;
+
+ 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 6f99d2cfcf3572d2815055ff2e3e75a314d9c7e3 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nat...@postgresql.org>
Date: Fri, 10 May 2024 15:55:24 -0500
Subject: [PATCH v4 1/1] Fix pg_sequence_last_value() for unlogged sequences on
standbys.
Presently, when this function is called for an unlogged sequence on
a standby server, it will error out with a message like
ERROR: could not open file "base/5/16388": No such file or directory
Since the pg_sequences system view uses pg_sequence_last_value(),
it can error similarly. To fix, modify the function to return NULL
for unlogged sequences on standby servers. Since this bug is
present on all versions since v15, this approach is preferable to
making the ERROR nicer because we need to repair the pg_sequences
view without modifying its definition on released versions. For
consistency, this commit also modifies the function to return NULL
for other sessions' temporary sequences. The pg_sequences view
already appropriately filters out such sequences, so there's no bug
there, but we might as well offer some defense in case someone
invokes this function directly.
Unlogged sequences were first introduced in v15, but temporary
sequences are much older, so while the fix for unlogged sequences
is only back-patched to v15, the temporary sequence portion is
back-patched to all supported versions.
We could also remove the privilege check in the pg_sequences view
definition in v18 if we modify this function to return NULL for
sequences for which the current user lacks privileges, but that is
left as a future exercise for when v18 development begins.
Reviewed-by: Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/20240501005730.GA594666%40nathanxps13
Backpatch-through: 12
---
doc/src/sgml/system-views.sgml | 34 +++++++++++++++++++++++----
src/backend/commands/sequence.c | 31 +++++++++++++++++-------
src/test/recovery/t/001_stream_rep.pl | 8 +++++++
3 files changed, 60 insertions(+), 13 deletions(-)
diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml
index 39815d5faf..82a56f6af4 100644
--- a/doc/src/sgml/system-views.sgml
+++ b/doc/src/sgml/system-views.sgml
@@ -3009,15 +3009,41 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts
ppx
<para>
The last sequence value written to disk. If caching is used,
this value can be greater than the last value handed out from the
- sequence. Null if the sequence has not been read from yet. Also, if
- the current user does not have <literal>USAGE</literal>
- or <literal>SELECT</literal> privilege on the sequence, the value is
- null.
+ sequence.
</para></entry>
</row>
</tbody>
</tgroup>
</table>
+
+ <para>
+ The <structfield>last_value</structfield> column will read as null if any of
+ the following are true:
+ <itemizedlist>
+ <listitem>
+ <para>
+ The sequence has not been read from yet.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ The current user does not have <literal>USAGE</literal> or
+ <literal>SELECT</literal> privilege on the sequence.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ The sequence is a temporary sequence created by another session.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ The sequence is unlogged and the server is a standby.
+ </para>
+ </listitem>
+ </itemizedlist>
+ </para>
+
</sect1>
<sect1 id="view-pg-settings">
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index c7e262c0fc..3fa4e78857 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -1795,11 +1795,8 @@ pg_sequence_last_value(PG_FUNCTION_ARGS)
Oid relid = PG_GETARG_OID(0);
SeqTable elm;
Relation seqrel;
- 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);
@@ -1810,12 +1807,28 @@ pg_sequence_last_value(PG_FUNCTION_ARGS)
errmsg("permission denied for sequence %s",
RelationGetRelationName(seqrel))));
- seq = read_seq_tuple(seqrel, &buf, &seqtuple);
+ /*
+ * 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.
+ */
+ if (!RELATION_IS_OTHER_TEMP(seqrel) &&
+ (RelationIsPermanent(seqrel) || !RecoveryInProgress()))
+ {
+ Buffer buf;
+ HeapTupleData seqtuple;
+ Form_pg_sequence_data seq;
+
+ 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);
+ }
relation_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 0c72ba0944..710bdd54da 100644
--- a/src/test/recovery/t/001_stream_rep.pl
+++ b/src/test/recovery/t/001_stream_rep.pl
@@ -76,6 +76,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 23e3ced85cfdcff5760e3ea32b962c009b50aede Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nat...@postgresql.org>
Date: Fri, 10 May 2024 15:55:24 -0500
Subject: [PATCH v4 1/1] Fix pg_sequence_last_value() for unlogged sequences on
standbys.
Presently, when this function is called for an unlogged sequence on
a standby server, it will error out with a message like
ERROR: could not open file "base/5/16388": No such file or directory
Since the pg_sequences system view uses pg_sequence_last_value(),
it can error similarly. To fix, modify the function to return NULL
for unlogged sequences on standby servers. Since this bug is
present on all versions since v15, this approach is preferable to
making the ERROR nicer because we need to repair the pg_sequences
view without modifying its definition on released versions. For
consistency, this commit also modifies the function to return NULL
for other sessions' temporary sequences. The pg_sequences view
already appropriately filters out such sequences, so there's no bug
there, but we might as well offer some defense in case someone
invokes this function directly.
Unlogged sequences were first introduced in v15, but temporary
sequences are much older, so while the fix for unlogged sequences
is only back-patched to v15, the temporary sequence portion is
back-patched to all supported versions.
We could also remove the privilege check in the pg_sequences view
definition in v18 if we modify this function to return NULL for
sequences for which the current user lacks privileges, but that is
left as a future exercise for when v18 development begins.
Reviewed-by: Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/20240501005730.GA594666%40nathanxps13
Backpatch-through: 12
---
doc/src/sgml/system-views.sgml | 34 +++++++++++++++++++++++----
src/backend/commands/sequence.c | 31 +++++++++++++++++-------
src/test/recovery/t/001_stream_rep.pl | 9 +++++++
3 files changed, 61 insertions(+), 13 deletions(-)
diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml
index 5f8b99bf69..e7284e2df5 100644
--- a/doc/src/sgml/system-views.sgml
+++ b/doc/src/sgml/system-views.sgml
@@ -2927,15 +2927,41 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts
ppx
<para>
The last sequence value written to disk. If caching is used,
this value can be greater than the last value handed out from the
- sequence. Null if the sequence has not been read from yet. Also, if
- the current user does not have <literal>USAGE</literal>
- or <literal>SELECT</literal> privilege on the sequence, the value is
- null.
+ sequence.
</para></entry>
</row>
</tbody>
</tgroup>
</table>
+
+ <para>
+ The <structfield>last_value</structfield> column will read as null if any of
+ the following are true:
+ <itemizedlist>
+ <listitem>
+ <para>
+ The sequence has not been read from yet.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ The current user does not have <literal>USAGE</literal> or
+ <literal>SELECT</literal> privilege on the sequence.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ The sequence is a temporary sequence created by another session.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ The sequence is unlogged and the server is a standby.
+ </para>
+ </listitem>
+ </itemizedlist>
+ </para>
+
</sect1>
<sect1 id="view-pg-settings">
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index acaf660c68..1a73a63d61 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -1810,11 +1810,8 @@ pg_sequence_last_value(PG_FUNCTION_ARGS)
Oid relid = PG_GETARG_OID(0);
SeqTable elm;
Relation seqrel;
- 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);
@@ -1825,12 +1822,28 @@ pg_sequence_last_value(PG_FUNCTION_ARGS)
errmsg("permission denied for sequence %s",
RelationGetRelationName(seqrel))));
- seq = read_seq_tuple(seqrel, &buf, &seqtuple);
+ /*
+ * 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.
+ */
+ if (!RELATION_IS_OTHER_TEMP(seqrel) &&
+ (RelationIsPermanent(seqrel) || !RecoveryInProgress()))
+ {
+ Buffer buf;
+ HeapTupleData seqtuple;
+ Form_pg_sequence_data seq;
+
+ 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);
+ }
relation_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 86864098f9..54c3d3bdf5 100644
--- a/src/test/recovery/t/001_stream_rep.pl
+++ b/src/test/recovery/t/001_stream_rep.pl
@@ -78,6 +78,15 @@ $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')");
+$primary_lsn = $node_primary->lsn('write');
+$node_primary->wait_for_catchup($node_standby_1, 'replay', $primary_lsn);
+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 9d454ea9fd11e42a2408cf35dd40957529c06308 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nat...@postgresql.org>
Date: Fri, 10 May 2024 15:55:24 -0500
Subject: [PATCH v4 1/1] Fix pg_sequence_last_value() for unlogged sequences on
standbys.
Presently, when this function is called for an unlogged sequence on
a standby server, it will error out with a message like
ERROR: could not open file "base/5/16388": No such file or directory
Since the pg_sequences system view uses pg_sequence_last_value(),
it can error similarly. To fix, modify the function to return NULL
for unlogged sequences on standby servers. Since this bug is
present on all versions since v15, this approach is preferable to
making the ERROR nicer because we need to repair the pg_sequences
view without modifying its definition on released versions. For
consistency, this commit also modifies the function to return NULL
for other sessions' temporary sequences. The pg_sequences view
already appropriately filters out such sequences, so there's no bug
there, but we might as well offer some defense in case someone
invokes this function directly.
Unlogged sequences were first introduced in v15, but temporary
sequences are much older, so while the fix for unlogged sequences
is only back-patched to v15, the temporary sequence portion is
back-patched to all supported versions.
We could also remove the privilege check in the pg_sequences view
definition in v18 if we modify this function to return NULL for
sequences for which the current user lacks privileges, but that is
left as a future exercise for when v18 development begins.
Reviewed-by: Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/20240501005730.GA594666%40nathanxps13
Backpatch-through: 12
---
doc/src/sgml/catalogs.sgml | 29 +++++++++++++++++++++++++----
src/backend/commands/sequence.c | 27 ++++++++++++++++++---------
2 files changed, 43 insertions(+), 13 deletions(-)
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index dba6479cf5..b444ca776c 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -12108,15 +12108,36 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts
ppx
<para>
The last sequence value written to disk. If caching is used,
this value can be greater than the last value handed out from the
- sequence. Null if the sequence has not been read from yet. Also, if
- the current user does not have <literal>USAGE</literal>
- or <literal>SELECT</literal> privilege on the sequence, the value is
- null.
+ sequence.
</para></entry>
</row>
</tbody>
</tgroup>
</table>
+
+ <para>
+ The <structfield>last_value</structfield> column will read as null if any of
+ the following are true:
+ <itemizedlist>
+ <listitem>
+ <para>
+ The sequence has not been read from yet.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ The current user does not have <literal>USAGE</literal> or
+ <literal>SELECT</literal> privilege on the sequence.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ The sequence is a temporary sequence created by another session.
+ </para>
+ </listitem>
+ </itemizedlist>
+ </para>
+
</sect1>
<sect1 id="view-pg-settings">
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index 98649986e1..ad34aaff6d 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -1856,11 +1856,8 @@ pg_sequence_last_value(PG_FUNCTION_ARGS)
Oid relid = PG_GETARG_OID(0);
SeqTable elm;
Relation seqrel;
- 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);
@@ -1871,12 +1868,24 @@ pg_sequence_last_value(PG_FUNCTION_ARGS)
errmsg("permission denied for sequence %s",
RelationGetRelationName(seqrel))));
- seq = read_seq_tuple(seqrel, &buf, &seqtuple);
+ /*
+ * 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.
+ */
+ if (!RELATION_IS_OTHER_TEMP(seqrel))
+ {
+ Buffer buf;
+ HeapTupleData seqtuple;
+ Form_pg_sequence_data seq;
- is_called = seq->is_called;
- result = seq->last_value;
+ seq = read_seq_tuple(seqrel, &buf, &seqtuple);
- UnlockReleaseBuffer(buf);
+ is_called = seq->is_called;
+ result = seq->last_value;
+
+ UnlockReleaseBuffer(buf);
+ }
relation_close(seqrel, NoLock);
if (is_called)
--
2.25.1
>From 8e614d1d1439d9b75e3c5218ccfb9e4123fb6d14 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nat...@postgresql.org>
Date: Fri, 10 May 2024 15:55:24 -0500
Subject: [PATCH v4 1/1] Fix pg_sequence_last_value() for unlogged sequences on
standbys.
Presently, when this function is called for an unlogged sequence on
a standby server, it will error out with a message like
ERROR: could not open file "base/5/16388": No such file or directory
Since the pg_sequences system view uses pg_sequence_last_value(),
it can error similarly. To fix, modify the function to return NULL
for unlogged sequences on standby servers. Since this bug is
present on all versions since v15, this approach is preferable to
making the ERROR nicer because we need to repair the pg_sequences
view without modifying its definition on released versions. For
consistency, this commit also modifies the function to return NULL
for other sessions' temporary sequences. The pg_sequences view
already appropriately filters out such sequences, so there's no bug
there, but we might as well offer some defense in case someone
invokes this function directly.
Unlogged sequences were first introduced in v15, but temporary
sequences are much older, so while the fix for unlogged sequences
is only back-patched to v15, the temporary sequence portion is
back-patched to all supported versions.
We could also remove the privilege check in the pg_sequences view
definition in v18 if we modify this function to return NULL for
sequences for which the current user lacks privileges, but that is
left as a future exercise for when v18 development begins.
Reviewed-by: Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/20240501005730.GA594666%40nathanxps13
Backpatch-through: 12
---
doc/src/sgml/catalogs.sgml | 29 +++++++++++++++++++++++++----
src/backend/commands/sequence.c | 27 ++++++++++++++++++---------
2 files changed, 43 insertions(+), 13 deletions(-)
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 0569093486..23dd3df40a 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -11794,15 +11794,36 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts
ppx
<para>
The last sequence value written to disk. If caching is used,
this value can be greater than the last value handed out from the
- sequence. Null if the sequence has not been read from yet. Also, if
- the current user does not have <literal>USAGE</literal>
- or <literal>SELECT</literal> privilege on the sequence, the value is
- null.
+ sequence.
</para></entry>
</row>
</tbody>
</tgroup>
</table>
+
+ <para>
+ The <structfield>last_value</structfield> column will read as null if any of
+ the following are true:
+ <itemizedlist>
+ <listitem>
+ <para>
+ The sequence has not been read from yet.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ The current user does not have <literal>USAGE</literal> or
+ <literal>SELECT</literal> privilege on the sequence.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ The sequence is a temporary sequence created by another session.
+ </para>
+ </listitem>
+ </itemizedlist>
+ </para>
+
</sect1>
<sect1 id="view-pg-settings">
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index ca0bdb5771..5106ef685b 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -1857,11 +1857,8 @@ pg_sequence_last_value(PG_FUNCTION_ARGS)
Oid relid = PG_GETARG_OID(0);
SeqTable elm;
Relation seqrel;
- 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);
@@ -1872,12 +1869,24 @@ pg_sequence_last_value(PG_FUNCTION_ARGS)
errmsg("permission denied for sequence %s",
RelationGetRelationName(seqrel))));
- seq = read_seq_tuple(seqrel, &buf, &seqtuple);
+ /*
+ * 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.
+ */
+ if (!RELATION_IS_OTHER_TEMP(seqrel))
+ {
+ Buffer buf;
+ HeapTupleData seqtuple;
+ Form_pg_sequence_data seq;
- is_called = seq->is_called;
- result = seq->last_value;
+ seq = read_seq_tuple(seqrel, &buf, &seqtuple);
- UnlockReleaseBuffer(buf);
+ is_called = seq->is_called;
+ result = seq->last_value;
+
+ UnlockReleaseBuffer(buf);
+ }
relation_close(seqrel, NoLock);
if (is_called)
--
2.25.1
>From 8078c01d39e2f66c9d1f6161799e19d4e5ceb8a5 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nat...@postgresql.org>
Date: Fri, 10 May 2024 15:55:24 -0500
Subject: [PATCH v4 1/1] Fix pg_sequence_last_value() for unlogged sequences on
standbys.
Presently, when this function is called for an unlogged sequence on
a standby server, it will error out with a message like
ERROR: could not open file "base/5/16388": No such file or directory
Since the pg_sequences system view uses pg_sequence_last_value(),
it can error similarly. To fix, modify the function to return NULL
for unlogged sequences on standby servers. Since this bug is
present on all versions since v15, this approach is preferable to
making the ERROR nicer because we need to repair the pg_sequences
view without modifying its definition on released versions. For
consistency, this commit also modifies the function to return NULL
for other sessions' temporary sequences. The pg_sequences view
already appropriately filters out such sequences, so there's no bug
there, but we might as well offer some defense in case someone
invokes this function directly.
Unlogged sequences were first introduced in v15, but temporary
sequences are much older, so while the fix for unlogged sequences
is only back-patched to v15, the temporary sequence portion is
back-patched to all supported versions.
We could also remove the privilege check in the pg_sequences view
definition in v18 if we modify this function to return NULL for
sequences for which the current user lacks privileges, but that is
left as a future exercise for when v18 development begins.
Reviewed-by: Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/20240501005730.GA594666%40nathanxps13
Backpatch-through: 12
---
doc/src/sgml/catalogs.sgml | 29 +++++++++++++++++++++++++----
src/backend/commands/sequence.c | 27 ++++++++++++++++++---------
2 files changed, 43 insertions(+), 13 deletions(-)
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 118e325464..2337ce8fd2 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -10381,14 +10381,35 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts
ppx
<entry></entry>
<entry>The last sequence value written to disk. If caching is used,
this value can be greater than the last value handed out from the
- sequence. Null if the sequence has not been read from yet. Also, if
- the current user does not have <literal>USAGE</literal>
- or <literal>SELECT</literal> privilege on the sequence, the value is
- null.</entry>
+ sequence.</entry>
</row>
</tbody>
</tgroup>
</table>
+
+ <para>
+ The <structfield>last_value</structfield> column will read as null if any of
+ the following are true:
+ <itemizedlist>
+ <listitem>
+ <para>
+ The sequence has not been read from yet.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ The current user does not have <literal>USAGE</literal> or
+ <literal>SELECT</literal> privilege on the sequence.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ The sequence is a temporary sequence created by another session.
+ </para>
+ </listitem>
+ </itemizedlist>
+ </para>
+
</sect1>
<sect1 id="view-pg-settings">
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index 0577184f82..daaf8ee3d4 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -1857,11 +1857,8 @@ pg_sequence_last_value(PG_FUNCTION_ARGS)
Oid relid = PG_GETARG_OID(0);
SeqTable elm;
Relation seqrel;
- 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);
@@ -1872,12 +1869,24 @@ pg_sequence_last_value(PG_FUNCTION_ARGS)
errmsg("permission denied for sequence %s",
RelationGetRelationName(seqrel))));
- seq = read_seq_tuple(seqrel, &buf, &seqtuple);
+ /*
+ * 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.
+ */
+ if (!RELATION_IS_OTHER_TEMP(seqrel))
+ {
+ Buffer buf;
+ HeapTupleData seqtuple;
+ Form_pg_sequence_data seq;
- is_called = seq->is_called;
- result = seq->last_value;
+ seq = read_seq_tuple(seqrel, &buf, &seqtuple);
- UnlockReleaseBuffer(buf);
+ is_called = seq->is_called;
+ result = seq->last_value;
+
+ UnlockReleaseBuffer(buf);
+ }
relation_close(seqrel, NoLock);
if (is_called)
--
2.25.1
