Remove unnecessary pfree() in g_intbig_compress().

tglsfdc · tglsfdc · commit 8aac9759bb85 · 2023-07-13T13:07:51.000-04:00
GiST compress functions (like all GiST opclass functions) are supposed to be called in short-lived memory contexts, so that minor memory leaks in them are not of concern, and indeed explicit pfree's are likely slightly counterproductive. But this one in g_intbig_compress() is more than slightly counterproductive, because it's guarded by "if (in != DatumGetArrayTypeP(entry->key))" which means that if this test succeeds, we've detoasted the datum twice. (And to add insult to injury, the extra detoast result is leaked.) Let's just drop the whole stanza, relying on the GiST temporary context mechanism to clean up in good time. The analogous bit in g_int_compress() is if (r != (ArrayType *) DatumGetPointer(entry->key)) pfree(r); which doesn't have the gratuitous-detoast problem so I left it alone. Perhaps there is a case for removing unnecessary pfree's more widely, but I'm not sure if it's worth the code churn. The potential extra decompress seems expensive enough to justify calling this a (minor) performance bug and back-patching. Konstantin Knizhnik, Matthias van de Meent, Tom Lane Discussion: https://postgr.es/m/CAEze2Wi86=DxErfvf+SCB2UKmU2amKOF60BKuJOX=w-RojRn0A@mail.gmail.com
diff --git a/contrib/intarray/_intbig_gist.c b/contrib/intarray/_intbig_gist.c
@@ -178,9 +178,6 @@ g_intbig_compress(PG_FUNCTION_ARGS)
 					  entry->rel, entry->page,
 					  entry->offset, false);
 
-		if (in != DatumGetArrayTypeP(entry->key))
-			pfree(in);
-
 		PG_RETURN_POINTER(retval);
 	}
 	else if (!ISALLTRUE(DatumGetPointer(entry->key)))

Original file line number	Diff line number	Diff line change
`@@ -178,9 +178,6 @@ g_intbig_compress(PG_FUNCTION_ARGS)`
`178`	`178`	`entry->rel, entry->page,`
`179`	`179`	`entry->offset, false);`
`180`	`180`
`181`		`- if (in != DatumGetArrayTypeP(entry->key))`
`182`		`- pfree(in);`
`183`		`-`
`184`	`181`	`PG_RETURN_POINTER(retval);`
`185`	`182`	`}`
`186`	`183`	`else if (!ISALLTRUE(DatumGetPointer(entry->key)))`