On Thu, Mar 28, 2024 at 1:23 PM Amit Langote <[email protected]> wrote: > > On Wed, Mar 27, 2024 at 1:34 PM Amit Langote <[email protected]> wrote: > > On Wed, Mar 27, 2024 at 12:42 PM jian he <[email protected]> > > 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
