PG_CATCH used without PG_RETHROW

Started by Bruce Momjianover 10 years ago3 messageshackers
Jump to latest
#1Bruce Momjian
bruce@momjian.us

My understanding is that PG_TRY/PG_CATCH doesn't save enough state to
avoid rethrowing errors and if you want to actually continue the
transaction you must use a subtransaction. As a result I was under the
impression it was mandatory to PG_RETHROW as a result.

If that's the case then I think I just came across a bug in
utils/adt/xml.c where there's no PG_RETHROW:

/*
* Functions for checking well-formed-ness
*/

#ifdef USE_LIBXML
static bool
wellformed_xml(text *data, XmlOptionType xmloption_arg)
{
bool result;
volatile xmlDocPtr doc = NULL;

/* We want to catch any exceptions and return false */
PG_TRY();
{
doc = xml_parse(data, xmloption_arg, true, GetDatabaseEncoding());
result = true;
}
PG_CATCH();
{
FlushErrorState();
result = false;
}
PG_END_TRY();

if (doc)
xmlFreeDoc(doc);

return result;
}
#endif

--
greg

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#1)
Re: PG_CATCH used without PG_RETHROW

Greg Stark <stark@mit.edu> writes:

My understanding is that PG_TRY/PG_CATCH doesn't save enough state to
avoid rethrowing errors and if you want to actually continue the
transaction you must use a subtransaction. As a result I was under the
impression it was mandatory to PG_RETHROW as a result.

If that's the case then I think I just came across a bug in
utils/adt/xml.c where there's no PG_RETHROW:

The reason we think that's OK is that we assume libxml2 does not call back
into the general backend code, so there is no PG state we'd have to undo.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3正华吕
kainwen@gmail.com
In reply to: Tom Lane (#2)
Re: PG_CATCH used without PG_RETHROW

Hi, I think this is a bug because this function's CATCH clause does not
restore memroycontext. Thus when leaving the function, the
CurrentMemroyContext
will be ErrorMemroyContext.

I see this part of code is refactored in master branch, but in
REL_16_STABLE, the code
is still there.

The following SQL might lead to PANIC (reference to memory that just reset)

zlyu=# select version();
version
-------------------------------------------------------------------------------------------------------------
PostgreSQL 15.3 on aarch64-unknown-linux-gnu, compiled by gcc (Ubuntu
11.4.0-1ubuntu1~22.04) 11.4.0, 64-bit
(1 row)

zlyu=# select xml_is_well_formed('<a>') or
array_length(array_append(array[random(), random()], case when
xml_is_well_formed('<a>') then random() else random() end), 1) > 1;
ERROR: cache lookup failed for type 2139062143
zlyu=#

Simple fix is to backport the change from master branch or restore the
context.

Tom Lane <tgl@sss.pgh.pa.us> 于2023年8月2日周三 16:32写道:

Show quoted text

Greg Stark <stark@mit.edu> writes:

My understanding is that PG_TRY/PG_CATCH doesn't save enough state to
avoid rethrowing errors and if you want to actually continue the
transaction you must use a subtransaction. As a result I was under the
impression it was mandatory to PG_RETHROW as a result.

If that's the case then I think I just came across a bug in
utils/adt/xml.c where there's no PG_RETHROW:

The reason we think that's OK is that we assume libxml2 does not call back
into the general backend code, so there is no PG state we'd have to undo.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers