| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
| Cc: | jian he <jian(dot)universality(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: Making jsonb_agg() faster |
| Date: | 2025-12-07 00:13:42 |
| Message-ID: | 0C8ACF97-C1F0-43EA-B8FD-CD7E7ECB8190@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Dec 7, 2025, at 03:00, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> writes:
>> On Dec 6, 2025, at 07:14, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> I'd kind of like to get this pushed soon, because it keeps getting
>>> sideswiped ... does anyone have further comments?
>
>> My only nit commit is still about the hard-coded 12:
>> I commented this before and you explained. But I still think it may deserve a comment for why 12 is here, otherwise future reader may also get the same confusion as when I first time read this code.
>
> I've been thinking of that as an independent issue. But what I'm
> inclined to do is add a symbol to date.h, say like
>
> diff --git a/src/include/utils/date.h b/src/include/utils/date.h
> index 7316ac0ff17..2aca785b65d 100644
> --- a/src/include/utils/date.h
> +++ b/src/include/utils/date.h
> @@ -30,6 +30,14 @@ typedef struct
> int32 zone; /* numeric time zone, in seconds */
> } TimeTzADT;
>
> +/*
> + * sizeof(TimeTzADT) will be 16 on most platforms due to alignment padding.
> + * However, timetz's typlen is 12 according to pg_type. In most places
> + * we can get away with using sizeof(TimeTzADT), but where it's important
> + * to match the declared typlen, use TIMETZ_TYPLEN.
> + */
> +#define TIMETZ_TYPLEN 12
> +
> /*
> * Infinity and minus infinity must be the max and min values of DateADT.
> */
>
> and then use that. (I poked around in other code using TimeTzADT,
> and could not find any other places where we have hard-wired "12",
> which seems a bit surprising perhaps. But pretty much all the
> references to sizeof(TimeTzADT) are in palloc's, where it's fine.)
That’s even better than resolving my comment.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Peter Geoghegan | 2025-12-07 02:44:46 | Re: Moving _bt_readpage and _bt_checkkeys into a new .c file |
| Previous Message | Marcos Magueta | 2025-12-06 23:38:24 | WIP - xmlvalidate implementation from TODO list |