PG-1437 Fix race condition in the event trigger

jeltz · jeltz · commit b223d2d25433 · 2025-04-04T19:48:52.000+02:00
If we look up something about a relation in the command start event
trigger we need to hold onto that lock until the end of the command
and not release it so no other commd is able to change the data we
looked at between now and when the DDL command itself actually locks
the relation.
diff --git a/contrib/pg_tde/src/pg_tde_event_capture.c b/contrib/pg_tde/src/pg_tde_event_capture.c
@@ -122,7 +122,7 @@ pg_tde_ddl_command_start_capture(PG_FUNCTION_ARGS)
 	if (IsA(parsetree, IndexStmt))
 	{
 		IndexStmt  *stmt = (IndexStmt *) parsetree;
-		Oid			relationId = RangeVarGetRelid(stmt->relation, NoLock, true);
+		Oid			relationId = RangeVarGetRelid(stmt->relation, AccessShareLock, true);
 
 		validateCurrentEventTriggerState(true);
 		tdeCurrentCreateEvent.tid = GetCurrentFullTransactionId();
@@ -132,16 +132,16 @@ pg_tde_ddl_command_start_capture(PG_FUNCTION_ARGS)
 
 		if (relationId != InvalidOid)
 		{
-			LOCKMODE	lockmode = AccessShareLock; /* TODO. Verify lock mode? */
-			Relation	rel = table_open(relationId, lockmode);
+			Relation	rel = table_open(relationId, NoLock);
 
 			if (rel->rd_rel->relam == get_tde_table_am_oid())
 			{
 				/* We are creating the index on encrypted table */
 				/* set the global state */
 				tdeCurrentCreateEvent.encryptMode = true;
 			}
-			table_close(rel, lockmode);
+
+			table_close(rel, NoLock);
 
 			if (tdeCurrentCreateEvent.encryptMode)
 			{
@@ -181,7 +181,7 @@ pg_tde_ddl_command_start_capture(PG_FUNCTION_ARGS)
 	{
 		AlterTableStmt *stmt = (AlterTableStmt *) parsetree;
 		ListCell   *lcmd;
-		Oid			relationId = RangeVarGetRelid(stmt->relation, NoLock, true);
+		Oid			relationId = RangeVarGetRelid(stmt->relation, AccessShareLock, true);
 
 		validateCurrentEventTriggerState(true);
 		tdeCurrentCreateEvent.tid = GetCurrentFullTransactionId();
@@ -216,8 +216,7 @@ pg_tde_ddl_command_start_capture(PG_FUNCTION_ARGS)
 
 			if (relationId != InvalidOid)
 			{
-				LOCKMODE	lockmode = AccessShareLock;
-				Relation	rel = relation_open(relationId, lockmode);
+				Relation	rel = relation_open(relationId, NoLock);
 
 				if (rel->rd_rel->relam == get_tde_table_am_oid())
 				{
@@ -227,7 +226,8 @@ pg_tde_ddl_command_start_capture(PG_FUNCTION_ARGS)
 					 */
 					tdeCurrentCreateEvent.encryptMode = true;
 				}
-				relation_close(rel, lockmode);
+
+				relation_close(rel, NoLock);
 
 				if (tdeCurrentCreateEvent.encryptMode)
 				{

Original file line number	Diff line number	Diff line change
`@@ -122,7 +122,7 @@ pg_tde_ddl_command_start_capture(PG_FUNCTION_ARGS)`
`122`	`122`	`if (IsA(parsetree, IndexStmt))`
`123`	`123`	`{`
`124`	`124`	`IndexStmt stmt = (IndexStmt ) parsetree;`
`125`		`- Oid relationId = RangeVarGetRelid(stmt->relation, NoLock, true);`
	`125`	`+ Oid relationId = RangeVarGetRelid(stmt->relation, AccessShareLock, true);`
`126`	`126`
`127`	`127`	`validateCurrentEventTriggerState(true);`
`128`	`128`	`tdeCurrentCreateEvent.tid = GetCurrentFullTransactionId();`
`@@ -132,16 +132,16 @@ pg_tde_ddl_command_start_capture(PG_FUNCTION_ARGS)`
`132`	`132`
`133`	`133`	`if (relationId != InvalidOid)`
`134`	`134`	`{`
`135`		`- LOCKMODE lockmode = AccessShareLock; /* TODO. Verify lock mode? */`
`136`		`- Relation rel = table_open(relationId, lockmode);`
	`135`	`+ Relation rel = table_open(relationId, NoLock);`
`137`	`136`
`138`	`137`	`if (rel->rd_rel->relam == get_tde_table_am_oid())`
`139`	`138`	`{`
`140`	`139`	`/* We are creating the index on encrypted table */`
`141`	`140`	`/* set the global state */`
`142`	`141`	`tdeCurrentCreateEvent.encryptMode = true;`
`143`	`142`	`}`
`144`		`- table_close(rel, lockmode);`
	`143`	`+`
	`144`	`+ table_close(rel, NoLock);`
`145`	`145`
`146`	`146`	`if (tdeCurrentCreateEvent.encryptMode)`
`147`	`147`	`{`
`@@ -181,7 +181,7 @@ pg_tde_ddl_command_start_capture(PG_FUNCTION_ARGS)`
`181`	`181`	`{`
`182`	`182`	`AlterTableStmt stmt = (AlterTableStmt ) parsetree;`
`183`	`183`	`ListCell *lcmd;`
`184`		`- Oid relationId = RangeVarGetRelid(stmt->relation, NoLock, true);`
	`184`	`+ Oid relationId = RangeVarGetRelid(stmt->relation, AccessShareLock, true);`
`185`	`185`
`186`	`186`	`validateCurrentEventTriggerState(true);`
`187`	`187`	`tdeCurrentCreateEvent.tid = GetCurrentFullTransactionId();`
`@@ -216,8 +216,7 @@ pg_tde_ddl_command_start_capture(PG_FUNCTION_ARGS)`
`216`	`216`
`217`	`217`	`if (relationId != InvalidOid)`
`218`	`218`	`{`
`219`		`- LOCKMODE lockmode = AccessShareLock;`
`220`		`- Relation rel = relation_open(relationId, lockmode);`
	`219`	`+ Relation rel = relation_open(relationId, NoLock);`
`221`	`220`
`222`	`221`	`if (rel->rd_rel->relam == get_tde_table_am_oid())`
`223`	`222`	`{`
`@@ -227,7 +226,8 @@ pg_tde_ddl_command_start_capture(PG_FUNCTION_ARGS)`
`227`	`226`	`*/`
`228`	`227`	`tdeCurrentCreateEvent.encryptMode = true;`
`229`	`228`	`}`
`230`		`- relation_close(rel, lockmode);`
	`229`	`+`
	`230`	`+ relation_close(rel, NoLock);`
`231`	`231`
`232`	`232`	`if (tdeCurrentCreateEvent.encryptMode)`
`233`	`233`	`{`