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-09-05T12:58:01Z
Lists: pgsql-hackers

Attachments

Hi,

On Tue, Sep 3, 2024 at 6:05 PM jian he <jian.universality@gmail.com> wrote:
> On Mon, Sep 2, 2024 at 4:18 PM Amit Langote <amitlangote09@gmail.com> wrote:
> v2-0001 looks good to me.
> +-- Test JSON_TABLE() column deparsing -- don't emit default ON ERROR / EMPTY
> +-- behavior
> +EXPLAIN VERBOSE SELECT * from JSON_TABLE('"a"', '$' COLUMNS (a text PATH '$'));
> +EXPLAIN VERBOSE SELECT * from JSON_TABLE('"a"', '$' COLUMNS (a text
> PATH '$') ERROR ON ERROR);
>
> Are these tests duplicated? appears both in v2-0002 and v2-0003.
>
> 0002 output is:
> +EXPLAIN VERBOSE SELECT * from JSON_TABLE('"a"', '$' COLUMNS (a text
> PATH '$') ERROR ON ERROR);
> +
> QUERY PLAN
> +------------------------------------------------------------------------------------------------------------------------------------------------
> + Table Function Scan on "json_table"  (cost=0.01..1.00 rows=100 width=32)
> +   Output: a
> +   Table Function Call: JSON_TABLE('"a"'::jsonb, '$' AS
> json_table_path_0 COLUMNS (a text PATH '$' NULL ON EMPTY NULL ON
> ERROR) ERROR ON ERROR)
> +(3 rows)
>
> 0003 output is:
> EXPLAIN VERBOSE SELECT * from JSON_TABLE('"a"', '$' COLUMNS (a text
> PATH '$') ERROR ON ERROR);
>                                                      QUERY PLAN
> --------------------------------------------------------------------------------------------------------------------
>  Table Function Scan on "json_table"  (cost=0.01..1.00 rows=100 width=32)
>    Output: a
>    Table Function Call: JSON_TABLE('"a"'::jsonb, '$' AS
> json_table_path_0 COLUMNS (a text PATH '$') ERROR ON ERROR)
> (3 rows)
>
> two patches with different output,
> overall we should merge 0002 and 0003?

Looks like I ordered the patches wrong.  I'd like to commit the two separately.

>     if (jsexpr->on_error &&
>         jsexpr->on_error->btype != JSON_BEHAVIOR_ERROR &&
>         (jsexpr->on_error->btype != JSON_BEHAVIOR_NULL || returning_domain))
>
> we can be simplified as
>       if ( jsexpr->on_error->btype != JSON_BEHAVIOR_ERROR &&
>          (jsexpr->on_error->btype != JSON_BEHAVIOR_NULL || returning_domain))
>
> since if jsexpr->on_error is NULL, then segfault will appear at the beginning of
> ExecInitJsonExpr

Ok, done.

> + *
> + * Only add the extra steps for a NULL-valued expression when RETURNING a
> + * domain type to check the constraints, if any.
>   */
> + jsestate->jump_error = state->steps_len;
>   if (jsexpr->on_error &&
> - jsexpr->on_error->btype != JSON_BEHAVIOR_ERROR)
> + jsexpr->on_error->btype != JSON_BEHAVIOR_ERROR &&
> + (jsexpr->on_error->btype != JSON_BEHAVIOR_NULL || returning_domain))
>
> + *
> + * Only add the extra steps for a NULL-valued expression when RETURNING a
> + * domain type to check the constraints, if any.
>   */
> + jsestate->jump_empty = state->steps_len;
>   if (jsexpr->on_empty != NULL &&
> - jsexpr->on_empty->btype != JSON_BEHAVIOR_ERROR)
> + jsexpr->on_empty->btype != JSON_BEHAVIOR_ERROR &&
> + (jsexpr->on_empty->btype != JSON_BEHAVIOR_NULL || returning_domain))
>
> I am a little bit confused with the comments.
> not sure the "NULL-valued expression" refers to.

It refers to on_error/on_empty->expr, which is a Const node encoding
the NULL (constisnull is true) for that behavior.

That's actually the case also for behaviors UNKNOWN and EMPTY, so the
condition should actually be checking whether the expr is a
NULL-valued expression.

I've updated the patch.

> i think it is:
> implicitly default for ON EMPTY | ERROR clause is NULL (JSON_BEHAVIOR_NULL)
> for that situation, we can skip the json coercion process,
> but this only applies when the returning type of JsonExpr is not domain,

I've reworded the comment to mention that this speeds up the default
ON ERROR / EMPTY handling for JSON_QUERY() and JSON_VALUE().

Plan to push these tomorrow.

--
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