Horton

Re: pgsql: Add more SQL/JSON constructor functions

Amit Langote <amitlangote09@gmail.com>

View thread View on postgr.es
From: Amit Langote <amitlangote09@gmail.com>
To: jian he <jian.universality@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Peter Eisentraut <peter@eisentraut.org>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2024-07-25T14:16:45Z
Lists: pgsql-hackers

Attachments

On Wed, Jul 24, 2024 at 3:25 PM jian he <jian.universality@gmail.com> wrote:
> transformJsonFuncExpr we have:
>         case JSON_QUERY_OP:
>             if (jsexpr->returning->typid != JSONBOID || jsexpr->omit_quotes)
>                 jsexpr->use_json_coercion = true;
>
>         case JSON_VALUE_OP:
>             if (jsexpr->returning->typid != TEXTOID)
>             {
>                 if (get_typtype(jsexpr->returning->typid) == TYPTYPE_DOMAIN &&
>                     DomainHasConstraints(jsexpr->returning->typid))
>                     jsexpr->use_json_coercion = true;
>                 else
>                     jsexpr->use_io_coercion = true;
>             }
>
> JSONBOID won't be a domain. for domain type, json_value, json_query
> will use jsexpr->use_json_coercion.
> jsexpr->use_json_coercion can handle whether the domain has constraints or not.
>
> so i don't know the purpose of following code in ExecInitJsonExpr
>     if (get_typtype(jsexpr->returning->typid) == TYPTYPE_DOMAIN &&
>         DomainHasConstraints(jsexpr->returning->typid))
>     {
>         Assert(jsexpr->use_json_coercion);
>         scratch->opcode = EEOP_JUMP;
>         scratch->d.jump.jumpdone = state->steps_len + 1;
>         ExprEvalPushStep(state, scratch);
>     }

Yeah, it's a useless JUMP.  I forget why it's there.  I have attached
a patch (0005) to remove it.

> json_table exits works fine with int4, not domain over int4. The
> following are test suites.
>
> drop domain if exists dint4, dint4_1,dint4_0;
> create domain dint4 as int;
> create domain dint4_1 as int check ( value <> 1 );
> create domain dint4_0 as int check ( value <> 0 );
> SELECT a, a::bool FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a dint4
> EXISTS PATH '$.a' ));
> SELECT a, a::bool FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a dint4
> EXISTS PATH '$.a' false ON ERROR));
> SELECT a, a::bool FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a dint4
> EXISTS PATH '$.a' ERROR ON ERROR));
> SELECT a, a::bool FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a dint4_0
> EXISTS PATH '$.a'));
> SELECT a, a::bool FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a dint4_0
> EXISTS PATH '$'));
> SELECT a,a::bool FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a dint4_1
> EXISTS PATH '$'));
> SELECT a,a::bool FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a dint4_1
> EXISTS PATH '$.a'));
> SELECT a,a::bool FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a dint4_1
> EXISTS PATH '$.a' ERROR ON ERROR));

Domain-over-integer case should be fixed with the attached updated 0002.

> I found out 2 issues for the above tests.
> 1. RETURNING types is jsonb/domain over jsonb, default expression does
> not respect omit/keep quotes,
> but other RETURNING types do. Maybe this will be fine.

Yeah, I am not sure whether and how we could implement OMIT/KEEP
QUOTES for the DEFAULT expression.  I might try later or simply
document that OMIT/KEEP QUOTE is only applied to the query result but
not the DEFAULT expression.

> 2. domain over jsonb should fail just like domain over other types?
> RETURNING djs keep quotes DEFAULT '"11"' ON empty
> should fail as
> ERROR:  could not coerce ON EMPTY expression (DEFAULT) to the RETURNING type
> DETAIL:  value for domain djs violates check constraint "djs_check""

I think this should be fixed with the attached patch 0004.

>                             errcode(ERRCODE_CANNOT_COERCE),
>                             errmsg("cannot cast behavior expression of
> type %s to %s",
>                                    format_type_be(exprType(expr)),
>                                    format_type_be(returning->typid)),
>                             errhint("You will need to cast the expression."),
>                             parser_errposition(pstate, exprLocation(expr)));
>
> maybe
> errhint("You will need to explicitly cast the expression to type %s",
> format_type_be(returning->typid))

OK, done.

Please check.

-- 
Thanks, Amit Langote

Commits

  1. SQL/JSON: Avoid initializing unnecessary ON ERROR / ON EMPTY steps

  2. SQL/JSON: Fix default ON ERROR behavior for JSON_TABLE

  3. SQL/JSON: Fix JSON_TABLE() column deparsing

  4. Update comment about ExprState.escontext

  5. SQL/JSON: Fix casting for integer EXISTS columns in JSON_TABLE

  6. SQL/JSON: Some fixes to JsonBehavior expression casting

  7. SQL/JSON: Remove useless code in ExecInitJsonExpr()

  8. SQL/JSON: Respect OMIT QUOTES when RETURNING domains over jsonb

  9. SQL/JSON: Improve error-handling of JsonBehavior expressions

  10. SQL/JSON: Fix error-handling of some JsonBehavior expressions

  11. SQL/JSON: Rethink c2d93c3802b

  12. SQL/JSON: Always coerce JsonExpr result at runtime

  13. SQL/JSON: Fix coercion of constructor outputs to types with typmod