Re: pg_get_indexdef() modification to use TxnSnapshot - Mailing list pgsql-hackers

From vignesh C
Subject Re: pg_get_indexdef() modification to use TxnSnapshot
Date
Msg-id CALDaNm2xwHc798aiZf7cvMEQ4iQcEVHSrU0cSoWyBvD8vSgDJw@mail.gmail.com
Whole thread Raw
In response to pg_get_indexdef() modification to use TxnSnapshot  (shveta malik <shveta.malik@gmail.com>)
List pgsql-hackers
On Fri, 26 May 2023 at 15:25, 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.
>
> 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?

I could reproduce this issue in HEAD with pg_dump dumping a database
having a table and an index like:
create table t1(c1 int);
create index idx1 on t1(c1);

Steps to reproduce:
a) ./pg_dump -d postgres -f dump.txt -- Debug this statement and hold
a breakpoint at getTables function just before it takes lock on the
table t1 b) when the breakpoint is hit, drop the index idx1 c)
Continue the pg_dump execution after dropping the index d) pg_dump
calls pg_get_indexdef to get the index definition e) since
pg_get_indexdef->pg_get_indexdef uses SearchSysCache1 which uses the
latest transaction, it will not get the index information as it sees
the latest catalog snapshot(it will not get the index information). e)
pg_dump will get an empty index statement in this case like:
---------------------------------------------------------------------------------------------
--
-- Name: idx; Type: INDEX; Schema: public; Owner: vignesh
--

;
---------------------------------------------------------------------------------------------

This issue is solved using shveta's patch as it registers the
transaction snapshot and calls systable_beginscan which will not see
the transactions that were started after pg_dump's transaction(the
drop index will not be seen) and gets the index definition as expected
like:
---------------------------------------------------------------------------------------------
--
-- Name: idx; Type: INDEX; Schema: public; Owner: vignesh
--

CREATE INDEX idx ON public.t1 USING btree (c1);
---------------------------------------------------------------------------------------------

Regards,
Vignesh



pgsql-hackers by date:

Previous
From: shveta malik
Date:
Subject: pg_get_indexdef() modification to use TxnSnapshot
Next
From: Alvaro Herrera
Date:
Subject: Re: PG 16 draft release notes ready