Re: pg_get_indexdef() modification to use TxnSnapshot

From: vignesh C <vignesh21(at)gmail(dot)com>
To: shveta malik <shveta(dot)malik(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: pg_get_indexdef() modification to use TxnSnapshot
Date: 2023-05-26 10:14:29
Message-ID: CALDaNm2xwHc798aiZf7cvMEQ4iQcEVHSrU0cSoWyBvD8vSgDJw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, 26 May 2023 at 15:25, 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.
>
> 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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2023-05-26 10:21:23 Re: PG 16 draft release notes ready
Previous Message shveta malik 2023-05-26 09:54:42 pg_get_indexdef() modification to use TxnSnapshot