From: | Gurjeet Singh <gurjeet(at)singh(dot)im> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Przemysław Sztoch <przemyslaw(at)sztoch(dot)pl>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: generate_series for timestamptz and time zone problem |
Date: | 2023-01-30 07:18:53 |
Message-ID: | CABwTF4WFX6vVRfev9vLE2PwZHcXJwWvT08eBP4UpaQDGax6eHQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sat, Nov 12, 2022 at 10:44 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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()).
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?
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.
Best regards,
Gurjeet
http://Gurje.et
Attachment | Content-Type | Size |
---|---|---|
generate_series_with_timezone.v6.patch | application/octet-stream | 21.7 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2023-01-30 07:28:54 | Re: generate_series for timestamptz and time zone problem |
Previous Message | Peter Smith | 2023-01-30 07:15:58 | Re: pub/sub - specifying optional parameters without values. |