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.
