Casts from jsonb to other types should cope with json null

I complained in the discussion of bug #18564 [1] that it's quite
inconsistent that you can cast a jsonb null to text and get
a SQL NULL:

=# select ('{"a": null}'::jsonb)->>'a';
?column?
----------

(1 row)

but if you cast it to any other type it's an error:

=# select (('{"a": null}'::jsonb)->'a')::float8;
ERROR: cannot cast jsonb null to type double precision

I think this should be allowed and should produce a SQL NULL.
It doesn't look hard: the attached POC patch fixes this for
the float8 case only. If there's not conceptual objections
I can flesh this out to cover the other jsonb-to-XXX
cast functions.

regards, tom lane

[1] https://www.postgresql.org/message-id/flat/18564-5985f90678ed7512%40postgresql.org

On Thu, Aug 1, 2024 at 3:52 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> I complained in the discussion of bug #18564 [1] that it's quite
> inconsistent that you can cast a jsonb null to text and get
> a SQL NULL:
>
> =# select ('{"a": null}'::jsonb)->>'a';
> ?column?
> ----------
>
> (1 row)

Oddly, it looks like you only get a null if you use the '->>'
operator. With '->' and a subsequent cast to text, you get the string
"null":

maciek=# select (('{"a":null}'::jsonb)->'a')::text;
text
------
null
(1 row)

Is that expected?

Maciek Sakrejda <maciek(at)pganalyze(dot)com> writes:
> Oddly, it looks like you only get a null if you use the '->>'
> operator. With '->' and a subsequent cast to text, you get the string
> "null":

> maciek=# select (('{"a":null}'::jsonb)->'a')::text;
> text
> ------
> null
> (1 row)

> Is that expected?

I think what is happening there is you're getting the fallback
"cast via I/O" behavior. There's no jsonb->text cast function
in the catalogs.

Perhaps it's worth adding one, so that it can be made to behave
similarly to the casts to other types. However, that would be
a compatibility break for a case that doesn't fail today, so
it might be a harder sell than changing cases that do fail.
I'm mildly in favor of doing it, but somebody else might
think differently.

regards, tom lane

On Thursday, August 1, 2024, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Maciek Sakrejda <maciek(at)pganalyze(dot)com> writes:
> > Oddly, it looks like you only get a null if you use the '->>'
> > operator. With '->' and a subsequent cast to text, you get the string
> > "null":
>
> > maciek=# select (('{"a":null}'::jsonb)->'a')::text;
> > text
> > ------
> > null
> > (1 row)
>
> > Is that expected?
>
> I think what is happening there is you're getting the fallback
> "cast via I/O" behavior. There's no jsonb->text cast function
> in the catalogs.
>
> Perhaps it's worth adding one, so that it can be made to behave
> similarly to the casts to other types.
>

I’m not too keen on opening Pandora’s box here even if I do regret our
current choices. Semantic casting of json scalar strings only, and doing
document serialization as a function, would have been better in hindsight.

I am fine with implementing the conversion of json null types to SQL null
for all casts that already do semantic value casting, and thus recognize
but prohibit the cast, as shown for float.

I read the discussion thread [1] that added this and while one person
mentioned json null no one replied to that point and seemingly no explicit
consideration for treating json null semantically was ever done - i.e. this
fails only because in json null has its own type, and the test were type,
not value, oriented. As SQL null is a value only, whose type is whatever
holds it, I’d argue our lack of doing this even constitutes a bug but
wouldn’t - and turning errors into non-errors has a lower “bug acceptance
threshold”.

David J.

[1]
https://www.postgresql.org/message-id/flat/0154d35a-24ae-f063-5273-9ffcdf1c7f2e%40postgrespro.ru

"David G. Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com> writes:
> On Thursday, August 1, 2024, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> I think what is happening there is you're getting the fallback
>> "cast via I/O" behavior. There's no jsonb->text cast function
>> in the catalogs.
>> Perhaps it's worth adding one, so that it can be made to behave
>> similarly to the casts to other types.

> I’m not too keen on opening Pandora’s box here even if I do regret our
> current choices. Semantic casting of json scalar strings only, and doing
> document serialization as a function, would have been better in hindsight.
> ...
> I read the discussion thread [1] that added this and while one person
> mentioned json null no one replied to that point and seemingly no explicit
> consideration for treating json null semantically was ever done - i.e. this
> fails only because in json null has its own type, and the test were type,
> not value, oriented. As SQL null is a value only, whose type is whatever
> holds it, I’d argue our lack of doing this even constitutes a bug but
> wouldn’t - and turning errors into non-errors has a lower “bug acceptance
> threshold”.

Yeah, it's clear that this wasn't thought about too hard, because
I discovered that changing this behavior affects exactly zero existing
regression test cases :-(. But I still think we should probably
change it. Aside from ->>, we have other operators/functions that
convert jsonb values to SQL types, such as #>>,
jsonb_array_elements_text, jsonb_each_text, and AFAICS every one
of those translates JSON null to SQL NULL.

Attached are some actual patches for this. 0001 just changes the
existing json-to-scalar casts; since those previously threw
an error, I think that change is not too controversial. 0002 is
POC for changing the behavior of jsonb::text. I don't think
it's committable as-is, because we have the same behavior for
jsonb::varchar and other string-category target types. That's
not all that hard to fix, but I've not done so pending a decision
on whether we want 0002.

It strikes me that there's also a question of whether json::text
should translate 'null' like this. I'm inclined to think not,
since json is in some sense a domain over text, but it's certainly
debatable.

regards, tom lane

On Sat, 3 Aug 2024 at 23:10, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> It strikes me that there's also a question of whether json::text
> should translate 'null' like this. I'm inclined to think not,
> since json is in some sense a domain over text, but it's certainly
> debatable.
>
I tend to agree with you here about the semantics of such casts. But
consistency and "predictability" of
behaviour for the casts are a bit more important in my view. At least,
based on my experience. So, I'm
for 0002 patch.

--
Best regards,
Maxim Orlov.

Maxim Orlov <orlovmg(at)gmail(dot)com> writes:
> I tend to agree with you here about the semantics of such casts. But
> consistency and "predictability" of
> behaviour for the casts are a bit more important in my view. At least,
> based on my experience. So, I'm
> for 0002 patch.

OK. Nobody has spoken against the 0001 patch (replace errors with
return-a-null), so I think I'll go ahead and commit that one.
Then I'll return to this thread with a fleshed-out patch for 0002.

regards, tom lane

I wrote:
> OK. Nobody has spoken against the 0001 patch (replace errors with
> return-a-null), so I think I'll go ahead and commit that one.
> Then I'll return to this thread with a fleshed-out patch for 0002.

0001 is pushed, and as promised, here's a version of 0002 extended
to cover all the string-category types.

However ... the more I play with 0002, the less enchanted I get.
The problem is that there are two different use-cases to serve:

1. General conversion of any jsonb value to string form.
2. Conversion of a jsonb scalar string to a SQL string.

For use-case #1, it makes sense that we do

regression=# select '"hello"'::jsonb::text;
text
---------
"hello"
(1 row)

but for use-case #2, you'd probably prefer that the quotes
weren't included. (We can't do that in use-case #1 because
the string contents might look too much like some other
sort of JSON value.) So it seems like two separate conversion
functions are needed to serve these two use-cases, and for
better or worse we've already decided that casting jsonb to text
is meant for use-case #1. (It was sort of a decision by default,
I suspect, but not deciding is still a decision.)

What I am realizing is that "JSON null becomes SQL NULL" is a rule
that is adapted to use-case #2 but not so much to use-case #1.
For example, the existing behavior

regression=# select null::jsonb::text;
text
------

(1 row)

regression=# select 'null'::jsonb::text;
text
------
null
(1 row)

actually makes plenty of sense if you hope to be able to round-trip
the result. It's only after rejecting non-scalar JSON values that
it makes sense to special-case a JSON null.

So here's the patch, just because I promised it, but I'm now
thinking about withdrawing it.

What would make more sense for use-case #2 is something that
produces NULL for JSON null, a de-quoted string for a JSON
string value, and an error otherwise. The ->> operator is
about halfway there (it won't throw an error for non-scalar
input), but of course that only works when the value you want
to extract is in a JSON object field. I guess what would
be wanted is a new function f(jsonb) returns text, but I'm
unsure about a good name.

regards, tom lane

On Fri, 24 Jan 2025 at 22:20, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

>
> What I am realizing is that "JSON null becomes SQL NULL" is a rule
> that is adapted to use-case #2 but not so much to use-case #1.
>
OK, I think I've got it.

> So here's the patch, just because I promised it, but I'm now
> thinking about withdrawing it.
>
Thanks for explanation, at your will.

--
Best regards,
Maxim Orlov.