Horton

Re: remaining sql/json patches

amit <amitlangote09@gmail.com>

View thread
From: Amit Langote <amitlangote09@gmail.com>
To: jian he <jian.universality@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, Tomas Vondra <tomas.vondra@enterprisedb.com>, Himanshu Upadhyaya <upadhyaya.himanshu@gmail.com>, Erik Rijkers <er@xs4all.nl>, Andres Freund <andres@anarazel.de>, Andrew Dunstan <andrew@dunslane.net>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2024-04-04T12:02:48Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. SQL/JSON: Various improvements to SQL/JSON query function docs

  2. SQL/JSON: Fix some obsolete comments.

  3. SQL/JSON: Fix issues with DEFAULT .. ON ERROR / EMPTY

  4. JSON_TABLE: Add support for NESTED paths and columns

  5. Fix JsonExpr deparsing to emit QUOTES and WRAPPER correctly

  6. Fix typo introduced in 6185c9737

  7. Add basic JSON_TABLE() functionality

  8. Avoid splitting errmsg string to span multiple lines

  9. Add SQL/JSON query functions

  10. Implement various jsonpath methods

  11. Add soft error handling to some expression nodes

  12. Adjust populate_record_field() to handle errors softly

  13. Refactor code used by jsonpath executor to fetch variables

  14. Test EXPLAIN (FORMAT JSON) ... XMLTABLE

  15. Simplify productions for FORMAT JSON [ ENCODING name ]

  16. Add trailing commas to enum definitions

  17. doc: add missing <returnvalue> and whitespace

  18. Add more SQL/JSON constructor functions

  19. Rename a nonterminal used in SQL/JSON grammar

  20. Some refactoring to export json(b) conversion functions

  21. Don't include CaseTestExpr in JsonValueExpr.formatted_expr

  22. Code review for commit b6e1157e7d

  23. Pass constructName to transformJsonValueExpr()

  24. Unify JSON categorize type API and export for external use

  25. Make some indentation in gram.y consistent

  26. Allow most keywords to be used as column labels without requiring AS.

  27. Reduce size of backend scanner's tables.

  28. Use perfect hashing, instead of binary search, for keyword lookup.

On Wed, Apr 3, 2024 at 11:48 PM jian he <jian.universality@gmail.com> wrote:
> hi.
> +  <para>
> +   <function>json_table</function> is an SQL/JSON function which
> +   queries <acronym>JSON</acronym> data
> +   and presents the results as a relational view, which can be accessed as a
> +   regular SQL table. You can only use
> <function>json_table</function> inside the
> +   <literal>FROM</literal> clause of a <literal>SELECT</literal>,
> +   <literal>UPDATE</literal>, <literal>DELETE</literal>, or
> <literal>MERGE</literal>
> +   statement.
> +  </para>
>
> the only issue is that <literal>MERGE</literal> Synopsis don't have
> <literal>FROM</literal> clause.
> other than that, it's quite correct.
> see following tests demo:
>
> drop table ss;
> create table ss(a int);
> insert into ss select 1;
> delete from ss using JSON_TABLE(jsonb '1', '$' COLUMNS (a int PATH '$'
> ) ERROR ON ERROR) jt where jt.a = 1;
> insert into ss select 2;
> update ss set a = 1 from JSON_TABLE(jsonb '2', '$' COLUMNS (a int PATH
> '$')) jt where jt.a = 2;
> DROP TABLE IF EXISTS target;
> CREATE TABLE target (tid integer, balance integer) WITH
> (autovacuum_enabled=off);
> INSERT INTO target VALUES (1, 10),(2, 20),(3, 30);
> MERGE INTO target USING JSON_TABLE(jsonb '2', '$' COLUMNS (a int PATH
> '$' ) ERROR ON ERROR) source(sid)
> ON target.tid = source.sid
> WHEN MATCHED THEN UPDATE SET balance = 0
> returning *;
> --------------------------------------------------------------------------------------------------
>
> +  <para>
> +   To split the row pattern into columns, <function>json_table</function>
> +   provides the <literal>COLUMNS</literal> clause that defines the
> +   schema of the created view. For each column, a separate path expression
> +   can be specified to be evaluated against the row pattern to get a
> +   SQL/JSON value that will become the value for the specified column in
> +   a given output row.
> +  </para>
> should be "an SQL/JSON".
>
> +    <para>
> +     Inserts a SQL/JSON value obtained by applying
> +     <replaceable>path_expression</replaceable> against the row pattern into
> +     the view's output row after coercing it to specified
> +     <replaceable>type</replaceable>.
> +    </para>
> should be "an SQL/JSON".
>
> "coercing it to specified <replaceable>type</replaceable>"
> should be
> "coercing it to the specified <replaceable>type</replaceable>"?
> ---------------------------------------------------------------------------------------------------------------
> +    <para>
> +     The value corresponds to whether evaluating the <literal>PATH</literal>
> +     expression yields any SQL/JSON values.
> +    </para>
> maybe we can change to
> +    <para>
> +     The value corresponds to whether applying the
> <replaceable>path_expression</replaceable>
> +     expression yields any SQL/JSON values.
> +    </para>
> so it looks more consistent with the preceding paragraph.
>
> +    <para>
> +     Optionally, <literal>ON ERROR</literal> can be used to specify whether
> +     to throw an error or return the specified value to handle structural
> +     errors, respectively.  The default is to return a boolean value
> +     <literal>FALSE</literal>.
> +    </para>
> we don't need "respectively" here?
>
> + if (jt->on_error &&
> + jt->on_error->btype != JSON_BEHAVIOR_ERROR &&
> + jt->on_error->btype != JSON_BEHAVIOR_EMPTY &&
> + jt->on_error->btype != JSON_BEHAVIOR_EMPTY_ARRAY)
> + ereport(ERROR,
> + errcode(ERRCODE_SYNTAX_ERROR),
> + errmsg("invalid ON ERROR behavior"),
> + errdetail("Only EMPTY or ERROR is allowed for ON ERROR in JSON_TABLE()."),
> + parser_errposition(pstate, jt->on_error->location));
>
> errdetail("Only EMPTY or ERROR is allowed for ON ERROR in JSON_TABLE()."),
> maybe change to something like:
> `
> errdetail("Only EMPTY or ERROR is allowed for ON ERROR in the
> top-level JSON_TABLE() ").
> `
> i guess mentioning "top-level" is fine.
> since "top-level", we have 19  appearances in functions-json.html.

Thanks for checking.

Pushed after fixing these and a few other issues.  I didn't include
the testing function you proposed in your other email.  It sounds
useful for testing locally but will need some work before we can
include it in the tree.

I'll post the rebased 0002 tomorrow after addressing your comments.


--
Thanks, Amit Langote