From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andrew Dunstan <andrew@dunslane.net>
cc: Amul Sul <sulamul@gmail.com>, Andres Freund <andres@anarazel.de>,
        Alexander Korotkov <aekorotkov@gmail.com>,
        Robert Haas <robertmhaas@gmail.com>,
        "David G. Johnston" <david.g.johnston@gmail.com>,
        Corey Huinker <corey.huinker@gmail.com>,
        Alvaro Herrera <alvherre@alvh.no-ip.org>,
        Joe Conway <mail@joeconway.com>,
        Nikita Glukhov <n.gluhov@postgrespro.ru>,
        PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Error-safe user functions
In-reply-to: <d231804a-5306-e2ce-f795-d243f0ebcd2c@dunslane.net>
References: <4009543.1670422833@sss.pgh.pa.us>
 <20221209023351.l362yejerfoq3lfp@awork3.anarazel.de>
 <633426.1670554790@sss.pgh.pa.us>
 <01ff2051-806d-af2f-4f9e-793f7ba9a3f8@dunslane.net>
 <778369.1670598993@sss.pgh.pa.us>
 <5aaf7c66-9df9-21a9-1ff4-02c7848def52@dunslane.net>
 <3bac9841-fe07-713d-fa42-606c225567d6@dunslane.net>
 <1436686.1670701118@sss.pgh.pa.us>
 <2b1e7d56-6f37-bad9-9702-053ab55ecf60@dunslane.net>
 <1805344.1670779451@sss.pgh.pa.us>
 <b54e97be-7316-2cab-6969-bfac6ec88ca6@dunslane.net>
 <1863335.1670783397@sss.pgh.pa.us> <CAAJ_b951-kZ=FzWpU31A_Lb!
 j6foJmy852T3YeCT6sAqRbqLPWQ@mail.gmail.com>
 <CAAJ_b97KeDWUdpTKGOaFYPv0OicjOu6EW+QYWj-Ywrgj_aEy1g@mail.gmail.com>
 <2687377.1671033604@sss.pgh.pa.us> <be2bb!
 2c4-d246-3cad-b969-7eba4c08336a@dunslane.net>
 <2966981.1671057469@sss.pgh.pa.us> <a!
 8dc5700-c341-3ba8-0507-cc09881e6200@dunslane.net>
 <1964968.1671727440@sss.pgh.pa.us> <9c8f22f2-7a37-f!
 c4d-8e9f-3431c0d09c5a@dunslane.net> <2769684.1671821604@sss.pgh.pa.us>
 <d231804a-5306!
 -e2ce-f795-d243f0ebcd2c@dunslane.net>
Comments: In-reply-to Andrew Dunstan <andrew@dunslane.net>
	message dated "Sat, 24 Dec 2022 07:51:25 -0500"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <2911706.1671896529.0@sss.pgh.pa.us>
Date: Sat, 24 Dec 2022 10:42:35 -0500
Message-ID: <2911749.1671896555@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/2911749.1671896555%40sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="UTF-8"
Content-ID: <2911706.1671896529.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable

Andrew Dunstan <andrew@dunslane.net> writes:
> As I was giving this a final polish I noticed this in jspConvertRegexFla=
gs:

> =C2=A0=C2=A0=C2=A0 /*
> =C2=A0=C2=A0 =C2=A0 * We'll never need sub-match details at execution.=C2=
=A0 While
> =C2=A0=C2=A0 =C2=A0 * RE_compile_and_execute would set this flag anyway,=
 force it on here to
> =C2=A0=C2=A0 =C2=A0 * ensure that the regex cache entries created by mak=
eItemLikeRegex are
> =C2=A0=C2=A0 =C2=A0 * useful.
> =C2=A0=C2=A0 =C2=A0 */
> =C2=A0=C2=A0 =C2=A0cflags |=3D REG_NOSUB;

> Clearly the comment would no longer be true. I guess I should just
> remove this?

Yeah, we can just drop that I guess.  I'm slightly worried that we might
need it again after some future refactoring; but it's not really worth
devising a re-worded comment to justify keeping it.

Also, I realized that I failed in my reviewerly duty by not noticing
that you'd forgotten to pg_regfree the regex after successful
compilation.  Running something like this exposes the memory leak
very quickly:

select pg_input_is_valid('$ ? (@ like_regex "pattern" flag "smixq")', 'jso=
npath')
  from generate_series(1,10000000);

The attached delta patch takes care of it.  (Per comment at pg_regcomp,
we don't need this after a failure return.)

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="dont-leak-compiled-regex.patch";
	charset="us-ascii"
Content-ID: <2911706.1671896529.2@sss.pgh.pa.us>
Content-Description: dont-leak-compiled-regex.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/utils/adt/jsonpath_gram.y b/src/backend/utils/adt=
/jsonpath_gram.y
index 8c3a0c7623..30179408f5 100644
--- a/src/backend/utils/adt/jsonpath_gram.y
+++ b/src/backend/utils/adt/jsonpath_gram.y
@@ -560,6 +560,8 @@ makeItemLikeRegex(JsonPathParseItem *expr, JsonPathStr=
ing *pattern,
 					(errcode(ERRCODE_INVALID_REGULAR_EXPRESSION),
 					 errmsg("invalid regular expression: %s", errMsg)));
 		}
+
+		pg_regfree(&re_tmp);
 	}
 =

 	*result =3D v;

------- =_aaaaaaaaaa0--