On Tue, Jan 29, 2019 at 4:03 AM Andres Freund <and...@anarazel.de> wrote: > On 2019-01-29 04:00:19 +0300, Alexander Korotkov wrote: > > + /* > > + * It is safe to use here PG_TRY/PG_CATCH without subtransaction > > because > > + * no function called inside performs data modification. > > + */ > > + PG_TRY(); > > + { > > + res = DirectFunctionCall2(func, ldatum, rdatum); > > + } > > + PG_CATCH(); > > + { > > + int errcode = geterrcode(); > > + > > + if (jspThrowErrors(cxt) || > > + ERRCODE_TO_CATEGORY(errcode) != > > ERRCODE_DATA_EXCEPTION) > > + PG_RE_THROW(); > > + > > + MemoryContextSwitchTo(mcxt); > > + FlushErrorState(); > > + > > + return jperError; > > + } > > + PG_END_TRY(); > > FWIW, I still think this is a terrible idea and shouldn't be merged this > way. The likelihood of introducing subtle bugs seems way too high - even > if it's possibly not buggy today, who says that it's not going to be in > the future?
I'm probably not yet understanding all the risks this code have. So far I see: 1) One of functions called here performs database modification, while it wasn't suppose to. So, it becomes not safe to skip subtransaction. 2) ERRCODE_DATA_EXCEPTION was thrown for unexpected reason. So, it might appear that ERRCODE_DATA_EXCEPTION is not safe to ignore. Could you complete this list? ------ Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company