Skip to content

Commit 48a39f1

Browse files
authored
[OSS-ONLY]Revert Output clause changes in Executor to match with PG Returning (babelfish-for-postgresql#668)
We are dealing with a critical crash bug in Babelfish's EPQ (EvalPlanQual) handling during concurrent UPDATE operations with OUTPUT clauses. The crash occurs when two sessions simultaneously modify the same row, where the column is of type "varchar" causing tuple alignment problems during EPQ re-evaluation. This is a memory corruption issue where PostgreSQL's EPQ mechanism fails to properly handle tuple structure changes when Babelfish processes OUTPUT clauses, leading to segmentation faults or assertion failures. ### The Problem Babelfish was processing the OUTPUT clause (RETURNING clause) BEFORE EPQ evaluation, which created a race condition crash because: 1. Original Design Rationale: OUTPUT clause was moved before EPQ to ensure it executed before triggers 2. Race Condition: When EPQ re-evaluation occurred, the OUTPUT clause had already processed using stale tuple data 3. Crash Point: EPQ tried to re-evaluate with changed tuple structure, but OUTPUT processing was already complete with wrong memory references ### The Solution Align Babelfish OUTPUT clause execution with PostgreSQL's native RETURNING clause flow: ### Key Insights from Fix - [ ] Trigger Execution Order: Analysis revealed that OUTPUT clause doesn't actually need to run before triggers in Babelfish context - [ ] PostgreSQL Alignment: Following PostgreSQL's native RETURNING execution flow eliminates the race condition - [ ] EPQ Compatibility: Processing OUTPUT after EPQ ensures it always works with the correct, final tuple data - [ ] Memory Safety: Eliminates tuple alignment crashes by ensuring OUTPUT clause processes current data structure ### Related PR - Extension PR - babelfish-for-postgresql/babelfish_extensions#4347 Task: BABEL-4880 Signed-off-by: Herambh Shah <herambhs@amazon.com>
1 parent b90bc07 commit 48a39f1

File tree

1 file changed

+3
-56
lines changed

1 file changed

+3
-56
lines changed

src/backend/executor/nodeModifyTable.c

Lines changed: 3 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -827,10 +827,6 @@ ExecInsert(ModifyTableContext *context,
827827
return NULL; /* "do nothing" */
828828
}
829829

830-
/* Process RETURNING if present */
831-
if (resultRelInfo->ri_projectReturning && sql_dialect == SQL_DIALECT_TSQL)
832-
result = ExecProcessReturning(resultRelInfo, slot, planSlot);
833-
834830
/* INSTEAD OF ROW INSERT Triggers */
835831
if (resultRelInfo->ri_TrigDesc &&
836832
resultRelInfo->ri_TrigDesc->trig_insert_instead_row)
@@ -1220,7 +1216,7 @@ ExecInsert(ModifyTableContext *context,
12201216
ExecWithCheckOptions(WCO_VIEW_CHECK, resultRelInfo, slot, estate);
12211217

12221218
/* Process RETURNING if present */
1223-
if (resultRelInfo->ri_projectReturning && sql_dialect != SQL_DIALECT_TSQL)
1219+
if (resultRelInfo->ri_projectReturning)
12241220
result = ExecProcessReturning(resultRelInfo, slot, planSlot);
12251221

12261222
if (inserted_tuple)
@@ -1459,7 +1455,6 @@ ExecDelete(ModifyTableContext *context,
14591455
Relation resultRelationDesc = resultRelInfo->ri_RelationDesc;
14601456
TupleTableSlot *slot = NULL;
14611457
TM_Result result;
1462-
TupleTableSlot *rslot_output = NULL;
14631458

14641459
if (tupleDeleted)
14651460
*tupleDeleted = false;
@@ -1472,43 +1467,6 @@ ExecDelete(ModifyTableContext *context,
14721467
epqreturnslot, tmresult))
14731468
return NULL;
14741469

1475-
/* Process RETURNING if present and if requested */
1476-
if (processReturning && resultRelInfo->ri_projectReturning && sql_dialect == SQL_DIALECT_TSQL)
1477-
{
1478-
/*
1479-
* We have to put the target tuple into a slot, which means first we
1480-
* gotta fetch it. We can use the trigger tuple slot.
1481-
*/
1482-
if (resultRelInfo->ri_FdwRoutine)
1483-
{
1484-
/* FDW must have provided a slot containing the deleted row */
1485-
Assert(!TupIsNull(slot));
1486-
}
1487-
else
1488-
{
1489-
slot = ExecGetReturningSlot(estate, resultRelInfo);
1490-
if (oldtuple != NULL)
1491-
{
1492-
ExecForceStoreHeapTuple(oldtuple, slot, false);
1493-
}
1494-
else
1495-
{
1496-
if (!table_tuple_fetch_row_version(resultRelationDesc, tupleid,
1497-
SnapshotAny, slot))
1498-
elog(ERROR, "failed to fetch deleted tuple for DELETE RETURNING");
1499-
}
1500-
}
1501-
rslot_output = ExecProcessReturning(resultRelInfo, slot, context->planSlot);
1502-
1503-
/*
1504-
* Before releasing the target tuple again, make sure rslot has a
1505-
* local copy of any pass-by-reference values.
1506-
*/
1507-
ExecMaterializeSlot(rslot_output);
1508-
1509-
ExecClearTuple(slot);
1510-
}
1511-
15121470
if (resultRelInfo->ri_TrigDesc &&
15131471
resultRelInfo->ri_TrigDesc->trig_delete_instead_statement &&
15141472
sql_dialect == SQL_DIALECT_TSQL &&
@@ -1744,7 +1702,7 @@ ldelete:;
17441702
ExecDeleteEpilogue(context, resultRelInfo, tupleid, oldtuple, changingPart);
17451703

17461704
/* Process RETURNING if present and if requested */
1747-
if (processReturning && resultRelInfo->ri_projectReturning && sql_dialect != SQL_DIALECT_TSQL)
1705+
if (processReturning && resultRelInfo->ri_projectReturning)
17481706
{
17491707
/*
17501708
* We have to put the target tuple into a slot, which means first we
@@ -1785,9 +1743,6 @@ ldelete:;
17851743
return rslot;
17861744
}
17871745

1788-
if (processReturning && resultRelInfo->ri_projectReturning && rslot_output)
1789-
return rslot_output;
1790-
17911746
return NULL;
17921747
}
17931748

@@ -2352,7 +2307,6 @@ ExecUpdate(ModifyTableContext *context, ResultRelInfo *resultRelInfo,
23522307
Relation resultRelationDesc = resultRelInfo->ri_RelationDesc;
23532308
UpdateContext updateCxt = {0};
23542309
TM_Result result;
2355-
TupleTableSlot *rslot = NULL;
23562310

23572311
/*
23582312
* abort the operation if not running transactions
@@ -2367,10 +2321,6 @@ ExecUpdate(ModifyTableContext *context, ResultRelInfo *resultRelInfo,
23672321
if (!ExecUpdatePrologue(context, resultRelInfo, tupleid, oldtuple, slot, NULL))
23682322
return NULL;
23692323

2370-
/* Process RETURNING if present */
2371-
if (resultRelInfo->ri_projectReturning && sql_dialect == SQL_DIALECT_TSQL)
2372-
rslot = ExecProcessReturning(resultRelInfo, slot, context->planSlot);
2373-
23742324
if (resultRelInfo->ri_TrigDesc &&
23752325
resultRelInfo->ri_TrigDesc->trig_update_instead_statement &&
23762326
sql_dialect == SQL_DIALECT_TSQL && bbfViewHasInsteadofTrigger_hook && (bbfViewHasInsteadofTrigger_hook)(resultRelationDesc, CMD_UPDATE) &&
@@ -2594,11 +2544,8 @@ ExecUpdate(ModifyTableContext *context, ResultRelInfo *resultRelInfo,
25942544
slot);
25952545

25962546
/* Process RETURNING if present */
2597-
if (resultRelInfo->ri_projectReturning && sql_dialect != SQL_DIALECT_TSQL)
2598-
return ExecProcessReturning(resultRelInfo, slot, context->planSlot);
2599-
26002547
if (resultRelInfo->ri_projectReturning)
2601-
return rslot;
2548+
return ExecProcessReturning(resultRelInfo, slot, context->planSlot);
26022549

26032550
return NULL;
26042551
}

0 commit comments

Comments
 (0)