diff options
| -rw-r--r-- | doc/src/sgml/ref/alter_sequence.sgml | 7 | ||||
| -rw-r--r-- | src/backend/commands/sequence.c | 69 | ||||
| -rw-r--r-- | src/test/isolation/expected/sequence-ddl.out | 85 | ||||
| -rw-r--r-- | src/test/isolation/isolation_schedule | 1 | ||||
| -rw-r--r-- | src/test/isolation/specs/sequence-ddl.spec | 39 |
5 files changed, 180 insertions, 21 deletions
diff --git a/doc/src/sgml/ref/alter_sequence.sgml b/doc/src/sgml/ref/alter_sequence.sgml index 3fb3400d6be..30e5316b8cb 100644 --- a/doc/src/sgml/ref/alter_sequence.sgml +++ b/doc/src/sgml/ref/alter_sequence.sgml @@ -305,6 +305,13 @@ ALTER SEQUENCE [ IF EXISTS ] <replaceable class="parameter">name</replaceable> S </para> <para> + <command>ALTER SEQUENCE</command> blocks + concurrent <function>nextval</function>, <function>currval</function>, + <function>lastval</function>, and <command>setval</command> calls, except + if only the <literal>RESTART</literal> clause is used. + </para> + + <para> For historical reasons, <command>ALTER TABLE</command> can be used with sequences too; but the only variants of <command>ALTER TABLE</command> that are allowed with sequences are equivalent to the forms shown above. diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c index 4a56926b530..0f7cf1dce8a 100644 --- a/src/backend/commands/sequence.c +++ b/src/backend/commands/sequence.c @@ -93,11 +93,12 @@ static HTAB *seqhashtab = NULL; /* hash table for SeqTable items */ static SeqTableData *last_used_seq = NULL; static void fill_seq_with_data(Relation rel, HeapTuple tuple); -static Relation open_share_lock(SeqTable seq); +static Relation lock_and_open_sequence(SeqTable seq); static void create_seq_hashtable(void); static void init_sequence(Oid relid, SeqTable *p_elm, Relation *p_rel); static Form_pg_sequence_data read_seq_tuple(Relation rel, Buffer *buf, HeapTuple seqdatatuple); +static LOCKMODE alter_sequence_get_lock_level(List *options); static void init_params(ParseState *pstate, List *options, bool for_identity, bool isInit, Form_pg_sequence seqform, @@ -427,7 +428,9 @@ AlterSequence(ParseState *pstate, AlterSeqStmt *stmt) HeapTuple tuple; /* Open and lock sequence. */ - relid = RangeVarGetRelid(stmt->sequence, AccessShareLock, stmt->missing_ok); + relid = RangeVarGetRelid(stmt->sequence, + alter_sequence_get_lock_level(stmt->options), + stmt->missing_ok); if (relid == InvalidOid) { ereport(NOTICE, @@ -443,12 +446,6 @@ AlterSequence(ParseState *pstate, AlterSeqStmt *stmt) aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS, stmt->sequence->relname); - /* lock page' buffer and read tuple into new sequence structure */ - seqdata = read_seq_tuple(seqrel, &buf, &seqdatatuple); - - /* Copy old sequence data into workspace */ - memcpy(&newseqdata, seqdata, sizeof(FormData_pg_sequence_data)); - rel = heap_open(SequenceRelationId, RowExclusiveLock); tuple = SearchSysCacheCopy1(SEQRELID, ObjectIdGetDatum(relid)); @@ -458,6 +455,12 @@ AlterSequence(ParseState *pstate, AlterSeqStmt *stmt) seqform = (Form_pg_sequence) GETSTRUCT(tuple); + /* lock page's buffer and read tuple into new sequence structure */ + seqdata = read_seq_tuple(seqrel, &buf, &seqdatatuple); + + /* Copy old sequence data into workspace */ + memcpy(&newseqdata, seqdata, sizeof(FormData_pg_sequence_data)); + /* Check and set new values */ init_params(pstate, stmt->options, stmt->for_identity, false, seqform, &changed_seqform, &newseqdata, &owned_by); @@ -508,12 +511,12 @@ AlterSequence(ParseState *pstate, AlterSeqStmt *stmt) ObjectAddressSet(address, RelationRelationId, relid); - relation_close(seqrel, NoLock); - if (changed_seqform) CatalogTupleUpdate(rel, &tuple->t_self, tuple); heap_close(rel, RowExclusiveLock); + relation_close(seqrel, NoLock); + return address; } @@ -594,7 +597,7 @@ nextval_internal(Oid relid, bool check_permissions) bool cycle; bool logit = false; - /* open and AccessShareLock sequence */ + /* open and lock sequence */ init_sequence(relid, &elm, &seqrel); if (check_permissions && @@ -829,7 +832,7 @@ currval_oid(PG_FUNCTION_ARGS) SeqTable elm; Relation seqrel; - /* open and AccessShareLock sequence */ + /* open and lock sequence */ init_sequence(relid, &elm, &seqrel); if (pg_class_aclcheck(elm->relid, GetUserId(), @@ -869,7 +872,7 @@ lastval(PG_FUNCTION_ARGS) (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), errmsg("lastval is not yet defined in this session"))); - seqrel = open_share_lock(last_used_seq); + seqrel = lock_and_open_sequence(last_used_seq); /* nextval() must have already been called for this sequence */ Assert(last_used_seq->last_valid); @@ -913,7 +916,7 @@ do_setval(Oid relid, int64 next, bool iscalled) int64 maxv, minv; - /* open and AccessShareLock sequence */ + /* open and lock sequence */ init_sequence(relid, &elm, &seqrel); if (pg_class_aclcheck(elm->relid, GetUserId(), ACL_UPDATE) != ACLCHECK_OK) @@ -1042,15 +1045,15 @@ setval3_oid(PG_FUNCTION_ARGS) /* - * Open the sequence and acquire AccessShareLock if needed + * Open the sequence and acquire lock if needed * * If we haven't touched the sequence already in this transaction, - * we need to acquire AccessShareLock. We arrange for the lock to + * we need to acquire a lock. We arrange for the lock to * be owned by the top transaction, so that we don't need to do it * more than once per xact. */ static Relation -open_share_lock(SeqTable seq) +lock_and_open_sequence(SeqTable seq) { LocalTransactionId thislxid = MyProc->lxid; @@ -1063,7 +1066,7 @@ open_share_lock(SeqTable seq) PG_TRY(); { CurrentResourceOwner = TopTransactionResourceOwner; - LockRelationOid(seq->relid, AccessShareLock); + LockRelationOid(seq->relid, RowExclusiveLock); } PG_CATCH(); { @@ -1078,7 +1081,7 @@ open_share_lock(SeqTable seq) seq->lxid = thislxid; } - /* We now know we have AccessShareLock, and can safely open the rel */ + /* We now know we have the lock, and can safely open the rel */ return relation_open(seq->relid, NoLock); } @@ -1135,7 +1138,7 @@ init_sequence(Oid relid, SeqTable *p_elm, Relation *p_rel) /* * Open the sequence relation. */ - seqrel = open_share_lock(elm); + seqrel = lock_and_open_sequence(elm); if (seqrel->rd_rel->relkind != RELKIND_SEQUENCE) ereport(ERROR, @@ -1217,6 +1220,30 @@ read_seq_tuple(Relation rel, Buffer *buf, HeapTuple seqdatatuple) } /* + * Check the sequence options list and return the appropriate lock level for + * ALTER SEQUENCE. + * + * Most sequence option changes require a self-exclusive lock and should block + * concurrent nextval() et al. But RESTART does not, because it's not + * transactional. Also take a lower lock if no option at all is present. + */ +static LOCKMODE +alter_sequence_get_lock_level(List *options) +{ + ListCell *option; + + foreach(option, options) + { + DefElem *defel = (DefElem *) lfirst(option); + + if (strcmp(defel->defname, "restart") != 0) + return ShareRowExclusiveLock; + } + + return RowExclusiveLock; +} + +/* * init_params: process the options list of CREATE or ALTER SEQUENCE, and * store the values into appropriate fields of seqform, for changes that go * into the pg_sequence catalog, and seqdataform for changes to the sequence @@ -1850,7 +1877,7 @@ pg_sequence_last_value(PG_FUNCTION_ARGS) bool is_called; int64 result; - /* open and AccessShareLock sequence */ + /* open and lock sequence */ init_sequence(relid, &elm, &seqrel); if (pg_class_aclcheck(relid, GetUserId(), ACL_SELECT | ACL_USAGE) != ACLCHECK_OK) diff --git a/src/test/isolation/expected/sequence-ddl.out b/src/test/isolation/expected/sequence-ddl.out new file mode 100644 index 00000000000..6b7119738ff --- /dev/null +++ b/src/test/isolation/expected/sequence-ddl.out @@ -0,0 +1,85 @@ +Parsed test spec with 2 sessions + +starting permutation: s1alter s1commit s2nv +step s1alter: ALTER SEQUENCE seq1 MAXVALUE 10; +step s1commit: COMMIT; +step s2nv: SELECT nextval('seq1') FROM generate_series(1, 15); +ERROR: nextval: reached maximum value of sequence "seq1" (10) + +starting permutation: s1alter s2nv s1commit +step s1alter: ALTER SEQUENCE seq1 MAXVALUE 10; +step s2nv: SELECT nextval('seq1') FROM generate_series(1, 15); <waiting ...> +step s1commit: COMMIT; +step s2nv: <... completed> +error in steps s1commit s2nv: ERROR: nextval: reached maximum value of sequence "seq1" (10) + +starting permutation: s2begin s2nv s1alter2 s2commit s1commit +step s2begin: BEGIN; +step s2nv: SELECT nextval('seq1') FROM generate_series(1, 15); +nextval + +1 +2 +3 +4 +5 +6 +7 +8 +9 +10 +11 +12 +13 +14 +15 +step s1alter2: ALTER SEQUENCE seq1 MAXVALUE 20; <waiting ...> +step s2commit: COMMIT; +step s1alter2: <... completed> +step s1commit: COMMIT; + +starting permutation: s1restart s2nv s1commit +step s1restart: ALTER SEQUENCE seq1 RESTART WITH 5; +step s2nv: SELECT nextval('seq1') FROM generate_series(1, 15); +nextval + +5 +6 +7 +8 +9 +10 +11 +12 +13 +14 +15 +16 +17 +18 +19 +step s1commit: COMMIT; + +starting permutation: s2begin s2nv s1restart s2commit s1commit +step s2begin: BEGIN; +step s2nv: SELECT nextval('seq1') FROM generate_series(1, 15); +nextval + +1 +2 +3 +4 +5 +6 +7 +8 +9 +10 +11 +12 +13 +14 +15 +step s1restart: ALTER SEQUENCE seq1 RESTART WITH 5; +step s2commit: COMMIT; +step s1commit: COMMIT; diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index fc1918dedc8..32c965b2a02 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -58,6 +58,7 @@ test: alter-table-1 test: alter-table-2 test: alter-table-3 test: create-trigger +test: sequence-ddl test: async-notify test: vacuum-reltuples test: timeouts diff --git a/src/test/isolation/specs/sequence-ddl.spec b/src/test/isolation/specs/sequence-ddl.spec new file mode 100644 index 00000000000..42ee3b06151 --- /dev/null +++ b/src/test/isolation/specs/sequence-ddl.spec @@ -0,0 +1,39 @@ +# Test sequence usage and concurrent sequence DDL + +setup +{ + CREATE SEQUENCE seq1; +} + +teardown +{ + DROP SEQUENCE seq1; +} + +session "s1" +setup { BEGIN; } +step "s1alter" { ALTER SEQUENCE seq1 MAXVALUE 10; } +step "s1alter2" { ALTER SEQUENCE seq1 MAXVALUE 20; } +step "s1restart" { ALTER SEQUENCE seq1 RESTART WITH 5; } +step "s1commit" { COMMIT; } + +session "s2" +step "s2begin" { BEGIN; } +step "s2nv" { SELECT nextval('seq1') FROM generate_series(1, 15); } +step "s2commit" { COMMIT; } + +permutation "s1alter" "s1commit" "s2nv" + +# Prior to PG10, the s2nv would see the uncommitted s1alter change, +# but now it waits. +permutation "s1alter" "s2nv" "s1commit" + +# nextval doesn't release lock until transaction end, so s1alter2 has +# to wait for s2commit. +permutation "s2begin" "s2nv" "s1alter2" "s2commit" "s1commit" + +# RESTART is nontransactional, so s2nv sees it right away +permutation "s1restart" "s2nv" "s1commit" + +# RESTART does not wait +permutation "s2begin" "s2nv" "s1restart" "s2commit" "s1commit" |