Hi, On 2019-06-11 10:49:28 -0400, Tom Lane wrote: > It doesn't reliably work to do so, and we have a project policy against > trying, and this code should never have been committed in this state.
I'll also note that I complained about this specific instance being introduced all the way back in 2013 and then again 2016: https://www.postgresql.org/message-id/20131118172748.GG20305%40awork2.anarazel.de On 2013-11-18 18:27:48 +0100, Andres Freund wrote: > * Why the PG_TRY/PG_CATCH in check_recovery_target_time? Besides being > really strangely formatted (multiline :? inside a function?) it > doesn't a) seem to be correct to ignore potential memory allocation > errors by just switching back into the context that just errored out, > and continue to work there b) forgo cleanup by just continuing as if > nothing happened. That's unlikely to be acceptable. > * You access recovery_target_name[0] unconditionally, although it's https://www.postgresql.org/message-id/20140123133424.GD29782%40awork2.anarazel.de On 2016-11-12 08:09:49 -0800, Andres Freund wrote: > > +static bool > > +check_recovery_target_time(char **newval, void **extra, GucSource source) > > +{ > > + TimestampTz time; > > + TimestampTz *myextra; > > + MemoryContext oldcontext = CurrentMemoryContext; > > + > > + PG_TRY(); > > + { > > + time = (strcmp(*newval, "") == 0) ? > > + 0 : > > + DatumGetTimestampTz(DirectFunctionCall3(timestamptz_in, > > + > > CStringGetDatum(*newval), > > + > > ObjectIdGetDatum(InvalidOid), > > + > > Int32GetDatum(-1))); > > + } > > + PG_CATCH(); > > + { > > + ErrorData *edata; > > + > > + /* Save error info */ > > + MemoryContextSwitchTo(oldcontext); > > + edata = CopyErrorData(); > > + FlushErrorState(); > > + > > + /* Pass the error message */ > > + GUC_check_errdetail("%s", edata->message); > > + FreeErrorData(edata); > > + return false; > > + } > > + PG_END_TRY(); > > Hm, I'm not happy to do that kind of thing. What if there's ever any > database interaction added to timestamptz_in? > > It's also problematic because the parsing of timestamps depends on the > timezone GUC - which might not yet have taken effect... I don't have particularly polite words about this. Greetings, Andres Freund
