On Sat, Aug 16, 2025 at 12:57 AM Tom Lane <tgl@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.
