> On Oct 11, 2021, at 10:10 AM, Peter Geoghegan <pg@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_amcheckconsiders that a failure and ultimately exists with a non-zero status. So, if we're going to have amcheck
handlethese cases, it will have to be with a NOTICE (or perhaps a WARNING) rather than an error. That's not what
happensnow, but if you'd rather we fixed this problem that way, I can go do that, or perhaps as the author of the
bt_*_checkfunctions, 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
