Re: pgsql: Add more SQL/JSON constructor functions

Hi,

On Wed, Jun 26, 2024 at 11:46 PM jian he <jian(dot)universality(at)gmail(dot)com> wrote:
> On Wed, Jun 26, 2024 at 8:39 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> >
> > >
> > > The RETURNING variant giving an error is what the standard asks us to
> > > do apparently. I read Tom's last message on this thread as agreeing
> > > to that, even though hesitantly. He can correct me if I got that
> > > wrong.
> > >
> > > > your patch will make domain and char(n) behavior inconsistent.
> > > > create domain char2 as char(2);
> > > > SELECT JSON_VALUE(jsonb '"aaa"', '$' RETURNING char2 ERROR ON ERROR);
> > > > SELECT JSON_VALUE(jsonb '"aaa"', '$' RETURNING char(2) ERROR ON ERROR);
> > > >
> > > >
> > > > another example:
> > > > SELECT JSON_query(jsonb '"aaa"', '$' RETURNING char(2) keep quotes
> > > > default '"aaa"'::jsonb ON ERROR);
> > > > same value (jsonb "aaa") error on error will yield error,
> > > > but `default expression on error` can coerce the value to char(2),
> > > > which looks a little bit inconsistent, I think.
> > >
> > > Interesting examples, thanks for sharing.
> > >
> > > Attached updated version should take into account that typmod may be
> > > hiding under domains. Please test.
> >
>
> SELECT JSON_VALUE(jsonb '111', '$' RETURNING queryfuncs_char2 default
> '13' on error);
> return
> ERROR: value too long for type character(2)
> should return 13
>
> I found out the source of the problem is in coerceJsonExprOutput
> /*
> * Use cast expression for domain types; we need CoerceToDomain here.
> */
> if (get_typtype(returning->typid) != TYPTYPE_DOMAIN)
> {
> jsexpr->use_io_coercion = true;
> return;
> }

Thanks for this test case and the analysis. Yes, using a cast
expression for coercion to the RETURNING type generally seems to be a
source of many problems that could've been solved by fixing things so
that only use_io_coercion and use_json_coercion are enough to handle
all the cases.

I've attempted that in the attached 0001, which removes
JsonExpr.coercion_expr and a bunch of code around it.

0002 is now the original patch minus the changes to make
JSON_EXISTS(), JSON_QUERY(), and JSON_VALUE() behave as we would like,
because the changes in 0001 covers them. The changes for JsonBehavior
expression coercion as they were in the last version of the patch are
still needed, but I decided to move those into 0001 so that the
changes for query functions are all in 0001 and those for constructors
in 0002. It would be nice to get rid of that coerce_to_target_type()
call to coerce the "behavior expression" to RETURNING type, but I'm
leaving that as a task for another day.

--
Thanks, Amit Langote