Re: generate_series for timestamptz and time zone problem
Przemysław Sztoch <przemyslaw@sztoch.pl>
From: Przemysław Sztoch <przemyslaw@sztoch.pl>
To: Gurjeet Singh <gurjeet@singh.im>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Pg Hackers <pgsql-hackers@postgresql.org>
Date: 2023-01-30T12:21:01Z
Lists: pgsql-hackers
Gurjeet Singh wrote on 30.01.2023 08:18: > On Sat, Nov 12, 2022 at 10:44 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: >> However, what it shouldn't be is part of this patch. It's worth >> pushing it separately to have a record of that decision. I've >> now done that, so you'll need to rebase to remove that delta. >> >> I looked over the v5 patch very briefly, and have two main >> complaints: >> >> * There's no documentation additions. You can't add a user-visible >> function without adding an appropriate entry to func.sgml. >> >> * I'm pretty unimpressed with the whole truncate-to-interval thing >> and would recommend you drop it. I don't think it's adding much >> useful functionality beyond what we can already do with the existing >> date_trunc variants; and the definition seems excessively messy >> (too many restrictions and special cases). > Please see attached v6 of the patch. > > The changes since v5 are: > .) Rebased and resolved conflicts caused by commit 533e02e92. > .) Removed code and tests related to new date_trunc() functions, as > suggested by Tom. > .) Added 3 more variants to accompany with date_add(tstz, interval, zone). > date_add(tstz, interval) > date_subtract(tstz, interval) > date_subtract(tstz, interval, zone) > > .) Eliminate duplication of code; use common function to implement > generate_series_timestamptz[_at_zone]() functions. > .) Fixed bug where in one of the new code paths, > generate_series_timestamptz_with_zone(), did not perform > TIMESTAMP_NOT_FINITE() check. > .) Replaced some DirectFunctionCall?() with direct calls to the > relevant *_internal() function; should be better for performance. > .) Added documentation all 5 functions (2 date_add(), 2 > date_subtract(), 1 overloaded version of generate_series()). Other work distracted me from this patch. I looked at your update v6 and it looks ok. For me the date_trunc function is important and I still have some corner cases. Now I will continue working with data_trunc in a separate patch. > I'm not sure of the convention around authorship. But since this was > not an insignificant amount of work, would this patch be considered as > co-authored by Przemyslaw and myself? Should I add myself to Authors > field in the Commitfest app? I see no obstacles for us to be co-authors. > Hi Przemyslaw, > I started working on this patch based on Tom's review a few days > ago, since you hadn't responded in a while, and I presumed you're not > working on this anymore. I should've consulted with/notified you of my > intent before starting to work on it, to avoid duplication of work. > Sorry if this submission obviates any work you have in progress. > Please feel free to provide your feedback on the v6 of the patch. I propose to get the approval of the current truncated version of the patch. As I wrote above, I will continue work on date_trunc later and as a separate patch. -- Przemysław Sztoch | Mobile +48 509 99 00 66
Commits
-
Add functions to do timestamptz arithmetic in a non-default timezone.
- 75bd846b68b0 16.0 landed
-
Refactor datetime functions' timezone lookup code to reduce duplication.
- 3e59e5048d0f 16.0 landed
-
Fix volatility marking of timestamptz_trunc_zone.
- 533e02e927c6 16.0 landed