Marc Cousin <cousinmarc@gmail.com> writes:
>>> - Does the feature work as advertised?
>>>
>>> Yes. It works consistently, isn't fooled by savepoints or multiple
>>> serials in a table, or concurrent transactions
I think there's a rather nasty problem here, which is what to do with
the cached nextval/currval state. As submitted, the patch does the same
thing as ALTER SEQUENCE RESTART (to wit, clear any cached unissued
nextval values, but don't touch currval) at the time of resetting the
sequence. That's fine, but what if the transaction later rolls back?
The cached state is untouched by rollback, so if the transaction had
done any nextval()s meanwhile, the cache will be out of step with the
rolled-back sequence contents.
We never had to worry about this before because sequence operations
didn't roll back, by definition. If we're going to add a situation
where they do roll back, we need to consider the case.
I think we can arrange to clear cached unissued values on the next
attempt to nextval() the sequence, by dint of adding the relfilenode
to SeqTable entries and clearing cached state whenever we note that
it doesn't match the current relfilenode of the sequence. However,
I'm unsure what ought to happen to currval. It doesn't seem too
practical to try to roll it back to its pre-transaction value.
Should we leave it alone (ie, possibly reflecting a value that was
assigned inside the failed transaction)? The other alternative would
be to clear it as though nextval had never been issued at all in the
session.
Thoughts?
regards, tom lane