Remove unnecessary pfree() in g_intbig_compress().
authorTom Lane <tgl@sss.pgh.pa.us>
Thu, 13 Jul 2023 17:07:51 +0000 (13:07 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Thu, 13 Jul 2023 17:07:51 +0000 (13:07 -0400)
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

contrib/intarray/_intbig_gist.c

index bfba5eef1d1f4e522f5ae3d14e2e1a7b02391080..8c6c4b5d051c739bbb9aab7b82f5999d2711e32d 100644 (file)
@@ -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)))