Re: Refactoring: Use soft error reporting for *_opt_error functions

On Tue, Sep 2, 2025 at 9:46 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Mon, Sep 01, 2025 at 12:21:18PM +0100, Dean Rasheed wrote:
> > On Mon, 1 Sept 2025 at 10:36, Amul Sul <sulamul(at)gmail(dot)com> wrote:
> >> I believe we should update all *_opt_error functions to use the new
> >> soft error reporting infrastructure instead of boolean flags -- did
> >> the same in the attached patch. I am not sure if this patch should be
> >> part of that thread[1]. It's a significant improvement in itself, as
> >> it would make the code more compact and consistent.
>
> Handling that as a separate patch seems OK here. Thanks for caring.
>
> > Agreed. That does look neater.
>
> Yep. More consistent.
>
> +/* forward declarations to avoid node.h include */
> +typedef struct Node Node;
>
> s/declarations/declaration/. Singular required, only one declaration.
>

Ok, will fix that.

> Looking at the surroundings, would it make sense to do the same for
> pg_lsn_in_internal()? It requires a safe fallback when parsing the
> LSN from the recovery_target_lsn GUC.

Yeah, it makes sense, will change in the next version.

Just a quick question regarding the naming conventions. It looks like
we have a choice between two options for consistency. Should we rename
the pg_lsn_in_internal function by replacing "_internal" with "_safe",
or should we rename all of the *_opt_error functions by replacing
"_opt_error" with "_internal"?

I would choose the latter option.

Regards,
Amul