Re: pgsql: Add more SQL/JSON constructor functions

On Wed, Jul 24, 2024 at 3:25 PM jian he <jian(dot)universality(at)gmail(dot)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