Re: BUG #11638: Transaction safety fails when constraints are dropped and analyze is done

On 2014-10-21 22:12:29 -0400, Tom Lane wrote:
> I don't much care for this patch.  Aside from cosmetic issues like having
> named the new argument backwards and failed to update the function header
> comment that the patch largely invalidates, it seems to me to be likely
> to have unforeseen side effects, in that there may now be assumptions
> elsewhere that we don't force a pg_class update for this type of change.
> The implications are particularly ticklish for pg_class itself...

I'm unconvinced that that's a problem. We already do transactional
updates of pg_class for stuff like CREATE INDEX - while the relation is
*not* locked exclusively.  And e.g. CLUSTER pg_class does transactional
updates of pg_class itself, without problems afaics. The latter is under
an exclusive lock admittedly.

What's the problem you're suspecting?

Even if it doesn't arrise to the level of data corruption, I suspect in
many cases updating the stats nontransactionally in an later aborted
transaction will surprise some users. The normal reason for doing a
ANALYZE in a transaction is that you changed the data dramatically.

> I think that a better answer is to continue to do this update
> nontransactionally, but to not let the code clear relhasindex etc
> if we're inside a transaction block.  It is certainly safe to put
> off clearing those flags if we're not sure that we're seeing a
> committed state of the table's schema.

That'd fix the corruption, so it'd certainly be an improvement. But I'm
unconvinced that it's the best way forward.

> An interesting question is whether it is ever possible for this function
> to be told to *set* relhasindex when it was clear (or likewise for the
> other flags).  Offhand I would say that that should never happen, because
> certainly neither VACUUM nor ANALYZE should be creating indexes etc.
> Should we make it throw an error if that happens, or just go ahead and
> apply the update, assuming that it's correcting somehow-corrupted data?

It probably should at least LOG/WARN, to make it clear that things were
in a bad shape. It might be helpful to allow VACUUM/ANALYZE to fix
existing corrupted relhas* flags. Although there could already be
existing corruption, in which case users might want to get alerted of
that more aggressively.

Greetings,

Andres Freund

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