Re: sql/json remaining issue

2024-04-25 Thread Amit Langote
On Thu, Apr 18, 2024 at 9:33 AM Amit Langote  wrote:
> On Mon, Apr 15, 2024 at 9:46 PM Amit Langote  wrote:
> > On Sat, Apr 13, 2024 at 11:12 PM jian he  
> > wrote:
> > > On Fri, Apr 12, 2024 at 5:44 PM Amit Langote  
> > > wrote:
> > > >
> > > > > elog(ERROR, "unrecognized json wrapper %d", wrapper);
> > > > > should be
> > > > > elog(ERROR, "unrecognized json wrapper %d", (int) wrapper);
> > > >
> > > > Fixed in 0003.
> > > >
> > > the fix seems not in 0003?
> > > other than that, everything looks fine.
>
> Oops, really fixed now in 0002.
>
> > I've combined these patches into one -- attached 0001.  Will push tomorrow.
>
> Decided to break the error message improvement patch into its own
> after all -- attached 0001.
>
> > Now studying the JsonBehavior DEFAULT expression issue and your patch.
>
> I found some more coercion-related expression nodes that must also be
> checked along with CoerceViaIO and CoerceToDomain.  Also, after fixing
> the code to allow them, I found that we'd need to also check
> recursively whether their argument expression is also one of the
> supported expression nodes.  Also, I decided that it's not necessary
> to include "cast" in the error message as one of the supported
> expressions.
>
> Will push all today.

Totally forgot to drop a note here that I pushed those and marked the
2 open items as resolved.

-- 
Thanks, Amit Langote




Re: sql/json remaining issue

2024-04-17 Thread Amit Langote
On Mon, Apr 15, 2024 at 9:46 PM Amit Langote  wrote:
> On Sat, Apr 13, 2024 at 11:12 PM jian he  wrote:
> > On Fri, Apr 12, 2024 at 5:44 PM Amit Langote  
> > wrote:
> > >
> > > > elog(ERROR, "unrecognized json wrapper %d", wrapper);
> > > > should be
> > > > elog(ERROR, "unrecognized json wrapper %d", (int) wrapper);
> > >
> > > Fixed in 0003.
> > >
> > the fix seems not in 0003?
> > other than that, everything looks fine.

Oops, really fixed now in 0002.

> I've combined these patches into one -- attached 0001.  Will push tomorrow.

Decided to break the error message improvement patch into its own
after all -- attached 0001.

> Now studying the JsonBehavior DEFAULT expression issue and your patch.

I found some more coercion-related expression nodes that must also be
checked along with CoerceViaIO and CoerceToDomain.  Also, after fixing
the code to allow them, I found that we'd need to also check
recursively whether their argument expression is also one of the
supported expression nodes.  Also, I decided that it's not necessary
to include "cast" in the error message as one of the supported
expressions.

Will push all today.

-- 
Thanks, Amit Langote


v5-0002-SQL-JSON-Miscellaneous-fixes-and-improvements.patch
Description: Binary data


v5-0001-SQL-JSON-Improve-some-error-messages.patch
Description: Binary data


v5-0003-SQL-JSON-Fix-issues-with-DEFAULT-.-ON-ERROR-EMPTY.patch
Description: Binary data


Re: sql/json remaining issue

2024-04-15 Thread Amit Langote
Hi,

On Sat, Apr 13, 2024 at 11:12 PM jian he  wrote:
> On Fri, Apr 12, 2024 at 5:44 PM Amit Langote  wrote:
> >
> > > elog(ERROR, "unrecognized json wrapper %d", wrapper);
> > > should be
> > > elog(ERROR, "unrecognized json wrapper %d", (int) wrapper);
> >
> > Fixed in 0003.
> >
> the fix seems not in 0003?
> other than that, everything looks fine.
>
>
> 
> SELECT * FROM JSON_TABLE (
> '{"favorites":
> {"movies":
>   [{"name": "One", "director": "John Doe"},
>{"name": "Two", "director": "Don Joe"}],
>  "books":
>   [{"name": "Mystery", "authors": [{"name": "Brown Dan"}]},
>{"name": "Wonder", "authors": [{"name": "Jun Murakami"},
> {"name":"Craig Doe"}]}]
> }}'::json, '$.favs[*]'
> COLUMNS (user_id FOR ORDINALITY,
>   NESTED '$.movies[*]'
> COLUMNS (
> movie_id FOR ORDINALITY,
> mname text PATH '$.name',
> director text),
>   NESTED '$.books[*]'
> COLUMNS (
>   book_id FOR ORDINALITY,
>   bname text PATH '$.name',
>   NESTED '$.authors[*]'
> COLUMNS (
>   author_id FOR ORDINALITY,
>   author_name text PATH '$.name';
> 
>
> I actually did run the query, it returns null.
> '$.favs[*]'
> should be
> '$.favorites[*]'

Oops, fixed.

I've combined these patches into one -- attached 0001.  Will push tomorrow.

> one more minor thing, I previously mentioned in getJsonPathVariable
> ereport(ERROR,
> (errcode(ERRCODE_UNDEFINED_OBJECT),
> errmsg("could not find jsonpath variable \"%s\"",
> pnstrdup(varName, varNameLength;
>
> do we need to remove pnstrdup?

Looking at this again, it seems like that's necessary because varName,
being a string extracted from JsonPathItem, is not necessarily
null-terminated.  There are many pndstrdup()s in jsonpath_exec.c
because of that aspect.

Now studying the JsonBehavior DEFAULT expression issue and your patch.

-- 
Thanks, Amit Langote


v4-0001-SQL-JSON-Miscellaneous-fixes-and-improvements.patch
Description: Binary data


Re: sql/json remaining issue

2024-04-14 Thread jian he
hi.
https://wiki.postgresql.org/wiki/PostgreSQL_17_Open_Items#Open_Issues
issue: Problems with deparsed SQL/JSON query function

original the bug report link:
https://postgr.es/m/cacjufxeqhqsfrg_p7emyo5zak3d767ifdl8vz_4%3dzbhpotr...@mail.gmail.com

forgive me for putting it in the new email thread.
I made the following change, added several tests on it.

--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -4636,10 +4636,10 @@ transformJsonBehavior(ParseState *pstate,
JsonBehavior *behavior,
  {
  expr = transformExprRecurse(pstate, behavior->expr);
  if (!IsA(expr, Const) && !IsA(expr, FuncExpr) &&
- !IsA(expr, OpExpr))
+ !IsA(expr, OpExpr) && !IsA(expr, CoerceViaIO) && !IsA(expr, CoerceToDomain))
  ereport(ERROR,
  (errcode(ERRCODE_DATATYPE_MISMATCH),
- errmsg("can only specify a constant, non-aggregate function, or
operator expression for DEFAULT"),
+ errmsg("can only specify a constant, non-aggregate function, or
operator expression or cast expression for DEFAULT"),
  parser_errposition(pstate, exprLocation(expr;
  if (contain_var_clause(expr))
  ereport(ERROR,

these two expression node also looks like Const:
CoerceViaIO:   "foo1"'::jsonb::text
CoerceToDomain: 'foo'::jsonb_test_domain

we need to deal with these two, otherwise we cannot use domain type in
DEFAULT expression.
also the following should not fail:
SELECT JSON_VALUE(jsonb '{"d1": "foo"}', '$.a2' returning text DEFAULT
'"foo1"'::text::json::text ON ERROR);


we have `if (contain_var_clause(expr))` further check it,
so it should be fine?
From 124cd4245266343daecdb4294b2013d9ebdd6b24 Mon Sep 17 00:00:00 2001
From: jian he 
Date: Mon, 15 Apr 2024 12:37:36 +0800
Subject: [PATCH v1 1/1] in transformJsonBehavior better handle default
 expression

sql/json query functions for process json can return empty or error.
We can specify DEFAULT expression for handling empty or error cases while applying
the path expression to the json or while type coercion.
the default expression can be just a plain constant,
however, a constant can be formed as a cast expression,eg (1::jsonb::text).

so allow the DEFAULT expression formed as CoerceViaIO node
or CoerceToDomain node in transformJsonBehavior
for better handling these cases.
Discussion: https://postgr.es/m/CACJufxEqhqsfrg_p7EMyo5zak3d767iFDL8vz_4%3DZBHpOtrghw@mail.gmail.com

---
 src/backend/parser/parse_expr.c   |  4 +--
 .../regress/expected/sqljson_jsontable.out| 25 ++
 .../regress/expected/sqljson_queryfuncs.out   | 26 +--
 src/test/regress/sql/sqljson_jsontable.sql| 13 ++
 src/test/regress/sql/sqljson_queryfuncs.sql   |  7 +
 5 files changed, 71 insertions(+), 4 deletions(-)

diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 4c98d7a0..94dbb531 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -4636,10 +4636,10 @@ transformJsonBehavior(ParseState *pstate, JsonBehavior *behavior,
 		{
 			expr = transformExprRecurse(pstate, behavior->expr);
 			if (!IsA(expr, Const) && !IsA(expr, FuncExpr) &&
-!IsA(expr, OpExpr))
+!IsA(expr, OpExpr) && !IsA(expr, CoerceViaIO) && !IsA(expr, CoerceToDomain))
 ereport(ERROR,
 		(errcode(ERRCODE_DATATYPE_MISMATCH),
-		 errmsg("can only specify a constant, non-aggregate function, or operator expression for DEFAULT"),
+		 errmsg("can only specify a constant, non-aggregate function, or operator expression or cast expression for DEFAULT"),
 		 parser_errposition(pstate, exprLocation(expr;
 			if (contain_var_clause(expr))
 ereport(ERROR,
diff --git a/src/test/regress/expected/sqljson_jsontable.out b/src/test/regress/expected/sqljson_jsontable.out
index a00eec8a..b45dd32a 100644
--- a/src/test/regress/expected/sqljson_jsontable.out
+++ b/src/test/regress/expected/sqljson_jsontable.out
@@ -113,6 +113,31 @@ FROM json_table_test vals
  [1, 1.23, "2", "aaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] | 11 | | "str"   | "str|  | | "str"   | "\"str\""| "\"str\""
 (14 rows)
 
+--check default expression
+SELECT * FROM JSON_TABLE(jsonb '{"d1": "H"}', '$'
+COLUMNS (js1 jsonb_test_domain path '$.a2' DEFAULT '"foo1"'::jsonb::text ON ERROR));
+  js1   
+
+ "foo1"
+(1 row)
+
+SELECT * FROM JSON_TABLE(jsonb '{"d1": "H"}', '$'
+COLUMNS (js1 jsonb_test_domain path '$.a2' DEFAULT 'foo'::jsonb_test_domain ON ERROR));
+ERROR:  value for domain jsonb_test_domain violates check constraint "jsonb_test_domain_check"
+SELECT * FROM JSON_TABLE(jsonb '{"d1": "H"}', '$'
+COLUMNS (js1 jsonb_test_domain path '$.a2' DEFAULT 'foo1'::jsonb_test_domain ON ERROR));
+ js1  
+--
+ foo1
+(1 row)
+
+SELECT * FROM JSON_TABLE(jsonb '{"d1": "foo"}', '$'
+COLUMNS (js1 jsonb_test_domain path '$.d1' DEFAULT 'foo2'::jsonb_test_domain ON ERROR));
+ js1  
+--
+ foo2
+(1 row)
+
 -- "formatted" columns
 SELECT *
 FROM json_table_test vals

Re: sql/json remaining issue

2024-04-13 Thread jian he
On Fri, Apr 12, 2024 at 5:44 PM Amit Langote  wrote:
>
> > elog(ERROR, "unrecognized json wrapper %d", wrapper);
> > should be
> > elog(ERROR, "unrecognized json wrapper %d", (int) wrapper);
>
> Fixed in 0003.
>
the fix seems not in 0003?
other than that, everything looks fine.



SELECT * FROM JSON_TABLE (
'{"favorites":
{"movies":
  [{"name": "One", "director": "John Doe"},
   {"name": "Two", "director": "Don Joe"}],
 "books":
  [{"name": "Mystery", "authors": [{"name": "Brown Dan"}]},
   {"name": "Wonder", "authors": [{"name": "Jun Murakami"},
{"name":"Craig Doe"}]}]
}}'::json, '$.favs[*]'
COLUMNS (user_id FOR ORDINALITY,
  NESTED '$.movies[*]'
COLUMNS (
movie_id FOR ORDINALITY,
mname text PATH '$.name',
director text),
  NESTED '$.books[*]'
COLUMNS (
  book_id FOR ORDINALITY,
  bname text PATH '$.name',
  NESTED '$.authors[*]'
COLUMNS (
  author_id FOR ORDINALITY,
  author_name text PATH '$.name';


I actually did run the query, it returns null.
'$.favs[*]'
should be
'$.favorites[*]'



one more minor thing, I previously mentioned in getJsonPathVariable
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg("could not find jsonpath variable \"%s\"",
pnstrdup(varName, varNameLength;

do we need to remove pnstrdup?




Re: sql/json remaining issue

2024-04-12 Thread Amit Langote
On Thu, Apr 11, 2024 at 12:02 PM jian he  wrote:
> On Wed, Apr 10, 2024 at 4:39 PM Amit Langote  wrote:
> > Attached is a bit more polished version of that, which also addresses
> > the error messages in JsonPathQuery() and JsonPathValue().  I noticed
> > that there was comment I had written at one point during JSON_TABLE()
> > hacking that said that we should be doing this.
> >
> > I've also added an open item for this.
>
> `
>  | NESTED [ PATH ] json_path_specification [ AS json_path_name ]
> COLUMNS ( json_table_column [, ...] )
> NESTED [ PATH ] json_path_specification [ AS json_path_name ] COLUMNS
> ( json_table_column [, ...] )
> `
>  "json_path_specification" should be "path_expression"?

Fixed in 0002.

> your explanation about memory usage is clear to me!
>
>
> The following are minor cosmetic issues while applying v2.
> +errmsg("JSON path expression in JSON_VALUE should return singleton
> scalar item")));
> "singleton" is not intuitive to me.
> Then I looked around.
> https://www.postgresql.org/search/?u=%2Fdocs%2F16%2F=singleton
> There is only one appearance of "singleton" in the manual.

Yes, singleton is a term used a lot in the source code but let's keep
it out of error messages and docs.  So fixed.

> errhint("Use WITH WRAPPER clause to wrap SQL/JSON item sequence into 
> array.")));
> maybe
> errhint("Use WITH WRAPPER clause to wrap SQL/JSON items into array.")));
> or
> errhint("Use WITH WRAPPER clause to wrap SQL/JSON item sequences into
> array.")));

Changed to use "SQL/JSON items into array.".

> then I wonder what's the difference between
> 22038 ERRCODE_SINGLETON_SQL_JSON_ITEM_REQUIRED
> 2203F ERRCODE_SQL_JSON_SCALAR_REQUIRED
>
> i assume '{"hello":"world"}'  is a singleton, but not a scalar item?
> if so, then I think the error message within the "if (count > 1)"
> branch in JsonPathValue
> should use ERRCODE_SINGLETON_SQL_JSON_ITEM_REQUIRED
> within the "if (!IsAJsonbScalar(res))" branch should use
> ERRCODE_SQL_JSON_SCALAR_REQUIRED
> ?
> + if (column_name)
> + ereport(ERROR,
> + (errcode(ERRCODE_MORE_THAN_ONE_SQL_JSON_ITEM),
> + errmsg("JSON path expression for column \"%s\" should return
> singleton scalar item",
> + column_name)));
> + else
> + ereport(ERROR,
> + (errcode(ERRCODE_SQL_JSON_SCALAR_REQUIRED),
> + errmsg("JSON path expression in JSON_VALUE should return singleton
> scalar item")));
> the error message seems similar, but the error code is different?
> both within "if (count > 1)" and "if (!IsAJsonbScalar(res))" branch.

Using different error codes for the same error is a copy-paste mistake
on my part.  Fixed.

> in src/include/utils/jsonpath.h, comments
> /* SQL/JSON item */
> should be
> /* SQL/JSON query functions */
>
>
> elog(ERROR, "unrecognized json wrapper %d", wrapper);
> should be
> elog(ERROR, "unrecognized json wrapper %d", (int) wrapper);

Fixed in 0003.

-- 
Thanks, Amit Langote


v3-0001-JSON_TABLE-mention-column-name-in-some-error-mess.patch
Description: Binary data


v3-0002-JSON_TABLE-Use-term-path_expression-more-consiste.patch
Description: Binary data


v3-0003-SQL-JSON-Fix-some-comments-in-jsonpath.h.patch
Description: Binary data


Re: sql/json remaining issue

2024-04-10 Thread jian he
On Wed, Apr 10, 2024 at 4:39 PM Amit Langote  wrote:
>
>
> Attached is a bit more polished version of that, which also addresses
> the error messages in JsonPathQuery() and JsonPathValue().  I noticed
> that there was comment I had written at one point during JSON_TABLE()
> hacking that said that we should be doing this.
>
> I've also added an open item for this.
>

`
 | NESTED [ PATH ] json_path_specification [ AS json_path_name ]
COLUMNS ( json_table_column [, ...] )
NESTED [ PATH ] json_path_specification [ AS json_path_name ] COLUMNS
( json_table_column [, ...] )
`
 "json_path_specification" should be "path_expression"?

your explanation about memory usage is clear to me!


The following are minor cosmetic issues while applying v2.
+errmsg("JSON path expression in JSON_VALUE should return singleton
scalar item")));
"singleton" is not intuitive to me.
Then I looked around.
https://www.postgresql.org/search/?u=%2Fdocs%2F16%2F=singleton
There is only one appearance of "singleton" in the manual.
then I wonder what's the difference between
22038 ERRCODE_SINGLETON_SQL_JSON_ITEM_REQUIRED
2203F ERRCODE_SQL_JSON_SCALAR_REQUIRED

i assume '{"hello":"world"}'  is a singleton, but not a scalar item?
if so, then I think the error message within the "if (count > 1)"
branch in JsonPathValue
should use ERRCODE_SINGLETON_SQL_JSON_ITEM_REQUIRED
within the "if (!IsAJsonbScalar(res))" branch should use
ERRCODE_SQL_JSON_SCALAR_REQUIRED
?


errhint("Use WITH WRAPPER clause to wrap SQL/JSON item sequence into array.")));
maybe
errhint("Use WITH WRAPPER clause to wrap SQL/JSON items into array.")));
or
errhint("Use WITH WRAPPER clause to wrap SQL/JSON item sequences into
array.")));


+ if (column_name)
+ ereport(ERROR,
+ (errcode(ERRCODE_MORE_THAN_ONE_SQL_JSON_ITEM),
+ errmsg("JSON path expression for column \"%s\" should return
singleton scalar item",
+ column_name)));
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_SQL_JSON_SCALAR_REQUIRED),
+ errmsg("JSON path expression in JSON_VALUE should return singleton
scalar item")));
the error message seems similar, but the error code is different?
both within "if (count > 1)" and "if (!IsAJsonbScalar(res))" branch.


in src/include/utils/jsonpath.h, comments
/* SQL/JSON item */
should be
/* SQL/JSON query functions */


elog(ERROR, "unrecognized json wrapper %d", wrapper);
should be
elog(ERROR, "unrecognized json wrapper %d", (int) wrapper);




Re: sql/json remaining issue

2024-04-10 Thread Amit Langote
On Tue, Apr 9, 2024 at 8:37 PM Amit Langote  wrote:
> On Tue, Apr 9, 2024 at 4:47 PM jian he  wrote:
> > the last query error message is:
> > `
> > ERROR:  no SQL/JSON item
> > `
> >
> > we are in ExecEvalJsonExprPath, can we output it to be:
> > `
> > ERROR:  after applying json_path "5s", no SQL/JSON item found
> > `
> > in a json_table query, we can have multiple path_expressions, like the
> > above query.
> > it's not easy to know applying which path_expression failed.
>
> Hmm, I'm not so sure about mentioning the details of the path because
> path names are optional and printing path expression itself is not a
> good idea.  Perhaps, we could mention the column name which would
> always be there, but we'd then need to add a new field column_name
> that's optionally set to JsonFuncExpr and JsonExpr, that is, when they
> are being set up for JSON_TABLE() columns.  As shown in the attached.
> With the patch you'll get:
>
> ERROR:  no SQL/JSON item found for column "b"

Attached is a bit more polished version of that, which also addresses
the error messages in JsonPathQuery() and JsonPathValue().  I noticed
that there was comment I had written at one point during JSON_TABLE()
hacking that said that we should be doing this.

I've also added an open item for this.

-- 
Thanks, Amit Langote


v2-0001-JSON_TABLE-mention-column-name-in-some-error-mess.patch
Description: Binary data


Re: sql/json remaining issue

2024-04-09 Thread Amit Langote
Hi,

On Tue, Apr 9, 2024 at 4:47 PM jian he  wrote:
>
> hi.
> `
>  | NESTED [ PATH ] json_path_specification [ AS json_path_name ]
> COLUMNS ( json_table_column [, ...] )
> NESTED [ PATH ] json_path_specification [ AS json_path_name ] COLUMNS
> ( json_table_column [, ...] )
> `
>  "json_path_specification" should be "path_expression"?
> --
> drop table s1;
> create or replace function random_text_1000() returns text
> as $$select string_agg(md5(random()::text),'') from
> generate_Series(1,1000) s $$ LANGUAGE SQL;
>
> create unlogged table s1(a int GENERATED BY DEFAULT AS IDENTITY, js jsonb);
> insert into s1(js)
> select jsonb ('{"a":{"za":[{"z1": [11,]},{"z21": [22, 234,' || g
> || ']},{"z22": [32, 204,145]}]},"c": ' || g
> || ',"id": "' || random_text_1000() || '"}')
> from generate_series(1_000_000, 1_000_000) g;
> insert into s1(js)
> select jsonb ('{"a":{"za":[{"z1": [11,]},{"z21": [22, 234,' || g
> || ']},{"z22": [32, 204,145]}]},"c": ' || g
> || ',"id": "' || random_text_1000() || '"}')
> from generate_series(235, 235 + 20,1) g;
>
> select count(*), pg_size_pretty(pg_total_relation_size('s1')) from s1;
>  count  | pg_size_pretty
> +
>  22 | 6398 MB
> --
>
> explain(analyze, costs off,buffers, timing)
> SELECT sub.*, s.a as s_a FROM s,
> (values(23)) x(x),
> generate_series(13, 13) y,
> JSON_TABLE(js, '$' AS c1 PASSING x AS x, y AS y
> COLUMNS (
> xx1 int PATH '$.c',
> NESTED PATH '$.a.za[2]' COLUMNS (NESTED PATH '$.z22[*]' as z22 COLUMNS
> (c int PATH '$')),
> NESTED PATH '$.a.za[1]' COLUMNS (d json PATH '$ ? (@.z21[*] == ($"x" -1))'),
> NESTED PATH '$.a.za[0]' COLUMNS (NESTED PATH '$.z1[*] ? (@ >= ($"x"
> -2))' as z1 COLUMNS (a int PATH '$')),
> NESTED PATH '$.a.za[1]' COLUMNS
> (NESTED PATH '$.z21[*] ? (@ >= ($"y" +121))' as z21 COLUMNS (b int
> PATH '$ ? (@ <= ($"x" + 76))' default -1000 ON EMPTY))
> )) sub;
>
> for one jsonb, it can expand to 7 rows, the above query will return
> around 1.4 million rows.
> i use the above query, and pg_log_backend_memory_contexts in another
> session to check the memory usage.
> didn't find memory over consumed issue.
>
> but I am not sure this is the right way to check the memory consumption.
> --
> begin;
> SELECT sub.*, s.a as s_a FROM s,
> (values(23)) x(x),
> generate_series(13, 13) y,
> JSON_TABLE(js, '$' AS c1 PASSING x AS x, y AS y
> COLUMNS (
> xx1 int PATH '$.c',
> NESTED PATH '$.a.za[2]' COLUMNS (NESTED PATH '$.z22[*]' as z22 COLUMNS
> (c int PATH '$')),
> NESTED PATH '$.a.za[1]' COLUMNS (d json PATH '$ ? (@.z21[*] == ($"x" -1))'),
> NESTED PATH '$.a.za[0]' COLUMNS (NESTED PATH '$.z1[*] ? (@ >= ($"x"
> -2))' as z1 COLUMNS (a int PATH '$')),
> NESTED PATH '$.a.za[1]' COLUMNS
> (NESTED PATH '$.z21[*] ? (@ >= ($"y" +121))' as z21 COLUMNS (b int
> PATH '$ ? (@ <= ($"x" + 76))' error ON EMPTY))
> )) sub;
> rollback;
>
> only the last row will fail, because of "error ON EMPTY", (1_000_000
> <= 23 + 76) is false.
> I remember the very previous patch, because of error cleanup, it took
> a lot of resources.
> does our current implementation, only the very last row fail, will it
> be easy to clean up the transaction?

I am not sure I understand your concern.  Could you please rephrase
it?  Which previous patch are you referring to and what problem did it
cause with respect to error cleanup?

Per-row memory allocated for each successful output row JSON_TABLE()
doesn't pile up, because it's allocated in a context that is reset
after evaluating each row; see tfuncLoadRows().  But again I may be
misunderstanding your concern.

> the last query error message is:
> `
> ERROR:  no SQL/JSON item
> `
>
> we are in ExecEvalJsonExprPath, can we output it to be:
> `
> ERROR:  after applying json_path "5s", no SQL/JSON item found
> `
> in a json_table query, we can have multiple path_expressions, like the
> above query.
> it's not easy to know applying which path_expression failed.

Hmm, I'm not so sure about mentioning the details of the path because
path names are optional and printing path expression itself is not a
good idea.  Perhaps, we could mention the column name which would
always be there, but we'd then need to add a new field column_name
that's optionally set to JsonFuncExpr and JsonExpr, that is, when they
are being set up for JSON_TABLE() columns.  As shown in the attached.
With the patch you'll get:

ERROR:  no SQL/JSON item found for column "b"

--
Thanks, Amit Langote


v1-0001-JSON_TABLE-mention-column-name-in-the-ON-EMPTY-er.patch
Description: Binary data