Subject: [PATCH] Fix wrong argument to SOFT_ERROR_OCCURRED in timestamptz_date

From: Jianghua Yang <[email-masked]>
Date: Tue, 24 Mar 2026 08:44:29 -0700

Hi hackers,

  I found a small bug in commit e2f289e5b9b ("Make many cast functions
error safe").

  In timestamptz_date(), the SOFT_ERROR_OCCURRED() check mistakenly
  uses fcinfo->args instead of fcinfo->context:

  result = timestamptz2date_safe(timestamp, fcinfo->context);
  if (SOFT_ERROR_OCCURRED(fcinfo->args))   /* should be fcinfo->context */
      PG_RETURN_NULL();

  fcinfo->args is a NullableDatum[] array, not a Node *. The
  SOFT_ERROR_OCCURRED macro casts its argument to Node * and reads
  the NodeTag field. When given fcinfo->args, it interprets the first
  argument's Datum value (a TimestampTz) as a NodeTag, which will
  almost never match T_ErrorSaveContext. This causes the soft error
  check to always evaluate to false.

  As a result, when the timestamptz-to-date conversion encounters an
  overflow in error-safe mode, the function returns a wrong date value
  instead of the expected NULL.

  All three sibling functions modified in the same commit (date_timestamp,
  timestamp_date, date_timestamptz) correctly use fcinfo->context.
  This appears to be a copy-paste oversight.

  The fix is a one-line change: fcinfo->args → fcinfo->context.

---

From: Nathan Bossart <[email-masked]>
Date: Tue, 24 Mar 2026 15:53:10 -0500

>   I found a small bug in commit e2f289e5b9b ("Make many cast functions

Nice find.  For future reference, since this was just committed, it
might've been better to report it directly in the thread where the change
was discussed.

>   The fix is a one-line change: fcinfo->args → fcinfo->context.

LGTM.  To prevent this from happening in the future, I think we ought to
change SOFT_ERROR_OCCURRED to a static inline function.  I tried that, and
I got the following warnings:

    execExprInterp.c:4964:27: warning: incompatible pointer types passing 'ErrorSaveContext *' (aka 'struct ErrorSaveContext *') to parameter of type 'Node *' (aka 'struct Node *') [-Wincompatible-pointer-types]
     4964 |                 if (SOFT_ERROR_OCCURRED(&jsestate->escontext))
          |                                         ^~~~~~~~~~~~~~~~~~~~
    ../../../src/include/nodes/miscnodes.h:54:27: note: passing argument to parameter 'escontext' here
       54 | SOFT_ERROR_OCCURRED(Node *escontext)
          |                           ^
    execExprInterp.c:5200:26: warning: incompatible pointer types passing 'ErrorSaveContext *' (aka 'struct ErrorSaveContext *') to parameter of type 'Node *' (aka 'struct Node *') [-Wincompatible-pointer-types]
     5200 |         if (SOFT_ERROR_OCCURRED(&jsestate->escontext))
          |                                 ^~~~~~~~~~~~~~~~~~~~
    ../../../src/include/nodes/miscnodes.h:54:27: note: passing argument to parameter 'escontext' here
       54 | SOFT_ERROR_OCCURRED(Node *escontext)
          |                           ^

I think we just need to add casts to "Node *" for those.  AFAICT there
isn't an actual bug.

[... looks for past discussions ...]

Ah, I noticed this thread, where the same lines of code were discussed:

	https://postgr.es/m/flat/20240724.155525.366150353176322967.ishii%40postgresql.org

-- 
nathan

---

From: Amit Langote <[email-masked]>
Date: Wed, 25 Mar 2026 12:13:55 +0900

On Wed, Mar 25, 2026 at 5:53 Nathan Bossart <[email-masked]>
wrote:

>     execExprInterp.c:4964:27: warning: incompatible pointer types passing
> 'ErrorSaveContext *' (aka 'struct ErrorSaveContext *') to parameter of type
> 'Node *' (aka 'struct Node *') [-Wincompatible-pointer-types]
>     execExprInterp.c:5200:26: warning: incompatible pointer types passing
> 'ErrorSaveContext *' (aka 'struct ErrorSaveContext *') to parameter of type
> 'Node *' (aka 'struct Node *') [-Wincompatible-pointer-types]

That seems ok to me.

> https://postgr.es/m/flat/20240724.155525.366150353176322967.ishii%40postgresql.org

ISTM the fix proposed by Ishii-san in that thread is the same thing, but
yours LGTM too.

- Amit

>
> <https://postgr.es/m/flat/20240724.155525.366150353176322967.ishii%40postgresql.org>

---

From: Peter Eisentraut <[email-masked]>
Date: Wed, 25 Mar 2026 07:16:26 +0100

>    The fix is a one-line change: fcinfo->args → fcinfo->context.

committed the fix, thanks

---

From: Nathan Bossart <[email-masked]>
Date: Wed, 25 Mar 2026 14:18:00 -0500

On Wed, Mar 25, 2026 at 07:17:15AM +0100, Peter Eisentraut wrote:
> On 24.03.26 21:53, Nathan Bossart wrote:
> Or maybe we change the escontext field to be of type Node *?

I started looking at this, but it seems to be a rather invasive change for
the level of gain.  Not only does it require more memory management, but we
then have to cast it many places like this:

    ((ErrorSaveContext *) jsestate->escontext)->error_occured = false;

If we instead make it an ErrorSaveContext *, we'd still need to cast it to
Node * for SOFT_ERROR_OCCURRED, unless we had it accept a void * or
something, which defeats the purpose.

-- 
nathan