Subject: Re: Postgresql 18beta1 and SPI changes

From: Achilleas Mantzios <[email-masked]>
Date: Mon, 12 May 2025 13:50:12 +0100

Dear All, Dear Tom

On 5/11/25 16:20, Tom Lane wrote:

> Achilleas Mantzios<[email-masked]> writes:
>> We use are own version of DBmirror, we run our replication in a highly
>> fine grained manner. So every upgrade I have to make the code compile
>> and test. Up to PostgreSQL 17, I only got minor compilation problems
>> that I managed to resolve fairly easily. However this didn't prove to be
>> the case with PostgreSQL 18beta1, it proved harder to compile and as my
>> fears were verified, it has serious problems.
>> My question : is 18's SPI stabilized ? Can I start work on our version
>> of DBmirror ? Or wait for 18beta2 or -RC ?
> If you think there are changes we need to make, you'd better get
> specific sooner not later.  I'm not aware of any large fixes that
> are pending, cf
>
> https://wiki.postgresql.org/wiki/PostgreSQL_18_Open_Items

I attach

a) our old source (pending.c.orig), as of PostgreSQL 17 (tested for some 
7 months, so pretty well tested),

b) the compilation errors when compiled against 18beta1, and

c) the patch that I came up with, which seems (in my minimal testing) to 
yield correct results on 18beta1.

The majority of serious warnings have to do with de-toasting arrays and 
the PK's int2vector , while the error has to do with getting column 
details such as attisdropped and attname.

Please have a look, and share your thoughts. I haven't touched serious C 
coding till I first wrote the above sometime in 2004 with a bunch of 
additions some years ago.

> 			regards, tom lane

diff --git a/code/dbmirror/pending.c b/code/dbmirror/pending.c
index fca76318..b1c61e02 100644
--- a/code/dbmirror/pending.c
+++ b/code/dbmirror/pending.c
@@ -98,7 +98,7 @@ char *get_namespace_name(Oid nspid);
 
 #define BUFFER_SIZE 256
 #define MAX_OID_LEN 10
-//#define DEBUG_OUTPUT 1
+#define DEBUG_OUTPUT 1
 extern Datum recordchange(PG_FUNCTION_ARGS);
 
 PG_FUNCTION_INFO_V1(recordchange);
@@ -399,7 +399,8 @@ getPrimaryKey(Oid tblOid)
 		return NULL;
 	}
 
-	tpResultKey = (int2vector *) DatumGetPointer(PG_DETOAST_DATUM(resDatum));
+	//tpResultKey = (int2vector *) DatumGetPointer(PG_DETOAST_DATUM(resDatum));
+	tpResultKey = (int2vector *) PG_DETOAST_DATUM(resDatum);
 	int n=tpResultKey->dim1;
 	resultKey = palloc(Int2VectorSize(n));
 	if (n > 0)
@@ -548,7 +549,8 @@ packageData(char *cpTableName,HeapTuple tTupleData, TupleDesc tTupleDesc, Oid ta
 		}						/* KeyUsage!=ALL */
 #ifndef  NODROPCOLUMN
                // this comment is for 11
-               if(tTupleDesc->attrs[iColumnCounter-1].attisdropped)
+               ////if(tTupleDesc->attrs[iColumnCounter-1].attisdropped)
+		if (TupleDescAttr(tTupleDesc,iColumnCounter-1)->attisdropped)
                //if(tTupleDesc->attrs[iColumnCounter-1]->attisdropped)
                  {
                    /**
@@ -568,7 +570,8 @@ eksairetea columns
 		}
 
 		// this comment is for 11
-		cpFieldName = DatumGetPointer(NameGetDatum(&tTupleDesc->attrs[iColumnCounter - 1].attname));
+		////cpFieldName = DatumGetPointer(NameGetDatum(&tTupleDesc->attrs[iColumnCounter - 1].attname));
+		cpFieldName = NameStr(TupleDescAttr(tTupleDesc,iColumnCounter - 1)->attname );
 		//cpFieldName = DatumGetPointer(NameGetDatum(&tTupleDesc->attrs[iColumnCounter - 1]->attname));
 #if defined DEBUG_OUTPUT
 		elog(NOTICE, "FieldName: %s", cpFieldName);
@@ -1441,7 +1444,8 @@ int handleParents(char *cpTableName,HeapTuple Btuple,HeapTuple Atuple, TupleDesc
 		confrelid = (Oid) DatumGetObjectId(resDatum);
 
 		resDatum = SPI_getbinval(resTuple,FK_tupTable->tupdesc,2,&isNull);
-		arr = (ArrayType *) DatumGetPointer(PG_DETOAST_DATUM(resDatum));
+		//arr = (ArrayType *) DatumGetPointer(PG_DETOAST_DATUM(resDatum));
+		arr = (ArrayType *) PG_DETOAST_DATUM(resDatum);
 		thisCols = (int16 *)ARR_DATA_PTR(arr);
 		numOfCols=ARR_DIMS(arr)[0];
 #if defined DEBUG_OUTPUT
@@ -1506,7 +1510,8 @@ int handleParents(char *cpTableName,HeapTuple Btuple,HeapTuple Atuple, TupleDesc
 #endif
 		if (FkHasNullValueORXcluded || !handleit) continue;
 		resDatum = SPI_getbinval(resTuple,FK_tupTable->tupdesc,3,&isNull);
-		arr = (ArrayType *) DatumGetPointer(PG_DETOAST_DATUM(resDatum));
+		//arr = (ArrayType *) DatumGetPointer(PG_DETOAST_DATUM(resDatum));
+		arr = (ArrayType *) PG_DETOAST_DATUM(resDatum);
 		fkCols = (int16 *)ARR_DATA_PTR(arr);
 		for (run=fkCols,colrun=0;colrun<numOfCols;run++,colrun++) {
 			SPITupleTable *PAR_tupTable;
@@ -1822,7 +1827,8 @@ int updateAccntParents(char *cpTableName,HeapTuple Btuple,HeapTuple Atuple, Tupl
 		confrelid = (Oid) DatumGetObjectId(resDatum);
 
 		resDatum = SPI_getbinval(resTuple,FK_tupTable->tupdesc,2,&isNull);
-		arr = (ArrayType *) DatumGetPointer(PG_DETOAST_DATUM(resDatum));
+		//arr = (ArrayType *) DatumGetPointer(PG_DETOAST_DATUM(resDatum));
+		arr = (ArrayType *) PG_DETOAST_DATUM(resDatum);
 		thisCols = (int16 *)ARR_DATA_PTR(arr);
 		numOfCols=ARR_DIMS(arr)[0];
 		for (run=thisCols,colrun=0;colrun<numOfCols;run++,colrun++)  {
@@ -1888,7 +1894,8 @@ int updateAccntParents(char *cpTableName,HeapTuple Btuple,HeapTuple Atuple, Tupl

 		resDatum = SPI_getbinval(resTuple,FK_tupTable->tupdesc,3,&isNull);
-		arr = (ArrayType *) DatumGetPointer(PG_DETOAST_DATUM(resDatum));
+		//arr = (ArrayType *) DatumGetPointer(PG_DETOAST_DATUM(resDatum));
+		arr = (ArrayType *) PG_DETOAST_DATUM(resDatum);
 		fkCols = (int16 *)ARR_DATA_PTR(arr);
 		for (run=fkCols,colrun=0;colrun<numOfCols;run++,colrun++) {
 			SPITupleTable *PAR_tupTable;

---

From: Achilleas Mantzios <[email-masked]>
Date: Tue, 13 May 2025 20:12:12 +0300

On 12/5/25 15:50, Achilleas Mantzios wrote:
> a) our old source (pending.c.orig), as of PostgreSQL 17 (tested for 
> some 7 months, so pretty well tested),
>
> c) the patch that I came up with, which seems (in my minimal testing) 
> to yield correct results on 18beta1.
>
> The majority of serious warnings have to do with de-toasting arrays 
> and the PK's int2vector , while the error has to do with getting 
> column details such as attisdropped and attname.
>
> Please have a look, and share your thoughts. I haven't touched serious 
> C coding till I first wrote the above sometime in 2004 with a bunch of 
Hi again

just to close this, it seems that the main issue for the compilation 
fail was a change introduced in this commit : 
d28dff3f6cd6a7562fb2c211ac0fb74a33ffd032 and had to do with access to 
TupleDesc->attrs which does not exist anymore. Some browsing in the 
contrib dir provided the hint I needed (to use : TupleDescAttr ).

The funny thing here is that deepseek spotted the issue before I even 
pasted any piece of code, just mentioning. It would be nice if those SPI 
level changes were mentioned somewhere.