Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions - Mailing list pgsql-hackers

From Amul Sul
Subject Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
Date
Msg-id CAAJ_b95rrHrY_RJqc=iJQkGrKhUUqTHGO1m3_hJ7A+aG_qxZHQ@mail.gmail.com
Whole thread Raw
In response to Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions  (jian he <jian.universality@gmail.com>)
List pgsql-hackers
On Tue, Nov 11, 2025 at 8:37 AM jian he <jian.universality@gmail.com> wrote:
>
> On Thu, Nov 6, 2025 at 6:00 AM Corey Huinker <corey.huinker@gmail.com> wrote:
> > [...]
>
> Currently, patches v9-0001 through v9-0018 focus solely on refactoring
> pg_cast.castfunc.

I had a quick look but haven't finished the full review due to lack of
time today. Here are a few initial comments:

v10-0003:

-   NO_XML_SUPPORT();
+   errsave(escontext,
+           errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+           errmsg("unsupported XML feature"),
+           errdetail("This functionality requires the server to be
built with libxml support."));
    return NULL;

Can use ereturn() instead of errsave() followed by return NULL.
--

10-0004:

+/* error safe version of textToQualifiedNameList */
+List *
+textToQualifiedNameListSafe(text *textval, Node *escontext)

If I am not mistaken, it looks like an exact copy of
textToQualifiedNameList(). I think you can simply keep only
textToQualifiedNameListSafe() and call that from
textToQualifiedNameList() with a NULL value for escontext. This way,
all errsave() or ereturn() calls will behave like ereport().

The same logic applies to RangeVarGetRelidExtendedSafe() and
makeRangeVarFromNameListSafe. These can be called from
RangeVarGetRelidExtended() and makeRangeVarFromNameList(),
respectively.
--

+   {
+       errsave(escontext,
+               errcode(ERRCODE_INVALID_NAME),
+               errmsg("invalid name syntax"));
+       return NIL;
+   }

I prefer ereturn() instead of errsave + return.
--

v10-0017

    for (i = 0; i < lengthof(messages); i++)
        if (messages[i].type == type)
-           ereport(ERROR,
+       {
+           errsave(escontext,
                    (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
                     errmsg(messages[i].msg, sqltype)));
+           return;
+       }

Here, I think, you can use ereturn() to return void.
--

v10-0018

+   if (unlikely(result == 0.0) && val1 != 0.0 && !isinf(val2))
+   {
+       errsave(escontext,
+               errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+               errmsg("value out of range: underflow"));
+
+       result = 0.0;
+       return result;
+   }

Here, you can use ereturn() to return 0.0. Similar changes are needed
at other places in the same patch.

Regards,
Amul



pgsql-hackers by date:

Previous
From: Bertrand Drouvot
Date:
Subject: Re: Consistently use the XLogRecPtrIsInvalid() macro
Next
From: Yugo Nagata
Date:
Subject: Re: Suggestion to add --continue-client-on-abort option to pgbench