Thread
Commits
-
SQL/JSON: Miscellaneous fixes and improvements
- ef744ebb7340 17.0 landed
-
SQL/JSON: Fix issues with DEFAULT .. ON ERROR / EMPTY
- c0fc0751862d 17.0 landed
-
SQL/JSON: Improve some error messages
- b4fad46b6bc8 17.0 landed
-
sql/json remaining issue
jian he <jian.universality@gmail.com> — 2024-04-09T07:47:03Z
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,2222]},{"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,2222]},{"z21": [22, 234,' || g || ']},{"z22": [32, 204,145]}]},"c": ' || g || ',"id": "' || random_text_1000() || '"}') from generate_series(235, 235 + 200000,1) g; select count(*), pg_size_pretty(pg_total_relation_size('s1')) from s1; count | pg_size_pretty --------+---------------- 200002 | 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" + 999976))' 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" + 999976))' error ON EMPTY)) )) sub; rollback; only the last row will fail, because of "error ON EMPTY", (1_000_000 <= 23 + 999976) 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? 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. -
Re: sql/json remaining issue
Amit Langote <amitlangote09@gmail.com> — 2024-04-09T11:37:29Z
Hi, On Tue, Apr 9, 2024 at 4:47 PM jian he <jian.universality@gmail.com> 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,2222]},{"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,2222]},{"z21": [22, 234,' || g > || ']},{"z22": [32, 204,145]}]},"c": ' || g > || ',"id": "' || random_text_1000() || '"}') > from generate_series(235, 235 + 200000,1) g; > > select count(*), pg_size_pretty(pg_total_relation_size('s1')) from s1; > count | pg_size_pretty > --------+---------------- > 200002 | 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" + 999976))' 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" + 999976))' error ON EMPTY)) > )) sub; > rollback; > > only the last row will fail, because of "error ON EMPTY", (1_000_000 > <= 23 + 999976) 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 -
Re: sql/json remaining issue
Amit Langote <amitlangote09@gmail.com> — 2024-04-10T08:39:21Z
On Tue, Apr 9, 2024 at 8:37 PM Amit Langote <amitlangote09@gmail.com> wrote: > On Tue, Apr 9, 2024 at 4:47 PM jian he <jian.universality@gmail.com> 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
-
Re: sql/json remaining issue
jian he <jian.universality@gmail.com> — 2024-04-11T03:01:00Z
On Wed, Apr 10, 2024 at 4:39 PM Amit Langote <amitlangote09@gmail.com> 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&q=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
Amit Langote <amitlangote09@gmail.com> — 2024-04-12T09:43:57Z
On Thu, Apr 11, 2024 at 12:02 PM jian he <jian.universality@gmail.com> wrote: > On Wed, Apr 10, 2024 at 4:39 PM Amit Langote <amitlangote09@gmail.com> 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&q=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 -
Re: sql/json remaining issue
jian he <jian.universality@gmail.com> — 2024-04-13T14:12:47Z
On Fri, Apr 12, 2024 at 5:44 PM Amit Langote <amitlangote09@gmail.com> 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. <programlisting> 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')))); </programlisting> 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
jian he <jian.universality@gmail.com> — 2024-04-15T05:03:30Z
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%3DZBHpOtrghw@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? -
Re: sql/json remaining issue
Amit Langote <amitlangote09@gmail.com> — 2024-04-15T12:46:31Z
Hi, On Sat, Apr 13, 2024 at 11:12 PM jian he <jian.universality@gmail.com> wrote: > On Fri, Apr 12, 2024 at 5:44 PM Amit Langote <amitlangote09@gmail.com> 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. > > > <programlisting> > 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')))); > </programlisting> > > 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 -
Re: sql/json remaining issue
Amit Langote <amitlangote09@gmail.com> — 2024-04-18T00:33:23Z
On Mon, Apr 15, 2024 at 9:46 PM Amit Langote <amitlangote09@gmail.com> wrote: > On Sat, Apr 13, 2024 at 11:12 PM jian he <jian.universality@gmail.com> wrote: > > On Fri, Apr 12, 2024 at 5:44 PM Amit Langote <amitlangote09@gmail.com> 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
-
Re: sql/json remaining issue
Amit Langote <amitlangote09@gmail.com> — 2024-04-26T03:34:10Z
On Thu, Apr 18, 2024 at 9:33 AM Amit Langote <amitlangote09@gmail.com> wrote: > On Mon, Apr 15, 2024 at 9:46 PM Amit Langote <amitlangote09@gmail.com> wrote: > > On Sat, Apr 13, 2024 at 11:12 PM jian he <jian.universality@gmail.com> wrote: > > > On Fri, Apr 12, 2024 at 5:44 PM Amit Langote <amitlangote09@gmail.com> 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