The Wednesday 17 November 2010 19:41:19, Tom Lane wrote :
> 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.
Yes, I completely missed testing with non default cache value. And it fails,
of course, some values are generated a second time twice after a rollback
>
> 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.
Should currval really be used after a failed transaction ? Right now, we can
have a value that has been generated inside a rollbacked transaction too. I'd
vote for leave it alone.