Re: [HACKERS] Concurrent ALTER SEQUENCE RESTART Regression

On May 24, 2017 6:57:08 AM EDT, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>On Tue, May 23, 2017 at 11:25 PM, Andres Freund <andres(at)anarazel(dot)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(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.
>>
>> 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 wrong values.

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.