From: | Daniel Gustafsson <daniel(at)yesql(dot)se> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, alvherre(at)alvh(dot)no-ip(dot)org |
Subject: | Re: Support json_errdetail in FRONTEND builds |
Date: | 2024-03-14 09:56:46 |
Message-ID: | FF5BB399-9DFB-434B-8C18-FEEBC706EAA8@yesql.se |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
> On 14 Mar 2024, at 09:06, Michael Paquier <michael(at)paquier(dot)xyz> 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).
I wouldn't worry about that, seems fine as is to me.
> See the attached with few tweaks to 0001, previously 0002 @-(at)(dot)
> I'd still need to do a more serious lookup of 0002, previously 0001.
A few small comments:
- *
+*
Whitespace
+ /* 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 use-after-free bugs.
+ * The returned allocation is either static or owned by the JsonLexContext and
+ * should not be freed.
The most important part of that comment is at the very end, to help readers I
would reword this to just "The returned pointer should not be freed.", or at
least put that part first.
+#define token_error(lex, format) \
I'm not sure this buys much more than reduced LoC, the expansion isn't
unreadable to the point that the format constraint encoded in the macro is
worth it IMO.
--
Daniel Gustafsson
From | Date | Subject | |
---|---|---|---|
Next Message | Andrei Lepikhov | 2024-03-14 10:11:15 | Re: POC, WIP: OR-clause support for indexes |
Previous Message | John Naylor | 2024-03-14 09:55:42 | Re: [PoC] Improve dead tuple storage for lazy vacuum |