Re: list_free() in index_get_partition() - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: list_free() in index_get_partition()
Date
Msg-id 20201106003235.GC21123@paquier.xyz
Whole thread Raw
In response to list_free() in index_get_partition()  (Justin Pryzby <pryzby@telsasoft.com>)
Responses Re: list_free() in index_get_partition()  (Alvaro Herrera <alvherre@alvh.no-ip.org>)
List pgsql-hackers
On Thu, Nov 05, 2020 at 02:36:06PM -0600, Justin Pryzby wrote:
> Seems to be missing.

Any code paths calling index_get_partition() is in tablecmds.c,
involving commands that are not that hot with such lookups, but that
could be an issue if this gets called by some out-of-core code if
memory context cleanup is sloppy.  So I agree to fix this one and
backpatch down to 12.  I'd like to apply your fix, except if Alvaro
thinks differently as that's his commit originally.  So let's wait a
bit first.

> The 2nd patch does some more cleanup - Before, a failed syscache lookup would
> ERROR, but I don't think that's supposed to happen.  get_rel_relispartition()
> would instead return false, and we won't call get_partition_parent().

The cache lookup error is here as a safeguard to remind that any code
path calling index_get_partition() needs a proper lock on the
relations involved before doing such lookups, so that's a defensive
move.  By switching the code as you do in 0002, you could mask such
logical problems as different errors could get raised.  So I don't
think that it is a good idea.
--
Michael

Attachment

pgsql-hackers by date:

Previous
From: Kyotaro Horiguchi
Date:
Subject: Re: shared-memory based stats collector
Next
From: "David G. Johnston"
Date:
Subject: Re: Why does to_json take "anyelement" rather than "any"?