Re: Patch: shouldn't timezone(text, timestamp[tz]) be STABLE?

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Aleksander Alekseev <aleksander(at)timescale(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Patch: shouldn't timezone(text, timestamp[tz]) be STABLE?
Date: 2021-09-06 15:49:12
Message-ID: 3065164.1630943352@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Aleksander Alekseev <aleksander(at)timescale(dot)com> writes:
>> Anyway, attached is a revised patch that gets rid of the antique
>> code, and it produces correct results AFAICT.

> I tested your patch against the current master branch 78aa616b on
> MacOS Catalina. I have nothing to add to the patch.

Thanks. Pushed, along with a quick-and-dirty patch to resolve the
DYNTZ problem in the back branches.

>> I'm fairly unhappy now that we don't have any
>> regression test coverage for this function.

> Yep, that's unfortunate. I see several tests for `AT TIME ZONE`
> syntax, which is a syntax sugar to timezone() with timestamp[tz]
> arguments. But considering how `timetz` type is broken in the first
> place [1], I'm not surprised few people feel motivated to do anything
> related to it. Do you think there is a possibility that one day we may
> be brave enough to get rid of this type?

I'm afraid not, seeing that it's required by the SQL standard.

I thought about adding tests based on the CLT example I showed upthread,
and just accepting the need for two variant result files. Maybe we
should do that. However, it still wouldn't be a great test, because
it would not prove that the DST switchover happens at the right time of
year, or indeed at all. So for the moment I didn't.

regards, tom lane

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Rahila Syed 2021-09-06 15:55:42 Re: Column Filtering in Logical Replication
Previous Message Dilip Kumar 2021-09-06 15:43:50 Re: [BUG] Failed Assertion in ReorderBufferChangeMemoryUpdate()