Re: Support json_errdetail in FRONTEND builds

On Mon, Mar 18, 2024 at 06:17:18AM -0700, Jacob Champion wrote: > Great! Thanks everyone. Cool. Thanks for the commit. -- Michael signature.asc Description: PGP signature

Re: Support json_errdetail in FRONTEND builds

On Sun, Mar 17, 2024 at 4:49 PM Daniel Gustafsson wrote: > I took another look at this tonight and committed it with some mostly cosmetic > changes. Great! Thanks everyone. --Jacob

Re: Support json_errdetail in FRONTEND builds

> On 18 Mar 2024, at 01:13, Tom Lane wrote: > > Daniel Gustafsson writes: >> I took another look at this tonight and committed it with some mostly >> cosmetic >> changes. Since then, tamandua failed the SSL test on this commit but I am >> unable to reproduce any error both on older OpenSSL and

Re: Support json_errdetail in FRONTEND builds

Daniel Gustafsson writes: > I took another look at this tonight and committed it with some mostly cosmetic > changes. Since then, tamandua failed the SSL test on this commit but I am > unable to reproduce any error both on older OpenSSL and matching the 3.1 > version on tamandua, so not sure if i

Re: Support json_errdetail in FRONTEND builds

> On 17 Mar 2024, at 00:00, Daniel Gustafsson wrote: > >> On 16 Mar 2024, at 00:59, Andrew Dunstan wrote: >>> On Mar 16, 2024, at 8:53 AM, Daniel Gustafsson wrote: On 15 Mar 2024, at 21:56, Andrew Dunstan wrote: > I'd like to get this done ASAP so I can rebase my incremental parse

Re: Support json_errdetail in FRONTEND builds

> On 16 Mar 2024, at 00:59, Andrew Dunstan wrote: >> On Mar 16, 2024, at 8:53 AM, Daniel Gustafsson wrote: >>> On 15 Mar 2024, at 21:56, Andrew Dunstan wrote: >>> I'd like to get this done ASAP so I can rebase my incremental parse >>> patchset. Daniel, do you want to commit it? If not, I can.

Re: Support json_errdetail in FRONTEND builds

> On Mar 16, 2024, at 8:53 AM, Daniel Gustafsson wrote: > >  >> >> On 15 Mar 2024, at 21:56, Andrew Dunstan wrote: >> On Fri, Mar 15, 2024 at 10:15 AM Tom Lane > > wrote: >> Daniel Gustafsson mailto:dan...@yesql.se>> writes: >>> I can't see how refusing to free me

Re: Support json_errdetail in FRONTEND builds

On Fri, Mar 15, 2024 at 11:23:07PM +0100, Daniel Gustafsson wrote: > On 15 Mar 2024, at 21:56, Andrew Dunstan wrote: >> On Fri, Mar 15, 2024 at 10:15 AM Tom Lane > > wrote: >> Yeah, I agree an Assert seems safest here. Cool. >> I'd like to get this done ASAP so I can r

Re: Support json_errdetail in FRONTEND builds

> On 15 Mar 2024, at 21:56, Andrew Dunstan wrote: > On Fri, Mar 15, 2024 at 10:15 AM Tom Lane > wrote: > Daniel Gustafsson mailto:dan...@yesql.se>> writes: > > I can't see how refusing to free memory owned and controlled by someone > > else, > > and throwing an error i

Re: Support json_errdetail in FRONTEND builds

On Fri, Mar 15, 2024 at 10:15 AM Tom Lane wrote: > Daniel Gustafsson writes: > > I can't see how refusing to free memory owned and controlled by someone > else, > > and throwing an error if attempted, wouldn't be a sound defensive > programming > > measure. > > I think the argument is about what

Re: Support json_errdetail in FRONTEND builds

Daniel Gustafsson writes: > I can't see how refusing to free memory owned and controlled by someone else, > and throwing an error if attempted, wouldn't be a sound defensive programming > measure. I think the argument is about what "refusal" looks like. An Assert seems OK to me, but anything base

Re: Support json_errdetail in FRONTEND builds

> On 15 Mar 2024, at 01:10, Michael Paquier wrote: > > On Thu, Mar 14, 2024 at 10:56:46AM +0100, Daniel Gustafsson wrote: >> +/* don't allow destroys of read-only StringInfos */ >> +Assert(str->maxlen != 0); >> Considering that StringInfo.c don't own the memory here I think it's >> warra

Re: Support json_errdetail in FRONTEND builds

> On 15 Mar 2024, at 09:38, Alvaro Herrera wrote: > > On 2024-Mar-14, Tom Lane wrote: > >> Michael Paquier writes: >>> Hmm. I am not sure how much protection this would offer, TBH. One >>> thing that I find annoying with common/stringinfo.c as it is currently >>> is that we have two exit() ca

Re: Support json_errdetail in FRONTEND builds

On 2024-Mar-14, Tom Lane wrote: > Michael Paquier writes: > > Hmm. I am not sure how much protection this would offer, TBH. One > > thing that I find annoying with common/stringinfo.c as it is currently > > is that we have two exit() calls in the enlarge path, and it does not > > seem wise to m

Re: Support json_errdetail in FRONTEND builds

Michael Paquier writes: > Hmm. I am not sure how much protection this would offer, TBH. One > thing that I find annoying with common/stringinfo.c as it is currently > is that we have two exit() calls in the enlarge path, and it does not > seem wise to me to spread that even more. > My last argu

Re: Support json_errdetail in FRONTEND builds

On Thu, Mar 14, 2024 at 10:56:46AM +0100, Daniel Gustafsson wrote: > + /* don't allow destroys of read-only StringInfos */ > + Assert(str->maxlen != 0); > Considering that StringInfo.c don't own the memory here I think it's warranted > to turn this assert into an elog() to avoid the risk of

Re: Support json_errdetail in FRONTEND builds

> On 14 Mar 2024, at 09:06, Michael Paquier wrote: > I think that it is cleaner to create the new API first, and then > rely on it, reversing the order of both patches I agree with this ordering. > (perhaps the extra destroyStringInfo in freeJsonLexContext() should > have been moved in 0001).

Re: Support json_errdetail in FRONTEND builds

On Wed, Mar 13, 2024 at 11:20:16AM -0700, Jacob Champion wrote: > Sounds good, split into v2-0002. (That gives me room to switch other > call sites to the new API, too.) Thanks both! That looks OK to me. I can see 7~8 remaining sites where StringInfo data is freed, like in the syslogger, but we s

Re: Support json_errdetail in FRONTEND builds

On Tue, Mar 12, 2024 at 11:38 PM Michael Paquier wrote: > On Tue, Mar 12, 2024 at 08:38:43PM -0400, Andrew Dunstan wrote: > > yeah, although maybe worth a different patch. > > I've wanted that a few times, FWIW. I would do a split, mainly for > clarity. Sounds good, split into v2-0002. (That giv

Re: Support json_errdetail in FRONTEND builds

On Tue, Mar 12, 2024 at 08:38:43PM -0400, Andrew Dunstan wrote: > On 2024-03-12 Tu 14:43, Jacob Champion wrote: >> Two notes that I wanted to point out explicitly: >> - On the other thread, Daniel contributed a destroyStringInfo() >> counterpart for makeStringInfo(), which is optional but seemed us

Re: Support json_errdetail in FRONTEND builds

On 2024-03-12 Tu 14:43, Jacob Champion wrote: Hello, Both the incremental JSON [1] and OAuth [2] patchsets would be improved by json_errdetail(), which was removed from FRONTEND builds in b44669b2ca: The routine providing a detailed error message based on the error code is made backe

Support json_errdetail in FRONTEND builds

Hello, Both the incremental JSON [1] and OAuth [2] patchsets would be improved by json_errdetail(), which was removed from FRONTEND builds in b44669b2ca: >The routine providing a detailed error message based on the error code >is made backend-only, the existing code being unsafe to use in