Do PG_TRY() inside a subtransaction block.

If we don't propagate the errors we are catching in PG_CATCH(), database's
internal state might not be clean. So we do PG_TRY() inside a subtransaction
so we can rollback to it after catching errors.
pull/1751/head
Hadi Moshayedi 2017-10-31 21:37:00 -04:00 committed by Hadi Moshayedi
parent 9bfbbf8a04
commit 7691991cb5
1 changed files with 31 additions and 0 deletions

View File

@ -97,6 +97,12 @@ CollectBasicUsageStatistics(void)
bool metadataCollectionFailed = false; bool metadataCollectionFailed = false;
memset(&unameData, 0, sizeof(unameData)); memset(&unameData, 0, sizeof(unameData));
/*
* Start a subtransaction so we can rollback database's state to it in case
* of error.
*/
BeginInternalSubTransaction(NULL);
PG_TRY(); PG_TRY();
{ {
distTableOids = DistTableOidList(); distTableOids = DistTableOidList();
@ -106,6 +112,13 @@ CollectBasicUsageStatistics(void)
metadataJsonbDatum = DistNodeMetadata(); metadataJsonbDatum = DistNodeMetadata();
metadataJsonbStr = DatumGetCString(DirectFunctionCall1(jsonb_out, metadataJsonbStr = DatumGetCString(DirectFunctionCall1(jsonb_out,
metadataJsonbDatum)); metadataJsonbDatum));
/*
* Releasing a subtransaction doesn't free its memory context, since the
* data it contains will be needed at upper commit. See the comments for
* AtSubCommit_Memory() at postgres/src/backend/access/transam/xact.c.
*/
ReleaseCurrentSubTransaction();
} }
PG_CATCH(); PG_CATCH();
{ {
@ -114,6 +127,8 @@ CollectBasicUsageStatistics(void)
edata = CopyErrorData(); edata = CopyErrorData();
FlushErrorState(); FlushErrorState();
RollbackAndReleaseCurrentSubTransaction();
/* rethrow as WARNING */ /* rethrow as WARNING */
edata->elevel = WARNING; edata->elevel = WARNING;
ThrowErrorData(edata); ThrowErrorData(edata);
@ -210,17 +225,25 @@ CheckForUpdatesCallback(char *contents, size_t size, size_t count, void *userDat
StringInfo responseNullTerminated = makeStringInfo(); StringInfo responseNullTerminated = makeStringInfo();
appendBinaryStringInfo(responseNullTerminated, contents, size * count); appendBinaryStringInfo(responseNullTerminated, contents, size * count);
/*
* Start a subtransaction so we can rollback database's state to it in case
* of error.
*/
BeginInternalSubTransaction(NULL);
/* jsonb_in can throw errors */ /* jsonb_in can throw errors */
PG_TRY(); PG_TRY();
{ {
Datum responseCStringDatum = CStringGetDatum(responseNullTerminated->data); Datum responseCStringDatum = CStringGetDatum(responseNullTerminated->data);
Datum responseJasonbDatum = DirectFunctionCall1(jsonb_in, responseCStringDatum); Datum responseJasonbDatum = DirectFunctionCall1(jsonb_in, responseCStringDatum);
responseJsonb = DatumGetJsonb(responseJasonbDatum); responseJsonb = DatumGetJsonb(responseJasonbDatum);
ReleaseCurrentSubTransaction();
} }
PG_CATCH(); PG_CATCH();
{ {
MemoryContextSwitchTo(savedContext); MemoryContextSwitchTo(savedContext);
FlushErrorState(); FlushErrorState();
RollbackAndReleaseCurrentSubTransaction();
responseJsonb = NULL; responseJsonb = NULL;
} }
PG_END_TRY(); PG_END_TRY();
@ -287,18 +310,26 @@ JsonbFieldInt32(Jsonb *jsonb, const char *fieldName, int32 *result)
return false; return false;
} }
/*
* Start a subtransaction so we can rollback database's state to it in case
* of error.
*/
BeginInternalSubTransaction(NULL);
/* numeric_int4 can throw errors */ /* numeric_int4 can throw errors */
PG_TRY(); PG_TRY();
{ {
Datum resultNumericDatum = NumericGetDatum(fieldValue->val.numeric); Datum resultNumericDatum = NumericGetDatum(fieldValue->val.numeric);
*result = DatumGetInt32(DirectFunctionCall1(numeric_int4, resultNumericDatum)); *result = DatumGetInt32(DirectFunctionCall1(numeric_int4, resultNumericDatum));
success = true; success = true;
ReleaseCurrentSubTransaction();
} }
PG_CATCH(); PG_CATCH();
{ {
MemoryContextSwitchTo(savedContext); MemoryContextSwitchTo(savedContext);
FlushErrorState(); FlushErrorState();
success = false; success = false;
RollbackAndReleaseCurrentSubTransaction();
} }
PG_END_TRY(); PG_END_TRY();