Re: Making jsonb_agg() faster

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.