On 2017-05-24 10:52:37 -0400, Robert Haas wrote:
> On Wed, May 24, 2017 at 10:32 AM, Andres Freund <andres@anarazel.de> wrote:
> > Well, but then we should just remove minval/maxval if we can't rely on
> > it.
>
> That seems like a drastic overreaction to me.
Well, either they work, or they don't. But since it turns out to be
easy enough to fix anyway...
> > I wonder if that's not actually very little new code, and I think we
> > might end up regretting having yet another inconsistent set of semantics
> > in v10, which we'll then end up changing again in v11.
>
> I'm not exercised enough about it to spend time on it or to demand
> that Peter do so, but feel free to propose something.
This turns out to be fairly simple patch. We already do precisely what
I describe when resetting a sequence for TRUNCATE, so it's an already
tested codepath. It also follows a lot more established practice around
transactional schema changes, so I think that's architecturally better
too. Peter, to my understanding, agreed with that reasoning at pgcon.
I just have two questions:
1) We've introduced, in 3d092fe540, infrastructure to avoid unnecessary catalog updates, in an attemt to fix the
"concurrentlyupdated" error. But that turned out to not be sufficient anyway, and it bulks up the code. I'd vote for
justremoving that piece of logic, it doesn't buy us anything meaningful, and it's bulky.
2) There's currently logic that takes a lower level lock for ALTER SEQUENCE RESET without other options. I think
that'sa bit confusing with the proposed change, because then it's unclear when ALTER SEQUENCE is transactional and
whennot. I think it's actually a lot easier to understand if we keep nextval()/setval() as non-transactional, and
ALTERSEQUENCE as transactional.
Comments?
- Andres