Re: sql/json remaining issue

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