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>
In-Reply-To: <f1084e8b-17f2-4e1f-9d75-fbb48eb75280@enterprisedb.com>
From: Amit Langote <amitlangote09@gmail.com>
Date: Thu, 7 Mar 2024 21:06:08 +0900
Message-ID: <CA+HiwqF5CHyRBO8WwOcQsKufFWZ1yjDaQQ7DJxsQNbm2nab3pg@mail.gmail.com>
Subject: Re: remaining sql/json patches
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: Himanshu Upadhyaya <upadhyaya.himanshu@gmail.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, 
	Erik Rijkers <er@xs4all.nl>, jian he <jian.universality@gmail.com>, 
	Andres Freund <andres@anarazel.de>, Andrew Dunstan <andrew@dunslane.net>, 
	PostgreSQL-development <pgsql-hackers@postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CA%2BHiwqF5CHyRBO8WwOcQsKufFWZ1yjDaQQ7DJxsQNbm2nab3pg%40mail.gmail.com>
Precedence: bulk

On Thu, Mar 7, 2024 at 8:13=E2=80=AFPM Tomas Vondra
<tomas.vondra@enterprisedb.com> wrote:
> On 3/7/24 06:18, Himanshu Upadhyaya wrote:

Thanks Himanshu for the testing.

> > On Wed, Mar 6, 2024 at 9:04=E2=80=AFPM Tomas Vondra <tomas.vondra@enter=
prisedb.com>
> > wrote:
> >>
> >> I'm pretty sure this is the correct & expected behavior. The second
> >> query treats the value as string (because that's what should happen fo=
r
> >> values in double quotes).
> >>
> >>  ok, Then why does the below query provide the correct conversion, eve=
n if
> > we enclose that in double quotes?
> > =E2=80=98postgres[102531]=3D#=E2=80=99SELECT * FROM JSON_TABLE(jsonb '{
> >          "id" : "1234567890",
> >          "FULL_NAME" : "JOHN DOE"}',
> >                 '$'
> >                 COLUMNS(
> >                      name varchar(20) PATH 'lax $.FULL_NAME',
> >                      id int PATH 'lax $.id'
> >       )
> >    )
> > ;
> >    name   |     id
> > ----------+------------
> >  JOHN DOE | 1234567890
> > (1 row)
> >
> > and for bigger input(string) it will leave as empty as below.
> > =E2=80=98postgres[102531]=3D#=E2=80=99SELECT * FROM JSON_TABLE(jsonb '{
> >          "id" : "12345678901",
> >          "FULL_NAME" : "JOHN DOE"}',
> >                 '$'
> >                 COLUMNS(
> >                      name varchar(20) PATH 'lax $.FULL_NAME',
> >                      id int PATH 'lax $.id'
> >       )
> >    )
> > ;
> >    name   | id
> > ----------+----
> >  JOHN DOE |
> > (1 row)
> >
> > seems it is not something to do with data enclosed in double quotes but
> > somehow related with internal casting it to integer and I think in case=
 of
> > bigger input it is not able to cast it to integer(as defined under COLU=
MNS
> > as id int PATH 'lax $.id')
> >
> > =E2=80=98postgres[102531]=3D#=E2=80=99SELECT * FROM JSON_TABLE(jsonb '{
> >          "id" : "12345678901",
> >          "FULL_NAME" : "JOHN DOE"}',
> >                 '$'
> >                 COLUMNS(
> >                      name varchar(20) PATH 'lax $.FULL_NAME',
> >                      id int PATH 'lax $.id'
> >       )
> >    )
> > ;
> >    name   | id
> > ----------+----
> >  JOHN DOE |
> > (1 row)
> > )
> >
> > if it is not able to represent it to integer because of bigger input, i=
t
> > should error out with a similar error message instead of leaving it emp=
ty.
> >
> > Thoughts?
> >
>
> Ah, I see! Yes, that's a bit weird. Put slightly differently:
>
> test=3D# SELECT * FROM JSON_TABLE(jsonb '{"id" : "2000000000"}',
>                 '$' COLUMNS(id int PATH '$.id'));
>      id
> ------------
>  2000000000
> (1 row)
>
> Time: 0.248 ms
> test=3D# SELECT * FROM JSON_TABLE(jsonb '{"id" : "3000000000"}',
>                 '$' COLUMNS(id int PATH '$.id'));
>  id
> ----
>
> (1 row)
>
> Clearly, when converting the string literal into int value, there's some
> sort of error handling that realizes 3B overflows, and returns NULL
> instead. I'm not sure if this is intentional.

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.

For the latter, it uses numeric_int4() which doesn't support
soft-error handling, so throws the error.  With quotes, the


--
Thanks, Amit Langote

--
Thanks, Amit Langote