Re: segmentation fault using currtid and partitioned tables

Michael Paquier <michael@paquier.xyz>

From: Michael Paquier <michael@paquier.xyz>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Jaime Casanova <jaime.casanova@2ndquadrant.com>, Pg Hackers <pgsql-hackers@postgresql.org>, Andres Freund <andres@anarazel.de>
Date: 2020-05-25T09:29:10Z
Lists: pgsql-hackers
On Fri, May 22, 2020 at 07:32:57PM -0400, Alvaro Herrera wrote:
> I don't know, but this stuff is so unused that your patch seems
> excessive ... and I think we'd rather not backpatch something so large.
> I propose we do something less invasive in the backbranches, like just
> throw elog() errors (nothing fancy) where necessary to avoid the
> crashes.  Even for pg12 it seems that that should be sufficient.

Even knowing that those trigger a bunch of elog()s which are not
something that should be user-triggerable?  :)

Perhaps you are right though, and that we don't need to spend this
much energy into improving the error messages so I am fine to discard
this part.  At the end, in order to remove the crashes, you just need
to keep around the two RELKIND_HAS_STORAGE() checks.  But I would
rather keep these two to use ereport(ERRCODE_FEATURE_NOT_SUPPORTED)
instead of elog(), and keep the test coverage of the previous patch
(including the tests for the aggregates I noticed were missing).
Would you be fine with that?

> For pg13 and beyond, I liked Tom's idea of installing dummy functions
> for tables without storage -- that seems safer.

Not sure about that for v13.  That would be invasive post-beta.
--
Michael

Commits

  1. Fix use-after-release mistake in currtid() and currtid2() for views

  2. Fix crashes with currtid() and currtid2()