Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1ww4A1-001igc-2S for pgsql-hackers@arkaria.postgresql.org; Mon, 17 Aug 2026 20:42:21 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.96) (envelope-from ) id 1ww49y-00CPHZ-0n for pgsql-hackers@arkaria.postgresql.org; Mon, 17 Aug 2026 20:42:19 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1ww49x-00CPHJ-2v for pgsql-hackers@lists.postgresql.org; Mon, 17 Aug 2026 20:42:19 +0000 Received: from sss.pgh.pa.us ([68.162.161.243]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.98.2) (envelope-from ) id 1ww49q-00000001GtG-3He6 for pgsql-hackers@lists.postgresql.org; Mon, 17 Aug 2026 20:42:13 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.18.1/8.18.1) with ESMTP id 67HKg2UI1132242; Mon, 17 Aug 2026 16:42:02 -0400 From: Tom Lane To: Andrey Rachitskiy cc: andrew@dunslane.net, pgsql-hackers@lists.postgresql.org Subject: Re: Residual cleanups for tied objects in PL/Perl In-reply-to: <756684.1786981296@sss.pgh.pa.us> References: <569769.1786901901@sss.pgh.pa.us> <579928.1786912017@sss.pgh.pa.us> <756684.1786981296@sss.pgh.pa.us> Comments: In-reply-to Tom Lane message dated "Mon, 17 Aug 2026 11:41:36 -0400" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <1132222.1786999304.0@sss.pgh.pa.us> Date: Mon, 17 Aug 2026 16:42:02 -0400 Message-ID: <1132241.1786999322@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <1132222.1786999304.1@sss.pgh.pa.us> I wrote: > We still have multiple > patterns for iterating through hashes: why do a couple of places use > hv_iternextsv when the others don't? And why do we use hek2cstr > in some places and not others? Here's a patch to try to clean that up. I think it is wrong that the extension modules don't use hek2cstr, so I made them do so. (But we probably shouldn't back-patch that: it's a user-visible behavioral change and we've not gotten actual field complaints AFAIR.) > (oh, and for extra credit: why is there a second call of hv_iterinit > in plperl_build_tuple_result and plperl_modify_tuple?) That seems to be just cargo-culted, so I removed those calls. regards, tom lane ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name*0="v1-0001-Make-plperl-s-handling-of-Perl-hashes-more-consis.p"; name*1="atch"; charset="us-ascii" Content-ID: <1132222.1786999304.2@sss.pgh.pa.us> Content-Description: v1-0001-Make-plperl-s-handling-of-Perl-hashes-more-consis.patch Content-Transfer-Encoding: quoted-printable =46rom d7b4ef67b382308f4ad3537afa1725acfce35720 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 17 Aug 2026 16:38:10 -0400 Subject: [PATCH v1] Make plperl's handling of Perl hashes more consistent. Make hek2cstr() available in plperl.h, so that it can be used in hstore_plperl and jsonb_plperl. Those modules were previously using different coding techniques that probably don't get conversion from Perl strings to the database encoding quite right. (I'd prefer to make hek2cstr() non-inline, but cross-extension calls are messy and plperl has avoided them up to now, so stick with the existing approach.) Consistently use hv_iternext, hek2cstr, and HeVAL for hash iterations, with one exception in plperl_trusted_init: there, hv_iternextsv is fine since we don't actually care about the hash keys. (A later patch will remove the HeVAL calls again, but for now we just want consistency.) Remove unnecessary extra calls of hv_iterinit. Remove duplicative pstrdup's in plperl_to_hstore. Author: Tom Lane Discussion: https://postgr.es/m/569769.1786901901@sss.pgh.pa.us Backpatch-through: 14 --- contrib/hstore_plperl/hstore_plperl.c | 6 +-- contrib/jsonb_plperl/jsonb_plperl.c | 13 +++--- src/pl/plperl/plperl.c | 59 +-------------------------- src/pl/plperl/plperl.h | 55 +++++++++++++++++++++++++ 4 files changed, 66 insertions(+), 67 deletions(-) diff --git a/contrib/hstore_plperl/hstore_plperl.c b/contrib/hstore_plperl= /hstore_plperl.c index d7f1b8ddb48..91dc0771fab 100644 --- a/contrib/hstore_plperl/hstore_plperl.c +++ b/contrib/hstore_plperl/hstore_plperl.c @@ -137,7 +137,7 @@ plperl_to_hstore(PG_FUNCTION_ARGS) i =3D 0; while ((he =3D hv_iternext(hv))) { - char *key =3D sv2cstr(HeSVKEY_force(he)); + char *key =3D hek2cstr(he); SV *value =3D HeVAL(he); = if (i >=3D pcount) @@ -146,7 +146,7 @@ plperl_to_hstore(PG_FUNCTION_ARGS) pairs =3D repalloc_array(pairs, Pairs, pcount); } = - pairs[i].key =3D pstrdup(key); + pairs[i].key =3D key; pairs[i].keylen =3D hstoreCheckKeyLen(strlen(pairs[i].key)); pairs[i].needfree =3D true; = @@ -158,7 +158,7 @@ plperl_to_hstore(PG_FUNCTION_ARGS) } else { - pairs[i].val =3D pstrdup(sv2cstr(value)); + pairs[i].val =3D sv2cstr(value); pairs[i].vallen =3D hstoreCheckValLen(strlen(pairs[i].val)); pairs[i].isnull =3D false; } diff --git a/contrib/jsonb_plperl/jsonb_plperl.c b/contrib/jsonb_plperl/js= onb_plperl.c index 00a99d303c6..b43101bd5a6 100644 --- a/contrib/jsonb_plperl/jsonb_plperl.c +++ b/contrib/jsonb_plperl/jsonb_plperl.c @@ -155,9 +155,7 @@ HV_to_JsonbValue(HV *obj, JsonbInState *jsonb_state) { dTHX; JsonbValue key; - SV *val; - char *kstr; - I32 klen; + HE *he; = key.type =3D jbvString; = @@ -165,10 +163,13 @@ HV_to_JsonbValue(HV *obj, JsonbInState *jsonb_state) = (void) hv_iterinit(obj); = - while ((val =3D hv_iternextsv(obj, &kstr, &klen))) + while ((he =3D hv_iternext(obj))) { - key.val.string.val =3D pnstrdup(kstr, klen); - key.val.string.len =3D klen; + char *k =3D hek2cstr(he); + SV *val =3D HeVAL(he); + + key.val.string.val =3D k; + key.val.string.len =3D strlen(k); pushJsonbValue(jsonb_state, WJB_KEY, &key); SV_to_JsonbValue(val, jsonb_state, false); } diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c index 8175407849e..ab1c60a0022 100644 --- a/src/pl/plperl/plperl.c +++ b/src/pl/plperl/plperl.c @@ -288,7 +288,6 @@ static void plperl_untrusted_init(void); static HV *plperl_spi_execute_fetch_result(SPITupleTable *tuptable, uint64 processed, int status); static void plperl_return_next_internal(SV *sv); -static char *hek2cstr(HE *he); static SV **hv_store_string(HV *hv, const char *key, SV *val); static SV **hv_fetch_string(HV *hv, const char *key); static void plperl_create_sub(plperl_proc_desc *prodesc, const char *s, @@ -322,60 +321,6 @@ SvREFCNT_dec_current(SV *sv) SvREFCNT_dec(sv); } = -/* - * convert a HE (hash entry) key to a cstr in the current database encodi= ng - */ -static char * -hek2cstr(HE *he) -{ - dTHX; - char *ret; - SV *sv; - - /* - * HeSVKEY_force will return a temporary mortal SV*, so we need to make - * sure to free it with ENTER/SAVE/FREE/LEAVE - */ - ENTER; - SAVETMPS; - - /*------------------------- - * Unfortunately, while HeUTF8 is true for most things > 256, for values - * 128..255 it's not, but perl will treat them as unicode code points if - * the utf8 flag is not set ( see The "Unicode Bug" in perldoc perlunico= de - * for more) - * - * So if we did the expected: - * if (HeUTF8(he)) - * utf_u2e(key...); - * else // must be ascii - * return HePV(he); - * we won't match columns with codepoints from 128..255 - * - * For a more concrete example given a column with the name of the unico= de - * codepoint U+00ae (registered sign) and a UTF8 database and the perl - * return_next { "\N{U+00ae}=3D>'text } would always fail as heUTF8 retu= rns - * 0 and HePV() would give us a char * with 1 byte contains the decimal - * value 174 - * - * Perl has the brains to know when it should utf8 encode 174 properly, = so - * here we force it into an SV so that perl will figure it out and do th= e - * right thing - *------------------------- - */ - - sv =3D HeSVKEY_force(he); - if (HeUTF8(he)) - SvUTF8_on(sv); - ret =3D sv2cstr(sv); - - /* free sv */ - FREETMPS; - LEAVE; - - return ret; -} - = /* * _PG_init() - library load-time initialization @@ -1093,8 +1038,8 @@ plperl_build_tuple_result(HV *perlhash, TupleDesc td= ) hv_iterinit(perlhash); while ((he =3D hv_iternext(perlhash))) { - SV *val =3D HeVAL(he); char *key =3D hek2cstr(he); + SV *val =3D HeVAL(he); int attn =3D SPI_fnumber(td, key); Form_pg_attribute attr; = @@ -1120,7 +1065,6 @@ plperl_build_tuple_result(HV *perlhash, TupleDesc td= ) = pfree(key); } - hv_iterinit(perlhash); = tup =3D heap_form_tuple(td, values, nulls); pfree(values); @@ -1857,7 +1801,6 @@ plperl_modify_tuple(HV *hvTD, TriggerData *tdata, He= apTuple otup) = pfree(key); } - hv_iterinit(hvNew); = rtup =3D heap_modify_tuple(otup, tupdesc, modvalues, modnulls, modrepls)= ; = diff --git a/src/pl/plperl/plperl.h b/src/pl/plperl/plperl.h index 4c03f9e0df6..33180b16b6e 100644 --- a/src/pl/plperl/plperl.h +++ b/src/pl/plperl/plperl.h @@ -163,6 +163,61 @@ cstr2sv(const char *str) return sv; } = +/* + * Convert a HE (hash entry) key to a cstr in the current database encodi= ng. + * The result is palloc'd. + */ +static inline char * +hek2cstr(HE *he) +{ + dTHX; + char *ret; + SV *sv; + + /* + * HeSVKEY_force will return a temporary mortal SV*, so we need to make + * sure to free it with ENTER/SAVE/FREE/LEAVE + */ + ENTER; + SAVETMPS; + + /*------------------------- + * Unfortunately, while HeUTF8 is true for most things > 256, for values + * 128..255 it's not, but perl will treat them as unicode code points if + * the utf8 flag is not set ( see The "Unicode Bug" in perldoc perlunico= de + * for more) + * + * So if we did the expected: + * if (HeUTF8(he)) + * utf_u2e(key...); + * else // must be ascii + * return HePV(he); + * we won't match columns with codepoints from 128..255 + * + * For a more concrete example given a column with the name of the unico= de + * codepoint U+00ae (registered sign) and a UTF8 database and the perl + * return_next { "\N{U+00ae}=3D>'text } would always fail as heUTF8 retu= rns + * 0 and HePV() would give us a char * with 1 byte contains the decimal + * value 174 + * + * Perl has the brains to know when it should utf8 encode 174 properly, = so + * here we force it into an SV so that perl will figure it out and do th= e + * right thing + *------------------------- + */ + + sv =3D HeSVKEY_force(he); + if (HeUTF8(he)) + SvUTF8_on(sv); + ret =3D sv2cstr(sv); + + /* free sv */ + FREETMPS; + LEAVE; + + return ret; +} + /* * croak() with specified message, which is given in the database encodin= g. * -- = 2.52.0 ------- =_aaaaaaaaaa0--