> On Oct 11, 2021, at 10:10 AM, Peter Geoghegan <p...@bowt.ie> wrote:
>
> This mostly looks good to me. Just one thing occurs to me: I suspect
> that we don't need to call pg_is_in_recovery() from SQL at all. What's
> wrong with just letting verify_heapam() (the C function from amcheck
> proper) show those notice messages where appropriate?
I thought a big part of the debate upthread was over exactly this point, that
pg_amcheck should not attempt to check (a) temporary relations, (b) indexes
that are invalid or unready, and (c) unlogged relations during recovery.
> In general I don't like the idea of making the behavior of pg_amcheck
> conditioned on the state of the system (e.g., whether we're in
> recovery) -- we should just let amcheck throw "invalid option" type
> errors when that's the logical outcome (e.g., when --parent-check is
> used on a replica). To me this seems rather different than not
> checking temporary tables, because that's something that inherently
> won't work. (Also, I consider the index-is-being-built stuff to be
> very similar to the temp table stuff -- same basic situation.)
I don't like having pg_amcheck parse the error message that comes back from
amcheck. If amcheck throws an error, pg_amcheck considers that a failure and
ultimately exists with a non-zero status. So, if we're going to have amcheck
handle these cases, it will have to be with a NOTICE (or perhaps a WARNING)
rather than an error. That's not what happens now, but if you'd rather we
fixed this problem that way, I can go do that, or perhaps as the author of the
bt_*_check functions, you can do that and I can just do the pg_amcheck changes.
How shall we proceed?
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
