On Wed, Oct 10, 2018 at 12:00 AM Thomas Munro <thomas.mu...@enterprisedb.com> wrote: > So I suppose we should just remove it, with something like 0002. I'm > a bit uneasy about existing code out there that might be not calling > CFI. OTOH I suspect that a lot of code copied worker_spi.c and > installed its own handler.
I agree -- I think worker_spi.c has probably been copied a lot, and that's not a good thing. > Maybe src/test/modules/worker_spi.c shouldn't use approach #1 (even if > it might technically be OK for that code)? I think I might have been > guilty of copying that. +1. It's not *really* OK unless all of the SQL queries it executes are super-short and can't possibly block ... which I guess is never really true of any SQL queries, right? > > Maybe we could replace this by a general-purpose hook. So instead of > > the various tests for process types that are there now, we would just > > have > > > > if (procdie_hook != NULL) > > (*procdie_hook)(); > > > > And that hook can do whatever you like (but probably including dying). > > Ok, patch 0001 is a sketch like that, for discussion. I was assuming that we'd replace the existing message-selection logic with customer proc-die handlers. Also, I think we should not indicate in the comments that the handler is expected to proc_exit() or ereport(FATAL), because you might want to do something else there, then return and let the usual thing happen. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
