Skip to content

Commit d708c4c

Browse files
committed
Don't capture hard errors for old cagg format
If an error occurs inside the deserialization function for continuous aggregates, we are using the old numeric format and this was captured inside `inner_agg_deserialize` and the format was "repaired". However, this captured hard errors as well, which can cause cascading errors if execution continues after the error. This commit fixes that by capturing data exception errors, internal error (which is the default for elog()) and protocol error (c.f. `pq_getmsgint64` used by `numeric_deserialize`), but re-throwing all other errors.
1 parent 4653c72 commit d708c4c

File tree

2 files changed

+40
-7
lines changed

2 files changed

+40
-7
lines changed

.unreleased/pr_7903

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fixes: #7903 Don't capture hard errors for old cagg format

tsl/src/partialize_finalize.c

+39-7
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include <utils/builtins.h>
1717
#include <utils/datum.h>
1818
#include <utils/fmgroids.h>
19+
#include <utils/resowner.h>
1920
#include <utils/syscache.h>
2021

2122
#include "partialize_finalize.h"
@@ -260,24 +261,55 @@ inner_agg_deserialize(FACombineFnMeta *combine_meta, bytea *volatile serialized_
260261
deser_fcinfo->isnull = false;
261262

262263
/*
263-
* When an exception is thrown and longjmp() is called, CurrentMemoryContext is potentially
264-
* different than what it was inside the PG_TRY() block below.
264+
* When an exception is thrown and longjmp() is called,
265+
* CurrentMemoryContext is potentially different than what it was
266+
* inside the PG_TRY() block below.
265267
*
266-
* Restore it to the old value so that the code in the subsequent PG_CATCH() block does not
267-
* corrupt the memory.
268+
* Restore it to the old value so that the code in the subsequent
269+
* PG_CATCH() block does not corrupt the memory.
268270
*
269-
* No need for volatile variables since we don't modify any of this function's stack frame
270-
* inside the PG_TRY() block.
271+
* No need for volatile variables since we don't modify any of this
272+
* function's stack frame inside the PG_TRY() block.
273+
*
274+
* We create a subtransaction to run the functions below, in case they
275+
* abort.
271276
*/
272277
MemoryContext oldcontext = CurrentMemoryContext;
278+
ResourceOwner oldowner = CurrentResourceOwner;
279+
BeginInternalSubTransaction(NULL);
273280
PG_TRY();
274281
{
275282
deserialized = FunctionCallInvoke(deser_fcinfo);
283+
ReleaseCurrentSubTransaction();
284+
MemoryContextSwitchTo(oldcontext);
285+
CurrentResourceOwner = oldowner;
276286
}
277287
PG_CATCH();
278288
{
279-
CurrentMemoryContext = oldcontext;
289+
const int sqlerrcode = geterrcode();
290+
291+
/*
292+
* If we get a data exception category error or a protocol
293+
* violation or an internal error (which is generated by default
294+
* when using elog()), the format is wrong and we can attempt to
295+
* repair it by switching to a different function for
296+
* deserializing.
297+
*
298+
* If the error is an insufficient resources category we should
299+
* *not* continue executing.
300+
*
301+
* Errors in other categories are not expected, but we cannot do
302+
* anything with them anyway, so just re-throw them too.
303+
*/
304+
if (ERRCODE_TO_CATEGORY(sqlerrcode) != ERRCODE_DATA_EXCEPTION &&
305+
sqlerrcode != ERRCODE_PROTOCOL_VIOLATION && sqlerrcode != ERRCODE_INTERNAL_ERROR)
306+
{
307+
PG_RE_THROW();
308+
}
280309
FlushErrorState();
310+
RollbackAndReleaseCurrentSubTransaction();
311+
MemoryContextSwitchTo(oldcontext);
312+
CurrentResourceOwner = oldowner;
281313
/* attempt to repair the serialized partial */
282314
serialized_partial =
283315
sanitize_serialized_partial(combine_meta->deserialfnoid, serialized_partial);

0 commit comments

Comments
 (0)