| From: | Amit Langote <amitlangote09(at)gmail(dot)com> |
|---|---|
| To: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
| Cc: | Jianghua Yang <yjhjstz(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, peter(at)eisentraut(dot)org, jian(dot)universality(at)gmail(dot)com |
| Subject: | Re: [PATCH] Fix wrong argument to SOFT_ERROR_OCCURRED in timestamptz_date |
| Date: | 2026-03-25 03:13:55 |
| Message-ID: | CA+HiwqFcyVFtUes6MbCW9Ek8=Fi5Mr-1b5v37a8fary=UfA4kA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, Mar 25, 2026 at 5:53 Nathan Bossart <nathandbossart(at)gmail(dot)com>
wrote:
> 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.
That seems ok to me.
[... 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
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 | Date | Subject | |
|---|---|---|---|
| Next Message | Bertrand Drouvot | 2026-03-25 03:15:19 | Re: Adding locks statistics |
| Previous Message | Bharath Rupireddy | 2026-03-25 03:09:48 | Re: Reduce log level of some logical decoding messages to DEBUG1 |