public inbox for [email protected]  

help / color / mirror / Atom feed

From: Amul Sul <[email protected]>
To: jian he <[email protected]>
Cc: Corey Huinker <[email protected]>
Cc: Vik Fearing <[email protected]>
Cc: Isaac Morland <[email protected]>
Cc: [email protected]
Subject: Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
Date: Mon, 17 Nov 2025 19:13:15 +0530
Message-ID: <CAAJ_b95rrHrY_RJqc=iJQkGrKhUUqTHGO1m3_hJ7A+aG_qxZHQ@mail.gmail.com> (raw)
In-Reply-To: <CACJufxEcrrcaeFW+zYsjgb6r+ijzwszyxeHk3wxGY+3idiA2ZA@mail.gmail.com>
References: <CADkLM=fv1JfY4Ufa-jcwwNbjQixNViskQ8jZu3Tz_p656i_4hQ@mail.gmail.com>
	<CAMsGm5dpfm2PHL8XZvC-JSd+UPkgx3rpReUA=G=4+rUCH+Ntcw@mail.gmail.com>
	<CADkLM=eD_S8mGhPfu5+hXXvXgR0-cxGpGd9dgPzD+nCuO7HFaQ@mail.gmail.com>
	<CACJufxHCMzrHOW=wRe8L30rMhB3sjwAv1LE928Fa7sxMu1Tx-g@mail.gmail.com>
	<[email protected]>
	<CACJufxGRAnwJzu7nMq4ZP=yqa1Sz=qR+mR1TmY0aCDjJoJRRtg@mail.gmail.com>
	<[email protected]>
	<CACJufxFy+DFpJ2e-czyCTAgSJXNFaQGWFKA4mjbW-LAMGc1YBA@mail.gmail.com>
	<CADkLM=f1Jv81=s5Ckazx3zZq=M5KoBJMJkOZux_-L+gezODCEQ@mail.gmail.com>
	<CACJufxGw_OY7K3rfG4kDb902O2guhT-wgTjTJQ=pWeVWRTHpHQ@mail.gmail.com>
	<CADkLM=cFSg3+6Sk00dLAF7Q7jnrKBk6+N5gRxT5BCxRvaGtR-g@mail.gmail.com>
	<CACJufxE_aO5FtBGwhDym-Fwe7k8oJY7a8jcYDx77=t3maPvG0g@mail.gmail.com>
	<CADkLM=chahh6ddZFjLL6AUdqzL_Px0raTu-5Jzn2WN8yELtmJw@mail.gmail.com>
	<CACJufxE053=bO3pDUpGba6Yz3VGpU_XCbg4HO6Rew5EJ7k7VnQ@mail.gmail.com>
	<CACJufxF--5d=fmoRBHfqJE9Vy38dCURNKYOKKpujRCnoTEQ7nQ@mail.gmail.com>
	<CACJufxHpMJn22Nu_wmG6eV_S8SAM6KM+GhMO7GuzVb=d9q5C4A@mail.gmail.com>
	<CACJufxHM2e3DQmbRdDZvWyG3ZCLyOg6XFifvOz_TGy1tGw7NHw@mail.gmail.com>
	<CADkLM=daTLuRcwzc6Egtwvh4XYgtABWuMBVnEznd-dXqmXfzUw@mail.gmail.com>
	<CACJufxEcrrcaeFW+zYsjgb6r+ijzwszyxeHk3wxGY+3idiA2ZA@mail.gmail.com>

On Tue, Nov 11, 2025 at 8:37 AM jian he <[email protected]> wrote:
>
> On Thu, Nov 6, 2025 at 6:00 AM Corey Huinker <[email protected]> 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

view thread (75+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
  In-Reply-To: <CAAJ_b95rrHrY_RJqc=iJQkGrKhUUqTHGO1m3_hJ7A+aG_qxZHQ@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox