From: | Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Justin Pryzby <pryzby(at)telsasoft(dot)com>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: list_free() in index_get_partition() |
Date: | 2020-11-06 23:40:05 |
Message-ID: | 20201106234005.GA23227@alvherre.pgsql |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2020-Nov-06, Michael Paquier wrote:
> 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.
Agreed; I'll get this pushed now. Thanks for reporting.
> > 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.
Yeah, I'm not so sure either. I have memories of purposefully not using
get_rel_relispartition there, but I don't remember exactly why and
it's not in the archives.
From | Date | Subject | |
---|---|---|---|
Next Message | Tomas Vondra | 2020-11-06 23:45:29 | Re: Fix brin_form_tuple to properly detoast data |
Previous Message | Daniel Gustafsson | 2020-11-06 23:11:15 | Re: Support for NSS as a libpq TLS backend |