| From: | Amit Langote <amitlangote09(at)gmail(dot)com> |
|---|---|
| To: | Michael Paquier <michael(at)paquier(dot)xyz> |
| Cc: | Amul Sul <sulamul(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Refactoring: Use soft error reporting for *_opt_overflow functions of date/timestamp |
| Date: | 2025-11-26 12:11:08 |
| Message-ID: | CA+HiwqEV7M_PpJXRLPsvi60hxHWEfUajqQjvKCJjA+zLhDA6ow@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On Wed, Nov 26, 2025 at 8:43 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> On Wed, Nov 26, 2025 at 03:09:25PM +0530, Amul Sul wrote:
> > This continues the previous refactoring commit [1] where we adopted
> > soft error reporting for some numeric functions. This patch applies
> > the same pattern to the date/timestamp function. The change ensures
> > consistency by utilizing the existing soft error reporting
> > infrastructure.
>
> Thanks for continuing this work.
+1
I see that Michael has now noticed this, I was looking at this earlier
today and thought of a couple of nitpicky things to share:
* The rename from *_opt_overflow to *_overflow_safe could be made a
separate patch (say 0002), so it can be discussed separately. For
example, whether to keep the old *_opt_overflow variants for backward
compatibility since they’re exported and possibly used by extensions.
* Maybe it's just me, but several function comments (for example
around date2timestamptz_overflow_safe()) lost detailed explanations of
overflow behavior. It’d be better to preserve those specifics and only
adjust the wording to describe how errors are reported via escontext:
/*
- * Promote date to timestamp with time zone.
- *
- * On successful conversion, *overflow is set to zero if it's not NULL.
- *
- * If the date is finite but out of the valid range for timestamptz, then:
- * if overflow is NULL, we throw an out-of-range error.
- * if overflow is not NULL, we store +1 or -1 there to indicate the sign
- * of the overflow, and return the appropriate timestamptz infinity.
+ * Promotes date to timestamp with time zone, including soft error reporting
+ * capabilities.
/*
- * Convert timestamp to date.
- *
- * On successful conversion, *overflow is set to zero if it's not NULL.
- *
- * If the timestamp is finite but out of the valid range for date, then:
- * if overflow is NULL, we throw an out-of-range error.
- * if overflow is not NULL, we store +1 or -1 there to indicate the sign
- * of the overflow, and return the appropriate date infinity.
+ * Convert timestamp to date, including soft error reporting capabilities.
/*
- * Convert timestamptz to date.
- *
- * On successful conversion, *overflow is set to zero if it's not NULL.
- *
- * If the timestamptz is finite but out of the valid range for date, then:
- * if overflow is NULL, we throw an out-of-range error.
- * if overflow is not NULL, we store +1 or -1 there to indicate the sign
- * of the overflow, and return the appropriate date infinity.
+ * Convert timestamptz to date, including soft error reporting capabilities.
--
Thanks, Amit Langote
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Kirill Reshke | 2025-11-26 12:13:38 | Re: Second RewriteQuery complains about first RewriteQuery in edge case |
| Previous Message | Álvaro Herrera | 2025-11-26 12:08:13 | Re: IPC/MultixactCreation on the Standby server |