On Thu, Apr 11, 2024 at 12:02 PM jian he <jian.universal...@gmail.com> wrote: > On Wed, Apr 10, 2024 at 4:39 PM Amit Langote <amitlangot...@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
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
