On Tue, 13 Jun 2023 at 11:49, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Fri, May 26, 2023 at 6:55 PM shveta malik <shveta.malik@gmail.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.
I feel registering an active snapshot before accessing the system
catalog as suggested by Sawada-san will be a better fix to solve the
problem. If this approach is fine, we will have to similarly fix the
other pg_get_*** functions like pg_get_constraintdef,
pg_get_function_arguments, pg_get_function_result,
pg_get_function_identity_arguments, pg_get_function_sqlbody,
pg_get_expr, pg_get_partkeydef, pg_get_statisticsobjdef and
pg_get_triggerdef.
The Attached patch has the changes for the same.
Regards,
Vignesh