On Thu, Mar 28, 2024 at 1:23 PM Amit Langote <amitlangot...@gmail.com> wrote: > > On Wed, Mar 27, 2024 at 1:34 PM Amit Langote <amitlangot...@gmail.com> wrote: > > On Wed, Mar 27, 2024 at 12:42 PM jian he <jian.universal...@gmail.com> > > wrote: > > > hi. > > > I don't fully understand all the code in json_table patch. > > > maybe we can split it into several patches, > > > > I'm working on exactly that atm. > > > > > like: > > > * no nested json_table_column. > > > * nested json_table_column, with PLAN DEFAULT > > > * nested json_table_column, with PLAN ( json_table_plan ) > > > > Yes, I think it will end up something like this. I'll try to post the > > breakdown tomorrow. > > Here's patch 1 for the time being that implements barebones > JSON_TABLE(), that is, without NESTED paths/columns and PLAN clause. > I've tried to shape the interfaces so that those features can be added > in future commits without significant rewrite of the code that > implements barebones JSON_TABLE() functionality. I'll know whether > that's really the case when I rebase the full patch over it. > > I'm still reading and polishing it and would be happy to get feedback > and testing. >
+static void +JsonValueListClear(JsonValueList *jvl) +{ + jvl->singleton = NULL; + jvl->list = NULL; +} jvl->list is a List structure, do we need to set it like "jvl->list = NIL"? + if (jperIsError(res)) + { + /* EMPTY ON ERROR case */ + Assert(!planstate->plan->errorOnError); + JsonValueListClear(&planstate->found); + } i am not sure the comment is right. `SELECT * FROM JSON_TABLE(jsonb'"1.23"', 'strict $.a' COLUMNS (js2 int PATH '$') );` will execute jperIsError branch. also SELECT * FROM JSON_TABLE(jsonb'"1.23"', 'strict $.a' COLUMNS (js2 int PATH '$') default '1' on error); I think it means applying path_expression, if the top level on_error behavior is not on error then ` if (jperIsError(res))` part may be executed. --- a/src/include/utils/jsonpath.h +++ b/src/include/utils/jsonpath.h @@ -15,6 +15,7 @@ #define JSONPATH_H #include "fmgr.h" +#include "executor/tablefunc.h" #include "nodes/pg_list.h" #include "nodes/primnodes.h" #include "utils/jsonb.h" should be: +#include "executor/tablefunc.h" #include "fmgr.h" +<synopsis> +JSON_TABLE ( + <replaceable>context_item</replaceable>, <replaceable>path_expression</replaceable> <optional> AS <replaceable>json_path_name</replaceable> </optional> <optional> PASSING { <replaceable>value</replaceable> AS <replaceable>varname</replaceable> } <optional>, ...</optional> </optional> + COLUMNS ( <replaceable class="parameter">json_table_column</replaceable> <optional>, ...</optional> ) + <optional> { <literal>ERROR</literal> | <literal>EMPTY</literal> } <literal>ON ERROR</literal> </optional> +) top level (not in the COLUMN clause) also allows <literal>NULL</literal> <literal>ON ERROR</literal>. SELECT JSON_VALUE(jsonb'"1.23"', 'strict $.a' null on error); returns one value. SELECT * FROM JSON_TABLE(jsonb'"1.23"', 'strict $.a' COLUMNS (js2 int PATH '$') NULL on ERROR); return zero rows. Is this what we expected? main changes are in jsonpath_exec.c, parse_expr.c, parse_jsontable.c overall the coverage seems pretty good. I added some tests to improve the coverage.
v46-0001-improve-regress-coverage-test-based-on-v46.no-cfbot
Description: Binary data