On Wed, Nov 26, 2025 at 5:41 PM Amit Langote <[email protected]> wrote:
>
> Hi,
>
> On Wed, Nov 26, 2025 at 8:43 PM Michael Paquier <[email protected]> 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

Thank you both for taking a look at the patch.

>
> 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.
>

I am probably okay with this, but it is up to the committer. In the
previous commit, however, we performed a rename within the same
commit. IIUC, the extensions are expected to be updated with respect
to the major release

> * 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:
>

The previous comments are no longer relevant now that we are utilizing
the established error-safe infrastructure, but, I would think more on
this later since I am out of time today.

Regards,
Amul


Reply via email to