jsonpath versus NaN

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: jsonpath versus NaN
Date: 2020-06-11 12:45:42
Message-ID: 203949.1591879542@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Commit 72b646033 inserted this into convertJsonbScalar:


case jbvNumeric:
+ /* replace numeric NaN with string "NaN" */
+ if (numeric_is_nan(scalarVal->val.numeric))
+ {
+ appendToBuffer(buffer, "NaN", 3);
+ *jentry = 3;
+ break;
+ }
numlen = VARSIZE_ANY(scalarVal->val.numeric);
padlen = padBufferToInt(buffer);

To characterize this as hack, slash, and burn programming would be
charitable. It is entirely clear from the code, the documentation,
and the relevant RFCs that JSONB does not allow NaNs as numeric
values. So it should be impossible for this code to do anything.

I tried taking it out, and found that this test case from the same
commit fails:

+select jsonb_path_query('"nan"', '$.double()');
+ jsonb_path_query
+ "NaN"
+(1 row)

However, seeing that the JSON RFC disallows NaNs, I do not understand
why it's important to accept this. The adjacent test case showing
that 'inf' isn't accepted:

+select jsonb_path_query('"inf"', '$.double()');
+ERROR: non-numeric SQL/JSON item
+DETAIL: jsonpath item method .double() can only be applied to a numeric value

seems like a saner approach.

In short, I think we should rip out the above code snippet and adjust
executeItemOptUnwrapTarget, at about line jsonpath_exec.c:1076 as of HEAD,
to reject NaNs the same way it already rejects infinities. Can you
explain why it was done like this?

(The reason I came across this was that I'm working on extending
type numeric to allow infinities, and it was not clear what to
do here. But allowing a jsonb to contain a numeric NaN, even
transiently, seems like a completely horrid idea.)

regards, tom lane


Browse pgsql-hackers by date

  From Date Subject
Next Message Ranier Vilela 2020-06-11 12:49:27 [PATCH] fix two shadow vars (src/backend/commands/sequence.c)
Previous Message Amit Kapila 2020-06-11 11:34:58 Re: Parallel Seq Scan vs kernel read ahead