MIME-Version: 1.0
References: <CACJufxFz=OsXQdsMJ-cqoqspD9aJrwntsQP-U2A-UaV_M+-S9g@mail.gmail.com>
 <CACJufxEKjGZ+9vjOMs0YM5VThZe5HFXrDbJQPm++RA7=HBj_xg@mail.gmail.com>
 <CACJufxEiCxM-GXHyPoW+s+Mq1sKcVBAem9DM94oN7H5dNzaFRA@mail.gmail.com>
 <CACJufxHT16yeEsYn=aN614W3AgufxnDMdhNkba0yjji33OOwtw@mail.gmail.com>
 <CA+vB=AELb=QjnpJRJC=m_uz=PGuRNgDJjazBvLbmOWuCgUqHQA@mail.gmail.com> <CACJufxEqnZL47VtUKsQ5CpmuQkk1wQ=A_DaxiCGkdKi+-5787Q@mail.gmail.com>
In-Reply-To: <CACJufxEqnZL47VtUKsQ5CpmuQkk1wQ=A_DaxiCGkdKi+-5787Q@mail.gmail.com>
From: Vaibhav Dalvi <vaibhav.dalvi@enterprisedb.com>
Date: Thu, 22 Jan 2026 09:43:32 +0530
Message-ID: <CA+vB=AGguhpEtH46QMSbGKU8PwwtVZuNDoNBn25XryfadUaGPQ@mail.gmail.com>
Subject: Re: finish TODOs in to_json_is_immutable, to_jsonb_is_immutable also
 add tests on it
To: jian he <jian.universality@gmail.com>
Cc: hackers@postgresql.org, pgsql-hackers@postgresql.org
Content-Type: multipart/alternative; boundary="000000000000a75dc30648f24536"
Archived-At: <https://www.postgresql.org/message-id/CA%2BvB%3DAGguhpEtH46QMSbGKU8PwwtVZuNDoNBn25XryfadUaGPQ%40mail.gmail.com>
Precedence: bulk

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

Hi Jian,

Thanks for your detailed explanation. This makes sense to me as well.

Regards,
Vaibhav

On Wed, Jan 21, 2026 at 2:09=E2=80=AFPM jian he <jian.universality@gmail.co=
m> wrote:

> On Tue, Jan 13, 2026 at 7:12=E2=80=AFPM Vaibhav Dalvi
> <vaibhav.dalvi@enterprisedb.com> wrote:
> >
> > Hi Jian,
> >
> > Thanks for working on this. The patch looks good, but I have a few
> observations:
> >
> > 1. Index creation fails for range types regardless of the base data typ=
e:
> >
> > postgres=3D# create type range_int as range(subtype=3Dint);
> > CREATE TYPE
> > postgres=3D# create table range_table(r1 range_int);
> > CREATE TABLE
> > postgres=3D# create index on range_table(json_array(r1 returning jsonb)=
);
> > ERROR: functions in index expression must be marked IMMUTABLE
> >
> > I believe this is because range_out is a stable function. Consequently,
> the following
> > code snippet in to_jsonb_is_immutable may be redundant, or should simpl=
y
> return
> > false without recursing for the sub-type:
> >
> > + else if (att_typtype =3D=3D TYPTYPE_RANGE)
> > + {
> > + to_jsonb_is_immutable(get_range_subtype(typoid), contain_mutable);
> > + }
> >
>
> yech, your observation is right.
>
> create index on range_table((r1::text));
>
> will fail, because pg_proc.provolatile is 's' for function range_out.
>
> but imagine the function range_out marked as immutable, then we would
> need to recurse to the range's subtype.
> also, it makes the logic more complite: if the type is container type,
> recurse to check its elemental/subtype.
>
> Therefore, I am inclined to leave it unchanged.
>
> > 2. Regarding the function names to_jsonb_is_immutable and
> to_json_is_immutable:
> > since these do not return a boolean value, "is" may no longer be
> appropriate. Perhaps
> > something like to_jsonb_check_mutable would be more accurate?
> >
>
> to_jsonb_is_immutable and to_json_is_immutable are marked as extern, so
> they may
> be used by other extensions. Renaming them would require extension author=
s
> to
> discover and update to the new symbol names. Adding an additional argumen=
t
> makes
> the change immediately visible through a signature mismatch.
>
> so I think we should stick to the current name.
>
> --
> jian
> https://www.enterprisedb.com
>

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

<div dir=3D"ltr"><div>Hi Jian,</div><div><br></div><div>Thanks for your det=
ailed explanation. This makes sense to me as well.</div><div><br></div><div=
>Regards,</div><div>Vaibhav</div></div><br><div class=3D"gmail_quote gmail_=
quote_container"><div dir=3D"ltr" class=3D"gmail_attr">On Wed, Jan 21, 2026=
 at 2:09=E2=80=AFPM jian he &lt;<a href=3D"mailto:jian.universality@gmail.c=
om">jian.universality@gmail.com</a>&gt; wrote:<br></div><blockquote class=
=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rg=
b(204,204,204);padding-left:1ex">On Tue, Jan 13, 2026 at 7:12=E2=80=AFPM Va=
ibhav Dalvi<br>
&lt;<a href=3D"mailto:vaibhav.dalvi@enterprisedb.com" target=3D"_blank">vai=
bhav.dalvi@enterprisedb.com</a>&gt; wrote:<br>
&gt;<br>
&gt; Hi Jian,<br>
&gt;<br>
&gt; Thanks for working on this. The patch looks good, but I have a few obs=
ervations:<br>
&gt;<br>
&gt; 1. Index creation fails for range types regardless of the base data ty=
pe:<br>
&gt;<br>
&gt; postgres=3D# create type range_int as range(subtype=3Dint);<br>
&gt; CREATE TYPE<br>
&gt; postgres=3D# create table range_table(r1 range_int);<br>
&gt; CREATE TABLE<br>
&gt; postgres=3D# create index on range_table(json_array(r1 returning jsonb=
));<br>
&gt; ERROR: functions in index expression must be marked IMMUTABLE<br>
&gt;<br>
&gt; I believe this is because range_out is a stable function. Consequently=
, the following<br>
&gt; code snippet in to_jsonb_is_immutable may be redundant, or should simp=
ly return<br>
&gt; false without recursing for the sub-type:<br>
&gt;<br>
&gt; + else if (att_typtype =3D=3D TYPTYPE_RANGE)<br>
&gt; + {<br>
&gt; + to_jsonb_is_immutable(get_range_subtype(typoid), contain_mutable);<b=
r>
&gt; + }<br>
&gt;<br>
<br>
yech, your observation is right.<br>
<br>
create index on range_table((r1::text));<br>
<br>
will fail, because pg_proc.provolatile is &#39;s&#39; for function range_ou=
t.<br>
<br>
but imagine the function range_out marked as immutable, then we would<br>
need to recurse to the range&#39;s subtype.<br>
also, it makes the logic more complite: if the type is container type,<br>
recurse to check its elemental/subtype.<br>
<br>
Therefore, I am inclined to leave it unchanged.<br>
<br>
&gt; 2. Regarding the function names to_jsonb_is_immutable and to_json_is_i=
mmutable:<br>
&gt; since these do not return a boolean value, &quot;is&quot; may no longe=
r be appropriate. Perhaps<br>
&gt; something like to_jsonb_check_mutable would be more accurate?<br>
&gt;<br>
<br>
to_jsonb_is_immutable and to_json_is_immutable are marked as extern, so the=
y may<br>
be used by other extensions. Renaming them would require extension authors =
to<br>
discover and update to the new symbol names. Adding an additional argument =
makes<br>
the change immediately visible through a signature mismatch.<br>
<br>
so I think we should stick to the current name.<br>
<br>
--<br>
jian<br>
<a href=3D"https://www.enterprisedb.com" rel=3D"noreferrer" target=3D"_blan=
k">https://www.enterprisedb.com</a><br>
</blockquote></div>

--000000000000a75dc30648f24536--