MIME-Version: 1.0
References: <1cee81f3-312f-30e3-0282-c4df7ee72f93@xs4all.nl>
 <202403041503.5momp6knupgo@alvherre.pgsql> <CA+HiwqGtdVCg-KboUUU=svwukwXGT4EOFOk3eCZLz6h-d6YLNw@mail.gmail.com>
 <CAPF61jAOJM6FqMsPGYHXbKxtX8DQ2a7kuOZ3AzR8oGXHy3fjFA@mail.gmail.com>
 <562e5ad9-1357-4752-ad28-7e56740f74f8@enterprisedb.com> <CAPF61jBm+_QCwDg32_K+BgNKf09isU9g3o8CLW2kUDLZ5=28Hw@mail.gmail.com>
 <f1084e8b-17f2-4e1f-9d75-fbb48eb75280@enterprisedb.com> <CA+HiwqF5CHyRBO8WwOcQsKufFWZ1yjDaQQ7DJxsQNbm2nab3pg@mail.gmail.com>
 <CACJufxGew-Y0nyBbBeaaFAndjjdu_b4WVSao_dtVM+-EUCVWyA@mail.gmail.com>
In-Reply-To: <CACJufxGew-Y0nyBbBeaaFAndjjdu_b4WVSao_dtVM+-EUCVWyA@mail.gmail.com>
From: Amit Langote <amitlangote09@gmail.com>
Date: Thu, 7 Mar 2024 23:11:03 +0900
Message-ID: <CA+HiwqF4nrbnLDPXqNHhiOQDj0tbRSf2oaF+Wvg8U2ckNd6H=A@mail.gmail.com>
Subject: Re: remaining sql/json patches
To: jian he <jian.universality@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, Andres Freund <andres@anarazel.de>, 
	Andrew Dunstan <andrew@dunslane.net>, Erik Rijkers <er@xs4all.nl>, 
	Himanshu Upadhyaya <upadhyaya.himanshu@gmail.com>, 
	PostgreSQL-development <pgsql-hackers@postgresql.org>, Tomas Vondra <tomas.vondra@enterprisedb.com>
Content-Type: multipart/alternative; boundary="000000000000f6f222061312a5fa"
Archived-At: <https://www.postgresql.org/message-id/CA%2BHiwqF4nrbnLDPXqNHhiOQDj0tbRSf2oaF%2BWvg8U2ckNd6H%3DA%40mail.gmail.com>
Precedence: bulk

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

On Thu, Mar 7, 2024 at 22:46 jian he <jian.universality@gmail.com> wrote:

> On Thu, Mar 7, 2024 at 8:06=E2=80=AFPM Amit Langote <amitlangote09@gmail.=
com>
> wrote:
> >
> >
> > Indeed.
> >
> > This boils down to the difference in the cast expression chosen to
> > convert the source value to int in the two cases.
> >
> > The case where the source value has no quotes, the chosen cast
> > expression is a FuncExpr for function numeric_int4(), which has no way
> > to suppress errors.  When the source value has quotes, the cast
> > expression is a CoerceViaIO expression, which can suppress the error.
> > The default behavior is to suppress the error and return NULL, so the
> > correct behavior is when the source value has quotes.
> >
> > I think we'll need either:
> >
> > * fix the code in 0001 to avoid getting numeric_int4() in this case,
> > and generally cast functions that don't have soft-error handling
> > support, in favor of using IO coercion.
> > * fix FuncExpr (like CoerceViaIO) to respect SQL/JSON's request to
> > suppress errors and fix downstream functions like numeric_int4() to
> > comply by handling errors softly.
> >
> > I'm inclined to go with the 1st option as we already have the
> > infrastructure in place -- input functions can all handle errors
> > softly.
>
> not sure this is the right way.
> but attached patches solved this problem.
>
> Also, can you share the previous memory-hogging bug issue
> when you are free, I want to know which part I am missing.....


Take a look at the json_populate_type() call in ExecEvalJsonCoercion() or
specifically compare the new way of passing its void *cache parameter with
the earlier patches.

>

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

<div>On Thu, Mar 7, 2024 at 22:46 jian he &lt;<a href=3D"mailto:jian.univer=
sality@gmail.com">jian.universality@gmail.com</a>&gt; wrote:<br></div><div>=
<div class=3D"gmail_quote"><blockquote class=3D"gmail_quote" style=3D"margi=
n:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;padding-l=
eft:1ex;border-left-color:rgb(204,204,204)">On Thu, Mar 7, 2024 at 8:06=E2=
=80=AFPM Amit Langote &lt;<a href=3D"mailto:amitlangote09@gmail.com" target=
=3D"_blank">amitlangote09@gmail.com</a>&gt; wrote:<br>
&gt;<br>
&gt;<br>
&gt; Indeed.<br>
&gt;<br>
&gt; This boils down to the difference in the cast expression chosen to<br>
&gt; convert the source value to int in the two cases.<br>
&gt;<br>
&gt; The case where the source value has no quotes, the chosen cast<br>
&gt; expression is a FuncExpr for function numeric_int4(), which has no way=
<br>
&gt; to suppress errors.=C2=A0 When the source value has quotes, the cast<b=
r>
&gt; expression is a CoerceViaIO expression, which can suppress the error.<=
br>
&gt; The default behavior is to suppress the error and return NULL, so the<=
br>
&gt; correct behavior is when the source value has quotes.<br>
&gt;<br>
&gt; I think we&#39;ll need either:<br>
&gt;<br>
&gt; * fix the code in 0001 to avoid getting numeric_int4() in this case,<b=
r>
&gt; and generally cast functions that don&#39;t have soft-error handling<b=
r>
&gt; support, in favor of using IO coercion.<br>
&gt; * fix FuncExpr (like CoerceViaIO) to respect SQL/JSON&#39;s request to=
<br>
&gt; suppress errors and fix downstream functions like numeric_int4() to<br=
>
&gt; comply by handling errors softly.<br>
&gt;<br>
&gt; I&#39;m inclined to go with the 1st option as we already have the<br>
&gt; infrastructure in place -- input functions can all handle errors<br>
&gt; softly.<br>
<br>
not sure this is the right way.<br>
but attached patches solved this problem.<br>
<br>
Also, can you share the previous memory-hogging bug issue<br>
when you are free, I want to know which part I am missing.....</blockquote>=
<div dir=3D"auto"><br></div><div dir=3D"auto">Take a look at the json_popul=
ate_type() call in ExecEvalJsonCoercion() or specifically compare the new w=
ay of passing its void *cache parameter with the earlier patches.</div><blo=
ckquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left=
-width:1px;border-left-style:solid;padding-left:1ex;border-left-color:rgb(2=
04,204,204)" dir=3D"auto"></blockquote></div></div>

--000000000000f6f222061312a5fa--