From: | jian he <jian(dot)universality(at)gmail(dot)com> |
---|---|
To: | Amit Langote <amitlangote09(at)gmail(dot)com> |
Cc: | Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, Himanshu Upadhyaya <upadhyaya(dot)himanshu(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Erik Rijkers <er(at)xs4all(dot)nl>, Andres Freund <andres(at)anarazel(dot)de>, Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: remaining sql/json patches |
Date: | 2024-03-29 03:20:00 |
Message-ID: | CACJufxEEt-rdX0dZaq_TXKVJWAAkyZCJ7s2=0fdZsRN4rkBemA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Mar 28, 2024 at 1:23 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
>
> On Wed, Mar 27, 2024 at 1:34 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> > On Wed, Mar 27, 2024 at 12:42 PM jian he <jian(dot)universality(at)gmail(dot)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.
Attachment | Content-Type | Size |
---|---|---|
v46-0001-improve-regress-coverage-test-based-on-v46.no-cfbot | application/octet-stream | 2.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Isaac Morland | 2024-03-29 03:27:48 | Re: PSQL Should \sv & \ev work with materialized views? |
Previous Message | Thomas Munro | 2024-03-29 03:14:37 | Re: AIX support |