Thread
Commits
-
Improve JsonLexContext's freeability
- 1c99cde2f344 17.0 landed
-
Some refactoring to export json(b) conversion functions
- b22391a2ff7b 17.0 cited
-
pgsql: Some refactoring to export json(b) conversion functions
Amit Langote <amitlan@postgresql.org> — 2023-07-26T08:09:45Z
Some refactoring to export json(b) conversion functions This is to export datum_to_json(), datum_to_jsonb(), and jsonb_from_cstring(), though the last one is exported as jsonb_from_text(). A subsequent commit to add new SQL/JSON constructor functions will need them for calling from the executor. Discussion: https://postgr.es/m/20230720160252.ldk7jy6jqclxfxkq%40alvherre.pgsql Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/b22391a2ff7bdfeff4438f7a9ab26de3e33fdeff Modified Files -------------- src/backend/utils/adt/json.c | 59 +++++++++++++++++++++++-------------- src/backend/utils/adt/jsonb.c | 67 +++++++++++++++++++++++++++++++------------ src/include/utils/jsonfuncs.h | 5 ++++ 3 files changed, 90 insertions(+), 41 deletions(-)
-
Re: pgsql: Some refactoring to export json(b) conversion functions
Alvaro Herrera <alvherre@alvh.no-ip.org> — 2023-08-08T17:41:10Z
On 2023-Jul-26, Amit Langote wrote: > Some refactoring to export json(b) conversion functions > > This is to export datum_to_json(), datum_to_jsonb(), and > jsonb_from_cstring(), though the last one is exported as > jsonb_from_text(). After this commit, Coverity started complaining that datum_to_jsonb_internal() leaks the JsonLexContext here 754 │ case JSONTYPE_CAST: 755 │ case JSONTYPE_JSON: 756 │ { 757 │ /* parse the json right into the existing result object */ 758 │ JsonLexContext *lex; 759 │ JsonSemAction sem; 760 │ text *json = DatumGetTextPP(val); 761 │ 762 │ lex = makeJsonLexContext(json, true); 763 │ 764 │ memset(&sem, 0, sizeof(sem)); 765 │ 766 │ sem.semstate = (void *) result; 767 │ 768 │ sem.object_start = jsonb_in_object_start; 769 │ sem.array_start = jsonb_in_array_start; 770 │ sem.object_end = jsonb_in_object_end; 771 │ sem.array_end = jsonb_in_array_end; 772 │ sem.scalar = jsonb_in_scalar; 773 │ sem.object_field_start = jsonb_in_object_field_start; 774 │ 775 │ pg_parse_json_or_ereport(lex, &sem); 776 │ } 777 │ break; Admittedly, our support code for this is not great, since we have no clean way to free those resources. Some places like json_object_keys are freeing everything manually (even though in that particular case it's unnecessary, since that one runs in a memcxt that's going to be cleaned up shortly afterwards). One idea that Tom floated was to allow the JsonLexContext to be optionally stack-allocated. That reduces palloc() traffic; but some callers do need it to be palloc'ed. Here's a patch that does it that way, and adds a freeing routine that knows what to do in either case. It may make sense to do some further analysis and remove useless free calls. It may make sense to change the structs that contain JsonLexContext * so that they directly embed JsonLexContext instead. That would further reduce palloc'ing. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "Thou shalt not follow the NULL pointer, for chaos and madness await thee at its end." (2nd Commandment for C programmers) -
Re: pgsql: Some refactoring to export json(b) conversion functions
Alvaro Herrera <alvherre@alvh.no-ip.org> — 2023-10-05T09:13:54Z
On 2023-Aug-08, Alvaro Herrera wrote: > One idea that Tom floated was to allow the JsonLexContext to be > optionally stack-allocated. That reduces palloc() traffic; but some > callers do need it to be palloc'ed. Here's a patch that does it that > way, and adds a freeing routine that knows what to do in either case. > It may make sense to do some further analysis and remove useless free > calls. Pushed this, after some further tweaking. -- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/ "La verdad no siempre es bonita, pero el hambre de ella sí"