On Mon, Oct 11, 2021 at 9:53 AM Mark Dilger <mark.dil...@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
