MIME-Version: 1.0
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>
 <04afcd1f-ed7d-4c0a-add1-50e3719ccbf9@postgresfriends.org>
 <CACJufxGRAnwJzu7nMq4ZP=yqa1Sz=qR+mR1TmY0aCDjJoJRRtg@mail.gmail.com>
 <762ae707-7fdc-43d8-a77a-3a10d12ce21d@postgresfriends.org>
 <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>
In-Reply-To: 
 <CACJufxEcrrcaeFW+zYsjgb6r+ijzwszyxeHk3wxGY+3idiA2ZA@mail.gmail.com>
From: Corey Huinker <corey.huinker@gmail.com>
Date: Mon, 10 Nov 2025 23:36:49 -0500
Message-ID: 
 <CADkLM=ehavqENDBCcYQufPFKboV90+o_uFdhcrh=Ymq_TNqo=A@mail.gmail.com>
Subject: Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User
 Functions
To: jian he <jian.universality@gmail.com>
Cc: Vik Fearing <vik@postgresfriends.org>,
 Isaac Morland <isaac.morland@gmail.com>,
	pgsql-hackers@lists.postgresql.org
Content-Type: multipart/alternative; boundary="000000000000fc10fc06434a32d8"
Archived-At: 
 <https://www.postgresql.org/message-id/CADkLM%3DehavqENDBCcYQufPFKboV90%2Bo_uFdhcrh%3DYmq_TNqo%3DA%40mail.gmail.com>
Precedence: bulk

--000000000000fc10fc06434a32d8
Content-Type: text/plain; charset="UTF-8"

>
> As mentioned before, to make
> CAST(source_expr AS target_type DEFAULT expr ON CONVERSION ERROR);
> work,
> we cannot just simply replace casting FuncExpr nodes with CoerceViaIO,
> since
> type modifiers behave differently in these two Nodes.
> (e.g., casting numeric 1.11 to integer is not equivalent to casting the
> literal
> "1.11" to integer).
>

I wasn't suggesting that. I was suggesting that we move tests that use a
given type's custom cast function into the same patch that makes that cast
safe. This would mean that the initial syntax+nodes+executor patch starts
out only with test cases known to not have custom functions.


>
> Also, are we settled on this new pg_cast column name
> (pg_cast.casterrorsafe)?
> Overall, I think it's better not to touch pg_cast.dat in each of these
> refactoring patches.
>

I'm fine with it. I can see having 'f' and 's' both mean cast functions,
but 's' means safe, but the extra boolean works too and we'll be fine with
either method.


>
> Without first refactoring pg_cast.castfunc (01 to 18), making CAST ...
> DEFAULT as the first patch (0001)
> won't work, since pg_cast.castfunc itself is not error safe yet, and we
> have no way to test the CAST DEFAULT syntax.
>
> So we have to *first* refactor pg_cast.castfunc, make it error safe
> then implement CAST DEFAULT.
>

Makes sense.


>
> However, I found out, to make
> SELECT CAST('five'::INTEGER AS INTEGER DEFAULT 6 ON CONVERSION ERROR);
> error safe is hard.
>

That's no problem at all. The CAST () function can't do anything about an
input that's already an error before the CAST node executes. Nor should it.
It should only concern itself with errors related to the actual casting of
a value to the specified type.


>
> ('five'::INTEGER) is a TypeCast node, normally it will error out in
> transformTypeCast->
> coerce_to_target_type->coerce_type ```(if (inputTypeId == UNKNOWNOID
> && IsA(node, Const)))```
> If we do not error out, then we need a Node to represent the failed cast,
> mainly
> for deparse purposes.
>

We _must_ error out in that case, before the CAST executes.


>
> that means for CAST(source_expr .... DEFAULT defexpr ON CONVERSION ERROR);
> The only corner case we handle is when source_expr is a simple Const node.
> In all other cases, source_expr is processed through transformExpr,
> which does all
> the normal parse analysis for a node.
>

I'll get to reviewing this patchset soon.

--000000000000fc10fc06434a32d8
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div class=3D"gmail_quote gmail_quote_container"><blockquo=
te class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px =
solid rgb(204,204,204);padding-left:1ex">As mentioned before, to make<br>
CAST(source_expr AS target_type DEFAULT expr ON CONVERSION ERROR);<br>
work,<br>
we cannot just simply replace casting FuncExpr nodes with CoerceViaIO, sinc=
e<br>
type modifiers behave differently in these two Nodes.<br>
(e.g., casting numeric 1.11 to integer is not equivalent to casting the lit=
eral<br>
&quot;1.11&quot; to integer).<br></blockquote><div><br></div><div>I wasn=
9;t suggesting that. I was suggesting that we move tests that use a given t=
ype&#39;s=C2=A0custom cast function into the same patch that makes that cas=
t safe. This would mean that the initial syntax+nodes+executor patch starts=
 out only with test cases known to not have custom functions.</div><div><br=
></div><div>=C2=A0</div><blockquote class=3D"gmail_quote" style=3D"margin:0=
px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Also, are we settled on this new pg_cast column name (pg_cast.casterrorsafe=
)?<br>
Overall, I think it&#39;s better not to touch pg_cast.dat in each of these<=
br>
refactoring patches.<br></blockquote><div><br></div><div>I&#39;m fine with =
it. I can see having &#39;f&#39; and &#39;s&#39; both mean cast functions, =
but &#39;s&#39; means safe, but the extra boolean works too and we&#39;ll b=
e fine with either method.</div><div></div><div>=C2=A0</div><blockquote cla=
ss=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid =
rgb(204,204,204);padding-left:1ex">
<br>
Without first refactoring pg_cast.castfunc (01 to 18), making CAST ...<br>
DEFAULT as the first patch (0001)<br>
won&#39;t work, since pg_cast.castfunc itself is not error safe yet, and we=
<br>
have no way to test the CAST DEFAULT syntax.<br>
<br>
So we have to *first* refactor pg_cast.castfunc, make it error safe<br>
then implement CAST DEFAULT.<br></blockquote><div><br></div><div>Makes sens=
e.</div><div>=C2=A0</div><blockquote class=3D"gmail_quote" style=3D"margin:=
0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">=
<br>
However, I found out, to make<br>
SELECT CAST(&#39;five&#39;::INTEGER AS INTEGER DEFAULT 6 ON CONVERSION ERRO=
R);<br>
error safe is hard.<br></blockquote><div><br></div><div>That&#39;s no probl=
em at all. The CAST () function can&#39;t do anything about an input that&#=
39;s already an error before the CAST node executes. Nor should it. It shou=
ld only concern itself with errors related to the actual casting of a value=
 to the specified type.</div><div>=C2=A0</div><blockquote class=3D"gmail_qu=
ote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,20=
4);padding-left:1ex">
<br>
(&#39;five&#39;::INTEGER) is a TypeCast node, normally it will error out in=
<br>
transformTypeCast-&gt;<br>
coerce_to_target_type-&gt;coerce_type ```(if (inputTypeId =3D=3D UNKNOWNOID=
<br>
&amp;&amp; IsA(node, Const)))```<br>
If we do not error out, then we need a Node to represent the failed cast, m=
ainly<br>
for deparse purposes.<br></blockquote><div><br></div><div>We _must_ error o=
ut in that case, before the CAST executes.</div><div></div><div>=C2=A0</div=
><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border=
-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
that means for CAST(source_expr .... DEFAULT defexpr ON CONVERSION ERROR);<=
br>
The only corner case we handle is when source_expr is a simple Const node.<=
br>
In all other cases, source_expr is processed through transformExpr,<br>
which does all<br>
the normal parse analysis for a node.<br></blockquote><div><br></div><div>I=
&#39;ll get to reviewing this patchset soon.</div></div></div>

--000000000000fc10fc06434a32d8--