Re: pgsql: Add more SQL/JSON constructor functions

On Tue, Jul 23, 2024 at 11:45 AM jian he <jian.universality@gmail.com> wrote:
> On Mon, Jul 22, 2024 at 4:46 PM Amit Langote <amitlangote09@gmail.com> wrote:
> >
> > On Thu, Jul 18, 2024 at 3:04 PM jian he <jian.universality@gmail.com> wrote:
> > > we still have problem in transformJsonBehavior
> > >
> > > currently transformJsonBehavior:
> > > SELECT JSON_VALUE(jsonb '1234', '$' RETURNING bit(3)  DEFAULT 010111 ON ERROR);
> > > ERROR:  cannot cast behavior expression of type text to bit
> > > LINE 1: ...VALUE(jsonb '1234', '$' RETURNING bit(3)  DEFAULT 010111 ON ...
> > >
> > > here, 010111 will default to int4, so "cannot cast behavior expression
> > > of type text to bit"
> > > is wrong?
> > > also int4/int8 can be explicitly cast to bit(3), in this case, it
> > > should return 111.
> >
> > I think we shouldn't try too hard in the code to "automatically" cast
> > the DEFAULT expression, especially if that means having to add special
> > case code for all sorts of source-target-type combinations.
> >
> > I'm inclined to just give a HINT to the user to cast the DEFAULT
> > expression by hand, because they *can* do that with the syntax that
> > exists.
>
> select typname, typinput, pg_get_function_identity_arguments(typinput)
> from pg_type pt join pg_proc proc  on proc.oid = pt.typinput
> where typtype = 'b' and typarray <> 0 and proc.pronargs > 1;
>
> As you can see from the query result, we only need to deal with bit
> and character type
> in this context.
>
> SELECT JSON_VALUE(jsonb '1234', '$.a' RETURNING bit(3) DEFAULT 10111 ON empty);
> SELECT JSON_VALUE(jsonb '1234', '$.a' RETURNING char(3) DEFAULT 10111
> ON empty) ;
>
> the single quote literal ', no explicit cast, resolve to text type.
> no single quote like 11, no explicit cast, resolve to int type.
> we actually can cast int to bit, also have pg_cast entry.
> so the above these 2 examples should behave the same, given there is
> no pg_cast entry for int to text.
>
> select castsource::regtype ,casttarget::regtype ,castfunc,castcontext,
> castmethod
> from pg_cast where 'int'::regtype in (castsource::regtype ,casttarget::regtype);
>
> but i won't insist on it, since bit/varbit don't use that much.

The cast from int to bit that exists in pg_cast is only good for
explicit casts, so would truncate user's value instead of flagging it
as invalid input, and this whole discussion is about not doing that.
With the DEFAULT expression specified or interpreted as a text string,
we don't have that problem because we can then use CoerceViaIO as an
assignment-level cast, whereby the invalid input *is* flagged as it
should, like this:

SELECT JSON_VALUE(jsonb '1234', '$' RETURNING bit(3)  DEFAULT '11111' ON ERROR);
ERROR:  bit string length 5 does not match type bit(3)

So it seems fair to me to flag it when the user specifies an integer
in DEFAULT we can't create a cast expression that does not truncate a
value to fit the RETURNING type.

> > I'm planning to push the attached 2 patches.  0001 is to fix
> > transformJsonBehavior() for these cases and 0002 to adjust the
> > behavior of casting the result of JSON_EXISTS() and EXISTS columns to
> > integer type.  I've included the tests in your patch in 0001.  I
> > noticed using cast expression to coerce the boolean constants to
> > fixed-length types would produce unexpected errors when the planner's
> > const-simplification calls the cast functions.  So in 0001, I've made
> > that case also use runtime coercion using json_populate_type().
> >
>
> +  <note>
> +   <para>
> +    If an <literal>ON ERROR</literal> or <literal>ON EMPTY</literal>
> +    expression can't be coerced to the <literal>RETURNING</literal> type
> +    successfully, an SQL NULL value will be returned.
> +   </para>
> +  </note>
>
> I think this change will have some controversy.

On second thought, I agree.  I've made some changes to *throw* the
error when the JsonBehavior values fail being coerced to the RETURNING
type.  Please check the attached.

In the attached patch, I've also taken care of the problem mentioned
in your latest email -- the solution I've chosen is not to produce the
error when ERROR ON ERROR is specified but to use runtime coercion
also for the jsonb type or any type that is not integer.  Also fixed
the typos.

Thanks for your attention!

--
Thanks, Amit Langote