From 449e798c77ed9a03f8bb04e5d324d4e3cfbbae8e Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Mon, 26 Feb 2024 16:04:59 +0900 Subject: [PATCH] Introduce sequence_*() access functions Similarly to tables and indexes, these functions are able to open relations with a sequence relkind, which is useful to make a distinction with the other relation kinds. Previously, commands/sequence.c used a mix of table_{close,open}() and relation_{close,open}() routines when manipulating sequence relations, so this clarifies the code. A direct effect of this change is to align the error messages produced when attempting DDLs for sequences on relations with an unexpected relkind, like a table or an index with ALTER SEQUENCE, providing an extra error detail about the relkind of the relation used in the DDL query. Author: Michael Paquier Reviewed-by: Tomas Vondra Discussion: https://postgr.es/m/ZWlohtKAs0uVVpZ3@paquier.xyz --- src/backend/access/Makefile | 2 +- src/backend/access/meson.build | 1 + src/backend/access/sequence/Makefile | 17 ++++++ src/backend/access/sequence/meson.build | 5 ++ src/backend/access/sequence/sequence.c | 78 +++++++++++++++++++++++++ src/backend/commands/sequence.c | 31 +++++----- src/include/access/sequence.h | 23 ++++++++ src/test/regress/expected/sequence.out | 3 +- 8 files changed, 140 insertions(+), 20 deletions(-) create mode 100644 src/backend/access/sequence/Makefile create mode 100644 src/backend/access/sequence/meson.build create mode 100644 src/backend/access/sequence/sequence.c create mode 100644 src/include/access/sequence.h diff --git a/src/backend/access/Makefile b/src/backend/access/Makefile index 0880e0a8bb..1932d11d15 100644 --- a/src/backend/access/Makefile +++ b/src/backend/access/Makefile @@ -9,6 +9,6 @@ top_builddir = ../../.. include $(top_builddir)/src/Makefile.global SUBDIRS = brin common gin gist hash heap index nbtree rmgrdesc spgist \ - table tablesample transam + sequence table tablesample transam include $(top_srcdir)/src/backend/common.mk diff --git a/src/backend/access/meson.build b/src/backend/access/meson.build index 621691ef34..62a371db7f 100644 --- a/src/backend/access/meson.build +++ b/src/backend/access/meson.build @@ -9,6 +9,7 @@ subdir('heap') subdir('index') subdir('nbtree') subdir('rmgrdesc') +subdir('sequence') subdir('spgist') subdir('table') subdir('tablesample') diff --git a/src/backend/access/sequence/Makefile b/src/backend/access/sequence/Makefile new file mode 100644 index 0000000000..9f9d31f542 --- /dev/null +++ b/src/backend/access/sequence/Makefile @@ -0,0 +1,17 @@ +#------------------------------------------------------------------------- +# +# Makefile +# Makefile for access/sequence +# +# IDENTIFICATION +# src/backend/access/sequence/Makefile +# +#------------------------------------------------------------------------- + +subdir = src/backend/access/sequence +top_builddir = ../../../.. +include $(top_builddir)/src/Makefile.global + +OBJS = sequence.o + +include $(top_srcdir)/src/backend/common.mk diff --git a/src/backend/access/sequence/meson.build b/src/backend/access/sequence/meson.build new file mode 100644 index 0000000000..1843aed38d --- /dev/null +++ b/src/backend/access/sequence/meson.build @@ -0,0 +1,5 @@ +# Copyright (c) 2022-2024, PostgreSQL Global Development Group + +backend_sources += files( + 'sequence.c', +) diff --git a/src/backend/access/sequence/sequence.c b/src/backend/access/sequence/sequence.c new file mode 100644 index 0000000000..9e30e6e08f --- /dev/null +++ b/src/backend/access/sequence/sequence.c @@ -0,0 +1,78 @@ +/*------------------------------------------------------------------------- + * + * sequence.c + * Generic routines for sequence-related code. + * + * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * + * IDENTIFICATION + * src/backend/access/sequence/sequence.c + * + * + * NOTES + * This file contains sequence_ routines that implement access to sequences + * (in contrast to other relation types like indexes). + * + *------------------------------------------------------------------------- + */ + +#include "postgres.h" + +#include "access/relation.h" +#include "access/sequence.h" +#include "storage/lmgr.h" + +static inline void validate_relation_kind(Relation r); + +/* ---------------- + * sequence_open - open a sequence relation by relation OID + * + * This is essentially relation_open plus check that the relation + * is a sequence. + * ---------------- + */ +Relation +sequence_open(Oid relationId, LOCKMODE lockmode) +{ + Relation r; + + r = relation_open(relationId, lockmode); + + validate_relation_kind(r); + + return r; +} + +/* ---------------- + * sequence_close - close a sequence + * + * If lockmode is not "NoLock", we then release the specified lock. + * + * Note that it is often sensible to hold a lock beyond relation_close; + * in that case, the lock is released automatically at xact end. + * ---------------- + */ +void +sequence_close(Relation relation, LOCKMODE lockmode) +{ + relation_close(relation, lockmode); +} + +/* ---------------- + * validate_relation_kind - check the relation's kind + * + * Make sure relkind is from an index + * ---------------- + */ +static inline void +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))); +} diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c index f7803744a5..d019fbdede 100644 --- a/src/backend/commands/sequence.c +++ b/src/backend/commands/sequence.c @@ -18,6 +18,7 @@ #include "access/htup_details.h" #include "access/multixact.h" #include "access/relation.h" +#include "access/sequence.h" #include "access/table.h" #include "access/transam.h" #include "access/xact.h" @@ -208,7 +209,7 @@ DefineSequence(ParseState *pstate, CreateSeqStmt *seq) seqoid = address.objectId; Assert(seqoid != InvalidOid); - rel = table_open(seqoid, AccessExclusiveLock); + rel = sequence_open(seqoid, AccessExclusiveLock); tupDesc = RelationGetDescr(rel); /* now initialize the sequence's data */ @@ -219,7 +220,7 @@ DefineSequence(ParseState *pstate, CreateSeqStmt *seq) if (owned_by) process_owned_by(rel, owned_by, seq->for_identity); - table_close(rel, NoLock); + sequence_close(rel, NoLock); /* fill in pg_sequence */ rel = table_open(SequenceRelationId, RowExclusiveLock); @@ -324,7 +325,7 @@ ResetSequence(Oid seq_relid) /* Note that we do not change the currval() state */ elm->cached = elm->last; - relation_close(seq_rel, NoLock); + sequence_close(seq_rel, NoLock); } /* @@ -531,7 +532,7 @@ AlterSequence(ParseState *pstate, AlterSeqStmt *stmt) ObjectAddressSet(address, RelationRelationId, relid); table_close(rel, RowExclusiveLock); - relation_close(seqrel, NoLock); + sequence_close(seqrel, NoLock); return address; } @@ -555,7 +556,7 @@ SequenceChangePersistence(Oid relid, char newrelpersistence) fill_seq_with_data(seqrel, &seqdatatuple); UnlockReleaseBuffer(buf); - relation_close(seqrel, NoLock); + sequence_close(seqrel, NoLock); } void @@ -662,7 +663,7 @@ nextval_internal(Oid relid, bool check_permissions) Assert(elm->last_valid); Assert(elm->increment != 0); elm->last += elm->increment; - relation_close(seqrel, NoLock); + sequence_close(seqrel, NoLock); last_used_seq = elm; return elm->last; } @@ -849,7 +850,7 @@ nextval_internal(Oid relid, bool check_permissions) UnlockReleaseBuffer(buf); - relation_close(seqrel, NoLock); + sequence_close(seqrel, NoLock); return result; } @@ -880,7 +881,7 @@ currval_oid(PG_FUNCTION_ARGS) result = elm->last; - relation_close(seqrel, NoLock); + sequence_close(seqrel, NoLock); PG_RETURN_INT64(result); } @@ -915,7 +916,7 @@ lastval(PG_FUNCTION_ARGS) RelationGetRelationName(seqrel)))); result = last_used_seq->last; - relation_close(seqrel, NoLock); + sequence_close(seqrel, NoLock); PG_RETURN_INT64(result); } @@ -1030,7 +1031,7 @@ do_setval(Oid relid, int64 next, bool iscalled) UnlockReleaseBuffer(buf); - relation_close(seqrel, NoLock); + sequence_close(seqrel, NoLock); } /* @@ -1095,7 +1096,7 @@ lock_and_open_sequence(SeqTable seq) } /* We now know we have the lock, and can safely open the rel */ - return relation_open(seq->relid, NoLock); + return sequence_open(seq->relid, NoLock); } /* @@ -1152,12 +1153,6 @@ init_sequence(Oid relid, SeqTable *p_elm, Relation *p_rel) */ seqrel = lock_and_open_sequence(elm); - if (seqrel->rd_rel->relkind != RELKIND_SEQUENCE) - ereport(ERROR, - (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("\"%s\" is not a sequence", - RelationGetRelationName(seqrel)))); - /* * If the sequence has been transactionally replaced since we last saw it, * discard any cached-but-unissued values. We do not touch the currval() @@ -1803,7 +1798,7 @@ pg_sequence_last_value(PG_FUNCTION_ARGS) result = seq->last_value; UnlockReleaseBuffer(buf); - relation_close(seqrel, NoLock); + sequence_close(seqrel, NoLock); if (is_called) PG_RETURN_INT64(result); diff --git a/src/include/access/sequence.h b/src/include/access/sequence.h new file mode 100644 index 0000000000..fb8708861b --- /dev/null +++ b/src/include/access/sequence.h @@ -0,0 +1,23 @@ +/*------------------------------------------------------------------------- + * + * sequence.h + * Generic routines for sequence-related code. + * + * + * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * src/include/access/sequence.h + * + *------------------------------------------------------------------------- + */ +#ifndef ACCESS_SEQUENCE_H +#define ACCESS_SEQUENCE_H + +#include "storage/lockdefs.h" +#include "utils/relcache.h" + +extern Relation sequence_open(Oid relationId, LOCKMODE lockmode); +extern void sequence_close(Relation relation, LOCKMODE lockmode); + +#endif /* ACCESS_SEQUENCE_H */ diff --git a/src/test/regress/expected/sequence.out b/src/test/regress/expected/sequence.out index 7cb2f7cc02..2b47b7796b 100644 --- a/src/test/regress/expected/sequence.out +++ b/src/test/regress/expected/sequence.out @@ -313,7 +313,8 @@ 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: "serialtest1" is not a sequence +ERROR: cannot open relation "serialtest1" +DETAIL: This operation is not supported for tables. CREATE SEQUENCE sequence_test2 START WITH 32; CREATE SEQUENCE sequence_test4 INCREMENT BY -1; SELECT nextval('sequence_test2');