On Sat, Aug 16, 2025 at 12:57 AM Tom Lane <t...@sss.pgh.pa.us> wrote: > > I think you're missing the point: per the commit message for 0001, > > The real reason for doing it is to provide a mechanism whereby > pushJsonbValue() can be told to construct the JsonbValue tree > in a context that is not CurrentMemoryContext. That happens > when a non-null "outcontext" is specified in the JsonbInState. > No callers exercise that option in this patch, but the next > patch in the series will make use of it. > > If we didn't add the outcontext option in this step, we'd just > have to do so in the next one. > ok, I didn't check the message. now I get it.
0003 looks very good to me.
I did some minor refactoring solely based on v1-0003, see attachment.
we can refactor datum_to_jsonb_internal,
so that ``switch (tcategory)`` order is more aligned with the order of
enum JsonTypeCategory.
maybe it's not worth the trouble.
about 0002:
jsonb_agg_finalfn
/*
* The final function can be called more than once, so we must not change
* the stored JsonbValue data structure. Fortunately, the WJB_END_ARRAY
* action will only change fields in the JsonbInState struct itself, so we
* can simply invoke pushJsonbValue on a local copy of that.
*/
I don't understand the above comments.
If I add another ``pushJsonbValue(&result, WJB_END_ARRAY, NULL);``
then it will cause segmentation fault, that means we can not call
WJB_END_ARRAY action twice.
in finalize_aggregate:
there is no foreach loop within
```if (OidIsValid(peragg->finalfn_oid))```
Overall, I can not come up with a case where the final function is
called more than once.
v1-0001-minor-change-in-datum_to_jsonb_internal.no-cfbot
Description: Binary data
