Subject: Re: More new SQL/JSON item methods

From: Andrew Dunstan <[email-masked]>
Date: Wed, 17 Jan 2024 11:53:23 -0500

On 2024-01-17 We 04:03, Jeevan Chalke wrote:
>
>
> On Mon, Jan 15, 2024 at 7:41 PM Peter Eisentraut 
> <[email-masked]> wrote:
>
>
>     Overall, I think it would be better if you combined all three of
>     these
>     patches into one.  Right now, you have arranged these as incremental
>     features, and as a result of that, the additions to the
>     JsonPathItemType
>     enum and the grammar keywords etc. are ordered in the way you
>     worked on
>     these features, I guess.  It would be good to maintain a bit of
>     sanity
>     to put all of this together and order all the enums and everything
>     else
>     for example in the order they are in the sql_features.txt file
>     (which is
>     alphabetical, I suppose).  At this point I suspect we'll end up
>     committing this whole feature set together anyway, so we might as
>     well
>     organize it that way.
>
>
> OK.
> I will merge them all into one and will try to keep them in the order 
> specified in sql_features.txt.
> However, for documentation, it makes more sense to keep them in 
> logical order than the alphabetical one. What are your views on this?
>

I agree that we should order the documentation logically. Users don't 
care how we organize the code etc, but they do care about docs have 
sensible structure.

cheers

andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

---

From: Peter Eisentraut <[email-masked]>
Date: Wed, 17 Jan 2024 20:33:14 +0100

On 17.01.24 10:03, Jeevan Chalke wrote:
> I added unary '+' and '-' support as well and thus thought of having 
> separate rules altogether rather than folding those in.
> 
>     Per SQL standard, the precision and scale arguments are unsigned
>     integers, so unary plus and minus signs are not supported.  So my patch
>     removes that support, but I didn't adjust the regression tests for that.
> 
> 
> However, PostgreSQL numeric casting does support a negative scale. Here 
> is an example:
> 
> # select '12345'::numeric(4,-2);
>   numeric
> ---------
>     12300
> (1 row)
> 
> And thus thought of supporting those.
> Do we want this JSON item method to behave differently here?

Ok, it would make sense to support this in SQL/JSON as well.

> However, for documentation, it makes more sense to keep them in logical 
> order than the alphabetical one. What are your views on this?

The documentation can be in a different order.

---

From: Jeevan Chalke <[email-masked]>
Date: Thu, 18 Jan 2024 19:55:28 +0530

> Ok, it would make sense to support this in SQL/JSON as well.

OK. So with this, we don't need changes done in your 0001 patches.

> The documentation can be in a different order.

Thanks, Andrew and Peter for the confirmation.

Attached merged single patch along these lines.

Peter, I didn't understand why the changes you did in your 0002 patch were
required here. I did run the pgindent, and it didn't complain to me. So,
just curious to know more about the changes. I have not merged those
changes in this single patch. However, if needed it can be cleanly applied
on top of this single patch.

Thanks

-- 
Jeevan Chalke

*Principal, ManagerProduct Development*

edbpostgres.com

---

From: Peter Eisentraut <[email-masked]>
Date: Thu, 18 Jan 2024 16:49:11 +0100

On 18.01.24 15:25, Jeevan Chalke wrote:
> Peter, I didn't understand why the changes you did in your 0002 patch 
> were required here. I did run the pgindent, and it didn't complain to 
> me. So, just curious to know more about the changes. I have not merged 
> those changes in this single patch. However, if needed it can be cleanly 
> applied on top of this single patch.

I just thought it was a bit wasteful with vertical space.  It's not 
essential.

---

From: Andrew Dunstan <[email-masked]>
Date: Thu, 25 Jan 2024 10:40:26 -0500

On 2024-01-18 Th 09:25, Jeevan Chalke wrote:
>
>
> On Thu, Jan 18, 2024 at 1:03 AM Peter Eisentraut 
>     logical

Thanks, I have pushed this.

cheers

andrew

---

From: Tom Lane <[email-masked]>
Date: Thu, 25 Jan 2024 14:31:09 -0500

> Thanks, I have pushed this.

The buildfarm is pretty widely unhappy, mostly failing on

select jsonb_path_query('1.23', '$.string()');

On a guess, I tried running that under valgrind, and behold it said

==00:00:00:05.637 435530== Conditional jump or move depends on uninitialised value(s)
==00:00:00:05.637 435530==    at 0x8FD131: executeItemOptUnwrapTarget (jsonpath_exec.c:1547)
==00:00:00:05.637 435530==    by 0x8FED03: executeItem (jsonpath_exec.c:626)
==00:00:00:05.637 435530==    by 0x8FED03: executeNextItem (jsonpath_exec.c:1604)
==00:00:00:05.637 435530==    by 0x8FCA58: executeItemOptUnwrapTarget (jsonpath_exec.c:956)
==00:00:00:05.637 435530==    by 0x8FFDE4: executeItem (jsonpath_exec.c:626)
==00:00:00:05.637 435530==    by 0x8FFDE4: executeJsonPath.constprop.30 (jsonpath_exec.c:612)
==00:00:00:05.637 435530==    by 0x8FFF8C: jsonb_path_query_internal (jsonpath_exec.c:438)

It's fairly obviously right about that:

                JsonbValue    jbv;
                ...
                jb = &jbv;
                Assert(tmp != NULL);    /* We must have set tmp above */
                jb->val.string.val = (jb->type == jbvString) ? tmp : pstrdup(tmp);
                                      ^^^^^^^^^^^^^^^^^^^^^

Presumably, this is a mistaken attempt to test the type
of the thing previously pointed to by "jb".

On the whole, what I'd be inclined to do here is get rid
of this test altogether and demand that every path through
the preceding "switch" deliver a value that doesn't need
pstrdup().  The only path that doesn't do that already is

                    case jbvBool:
                        tmp = (jb->val.boolean) ? "true" : "false";
                        break;

and TBH I'm not sure that we really need a pstrdup there
either.  The constants are immutable enough.  Is something
likely to try to pfree the pointer later?  I tried

@@ -1544,7 +1544,7 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt, JsonPathItem *jsp,
 
                jb = &jbv;
                Assert(tmp != NULL);    /* We must have set tmp above */
-               jb->val.string.val = (jb->type == jbvString) ? tmp : pstrdup(tmp);
+               jb->val.string.val = tmp;
                jb->val.string.len = strlen(jb->val.string.val);
                jb->type = jbvString;
 
and that quieted valgrind for this particular query and still
passes regression.

(The reported crashes seem to be happening later during a
recursive invocation, seemingly because JsonbType(jb) is
returning garbage.  So there may be another bug after this one.)

			regards, tom lane

---

From: Andrew Dunstan <[email-masked]>
Date: Thu, 25 Jan 2024 14:41:07 -0500

>                  jb->val.string.val = (jb->type == jbvString) ? tmp : pstrdup(tmp);

Argh! Will look, thanks.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

---

From: Andrew Dunstan <[email-masked]>
Date: Thu, 25 Jan 2024 15:25:28 -0500

>                  jb->val.string.val = (jb->type == jbvString) ? tmp : pstrdup(tmp);

Your fix looks sane. I also don't see why we need the pstrdup.

> returning garbage.  So there may be another bug after this one.)

I don't think so. AIUI The first call deals with the '$' and the second 
one deals with the '.string()', which is why we see the error on the 
second call.

cheers

andrew

---

From: Tom Lane <[email-masked]>
Date: Thu, 25 Jan 2024 15:33:51 -0500

> one deals with the '.string()', which is why we see the error on the 

There's something else going on, because I'm still getting the
assertion failure on my Mac with this fix in place.  Annoyingly,
it goes away if I compile with -O0, so it's kind of hard to
identify what's going wrong.

			regards, tom lane

---

From: Tom Lane <[email-masked]>
Date: Thu, 25 Jan 2024 15:58:20 -0500

> identify what's going wrong.

No, belay that: I must've got confused about which version I was
testing.  It's very unclear to me why the undefined reference
causes the preceding Assert to misbehave, but that is clearly
what's happening.  Compiler bug maybe?  My Mac has clang 15.0.0,
and the unhappy buildfarm members are also late-model clang.

Anyway, I did note that the preceding line

	res = jperOk;

is dead code and might as well get removed while you're at it.

			regards, tom lane

---

From: Andrew Dunstan <[email-masked]>
Date: Thu, 25 Jan 2024 16:01:39 -0500

> identify what's going wrong.

Curiouser and curiouser. On my Mac the error is manifest but the fix you 
suggested cures it. Built with -O2 -g, clang 15.0.0, Apple Silicon.

cheers

andrew

---

From: Andrew Dunstan <[email-masked]>
Date: Thu, 25 Jan 2024 16:27:41 -0500

> is dead code and might as well get removed while you're at it.

OK, pushed those. Thanks.

cheers

andrew

---

From: Jeevan Chalke <[email-masked]>
Date: Fri, 26 Jan 2024 18:07:15 +0530

> OK, pushed those. Thanks.

Thank you all.

-- 
Jeevan Chalke

edbpostgres.com

---

From: Kyotaro Horiguchi <[email-masked]>
Date: Mon, 29 Jan 2024 12:12:00 +0900 (JST)

I have two possible issues in a recent commit.

Commit 66ea94e8e6 has introduced the following messages:
(Aplogizies in advance if the commit is not related to this thread.)

jsonpath_exec.c:1287
> if (numeric_is_nan(num) || numeric_is_inf(num))
>   RETURN_ERROR(ereport(ERROR,
>              (errcode(ERRCODE_NON_NUMERIC_SQL_JSON_ITEM),
>               errmsg("numeric argument of jsonpath item method .%s() is out of range for type decimal or number",
>                            jspOperationName(jsp->type)))));

:1387
>   noerr = DirectInputFunctionCallSafe(numeric_in, numstr,
...
> if (!noerr || escontext.error_occurred)
>   RETURN_ERROR(ereport(ERROR,
>              (errcode(ERRCODE_NON_NUMERIC_SQL_JSON_ITEM),
>               errmsg("string argument of jsonpath item method .%s() is not a valid representation of a decimal or number",

They seem to be suggesting that PostgreSQL has the types "decimal" and
"number". I know of the former, but I don't think PostgreSQL has the
 latter type. Perhaps the "number" was intended to refer to "numeric"?
(And I think it is largely helpful if the given string were shown in
the error message, but it would be another issue.)

The same commit has introduced the following set of messages:

> %s format is not recognized: "%s"
> date format is not recognized: "%s"
> time format is not recognized: "%s"
> time_tz format is not recognized: "%s"
> timestamp format is not recognized: "%s"
> timestamp_tz format is not recognized: "%s"

I believe that the first line was intended to cover all the others:p

They are attached to this message separately.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index 22f598cd35..c10926a8a2 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -1287,7 +1287,7 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt, JsonPathItem *jsp,
 					if (numeric_is_nan(num) || numeric_is_inf(num))
 						RETURN_ERROR(ereport(ERROR,
 											 (errcode(ERRCODE_NON_NUMERIC_SQL_JSON_ITEM),
-											  errmsg("numeric argument of jsonpath item method .%s() is out of range for type decimal or number",
+											  errmsg("numeric argument of jsonpath item method .%s() is out of range for type decimal or numeric",
 													 jspOperationName(jsp->type)))));
 
 					if (jsp->type == jpiDecimal)
@@ -1312,14 +1312,14 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt, JsonPathItem *jsp,
 					if (!noerr || escontext.error_occurred)
 						RETURN_ERROR(ereport(ERROR,
 											 (errcode(ERRCODE_NON_NUMERIC_SQL_JSON_ITEM),
-											  errmsg("string argument of jsonpath item method .%s() is not a valid representation of a decimal or number",
+											  errmsg("string argument of jsonpath item method .%s() is not a valid representation of a decimal or numeric",
 													 jspOperationName(jsp->type)))));
 
 					num = DatumGetNumeric(datum);
 					if (numeric_is_nan(num) || numeric_is_inf(num))
 						RETURN_ERROR(ereport(ERROR,
 											 (errcode(ERRCODE_NON_NUMERIC_SQL_JSON_ITEM),
-											  errmsg("string argument of jsonpath item method .%s() is not a valid representation of a decimal or number",
+											  errmsg("string argument of jsonpath item method .%s() is not a valid representation of a decimal or numeric",
 													 jspOperationName(jsp->type)))));
 
 					res = jperOk;
@@ -1401,7 +1401,7 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt, JsonPathItem *jsp,
 					if (!noerr || escontext.error_occurred)
 						RETURN_ERROR(ereport(ERROR,
 											 (errcode(ERRCODE_NON_NUMERIC_SQL_JSON_ITEM),
-											  errmsg("string argument of jsonpath item method .%s() is not a valid representation of a decimal or number",
+											  errmsg("string argument of jsonpath item method .%s() is not a valid representation of a decimal or numeric",
 													 jspOperationName(jsp->type)))));
 
 					num = DatumGetNumeric(numdatum);
diff --git a/src/test/regress/expected/jsonb_jsonpath.out b/src/test/regress/expected/jsonb_jsonpath.out
index eea2af30c8..9d8ce48a25 100644
--- a/src/test/regress/expected/jsonb_jsonpath.out
+++ b/src/test/regress/expected/jsonb_jsonpath.out
@@ -2144,7 +2144,7 @@ select jsonb_path_query('"1.23"', '$.decimal()');
 (1 row)
 
 select jsonb_path_query('"1.23aaa"', '$.decimal()');
-ERROR:  string argument of jsonpath item method .decimal() is not a valid representation of a decimal or number
+ERROR:  string argument of jsonpath item method .decimal() is not a valid representation of a decimal or numeric
 select jsonb_path_query('1e1000', '$.decimal()');
                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                              jsonb_path_query                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                              
 -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@@ -2152,13 +2152,13 @@ select jsonb_path_query('1e1000', '$.decimal()');
 (1 row)
 
 select jsonb_path_query('"nan"', '$.decimal()');
-ERROR:  string argument of jsonpath item method .decimal() is not a valid representation of a decimal or number
+ERROR:  string argument of jsonpath item method .decimal() is not a valid representation of a decimal or numeric
 select jsonb_path_query('"NaN"', '$.decimal()');
-ERROR:  string argument of jsonpath item method .decimal() is not a valid representation of a decimal or number
+ERROR:  string argument of jsonpath item method .decimal() is not a valid representation of a decimal or numeric
 select jsonb_path_query('"inf"', '$.decimal()');
-ERROR:  string argument of jsonpath item method .decimal() is not a valid representation of a decimal or number
+ERROR:  string argument of jsonpath item method .decimal() is not a valid representation of a decimal or numeric
 select jsonb_path_query('"-inf"', '$.decimal()');
-ERROR:  string argument of jsonpath item method .decimal() is not a valid representation of a decimal or number
+ERROR:  string argument of jsonpath item method .decimal() is not a valid representation of a decimal or numeric
 select jsonb_path_query('"inf"', '$.decimal()', silent => true);
  jsonb_path_query 
 ------------------
@@ -2224,7 +2224,7 @@ select jsonb_path_query('12345.678', '$.decimal(6, 1)');
 (1 row)
 
 select jsonb_path_query('12345.678', '$.decimal(6, 2)');
-ERROR:  string argument of jsonpath item method .decimal() is not a valid representation of a decimal or number
+ERROR:  string argument of jsonpath item method .decimal() is not a valid representation of a decimal or numeric
 select jsonb_path_query('1234.5678', '$.decimal(6, 2)');
  jsonb_path_query 
 ------------------
@@ -2232,7 +2232,7 @@ select jsonb_path_query('1234.5678', '$.decimal(6, 2)');
 (1 row)
 
 select jsonb_path_query('12345.678', '$.decimal(4, 6)');
-ERROR:  string argument of jsonpath item method .decimal() is not a valid representation of a decimal or number
+ERROR:  string argument of jsonpath item method .decimal() is not a valid representation of a decimal or numeric
 select jsonb_path_query('12345.678', '$.decimal(0, 6)');
 ERROR:  NUMERIC precision 0 must be between 1 and 1000
 select jsonb_path_query('12345.678', '$.decimal(1001, 6)');
@@ -2440,7 +2440,7 @@ select jsonb_path_query('"1.23"', '$.number()');
 (1 row)
 
 select jsonb_path_query('"1.23aaa"', '$.number()');
-ERROR:  string argument of jsonpath item method .number() is not a valid representation of a decimal or number
+ERROR:  string argument of jsonpath item method .number() is not a valid representation of a decimal or numeric
 select jsonb_path_query('1e1000', '$.number()');
                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                              jsonb_path_query                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                              
 -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@@ -2448,13 +2448,13 @@ select jsonb_path_query('1e1000', '$.number()');
 (1 row)
 
 select jsonb_path_query('"nan"', '$.number()');
-ERROR:  string argument of jsonpath item method .number() is not a valid representation of a decimal or number
+ERROR:  string argument of jsonpath item method .number() is not a valid representation of a decimal or numeric
 select jsonb_path_query('"NaN"', '$.number()');
-ERROR:  string argument of jsonpath item method .number() is not a valid representation of a decimal or number
+ERROR:  string argument of jsonpath item method .number() is not a valid representation of a decimal or numeric
 select jsonb_path_query('"inf"', '$.number()');
-ERROR:  string argument of jsonpath item method .number() is not a valid representation of a decimal or number
+ERROR:  string argument of jsonpath item method .number() is not a valid representation of a decimal or numeric
 select jsonb_path_query('"-inf"', '$.number()');
-ERROR:  string argument of jsonpath item method .number() is not a valid representation of a decimal or number
+ERROR:  string argument of jsonpath item method .number() is not a valid representation of a decimal or numeric
 select jsonb_path_query('"inf"', '$.number()', silent => true);
  jsonb_path_query 
 ------------------
diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index c10926a8a2..4a2a995325 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -2368,8 +2368,8 @@ executeDateTimeMethod(JsonPathExecContext *cxt, JsonPathItem *jsp,
 			if (jsp->type == jpiDatetime)
 				RETURN_ERROR(ereport(ERROR,
 									 (errcode(ERRCODE_INVALID_ARGUMENT_FOR_SQL_JSON_DATETIME_FUNCTION),
-									  errmsg("datetime format is not recognized: \"%s\"",
-											 text_to_cstring(datetime)),
+									  errmsg("%s format is not recognized: \"%s\"",
+											 "datetime", text_to_cstring(datetime)),
 									  errhint("Use a datetime template argument to specify the input data format."))));
 			else
 				RETURN_ERROR(ereport(ERROR,
@@ -2401,8 +2401,8 @@ executeDateTimeMethod(JsonPathExecContext *cxt, JsonPathItem *jsp,
 					case TIMETZOID:
 						RETURN_ERROR(ereport(ERROR,
 											 (errcode(ERRCODE_INVALID_ARGUMENT_FOR_SQL_JSON_DATETIME_FUNCTION),
-											  errmsg("date format is not recognized: \"%s\"",
-													 text_to_cstring(datetime)))));
+											  errmsg("%s format is not recognized: \"%s\"",
+													 "date", text_to_cstring(datetime)))));
 						break;
 					case TIMESTAMPOID:
 						value = DirectFunctionCall1(timestamp_date,
@@ -2427,8 +2427,8 @@ executeDateTimeMethod(JsonPathExecContext *cxt, JsonPathItem *jsp,
 					case DATEOID:
 						RETURN_ERROR(ereport(ERROR,
 											 (errcode(ERRCODE_INVALID_ARGUMENT_FOR_SQL_JSON_DATETIME_FUNCTION),
-											  errmsg("time format is not recognized: \"%s\"",
-													 text_to_cstring(datetime)))));
+											  errmsg("%s format is not recognized: \"%s\"",
+													 "time", text_to_cstring(datetime)))));
 						break;
 					case TIMEOID:	/* Nothing to do for TIME */
 						break;
@@ -2476,8 +2476,8 @@ executeDateTimeMethod(JsonPathExecContext *cxt, JsonPathItem *jsp,
 					case TIMESTAMPOID:
 						RETURN_ERROR(ereport(ERROR,
 											 (errcode(ERRCODE_INVALID_ARGUMENT_FOR_SQL_JSON_DATETIME_FUNCTION),
-											  errmsg("time_tz format is not recognized: \"%s\"",
-													 text_to_cstring(datetime)))));
+											  errmsg("%s format is not recognized: \"%s\"",
+													 "time_tz", text_to_cstring(datetime)))));
 						break;
 					case TIMEOID:
 						value = DirectFunctionCall1(time_timetz,
@@ -2525,8 +2525,8 @@ executeDateTimeMethod(JsonPathExecContext *cxt, JsonPathItem *jsp,
 					case TIMETZOID:
 						RETURN_ERROR(ereport(ERROR,
 											 (errcode(ERRCODE_INVALID_ARGUMENT_FOR_SQL_JSON_DATETIME_FUNCTION),
-											  errmsg("timestamp format is not recognized: \"%s\"",
-													 text_to_cstring(datetime)))));
+											  errmsg("%s format is not recognized: \"%s\"",
+													 "timestamp", text_to_cstring(datetime)))));
 						break;
 					case TIMESTAMPOID:	/* Nothing to do for TIMESTAMP */
 						break;
@@ -2577,8 +2577,8 @@ executeDateTimeMethod(JsonPathExecContext *cxt, JsonPathItem *jsp,
 					case TIMETZOID:
 						RETURN_ERROR(ereport(ERROR,
 											 (errcode(ERRCODE_INVALID_ARGUMENT_FOR_SQL_JSON_DATETIME_FUNCTION),
-											  errmsg("timestamp_tz format is not recognized: \"%s\"",
-													 text_to_cstring(datetime)))));
+											  errmsg("%s format is not recognized: \"%s\"",
+													 "timestamp_tz", text_to_cstring(datetime)))));
 						break;
 					case TIMESTAMPOID:
 						value = DirectFunctionCall1(timestamp_timestamptz,

---

From: Tom Lane <[email-masked]>
Date: Sun, 28 Jan 2024 22:47:02 -0500

>  latter type. Perhaps the "number" was intended to refer to "numeric"?

Probably.  But I would write just "type numeric".  We do not generally
acknowledge "decimal" as a separate type, because for us it's only an
alias for numeric (there is not a pg_type entry for it).

Also, that leads to the thought that "numeric argument ... is out of
range for type numeric" seems either redundant or contradictory
depending on how you look at it.  So I suggest wording like

argument "...input string here..." of jsonpath item method .%s() is out of range for type numeric

> the error message, but it would be another issue.)

Agreed, so I suggest the above.

> I believe that the first line was intended to cover all the others:p

+1

			regards, tom lane

---

From: Kyotaro Horiguchi <[email-masked]>
Date: Mon, 29 Jan 2024 14:12:40 +0900 (JST)

At Sun, 28 Jan 2024 22:47:02 -0500, Tom Lane <[email-masked]> 
wrote in
> Agreed, so I suggest the above.

Having said that, I'm a bit concerned about the case where an overly
long string is given there. However, considering that we already have
"invalid input syntax for type xxx: %x" messages (including for the
numeric), this concern might be unnecessary.

Another concern is that the input value is already a numeric, not a
string. This situation occurs when the input is NaN of +-Inf. Although
numeric_out could be used, it might cause another error. Therefore,
simply writing out as "argument NaN and Infinity.." would be better.

Once we resolve these issues, another question arises regarding on of
the messages. In the case of NaN of Infinity, the message will be as
the follows:

"argument NaN or Infinity of jsonpath item method .%s() is out of 
range for type numeric"

This message appears quite strange and puzzling. I suspect that the
intended message was somewhat different.

regards.

---

From: Tom Lane <[email-masked]>
Date: Mon, 29 Jan 2024 00:33:22 -0500

> numeric), this concern might be unnecessary.

Yeah, we have not worried about that in the past.

> simply writing out as "argument NaN and Infinity.." would be better.

Oh!  I'd assumed that we were discussing a string that we'd failed to
convert to numeric.  If the input is already numeric, then either
the error is unreachable or what we're really doing is rejecting
special values such as NaN on policy grounds.  I would ask first
if that policy is sane at all.  (I'd lean to "not" --- if we allow
it in the input JSON, why not in the output?)  If it is sane, the
error message needs to be far more specific.

			regards, tom lane

---

From: Jeevan Chalke <[email-masked]>
Date: Tue, 30 Jan 2024 13:46:17 +0530

> error message needs to be far more specific.

*Consistent error message related to type:*

Agree that the number is not a PostgreSQL type and needs a change. As Tom
suggested, we can say "type numeric" here. However, I have seen two
variants of error messages here: (1) When the input is numeric and (2) when
the input is string. For first, we have error messages like:
numeric argument of jsonpath item method .%s() is out of range for type
double precision

and for the second, it is like:
string argument of jsonpath item method .%s() is not a valid representation
of a double precision number

The second form says "double precision number". So, in the decimal/number
case, should we use "numeric number" and then similarly "big integer
number"?

What if we use phrases like "for type double precision" at both places,
like:
numeric argument of jsonpath item method .%s() is out of range for type
double precision
string argument of jsonpath item method .%s() is not a valid representation
for type double precision

With this, the rest will be like:
for type numeric
for type bigint
for type integer

Suggestions?

---

*Showing input string in the error message:*

argument "...input string here..." of jsonpath item method .%s() is out of
range for type numeric

If we add the input string in the error, then for some errors, it will be
too big, for example:

-ERROR:  numeric argument of jsonpath item method .double() is out of range
for type double precision
+ERROR:  argument
"10000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"
of jsonpath item method .double() is out of range for type double precision

Also, for non-string input, we need to convert numeric to string just for
the error message, which seems overkill.

On another note, irrespective of these changes, is it good to show the
given input in the error messages? Error messages are logged and may leak
some details.

I think the existing way seems ok.

---

*NaN and Infinity restrictions:*

I am not sure why NaN and Infinity are not allowed in conversion to double
precision (.double() method). I have used the same restriction for
.decimal() and .number(). However, as you said, we should have error
messages more specific. I tried that in the attached patch; please have
your views. I have the following wordings for that error message:
"NaN or Infinity is not allowed for jsonpath item method .%s()"

Suggestions...

Thanks
-- 
Jeevan Chalke

edbpostgres.com