On Wed, Nov 26, 2025 at 06:40:38PM +0530, Amul Sul wrote:
> On Wed, Nov 26, 2025 at 5:41 PM Amit Langote <amitlangote09@gmail.com> wrote:
>> * 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
I am not sure to see a point in doing a rename of the routines
separately. We are changing one of the argument type of the
functions, replacing the "overflow" integer with an error context
Node. If we were to do a rename in one patch, then a redesign of the
arguments, this leads to more code churn at the same end as the same
code paths would get rewritten twice instead of once. This move could
have made more sense if the existing popo_opt_overflow() used NULL for
the overflow value like in btree_gin.c in the past, but that makes the
change less appealing with the soft reporting APIs available in the
core backend.
>> * 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.
It seems to me that it is important to keep documented the overflow
check in some way rather than removing it as the patch is doing,
particularly regarding the finite vs infinite value behaviors. We do
not need anymore the documentation about how "overflow" is set in this
routines, of course, but keeping these expectations documented would
be better.
--
Michael
