MIME-Version: 1.0
References: <CADkLM=dpz3KFnqP-dgJ-zvRvtjsa8UZv8wDAQdqho=qN3kX0Zg@mail.gmail.com>
 <4630636f-7720-4ce4-ab1c-253b390e5174@vondra.me> <CADkLM=cWdTvyAEQcxw-6er3umo=R1ZKU_9jf2ZinNxRf9MbQZg@mail.gmail.com>
 <b4ba8ed9-f1b3-4b6b-8612-ff42d65c18c6@vondra.me> <CADkLM=dnknZtCgdp2CmRp8gjkau1axYQ7SBMgcj74KgvjQzOwQ@mail.gmail.com>
 <CACJufxEZYqocFdgn-x-bJMRBSk_zkS=ziGGkaSumteiPDksnsg@mail.gmail.com> <CACJufxF0hnpjdsjrx4zuktVMaFOqM=mx3xQXctn2262vmwCapg@mail.gmail.com>
In-Reply-To: <CACJufxF0hnpjdsjrx4zuktVMaFOqM=mx3xQXctn2262vmwCapg@mail.gmail.com>
From: Corey Huinker <corey.huinker@gmail.com>
Date: Wed, 29 Jan 2025 15:04:40 -0500
Message-ID: <CADkLM=e=ZhNbHQ-QfLRxpSN1d9kFQ3vn4yBZhw0uWr6usPA96A@mail.gmail.com>
Subject: Re: Extended Statistics set/restore/clear functions.
To: jian he <jian.universality@gmail.com>
Cc: Tomas Vondra <tomas@vondra.me>, pgsql-hackers@lists.postgresql.org
Content-Type: multipart/alternative; boundary="00000000000096e71f062cddd2cb"
Archived-At: <https://www.postgresql.org/message-id/CADkLM%3De%3DZhNbHQ-QfLRxpSN1d9kFQ3vn4yBZhw0uWr6usPA96A%40mail.gmail.com>
Precedence: bulk

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

On Wed, Jan 29, 2025 at 2:50=E2=80=AFAM jian he <jian.universality@gmail.co=
m> wrote:

> hi.
>
> select '{"1, 0B100101":"NaN"}'::pg_ndistinct;
>       pg_ndistinct
> ------------------------
>  {"1, 37": -2147483648}
> (1 row)
>

I think my initial reaction is to just refuse those special values, but
I'll look into the parsing code to see what can be done.


this is not what we expected?
> For the VALUE part of pg_ndistinct, float8 has 3 special values: inf,
> -inf, NaN.
>
> For the key part of pg_ndistinct, see example.
> select '{"1, 16\t":"1"}'::pg_ndistinct;
> here \t is not tab character, ascii 9. it's two characters: backslash
> and character "t".
> so here it should error out?
> (apply this to \n, \r, \b)
>

I don't have a good answer as to what should happen here. Special cases
like this make Tomas' suggestion to change the in/out format more
attractive.


>
>
> pg_ndistinct_in(PG_FUNCTION_ARGS)
> ending part should be:
>
>     freeJsonLexContext(lex);
>     if (result =3D=3D JSON_SUCCESS)
>     {
>         ......
>     }
>     else
>     {
>        ereturn(parse_state.escontext, (Datum) 0,
>                     errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
>                     errmsg("malformed pg_ndistinct: \"%s\"", str),
>                     errdetail("Must be valid JSON."));
>        PG_RETURN_NULL();
>     }
> result should be either JSON_SUCCESS or anything else.
>
>
>
> all these functions:
> ndistinct_object_start, ndistinct_array_start,
> ndistinct_object_field_start, ndistinct_array_element_start
> have
> ndistinctParseState *parse =3D state;
>
> do we need to change it to
> ndistinctParseState *parse =3D (ndistinctParseState *)state;
> ?
>

The compiler isn't complaining so far, but I see no harm in it.

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

<div dir=3D"ltr"><div dir=3D"ltr">On Wed, Jan 29, 2025 at 2:50=E2=80=AFAM j=
ian he &lt;<a href=3D"mailto:jian.universality@gmail.com">jian.universality=
@gmail.com</a>&gt; wrote:</div><div class=3D"gmail_quote gmail_quote_contai=
ner"><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;bo=
rder-left:1px solid rgb(204,204,204);padding-left:1ex">hi.<br>
<br>
select &#39;{&quot;1, 0B100101&quot;:&quot;NaN&quot;}&#39;::pg_ndistinct;<b=
r>
=C2=A0 =C2=A0 =C2=A0 pg_ndistinct<br>
------------------------<br>
=C2=A0{&quot;1, 37&quot;: -2147483648}<br>
(1 row)<br></blockquote><div><br></div><div>I think my initial reaction is =
to just refuse those special values, but I&#39;ll look into the parsing cod=
e to see what can be done.<br><br><br><br></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">
this is not what we expected?<br>
For the VALUE part of pg_ndistinct, float8 has 3 special values: inf, -inf,=
 NaN.<br>
<br>
For the key part of pg_ndistinct, see example.<br>
select &#39;{&quot;1, 16\t&quot;:&quot;1&quot;}&#39;::pg_ndistinct;<br>
here \t is not tab character, ascii 9. it&#39;s two characters: backslash<b=
r>
and character &quot;t&quot;.<br>
so here it should error out?<br>
(apply this to \n, \r, \b)<br></blockquote><div><br></div><div>I don&#39;t =
have a good answer as to what should happen here. Special cases like this m=
ake Tomas&#39; suggestion to change the in/out format more attractive.</div=
><div><br></div><div><br></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>
<br>
pg_ndistinct_in(PG_FUNCTION_ARGS)<br>
ending part should be:<br>
<br>
=C2=A0 =C2=A0 freeJsonLexContext(lex);<br>
=C2=A0 =C2=A0 if (result =3D=3D JSON_SUCCESS)<br>
=C2=A0 =C2=A0 {<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 ......<br>
=C2=A0 =C2=A0 }<br>
=C2=A0 =C2=A0 else<br>
=C2=A0 =C2=A0 {<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0ereturn(parse_state.escontext, (Datum) 0,<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 errco=
de(ERRCODE_INVALID_TEXT_REPRESENTATION),<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 errms=
g(&quot;malformed pg_ndistinct: \&quot;%s\&quot;&quot;, str),<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 errde=
tail(&quot;Must be valid JSON.&quot;));<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0PG_RETURN_NULL();<br>
=C2=A0 =C2=A0 }<br>
result should be either JSON_SUCCESS or anything else.<br>
<br>
<br>
<br>
all these functions:<br>
ndistinct_object_start, ndistinct_array_start,<br>
ndistinct_object_field_start, ndistinct_array_element_start<br>
have<br>
ndistinctParseState *parse =3D state;<br>
<br>
do we need to change it to<br>
ndistinctParseState *parse =3D (ndistinctParseState *)state;<br>
?<br></blockquote><div><br></div><div>The compiler isn&#39;t complaining=C2=
=A0so far, but I see no harm in it.</div><div><br></div></div></div>

--00000000000096e71f062cddd2cb--