Re: BUG #17212: pg_amcheck fails on checking temporary relations

> On Oct 11, 2021, at 10:10 AM, Peter Geoghegan <pg(at)bowt(dot)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