On Mon, Oct 11, 2021 at 9:53 AM Mark Dilger
<mark.dilger@enterprisedb.com> wrote:
> > Right. I never meant anything like making a would-be
> > bt_index_parent_check() call into a bt_index_check() call, just
> > because of the state of the system (e.g., it's in recovery). That
> > seems awful, in fact.
>
> Please find attached the latest version of the patch which includes the changes we discussed.
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?
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.)
--
Peter Geoghegan
