Re: [HACKERS] Concurrent ALTER SEQUENCE RESTART Regression

On Mon, May 22, 2017 at 11:42 AM, Andres Freund <andres(at)anarazel(dot)de> wrote:
> Ooops.
>
> Two issues: Firstly, we get a value smaller than seqmin, obviously
> that's not ok. But even if we'd error out, it'd imo still not be ok,
> because we have a command that behaves partially transactionally
> (keeping the seqmax/min transactionally), partially not (keeping the
> current sequence state at -9).

I don't really agree that this is broken. In 9.6, the update appears
to be fully non-transactional, which you could argue is more
consistent, but I don't think I'd agree. In other cases where we
can't perform an operation transactionally, we call
PreventTransactionChain(), but in 9.6, ALTER SEQUENCE oobounds
MAXVALUE -10 START -10 MINVALUE -1000 INCREMENT BY -1 RESTART seems to
be allowed in a transaction but a subsequent ROLLBACK has no effect,
which seems fairly awful.

I think it's important to keep in mind that nextval() more or less has
to be non-transactional. From a certain point of view, that's why we
have sequences. If nextval() didn't notice actions performed by
uncommitted transactions, then sequences wouldn't deliver unique
values; if it blocked waiting to see whether the other transaction
committed and didn't return a value until it either committed or
aborted, then sequences would have terrible performance. However,
just because nextval() has to be non-transactional doesn't mean that
ALL sequence operations have to be non-transactional.

I don't really know whether the behavior Peter has chosen here is the
best possible choice, so I'm not intending to mount any sort of
vigorous defense of it. However, this thread started with the
complaint about occasional "ERROR: tuple concurrently updated"
messages; in my opinion, any such behavior in new code is
unacceptable, and now it's been fixed. "Totally broken" != "not the
behavior I prefer".

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company