Date: Tue, 24 Mar 2026 15:53:10 -0500
From: Nathan Bossart <nathandbossart@gmail.com>
To: Jianghua Yang <yjhjstz@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org, peter@eisentraut.org,
	jian.universality@gmail.com, amitlangote09@gmail.com
Subject: Re: [PATCH] Fix wrong argument to SOFT_ERROR_OCCURRED in
 timestamptz_date
Message-ID: <acL5tt3tfsyDql9x@nathan>
References: 
 <CAAZLFmSGti716gWeY=DCZ9TTVOixnHZ4_4V4tDzoeE86D64vOA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="X83H52P7q57jdRYd"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CAAZLFmSGti716gWeY=DCZ9TTVOixnHZ4_4V4tDzoeE86D64vOA@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/acL5tt3tfsyDql9x%40nathan>
Precedence: bulk


--X83H52P7q57jdRYd
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On Tue, Mar 24, 2026 at 08:44:29AM -0700, Jianghua Yang wrote:
>   I found a small bug in commit e2f289e5b9b ("Make many cast functions
> error safe").

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

--X83H52P7q57jdRYd
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename=v2-0001-change-SOFT_ERROR_OCCURRED-to-a-static-inline-fun.patch