Re: Review: rollback sequence reset for TRUNCATE ... RESTART IDENTITY

On 10-11-17 03:00 PM, Marc Cousin wrote:
> 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
>

I will look at addressing this in an updated patch.

>>
>> 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.
>

I agree  probably shouldn't be using curval after a failed transaction 
which is why having it return as if it hadn't been issued sounds like a 
more reasonable behaviour.  If an application tries a currval following 
the rollback then at least the application won't get a bogus value.  It 
is better to return an error than to let the application continuing on 
thinking it has a sequence value that won't be (or has not) been 
assigned to some other session.