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
