Re: ALTER TABLE lock strength reduction patch is unsafe

On 2014-02-26 15:15:00 +0000, Simon Riggs wrote:
> On 26 February 2014 13:38, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> > Hi,
> >
> > On 2014-02-26 07:32:45 +0000, Simon Riggs wrote:
> >> > * This definitely should include isolationtester tests actually
> >> > performing concurrent ALTER TABLEs. All that's currently there is
> >> > tests that the locklevel isn't too high, but not that it actually works.
> >>
> >> There is no concurrent behaviour here, hence no code that would be
> >> exercised by concurrent tests.
> >
> > Huh? There's most definitely new concurrent behaviour. Previously no
> > other backends could have a relation open (and locked) while it got
> > altered (which then sends out relcache invalidations). That's something
> > that should be tested.
>
> It has been. High volume concurrent testing has been performed, per
> Tom's original discussion upthread, but that's not part of the test
> suite.

Yea, that's not what I am looking for.

> For other tests I have no guide as to how to write a set of automated
> regression tests. Anything could cause a failure, so I'd need to write
> an infinite set of tests to prove there is no bug *somewhere*. How
> many tests are required? 0, 1, 3, 30?

I think some isolationtester tests for the most important changes in
lock levels are appropriate. Say, create a PRIMARY KEY, DROP INHERIT,
... while a query is in progress in a nother session.

> >> > * Why does ChangeOwner need AEL?
> >>
> >> Ownership affects privileges, which includes SELECTs, hence AEL.
> >
> > So?
>
> That reply could be added to any post. Please explain your concern.

I don't understand why that means it needs an AEL. After all,
e.g. changes in table inheritance do *not* require an AEL. I think it's
perfectly ok to not go for the minimally required locklevel for all
subcommands, but then it should be commented as such and not with
"change visible to SELECT" where other operations that do so as well
require lower locklevels.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services