Re: pg_get_indexdef() modification to use TxnSnapshot

On Fri, May 26, 2023 at 6:55 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> I have attempted to convert pg_get_indexdef() to use
> systable_beginscan() based on transaction-snapshot rather than using
> SearchSysCache(). The latter does not have any old info and thus
> provides only the latest info as per the committed txns, which could
> result in errors in some scenarios. One such case is mentioned atop
> pg_dump.c. The patch is an attempt to fix the same pg_dump's issue.
> Any feedback is welcome.

Even only in pg_get_indexdef_worker(), there are still many places
where we use syscache lookup: generate_relation_name(),
get_relation_name(), get_attname(), get_atttypetypmodcoll(),
get_opclass_name() etc.

>
> There is a long list of pg_get_* functions which use SearchSysCache()
> and thus may expose similar issues. I can give it a try to review the
> possibility of converting all of them. Thoughts?
>

it would be going to be a large refactoring and potentially make the
future implementations such as pg_get_tabledef() etc hard. Have you
considered changes to the SearchSysCache() family so that they
internally use a transaction snapshot that is registered in advance.
Since we are already doing similar things for catalog lookup in
logical decoding, it might be feasible. That way, the changes to
pg_get_XXXdef() functions would be much easier.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com