Feedback-ID: i0fe9450f:Fastmail
Date: Sun, 17 Sep 2023 08:50:13 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Alexander Lakhin <exclusion@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18070: Assertion failed when processing error from plpy's
 iterator
Message-ID: <ZQY/Nfv20tt4PBzA@paquier.xyz>
References: <18070-ab9c171cbf4ebb0f@postgresql.org>
 <9fa9c154-d084-07de-8224-aab7fc8a119e@gmail.com>
 <ZQVm8KponTxqj8G3@paquier.xyz>
 <3920993.1694874885@sss.pgh.pa.us>
 <5eba3ea5-ae13-aab2-35d6-cd3e58dab08b@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="lDNuXnKAEOdG6OxK"
Content-Disposition: inline
In-Reply-To: <5eba3ea5-ae13-aab2-35d6-cd3e58dab08b@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/ZQY/Nfv20tt4PBzA%40paquier.xyz>
Precedence: bulk


--lDNuXnKAEOdG6OxK
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Sat, Sep 16, 2023 at 08:00:00PM +0300, Alexander Lakhin wrote:
> This lengthy output is similar to what we get with a recursion in plperl,
> so I don't consider this as an issue, but may be we could just check
> the length of an error message before concatenation...
>=20
> Please look at the v2 patch, which contains an addition to regression tes=
ts for the master.
> If you agree with this approach, I'll make patches for back branches as w=
ell...

+DETAIL:  spiexceptions.InvalidDatetimeFormat: invalid value "DD" for "DD"
+Value must be an integer.

That's the kind of things we've been doing for years in libpq.  So
even if there are no proper sentence terminations, that may be enough
for the user.

+		if (detail =3D=3D NULL)
 			detail =3D xmsg;
+		else if (xmsg !=3D NULL) {
+			initStringInfo(&detailstr);
+			appendStringInfoString(&detailstr, xmsg);
+			appendStringInfoChar(&detailstr, '\n');
+			appendStringInfoString(&detailstr, detail);
+			detail =3D detailstr.data;
+		}

Perhaps it would be slightly cleaner to use a StringInfoData all the
time and rely on the presence of fmt to free the detail string?

Btw, backpatching a change in the error infrastructure does not seem a
good idea to me, as there could be applications that depend on the
existing behaviors?  It looks like we'd better just remove the
assertion in back-branches.
--
Michael

--lDNuXnKAEOdG6OxK
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmUGPzUACgkQnvQgOdby
QH1l+w//czNrl+ZF/eLNTv/bwPZoohm1RtlCPHj4PCxs+OILQzgbWQy/zqfeyyZO
MFi+Qn4ZqbMUClXfFv43/0rNFLyNJbN4Y+cTGCpDG5SJHm+2mNnno6CGMjgB8PCp
qi4viDZa8bUl/jV1Hpp5JipKIulFHhX/RILZbjx7AQ4CRtEULwcPxCUpJeNNzBot
Kt2Jn92WlLHD5nv3xqDzUdBsTxcsfWFAwoxRTgwxN/Zq13p4c2hQdh+XZVDniBMw
tAbn31JdTbGldYoYbE0P4vOI84NgyVLCSMRXn6DnkeAh0txMEr1dLBACW/S6IWTf
tQkM4SEg/m7uyegwF5AydRJBSKjQ5oAp2Kpi/buwclrOyuAV+aoGemrM9on1dXSC
liYj98tMhmzyLT/cYi00FZIB+boxvhAQBwBcZymXYxLCNjyontkbM7GzfoCs4vjW
8jvjuRayUoeZ+syX/LV9sYkPNVtCP5r5YiAzi3mk5tzWuLRlb83Cv+2jvmWNfa3S
BPPcZW3xNL/4TTLdQgIONU3tlHWsGqESv9Ajkr/0is1YaBPVM+SdDS9yOOvB+gGZ
+hCgviDtCF/9BbcIpAW5GOPnkgD/QzgZAO+xJombvk3YWD6177OKrBj0VKlz9oid
OpLlXBtY3bjNCbaJ86CsoEYGUY/24afVO2TFnxKtndgXekFzLZs=
=5GfV
-----END PGP SIGNATURE-----

--lDNuXnKAEOdG6OxK--