Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On May 24, 2017 6:57:08 AM EDT, Robert Haas <robertmhaas@gmail.com> wrote:
>On Tue, May 23, 2017 at 11:25 PM, Andres Freund <andres@anarazel.de>
>wrote:
>> On 2017-05-23 22:47:07 -0400, Robert Haas wrote:
>>> On Mon, May 22, 2017 at 11:42 AM, Andres Freund <andres@anarazel.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.
>>
>> Just a quick clarification question: You did notice that nextval() in
>S1
>> after the rollback returned -9, despite seqmin being 0?  I can see
>> erroring out being acceptable, but returning flat out wrong
>values....?
>
>I did see that.  I'm unclear what you think it should do instead.  I
>mean, it could notice that cur_val < min_val and return min_val, but
>that doesn't have a very good chance of being correct either.  I
>suppose the aborted transaction could try to restore the old cur_val
>when it rolls back, but that's clearly the wrong thing when there's no
>DDL involved (plus I'm not sure it would even be safe to try to do
>that).  Do you have an idea?

At the very least we'll have to error out. That's still not nice usability wise, but it sure beats returning flat out
wrongvalues. 

I suspect that the proper fix would be to use a different relfilenode after ddl, when changing the seq file itself
(I.e.setval and restart).  That seems like it'd be architecturally more appropriate, but also some work. 

Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.