Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Update some timestamp[tz] functions to use soft-error reporting

  2. Switch some date/timestamp functions to use the soft error reporting

  3. Switch some numeric-related functions to use soft error reporting

  1. Refactoring: Use soft error reporting for *_opt_overflow functions of date/timestamp

    amul sul <sulamul@gmail.com> — 2025-11-26T09:39:25Z

    Hi,
    
    The attached patch proposes to use soft error reporting infrastructure
    for the date/timestamp conversion function, which currently depends on
    integer variables to control error throwing.
    
    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.
    
    Note that in the patch, I renamed the function by replacing the
    "no_overflow" extension in the function name with "overflow_safe".
    Alternatively, we could just use "safe" alone. Suggestions are
    welcome.
    
    1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=4246a977bad6e76c4276a0d52def8a3dced154bb
    
    --
    Regards,
    Amul Sul
    EDB: http://www.enterprisedb.com
    
  2. Re: Refactoring: Use soft error reporting for *_opt_overflow functions of date/timestamp

    Michael Paquier <michael@paquier.xyz> — 2025-11-26T11:42:55Z

    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.
    
    > Note that in the patch, I renamed the function by replacing the
    > "no_overflow" extension in the function name with "overflow_safe".
    > Alternatively, we could just use "safe" alone. Suggestions are
    > welcome.
    
    Hmm.  Following the previous example you have quoted, I am wondering
    if we'd tweak the names a bit differently.  Rather than the
    popo_overflow_safe() pattern from your patch, I would choose a simpler
    popo_safe() as naming convention.  That would be also more consistent
    with the names applied to the refactored routines of 4246a977bad6.
    
    -   result = date2timestamp_opt_overflow(val, &overflow);
    +   result = date2timestamp_overflow_safe(val, (Node *) &escontext);
        /* We can ignore the overflow result, since result is useful as-is */ 
    
    In these cases, why don't you just pass NULL to the routines for the
    error context?  (Sorry, I don't have my eyes on the code now, but I
    recall that NULL should work as well, meaning the same as "ignore
    me".)
    --
    Michael
    
  3. Re: Refactoring: Use soft error reporting for *_opt_overflow functions of date/timestamp

    amit <amitlangote09@gmail.com> — 2025-11-26T12:11:08Z

    Hi,
    
    On Wed, Nov 26, 2025 at 8:43 PM Michael Paquier <michael@paquier.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
    
    
    
    
  4. Re: Refactoring: Use soft error reporting for *_opt_overflow functions of date/timestamp

    amul sul <sulamul@gmail.com> — 2025-11-26T13:01:03Z

    On Wed, Nov 26, 2025 at 5:13 PM Michael Paquier <michael@paquier.xyz> wrote:
    >
    >
    > Hmm.  Following the previous example you have quoted, I am wondering
    > if we'd tweak the names a bit differently.  Rather than the
    > popo_overflow_safe() pattern from your patch, I would choose a simpler
    > popo_safe() as naming convention.  That would be also more consistent
    > with the names applied to the refactored routines of 4246a977bad6.
    >
    
    The reason for this naming was to maintain consistency with the
    function date2timestamp_no_overflow() in date.h. I am now uncertain
    whether we should rename date2timestamp_no_overflow() as well to align
    with the current change. I also lean towards popo_safe() as a naming
    convention.
    
    > -   result = date2timestamp_opt_overflow(val, &overflow);
    > +   result = date2timestamp_overflow_safe(val, (Node *) &escontext);
    >     /* We can ignore the overflow result, since result is useful as-is */
    >
    > In these cases, why don't you just pass NULL to the routines for the
    > error context?  (Sorry, I don't have my eyes on the code now, but I
    > recall that NULL should work as well, meaning the same as "ignore
    > me".)
    
    Won't that result in an error that we are trying to avoid?
    
    Regards,
    Amul
    
    
    
    
  5. Re: Refactoring: Use soft error reporting for *_opt_overflow functions of date/timestamp

    amul sul <sulamul@gmail.com> — 2025-11-26T13:10:38Z

    On Wed, Nov 26, 2025 at 5:41 PM Amit Langote <amitlangote09@gmail.com> wrote:
    >
    > Hi,
    >
    > On Wed, Nov 26, 2025 at 8:43 PM Michael Paquier <michael@paquier.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
    
    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
    
    
    
    
  6. Re: Refactoring: Use soft error reporting for *_opt_overflow functions of date/timestamp

    Michael Paquier <michael@paquier.xyz> — 2025-11-26T22:59:22Z

    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
    
  7. Re: Refactoring: Use soft error reporting for *_opt_overflow functions of date/timestamp

    amit <amitlangote09@gmail.com> — 2025-11-27T01:18:38Z

    On Thu, Nov 27, 2025 at 7:59 AM Michael Paquier <michael@paquier.xyz> wrote:
    > 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.
    
    Ok, let's drop the patch breakdown part of my comment then.
    
    >  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.
    
    I’m fine with updating all core callers to use the new *_safe(... Node
    *escontext) APIs all in one patch. However, we could consider keeping
    the existing *_opt_overflow() functions as thin wrappers over the new
    ones, to avoid breaking third-party extensions immediately for what is
    primarily a refactoring change.
    
    > >> * 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.
    
    Yeah, I meant we should expand "including soft error reporting
    capabilities" somehow, something like this:
    
    - * 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 timestamp, 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 timestamp infinity.
    + * If the date is finite but out of the valid range for timestamp, an
    + * out-of-range error is reported.  When escontext is NULL this is a
    + * normal ERROR; when escontext points to an ErrorSaveContext, the error
    + * is reported softly and TIMESTAMP_NOEND is returned.
    
    -- 
    Thanks, Amit Langote
    
    
    
    
  8. Re: Refactoring: Use soft error reporting for *_opt_overflow functions of date/timestamp

    Michael Paquier <michael@paquier.xyz> — 2025-11-27T01:27:53Z

    On Thu, Nov 27, 2025 at 10:18:38AM +0900, Amit Langote wrote:
    > I’m fine with updating all core callers to use the new *_safe(... Node
    > *escontext) APIs all in one patch. However, we could consider keeping
    > the existing *_opt_overflow() functions as thin wrappers over the new
    > ones, to avoid breaking third-party extensions immediately for what is
    > primarily a refactoring change.
    
    Like 4246a977bad6, one point is to be able to make the internals of
    these functions leaner, by relying on the fact that the econtext node
    includes an error to let the callers of the functions decide what to
    do.  If we keep some function wrappers as the ones we have now, this
    partially defeats the purpose of the change, which is to simplify
    in-core code at the end.  This move also encourages users to switch to
    the new facility to be fed error reports generated by the in-core
    routines, meaning less duplication with error strings outside of the
    core code, hence less maintenance for out-of-core code.
    --
    Michael
    
  9. Re: Refactoring: Use soft error reporting for *_opt_overflow functions of date/timestamp

    amit <amitlangote09@gmail.com> — 2025-11-27T03:10:54Z

    On Thu, Nov 27, 2025 at 10:28 AM Michael Paquier <michael@paquier.xyz> wrote:
    > On Thu, Nov 27, 2025 at 10:18:38AM +0900, Amit Langote wrote:
    > > I’m fine with updating all core callers to use the new *_safe(... Node
    > > *escontext) APIs all in one patch. However, we could consider keeping
    > > the existing *_opt_overflow() functions as thin wrappers over the new
    > > ones, to avoid breaking third-party extensions immediately for what is
    > > primarily a refactoring change.
    >
    > Like 4246a977bad6, one point is to be able to make the internals of
    > these functions leaner, by relying on the fact that the econtext node
    > includes an error to let the callers of the functions decide what to
    > do.  If we keep some function wrappers as the ones we have now, this
    > partially defeats the purpose of the change, which is to simplify
    > in-core code at the end.  This move also encourages users to switch to
    > the new facility to be fed error reports generated by the in-core
    > routines, meaning less duplication with error strings outside of the
    > core code, hence less maintenance for out-of-core code.
    
    Ok, I see, the idea is to encourage out-of-core code to move to the
    new ErrorSaveContext-based handling, rather than keep wrappers around
    just to ease the transition. IOW, we shouldn’t carry redundant code in
    core once the new, simpler APIs exist. Sounds ok to me.  Some
    extension authors might grumble when their code stops compiling with
    v19 though,only to find that the change was mostly stylistic ;-).
    
    -- 
    Thanks, Amit Langote
    
    
    
    
  10. Re: Refactoring: Use soft error reporting for *_opt_overflow functions of date/timestamp

    amul sul <sulamul@gmail.com> — 2025-11-27T07:57:46Z

    On Thu, Nov 27, 2025 at 6:48 AM Amit Langote <amitlangote09@gmail.com> wrote:
    >
    > On Thu, Nov 27, 2025 at 7:59 AM Michael Paquier <michael@paquier.xyz> wrote:
    > > 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:
    > > >> [....]
    > >
    > > 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.
    >
    > Yeah, I meant we should expand "including soft error reporting
    > capabilities" somehow, something like this:
    >
    > - * 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 timestamp, 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 timestamp infinity.
    > + * If the date is finite but out of the valid range for timestamp, an
    > + * out-of-range error is reported.  When escontext is NULL this is a
    > + * normal ERROR; when escontext points to an ErrorSaveContext, the error
    > + * is reported softly and TIMESTAMP_NOEND is returned.
    >
    
    Okay, I have attached an updated version -- added the necessary
    comments and renamed the function, replacing "opt_overflow" with the
    "_safe".
    
    One question: Regarding date2timestamp_no_overflow(), should we rename
    it to date2double? We can't use date2timestamp_safe because we already
    have that function. The renaming is relevant because this function
    converts a date to the double data type, which allows us to remove the
    "_no_overflow" extension.
    
    Regards,
    Amul
    
  11. Re: Refactoring: Use soft error reporting for *_opt_overflow functions of date/timestamp

    amit <amitlangote09@gmail.com> — 2025-11-27T09:18:32Z

    On Thu, Nov 27, 2025 at 4:58 PM Amul Sul <sulamul@gmail.com> wrote:
    > On Thu, Nov 27, 2025 at 6:48 AM Amit Langote <amitlangote09@gmail.com> wrote:
    > > On Thu, Nov 27, 2025 at 7:59 AM Michael Paquier <michael@paquier.xyz> wrote:
    > > > On Wed, Nov 26, 2025 at 06:40:38PM +0530, Amul Sul wrote:
    > > > 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.
    > >
    > > Yeah, I meant we should expand "including soft error reporting
    > > capabilities" somehow, something like this:
    > >
    > > - * 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 timestamp, 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 timestamp infinity.
    > > + * If the date is finite but out of the valid range for timestamp, an
    > > + * out-of-range error is reported.  When escontext is NULL this is a
    > > + * normal ERROR; when escontext points to an ErrorSaveContext, the error
    > > + * is reported softly and TIMESTAMP_NOEND is returned.
    > >
    >
    > Okay, I have attached an updated version -- added the necessary
    > comments and renamed the function, replacing "opt_overflow" with the
    > "_safe".
    
    Thanks.
    
    LGTM beside minor nits:
    
    + * Promotes date to timestamp, including soft error reporting capabilities.
    
    The part after the comma looks unnecessary IMO.  It's clear from the
    rest of the description that the function has soft error reporting
    capabilities.
    
    + * handling proceeds based on the error save context:
    
    I don't see "error save context" anywhere else in the code.  Why not
    just use 'escontext' instead or just say "proceeds as follows:"?
    
    > One question: Regarding date2timestamp_no_overflow(), should we rename
    > it to date2double? We can't use date2timestamp_safe because we already
    > have that function. The renaming is relevant because this function
    > converts a date to the double data type, which allows us to remove the
    > "_no_overflow" extension.
    
    Makes sense to me.
    
    --
    Thanks, Amit Langote
    
    
    
    
  12. Re: Refactoring: Use soft error reporting for *_opt_overflow functions of date/timestamp

    Michael Paquier <michael@paquier.xyz> — 2025-11-28T00:46:07Z

    On Thu, Nov 27, 2025 at 06:18:32PM +0900, Amit Langote wrote:
    > On Thu, Nov 27, 2025 at 4:58 PM Amul Sul <sulamul@gmail.com> wrote:
    >> One question: Regarding date2timestamp_no_overflow(), should we rename
    >> it to date2double? We can't use date2timestamp_safe because we already
    >> have that function. The renaming is relevant because this function
    >> converts a date to the double data type, which allows us to remove the
    >> "_no_overflow" extension.
    > 
    > Makes sense to me.
    
    Yes, it would be nice to change all this area at once to remain
    consistent across the board.  date2timestamp_no_overflow() is the last
    "no_overflow" routine in date.h.
    --
    Michael
    
  13. Re: Refactoring: Use soft error reporting for *_opt_overflow functions of date/timestamp

    amul sul <sulamul@gmail.com> — 2025-11-28T04:16:43Z

    On Fri, Nov 28, 2025 at 6:16 AM Michael Paquier <michael@paquier.xyz> wrote:
    >
    > On Thu, Nov 27, 2025 at 06:18:32PM +0900, Amit Langote wrote:
    > > On Thu, Nov 27, 2025 at 4:58 PM Amul Sul <sulamul@gmail.com> wrote:
    > >> One question: Regarding date2timestamp_no_overflow(), should we rename
    > >> it to date2double? We can't use date2timestamp_safe because we already
    > >> have that function. The renaming is relevant because this function
    > >> converts a date to the double data type, which allows us to remove the
    > >> "_no_overflow" extension.
    > >
    > > Makes sense to me.
    >
    > Yes, it would be nice to change all this area at once to remain
    > consistent across the board.  date2timestamp_no_overflow() is the last
    > "no_overflow" routine in date.h.
    
    I have attached patch 0002 that renames it. I also updated patch 0001
    to accommodate Amit's comment suggestions.
    
    Regards,
    Amul
    
  14. Re: Refactoring: Use soft error reporting for *_opt_overflow functions of date/timestamp

    Michael Paquier <michael@paquier.xyz> — 2025-12-01T07:01:52Z

    On Fri, Nov 28, 2025 at 09:46:43AM +0530, Amul Sul wrote:
    > I have attached patch 0002 that renames it. I also updated patch 0001
    > to accommodate Amit's comment suggestions.
    
    Thanks, applied this one after more tweaks.  Regarding 0002, just
    doing a renaming makes me a bit uncomfortable after a second look.
    Another way to look at the problem while being consistent would be to
    convert date2timestamp_no_overflow() to use soft error reports,
    requiring its caller in selfuncs.c to use an error context node.  I
    cannot get really excited at the end just for the sake of the planner
    stats.
    
    There were two more functions that btree_gin.c is pointing at that
    could to the switch: timestamp->timestamptz and its opposite.  This
    also shaves some code, which is nice.  Please see the attached.
    --
    Michael
    
  15. Re: Refactoring: Use soft error reporting for *_opt_overflow functions of date/timestamp

    amul sul <sulamul@gmail.com> — 2025-12-01T12:01:43Z

    On Mon, Dec 1, 2025 at 12:32 PM Michael Paquier <michael@paquier.xyz> wrote:
    >
    > On Fri, Nov 28, 2025 at 09:46:43AM +0530, Amul Sul wrote:
    > > I have attached patch 0002 that renames it. I also updated patch 0001
    > > to accommodate Amit's comment suggestions.
    >
    > Thanks, applied this one after more tweaks.  Regarding 0002, just
    > doing a renaming makes me a bit uncomfortable after a second look.
    > Another way to look at the problem while being consistent would be to
    > convert date2timestamp_no_overflow() to use soft error reports,
    > requiring its caller in selfuncs.c to use an error context node.  I
    > cannot get really excited at the end just for the sake of the planner
    > stats.
    >
    
    Understood. Thanks for committing the patch.
    
    > There were two more functions that btree_gin.c is pointing at that
    > could to the switch: timestamp->timestamptz and its opposite.  This
    > also shaves some code, which is nice.  Please see the attached.
    
    Yes, the patch looks good to me.
    
    Regards,
    Amul
    
    
    
    
  16. Re: Refactoring: Use soft error reporting for *_opt_overflow functions of date/timestamp

    Michael Paquier <michael@paquier.xyz> — 2025-12-02T00:32:15Z

    On Mon, Dec 01, 2025 at 05:31:43PM +0530, Amul Sul wrote:
    > On Mon, Dec 1, 2025 at 12:32 PM Michael Paquier <michael@paquier.xyz> wrote:
    >> There were two more functions that btree_gin.c is pointing at that
    >> could to the switch: timestamp->timestamptz and its opposite.  This
    >> also shaves some code, which is nice.  Please see the attached.
    > 
    > Yes, the patch looks good to me.
    
    Thanks for double-checking.  Applied this one as well.
    --
    Michael
    
  17. Re: Refactoring: Use soft error reporting for *_opt_overflow functions of date/timestamp

    amit <amitlangote09@gmail.com> — 2025-12-02T08:21:34Z

    On Mon, Dec 1, 2025 at 9:02 PM Amul Sul <sulamul@gmail.com> wrote:
    > On Mon, Dec 1, 2025 at 12:32 PM Michael Paquier <michael@paquier.xyz> wrote:
    > >
    > > On Fri, Nov 28, 2025 at 09:46:43AM +0530, Amul Sul wrote:
    > > > I have attached patch 0002 that renames it. I also updated patch 0001
    > > > to accommodate Amit's comment suggestions.
    > >
    > > Thanks, applied this one after more tweaks.  Regarding 0002, just
    > > doing a renaming makes me a bit uncomfortable after a second look.
    > > Another way to look at the problem while being consistent would be to
    > > convert date2timestamp_no_overflow() to use soft error reports,
    > > requiring its caller in selfuncs.c to use an error context node.  I
    > > cannot get really excited at the end just for the sake of the planner
    > > stats.
    > >
    >
    > Understood. Thanks for committing the patch.
    
    +1, thanks Michael for taking care of this and Amul too.
    
    -- 
    Thanks, Amit Langote