Thread

Re: BUG #19367: typos in backend/utils/adt/timestamp.c

Japin Li <japinli@hotmail.com> — 2025-12-30T02:30:24Z

On Mon, 29 Dec 2025 at 10:37, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Japin Li <japinli@hotmail.com> writes:
>> On Mon, 29 Dec 2025 at 15:55, Rahila Syed <rahilasyed90@gmail.com> wrote:
>>>> There are typos in return type of these functions:
>
>>> You’re right — these are just typos, and they don’t affect correctness since both 
>>> ultimately call Int64GetDatum(). 
>>> Still, +1 for fixing them for clarity.
>
>> The functions timestamptz_pl_interval() and timestamptz_mi_interval() have the
>> same typos, right?
>
> My recollection is that this code deliberately fuzzes the difference
> between timestamp and timestamptz in many places.  An example is that
> timestamp_eq and its sibling comparison functions are used as-is for
> both timestamp and timestamptz --- note the comment in front of
> timestamp_cmp_internal in timestamp.c.  (BTW, "thomas" there is
> Lockhart not me.)
>
> So I think that being cavalier about PG_RETURN_TIMESTAMP versus
> PG_RETURN_TIMESTAMPTZ stems from that; in fact, if you go back
> far enough in our git history you will find we didn't even have
> the latter to begin with.
>
> There may be places where we can clean this up and not simply be
> reversing the direction in which we're type-punning, but I doubt
> we'll be able to fix every one without duplicating functions.
> So I can't get hugely excited about it.
>

After greping, I found the following:

  $ grep -e '^timestamp_pl_interval' -e '^timestamptz_pl_interval' -rn src/
  src/backend/utils/adt/timestamp.c:3090:timestamp_pl_interval(PG_FUNCTION_ARGS)
  src/backend/utils/adt/timestamp.c:3233:timestamptz_pl_interval_internal(TimestampTz timestamp,
  src/backend/utils/adt/timestamp.c:3380:timestamptz_pl_interval(PG_FUNCTION_ARGS)
  src/backend/utils/adt/timestamp.c:3401:timestamptz_pl_interval_at_zone(PG_FUNCTION_ARGS)
  $ grep -e '^timestamp_mi_interval' -e '^timestamptz_mi_interval' -rn src/
  src/backend/utils/adt/timestamp.c:3207:timestamp_mi_interval(PG_FUNCTION_ARGS)
  src/backend/utils/adt/timestamp.c:3365:timestamptz_mi_interval_internal(TimestampTz timestamp,
  src/backend/utils/adt/timestamp.c:3389:timestamptz_mi_interval(PG_FUNCTION_ARGS)
  src/backend/utils/adt/timestamp.c:3412:timestamptz_mi_interval_at_zone(PG_FUNCTION_ARGS)

Since timestamptz_pl_interval() and timestamptz_mi_interval() are independent
functions (not just aliases or direct calls to the timestamp versions), their
return macros should be updated for accuracy: change any PG_RETURN_TIMESTAMP()
to the correct PG_RETURN_TIMESTAMPTZ().

The timestamptz_pl_interval_at_zone() and timestamptz_mi_interval_at_zone() may
still intentionally blur the distinction between timestamp and timestamptz
-- consistent with the historical design choices discussed earlier -- so they
can remain unchanged for now.

-- 
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.