public inbox for [email protected]
help / color / mirror / Atom feedFrom: Peter Eisentraut <[email protected]>
To: pgsql-hackers <[email protected]>
Subject: some validate_relation_kind() tidying
Date: Wed, 18 Feb 2026 16:15:09 +0100
Message-ID: <[email protected]> (raw)
We have three different functions called validate_relation_kind(), namely in
src/backend/access/index/indexam.c
src/backend/access/sequence/sequence.c
src/backend/access/table/table.c
These all check which relkinds are permitted by index_open(),
sequence_open(), and table_open(), respectively, which are each wrappers
around relation_open() (which accepts any relkind).
I always found the one in table.c a little too mysterious, because it
just checks which relkinds it does *not* want, and so if you want to
know whether a particular relkind is suitable for table_open(), you need
to do additional research and check what all the possible relkinds are
and so on, and there is no real explanation why those choices were made.
I think it would be clearer and more robust and also more consistent
with the other ones if we flipped that around and listed the ones that
are acceptable and why.
Secondly, the sequence.c one was probably copied from the table.c one,
but I think we can make the error message a bit more direct by just
saying "... is not a sequence" instead of "cannot open relation".
These are the two attached patches. This is just something I found
while working on something else nearby.
From 12e4eb279cf64ac23131c40ae0420f6b7e2db33a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Wed, 18 Feb 2026 11:48:37 +0100
Subject: [PATCH 1/2] Change error message for sequence
validate_relation_kind()
We can just say "... is not a sequence" instead of the more
complicated variant from before, which was probably copied from
src/backend/access/table/table.c.
Fix a typo in a comment in passing.
---
src/backend/access/sequence/sequence.c | 7 +++----
src/test/regress/expected/sequence.out | 3 +--
2 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/src/backend/access/sequence/sequence.c b/src/backend/access/sequence/sequence.c
index 106af1477e9..6897f8432d6 100644
--- a/src/backend/access/sequence/sequence.c
+++ b/src/backend/access/sequence/sequence.c
@@ -63,7 +63,7 @@ sequence_close(Relation relation, LOCKMODE lockmode)
/* ----------------
* validate_relation_kind - check the relation's kind
*
- * Make sure relkind is from a sequence.
+ * Make sure relkind is a sequence.
* ----------------
*/
static inline void
@@ -72,7 +72,6 @@ validate_relation_kind(Relation r)
if (r->rd_rel->relkind != RELKIND_SEQUENCE)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot open relation \"%s\"",
- RelationGetRelationName(r)),
- errdetail_relkind_not_supported(r->rd_rel->relkind)));
+ errmsg("\"%s\" is not a sequence",
+ RelationGetRelationName(r))));
}
diff --git a/src/test/regress/expected/sequence.out b/src/test/regress/expected/sequence.out
index c4454e5b435..a0883b11007 100644
--- a/src/test/regress/expected/sequence.out
+++ b/src/test/regress/expected/sequence.out
@@ -313,8 +313,7 @@ ALTER SEQUENCE IF EXISTS sequence_test2 RESTART WITH 24
INCREMENT BY 4 MAXVALUE 36 MINVALUE 5 CYCLE;
NOTICE: relation "sequence_test2" does not exist, skipping
ALTER SEQUENCE serialTest1 CYCLE; -- error, not a sequence
-ERROR: cannot open relation "serialtest1"
-DETAIL: This operation is not supported for tables.
+ERROR: "serialtest1" is not a sequence
CREATE SEQUENCE sequence_test2 START WITH 32;
CREATE SEQUENCE sequence_test4 INCREMENT BY -1;
SELECT nextval('sequence_test2');
--
2.53.0
From 2cd2bf31d5adb6fcdbb4528018de700094075936 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Wed, 18 Feb 2026 14:32:45 +0100
Subject: [PATCH 2/2] Flip logic in table validate_relation_kind
It instead of checking which relkinds it shouldn't be, explicitly list
the ones we accept. This is used to check which relkinds are accepted
in table_open() and related functions. Before this change, figuring
that out was always a few steps too complicated. This also makes
changes for new relkinds more explicit instead of accidental.
Finally, this makes this more aligned with the functions of the same
name in src/backend/access/index/indexam.c and
src/backend/access/sequence/sequence.c.
---
src/backend/access/table/table.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/backend/access/table/table.c b/src/backend/access/table/table.c
index fe330ae862a..ef33d75d6fa 100644
--- a/src/backend/access/table/table.c
+++ b/src/backend/access/table/table.c
@@ -131,15 +131,20 @@ table_close(Relation relation, LOCKMODE lockmode)
/* ----------------
* validate_relation_kind - check the relation's kind
*
- * Make sure relkind is not index or composite type
+ * Make sure relkind is table-like. In particular, this excludes indexes
+ * and composite types, which cannot be read from in a query.
* ----------------
*/
static inline void
validate_relation_kind(Relation r)
{
- if (r->rd_rel->relkind == RELKIND_INDEX ||
- r->rd_rel->relkind == RELKIND_PARTITIONED_INDEX ||
- r->rd_rel->relkind == RELKIND_COMPOSITE_TYPE)
+ if (r->rd_rel->relkind != RELKIND_RELATION ||
+ r->rd_rel->relkind != RELKIND_SEQUENCE ||
+ r->rd_rel->relkind != RELKIND_TOASTVALUE ||
+ r->rd_rel->relkind != RELKIND_VIEW ||
+ r->rd_rel->relkind != RELKIND_MATVIEW ||
+ r->rd_rel->relkind != RELKIND_FOREIGN_TABLE ||
+ r->rd_rel->relkind != RELKIND_PARTITIONED_TABLE)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("cannot open relation \"%s\"",
--
2.53.0
Attachments:
[text/plain] 0001-Change-error-message-for-sequence-validate_relation_.patch (2.2K, ../[email protected]/2-0001-Change-error-message-for-sequence-validate_relation_.patch)
download | inline diff:
From 12e4eb279cf64ac23131c40ae0420f6b7e2db33a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Wed, 18 Feb 2026 11:48:37 +0100
Subject: [PATCH 1/2] Change error message for sequence
validate_relation_kind()
We can just say "... is not a sequence" instead of the more
complicated variant from before, which was probably copied from
src/backend/access/table/table.c.
Fix a typo in a comment in passing.
---
src/backend/access/sequence/sequence.c | 7 +++----
src/test/regress/expected/sequence.out | 3 +--
2 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/src/backend/access/sequence/sequence.c b/src/backend/access/sequence/sequence.c
index 106af1477e9..6897f8432d6 100644
--- a/src/backend/access/sequence/sequence.c
+++ b/src/backend/access/sequence/sequence.c
@@ -63,7 +63,7 @@ sequence_close(Relation relation, LOCKMODE lockmode)
/* ----------------
* validate_relation_kind - check the relation's kind
*
- * Make sure relkind is from a sequence.
+ * Make sure relkind is a sequence.
* ----------------
*/
static inline void
@@ -72,7 +72,6 @@ validate_relation_kind(Relation r)
if (r->rd_rel->relkind != RELKIND_SEQUENCE)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot open relation \"%s\"",
- RelationGetRelationName(r)),
- errdetail_relkind_not_supported(r->rd_rel->relkind)));
+ errmsg("\"%s\" is not a sequence",
+ RelationGetRelationName(r))));
}
diff --git a/src/test/regress/expected/sequence.out b/src/test/regress/expected/sequence.out
index c4454e5b435..a0883b11007 100644
--- a/src/test/regress/expected/sequence.out
+++ b/src/test/regress/expected/sequence.out
@@ -313,8 +313,7 @@ ALTER SEQUENCE IF EXISTS sequence_test2 RESTART WITH 24
INCREMENT BY 4 MAXVALUE 36 MINVALUE 5 CYCLE;
NOTICE: relation "sequence_test2" does not exist, skipping
ALTER SEQUENCE serialTest1 CYCLE; -- error, not a sequence
-ERROR: cannot open relation "serialtest1"
-DETAIL: This operation is not supported for tables.
+ERROR: "serialtest1" is not a sequence
CREATE SEQUENCE sequence_test2 START WITH 32;
CREATE SEQUENCE sequence_test4 INCREMENT BY -1;
SELECT nextval('sequence_test2');
--
2.53.0
[text/plain] 0002-Flip-logic-in-table-validate_relation_kind.patch (2.0K, ../[email protected]/3-0002-Flip-logic-in-table-validate_relation_kind.patch)
download | inline diff:
From 2cd2bf31d5adb6fcdbb4528018de700094075936 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Wed, 18 Feb 2026 14:32:45 +0100
Subject: [PATCH 2/2] Flip logic in table validate_relation_kind
It instead of checking which relkinds it shouldn't be, explicitly list
the ones we accept. This is used to check which relkinds are accepted
in table_open() and related functions. Before this change, figuring
that out was always a few steps too complicated. This also makes
changes for new relkinds more explicit instead of accidental.
Finally, this makes this more aligned with the functions of the same
name in src/backend/access/index/indexam.c and
src/backend/access/sequence/sequence.c.
---
src/backend/access/table/table.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/backend/access/table/table.c b/src/backend/access/table/table.c
index fe330ae862a..ef33d75d6fa 100644
--- a/src/backend/access/table/table.c
+++ b/src/backend/access/table/table.c
@@ -131,15 +131,20 @@ table_close(Relation relation, LOCKMODE lockmode)
/* ----------------
* validate_relation_kind - check the relation's kind
*
- * Make sure relkind is not index or composite type
+ * Make sure relkind is table-like. In particular, this excludes indexes
+ * and composite types, which cannot be read from in a query.
* ----------------
*/
static inline void
validate_relation_kind(Relation r)
{
- if (r->rd_rel->relkind == RELKIND_INDEX ||
- r->rd_rel->relkind == RELKIND_PARTITIONED_INDEX ||
- r->rd_rel->relkind == RELKIND_COMPOSITE_TYPE)
+ if (r->rd_rel->relkind != RELKIND_RELATION ||
+ r->rd_rel->relkind != RELKIND_SEQUENCE ||
+ r->rd_rel->relkind != RELKIND_TOASTVALUE ||
+ r->rd_rel->relkind != RELKIND_VIEW ||
+ r->rd_rel->relkind != RELKIND_MATVIEW ||
+ r->rd_rel->relkind != RELKIND_FOREIGN_TABLE ||
+ r->rd_rel->relkind != RELKIND_PARTITIONED_TABLE)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("cannot open relation \"%s\"",
--
2.53.0
view thread (2+ messages)
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: [email protected]
Cc: [email protected]
Subject: Re: some validate_relation_kind() tidying
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox