Re: Making jsonb_agg() faster

Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> writes:
>> On Nov 3, 2025, at 04:56, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> PFA v3, rebased over 8a27d418f, no substantive changes whatever.

> I am on vacation this week, I only find a hour in the evening and did an eyeball review without actually tracing and testing this patch set.

Thanks for looking at it!

> In elsewhere of the patch, you all use “JsonbInState ps = {0}” to do the initialization, only this place uses memset(). Can we keep consistent and use {0} here also. I see there is a “continue” and a “break” before the memset(), maybe you want to avoid unnecessary initialization. I guess that is a tiny saving, but if you think that saving is worthwhile, I’m fine with keeping the current code.

I intentionally made the 0001 patch as mechanical as possible,
and in particular did not second-guess existing decisions about
whether to initialize a variable in the declaration or later.
So I'm not excited about doing it differently in this one place.
There are some other memsets in the same area that could also
be replaced by "= {0}" if we cared to, but that seems like
material for a separate cleanup patch.

> Instead of hard-coding 12, can we use "sizeof(TimeTzADT)” for better readability?

No, because that's not the same :-(. sizeof() would produce 16,
on most machines anyway, thanks to alignment. But 12 is the
type's size according to pg_type.

> You left two “ugly hack” comments. Maybe add a short description for why they are hack and what can be improved in the future.

It's the same ugly hack as before, I just formatted the code more
legibly ... I didn't stop to look at whether there's a better way to
do it. Again, that seems like material for a different patch.

regards, tom lane