Обсуждение: Casts from jsonb to other types should cope with json null

Поиск
Список
Период
Сортировка

Casts from jsonb to other types should cope with json null

От
Tom Lane
Дата:
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

diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c
index 928552d551..91f1059e4c 100644
--- a/src/backend/utils/adt/jsonb.c
+++ b/src/backend/utils/adt/jsonb.c
@@ -2148,7 +2148,16 @@ jsonb_float8(PG_FUNCTION_ARGS)
     JsonbValue    v;
     Datum        retValue;
 
-    if (!JsonbExtractScalar(&in->root, &v) || v.type != jbvNumeric)
+    if (!JsonbExtractScalar(&in->root, &v))
+        cannotCastJsonbValue(v.type, "double precision");
+
+    if (v.type == jbvNull)
+    {
+        PG_FREE_IF_COPY(in, 0);
+        PG_RETURN_NULL();
+    }
+
+    if (v.type != jbvNumeric)
         cannotCastJsonbValue(v.type, "double precision");
 
     retValue = DirectFunctionCall1(numeric_float8,

Re: Casts from jsonb to other types should cope with json null

От
Maciek Sakrejda
Дата:
On Thu, Aug 1, 2024 at 3:52 PM Tom Lane <tgl@sss.pgh.pa.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?



Re: Casts from jsonb to other types should cope with json null

От
Tom Lane
Дата:
Maciek Sakrejda <maciek@pganalyze.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



Casts from jsonb to other types should cope with json null

От
"David G. Johnston"
Дата:
On Thursday, August 1, 2024, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Maciek Sakrejda <maciek@pganalyze.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.

Re: Casts from jsonb to other types should cope with json null

От
Tom Lane
Дата:
"David G. Johnston" <david.g.johnston@gmail.com> writes:
> On Thursday, August 1, 2024, Tom Lane <tgl@sss.pgh.pa.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

From e3c78ab3e9f1f7cbfec9b04933fd774ae2008780 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sat, 3 Aug 2024 14:43:46 -0400
Subject: [PATCH v1 1/2] Make jsonb casts to scalar types translate JSON null
 to SQL NULL.

Formerly, these cases threw an error "cannot cast jsonb null to type
<whatever>".  That seems less than helpful though.  It's also
inconsistent with the behavior of the ->> operator, which translates
JSON null to SQL NULL rather than 'null'.

Discussion: https://postgr.es/m/3851203.1722552717@sss.pgh.pa.us
---
 src/backend/utils/adt/jsonb.c       | 77 ++++++++++++++++++++++++++---
 src/test/regress/expected/jsonb.out | 66 +++++++++++++++++++++++++
 src/test/regress/sql/jsonb.sql      | 13 +++++
 3 files changed, 149 insertions(+), 7 deletions(-)

diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c
index 928552d551..ed054d5d42 100644
--- a/src/backend/utils/adt/jsonb.c
+++ b/src/backend/utils/adt/jsonb.c
@@ -2040,7 +2040,16 @@ jsonb_bool(PG_FUNCTION_ARGS)
     Jsonb       *in = PG_GETARG_JSONB_P(0);
     JsonbValue    v;

-    if (!JsonbExtractScalar(&in->root, &v) || v.type != jbvBool)
+    if (!JsonbExtractScalar(&in->root, &v))
+        cannotCastJsonbValue(v.type, "boolean");
+
+    if (v.type == jbvNull)
+    {
+        PG_FREE_IF_COPY(in, 0);
+        PG_RETURN_NULL();
+    }
+
+    if (v.type != jbvBool)
         cannotCastJsonbValue(v.type, "boolean");

     PG_FREE_IF_COPY(in, 0);
@@ -2055,7 +2064,16 @@ jsonb_numeric(PG_FUNCTION_ARGS)
     JsonbValue    v;
     Numeric        retValue;

-    if (!JsonbExtractScalar(&in->root, &v) || v.type != jbvNumeric)
+    if (!JsonbExtractScalar(&in->root, &v))
+        cannotCastJsonbValue(v.type, "numeric");
+
+    if (v.type == jbvNull)
+    {
+        PG_FREE_IF_COPY(in, 0);
+        PG_RETURN_NULL();
+    }
+
+    if (v.type != jbvNumeric)
         cannotCastJsonbValue(v.type, "numeric");

     /*
@@ -2076,7 +2094,16 @@ jsonb_int2(PG_FUNCTION_ARGS)
     JsonbValue    v;
     Datum        retValue;

-    if (!JsonbExtractScalar(&in->root, &v) || v.type != jbvNumeric)
+    if (!JsonbExtractScalar(&in->root, &v))
+        cannotCastJsonbValue(v.type, "smallint");
+
+    if (v.type == jbvNull)
+    {
+        PG_FREE_IF_COPY(in, 0);
+        PG_RETURN_NULL();
+    }
+
+    if (v.type != jbvNumeric)
         cannotCastJsonbValue(v.type, "smallint");

     retValue = DirectFunctionCall1(numeric_int2,
@@ -2094,7 +2121,16 @@ jsonb_int4(PG_FUNCTION_ARGS)
     JsonbValue    v;
     Datum        retValue;

-    if (!JsonbExtractScalar(&in->root, &v) || v.type != jbvNumeric)
+    if (!JsonbExtractScalar(&in->root, &v))
+        cannotCastJsonbValue(v.type, "integer");
+
+    if (v.type == jbvNull)
+    {
+        PG_FREE_IF_COPY(in, 0);
+        PG_RETURN_NULL();
+    }
+
+    if (v.type != jbvNumeric)
         cannotCastJsonbValue(v.type, "integer");

     retValue = DirectFunctionCall1(numeric_int4,
@@ -2112,7 +2148,16 @@ jsonb_int8(PG_FUNCTION_ARGS)
     JsonbValue    v;
     Datum        retValue;

-    if (!JsonbExtractScalar(&in->root, &v) || v.type != jbvNumeric)
+    if (!JsonbExtractScalar(&in->root, &v))
+        cannotCastJsonbValue(v.type, "bigint");
+
+    if (v.type == jbvNull)
+    {
+        PG_FREE_IF_COPY(in, 0);
+        PG_RETURN_NULL();
+    }
+
+    if (v.type != jbvNumeric)
         cannotCastJsonbValue(v.type, "bigint");

     retValue = DirectFunctionCall1(numeric_int8,
@@ -2130,7 +2175,16 @@ jsonb_float4(PG_FUNCTION_ARGS)
     JsonbValue    v;
     Datum        retValue;

-    if (!JsonbExtractScalar(&in->root, &v) || v.type != jbvNumeric)
+    if (!JsonbExtractScalar(&in->root, &v))
+        cannotCastJsonbValue(v.type, "real");
+
+    if (v.type == jbvNull)
+    {
+        PG_FREE_IF_COPY(in, 0);
+        PG_RETURN_NULL();
+    }
+
+    if (v.type != jbvNumeric)
         cannotCastJsonbValue(v.type, "real");

     retValue = DirectFunctionCall1(numeric_float4,
@@ -2148,7 +2202,16 @@ jsonb_float8(PG_FUNCTION_ARGS)
     JsonbValue    v;
     Datum        retValue;

-    if (!JsonbExtractScalar(&in->root, &v) || v.type != jbvNumeric)
+    if (!JsonbExtractScalar(&in->root, &v))
+        cannotCastJsonbValue(v.type, "double precision");
+
+    if (v.type == jbvNull)
+    {
+        PG_FREE_IF_COPY(in, 0);
+        PG_RETURN_NULL();
+    }
+
+    if (v.type != jbvNumeric)
         cannotCastJsonbValue(v.type, "double precision");

     retValue = DirectFunctionCall1(numeric_float8,
diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out
index e66d760189..c0bd0e76ae 100644
--- a/src/test/regress/expected/jsonb.out
+++ b/src/test/regress/expected/jsonb.out
@@ -5599,6 +5599,12 @@ select 'true'::jsonb::bool;
  t
 (1 row)

+select 'null'::jsonb::bool;
+ bool
+------
+
+(1 row)
+
 select '[]'::jsonb::bool;
 ERROR:  cannot cast jsonb array to type boolean
 select '1.0'::jsonb::float;
@@ -5607,22 +5613,82 @@ select '1.0'::jsonb::float;
       1
 (1 row)

+select 'null'::jsonb::float;
+ float8
+--------
+
+(1 row)
+
 select '[1.0]'::jsonb::float;
 ERROR:  cannot cast jsonb array to type double precision
+select '1.0'::jsonb::float4;
+ float4
+--------
+      1
+(1 row)
+
+select 'null'::jsonb::float4;
+ float4
+--------
+
+(1 row)
+
+select '[1.0]'::jsonb::float4;
+ERROR:  cannot cast jsonb array to type real
+select '12345'::jsonb::int2;
+ int2
+-------
+ 12345
+(1 row)
+
+select 'null'::jsonb::int2;
+ int2
+------
+
+(1 row)
+
+select '"hello"'::jsonb::int2;
+ERROR:  cannot cast jsonb string to type smallint
 select '12345'::jsonb::int4;
  int4
 -------
  12345
 (1 row)

+select 'null'::jsonb::int4;
+ int4
+------
+
+(1 row)
+
 select '"hello"'::jsonb::int4;
 ERROR:  cannot cast jsonb string to type integer
+select '12345'::jsonb::int8;
+ int8
+-------
+ 12345
+(1 row)
+
+select 'null'::jsonb::int8;
+ int8
+------
+
+(1 row)
+
+select '"hello"'::jsonb::int8;
+ERROR:  cannot cast jsonb string to type bigint
 select '12345'::jsonb::numeric;
  numeric
 ---------
    12345
 (1 row)

+select 'null'::jsonb::numeric;
+ numeric
+---------
+
+(1 row)
+
 select '{}'::jsonb::numeric;
 ERROR:  cannot cast jsonb object to type numeric
 select '12345.05'::jsonb::numeric;
diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql
index 97bc2242a1..1bcafe8cfb 100644
--- a/src/test/regress/sql/jsonb.sql
+++ b/src/test/regress/sql/jsonb.sql
@@ -1537,12 +1537,25 @@ select ts_headline('[]'::jsonb, tsquery('aaa & bbb'));

 -- casts
 select 'true'::jsonb::bool;
+select 'null'::jsonb::bool;
 select '[]'::jsonb::bool;
 select '1.0'::jsonb::float;
+select 'null'::jsonb::float;
 select '[1.0]'::jsonb::float;
+select '1.0'::jsonb::float4;
+select 'null'::jsonb::float4;
+select '[1.0]'::jsonb::float4;
+select '12345'::jsonb::int2;
+select 'null'::jsonb::int2;
+select '"hello"'::jsonb::int2;
 select '12345'::jsonb::int4;
+select 'null'::jsonb::int4;
 select '"hello"'::jsonb::int4;
+select '12345'::jsonb::int8;
+select 'null'::jsonb::int8;
+select '"hello"'::jsonb::int8;
 select '12345'::jsonb::numeric;
+select 'null'::jsonb::numeric;
 select '{}'::jsonb::numeric;
 select '12345.05'::jsonb::numeric;
 select '12345.05'::jsonb::float4;
--
2.43.5

From 8fca52bafc5b6b8dfc8d757c80a359202a56226c Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sat, 3 Aug 2024 15:53:11 -0400
Subject: [PATCH v1 2/2] Make jsonb cast to text translate JSON null to SQL
 NULL.

Previously, if the input was a JSON scalar null, you got the
string 'null'.  While that's not unreasonable in isolation, it's
inconsistent with the behavior of the ->> operator.  I think
that (jsonb->'fld')::text should produce results identical to
jsonb->>'fld', but up to now they disagree on this point.
Some other jsonb processing functions such as jsonb_array_elements
also translate JSON null to SQL NULL, and the preceding patch
in this series applies that rule to other jsonb-to-scalar casts.

This patch is incomplete: it only touches the behavior for casting
to text.  The pre-existing cast-via-I/O behavior applies to any
string-category result type, so if we want consistency we'd better
create explicit cast rules for varchar, bpchar, and name as well
as text.  (The regression tests added here demonstrate this
inconsistency.)  I've not done that, pending agreement on whether
we want this behavioral change at all.

Discussion: https://postgr.es/m/3851203.1722552717@sss.pgh.pa.us
---
 src/backend/utils/adt/jsonb.c       | 23 +++++++++
 src/include/catalog/pg_cast.dat     |  5 +-
 src/include/catalog/pg_proc.dat     |  3 ++
 src/test/regress/expected/jsonb.out | 72 +++++++++++++++++++++++++++++
 src/test/regress/sql/jsonb.sql      | 12 +++++
 5 files changed, 114 insertions(+), 1 deletion(-)

diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c
index ed054d5d42..c4e8db0d24 100644
--- a/src/backend/utils/adt/jsonb.c
+++ b/src/backend/utils/adt/jsonb.c
@@ -2222,6 +2222,29 @@ jsonb_float8(PG_FUNCTION_ARGS)
     PG_RETURN_DATUM(retValue);
 }

+Datum
+jsonb_text(PG_FUNCTION_ARGS)
+{
+    Jsonb       *in = PG_GETARG_JSONB_P(0);
+    JsonbValue    v;
+    StringInfoData jtext;
+
+    /* Convert scalar null to SQL null */
+    if (JsonbExtractScalar(&in->root, &v) && v.type == jbvNull)
+    {
+        PG_FREE_IF_COPY(in, 0);
+        PG_RETURN_NULL();
+    }
+
+    /* Every other case acts like jsonb_out() */
+    initStringInfo(&jtext);
+    (void) JsonbToCString(&jtext, &in->root, VARSIZE(in));
+
+    PG_FREE_IF_COPY(in, 0);
+
+    PG_RETURN_TEXT_P(cstring_to_text_with_len(jtext.data, jtext.len));
+}
+
 /*
  * Convert jsonb to a C-string stripping quotes from scalar strings.
  */
diff --git a/src/include/catalog/pg_cast.dat b/src/include/catalog/pg_cast.dat
index ca7b6d7191..bffe17ead0 100644
--- a/src/include/catalog/pg_cast.dat
+++ b/src/include/catalog/pg_cast.dat
@@ -512,7 +512,7 @@
 { castsource => 'jsonb', casttarget => 'json', castfunc => '0',
   castcontext => 'a', castmethod => 'i' },

-# jsonb to numeric and bool types
+# jsonb to various scalar types
 { castsource => 'jsonb', casttarget => 'bool', castfunc => 'bool(jsonb)',
   castcontext => 'e', castmethod => 'f' },
 { castsource => 'jsonb', casttarget => 'numeric', castfunc => 'numeric(jsonb)',
@@ -527,6 +527,9 @@
   castcontext => 'e', castmethod => 'f' },
 { castsource => 'jsonb', casttarget => 'float8', castfunc => 'float8(jsonb)',
   castcontext => 'e', castmethod => 'f' },
+# this cast replaces an implicit COERCEVIAIO cast, so must be assignment-level:
+{ castsource => 'jsonb', casttarget => 'text', castfunc => 'text(jsonb)',
+  castcontext => 'a', castmethod => 'f' },

 # range to multirange
 { castsource => 'int4range', casttarget => 'int4multirange',
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index d36f6001bb..c91824724c 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -4659,6 +4659,9 @@
 { oid => '2580', descr => 'convert jsonb to float8',
   proname => 'float8', prorettype => 'float8', proargtypes => 'jsonb',
   prosrc => 'jsonb_float8' },
+{ oid => '8079', descr => 'convert jsonb to text',
+  proname => 'text', prorettype => 'text', proargtypes => 'jsonb',
+  prosrc => 'jsonb_text' },

 # formatting
 { oid => '1770', descr => 'format timestamp with time zone to text',
diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out
index c0bd0e76ae..dda3ee5b95 100644
--- a/src/test/regress/expected/jsonb.out
+++ b/src/test/regress/expected/jsonb.out
@@ -5763,3 +5763,75 @@ select '12345.0000000000000000000000000000000000000000000005'::jsonb::int8;
  12345
 (1 row)

+select 'true'::jsonb::text;
+ text
+------
+ true
+(1 row)
+
+select '1.0'::jsonb::text;
+ text
+------
+ 1.0
+(1 row)
+
+select '"hello"'::jsonb::text;
+  text
+---------
+ "hello"
+(1 row)
+
+select 'null'::jsonb::text;
+ text
+------
+
+(1 row)
+
+select '[1.0]'::jsonb::text;
+ text
+-------
+ [1.0]
+(1 row)
+
+select '{"a": "b"}'::jsonb::text;
+    text
+------------
+ {"a": "b"}
+(1 row)
+
+select 'true'::jsonb::varchar;
+ varchar
+---------
+ true
+(1 row)
+
+select '1.0'::jsonb::varchar;
+ varchar
+---------
+ 1.0
+(1 row)
+
+select '"hello"'::jsonb::varchar;
+ varchar
+---------
+ "hello"
+(1 row)
+
+select 'null'::jsonb::varchar;  -- not the desired behavior
+ varchar
+---------
+ null
+(1 row)
+
+select '[1.0]'::jsonb::varchar;
+ varchar
+---------
+ [1.0]
+(1 row)
+
+select '{"a": "b"}'::jsonb::varchar;
+  varchar
+------------
+ {"a": "b"}
+(1 row)
+
diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql
index 1bcafe8cfb..57240c4146 100644
--- a/src/test/regress/sql/jsonb.sql
+++ b/src/test/regress/sql/jsonb.sql
@@ -1569,3 +1569,15 @@ select '12345.0000000000000000000000000000000000000000000005'::jsonb::float8;
 select '12345.0000000000000000000000000000000000000000000005'::jsonb::int2;
 select '12345.0000000000000000000000000000000000000000000005'::jsonb::int4;
 select '12345.0000000000000000000000000000000000000000000005'::jsonb::int8;
+select 'true'::jsonb::text;
+select '1.0'::jsonb::text;
+select '"hello"'::jsonb::text;
+select 'null'::jsonb::text;
+select '[1.0]'::jsonb::text;
+select '{"a": "b"}'::jsonb::text;
+select 'true'::jsonb::varchar;
+select '1.0'::jsonb::varchar;
+select '"hello"'::jsonb::varchar;
+select 'null'::jsonb::varchar;  -- not the desired behavior
+select '[1.0]'::jsonb::varchar;
+select '{"a": "b"}'::jsonb::varchar;
--
2.43.5


Re: Casts from jsonb to other types should cope with json null

От
Maxim Orlov
Дата:


On Sat, 3 Aug 2024 at 23:10, Tom Lane <tgl@sss.pgh.pa.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.

Re: Casts from jsonb to other types should cope with json null

От
Tom Lane
Дата:
Maxim Orlov <orlovmg@gmail.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



Re: Casts from jsonb to other types should cope with json null

От
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

diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c
index 8394a20e0e..e38aebd848 100644
--- a/src/backend/utils/adt/jsonb.c
+++ b/src/backend/utils/adt/jsonb.c
@@ -2034,6 +2034,10 @@ cannotCastJsonbValue(enum jbvType type, const char *sqltype)
     elog(ERROR, "unknown jsonb type: %d", (int) type);
 }

+/*
+ * Assorted jsonb-to-scalar-type conversion functions.
+ */
+
 Datum
 jsonb_bool(PG_FUNCTION_ARGS)
 {
@@ -2222,6 +2226,74 @@ jsonb_float8(PG_FUNCTION_ARGS)
     PG_RETURN_DATUM(retValue);
 }

+/* Note: this is also used for jsonb-to-varchar */
+Datum
+jsonb_text(PG_FUNCTION_ARGS)
+{
+    Jsonb       *in = PG_GETARG_JSONB_P(0);
+    JsonbValue    v;
+    StringInfoData jtext;
+
+    /* Convert scalar null to SQL null */
+    if (JsonbExtractScalar(&in->root, &v) && v.type == jbvNull)
+    {
+        PG_FREE_IF_COPY(in, 0);
+        PG_RETURN_NULL();
+    }
+
+    /* Every other case acts like jsonb_out() */
+    initStringInfo(&jtext);
+    (void) JsonbToCString(&jtext, &in->root, VARSIZE(in));
+
+    PG_FREE_IF_COPY(in, 0);
+
+    PG_RETURN_TEXT_P(cstring_to_text_with_len(jtext.data, jtext.len));
+}
+
+Datum
+jsonb_bpchar(PG_FUNCTION_ARGS)
+{
+    /*
+     * This is really equivalent to jsonb_text, but it must be a separate C
+     * function to keep opr_sanity.sql from complaining.
+     */
+    return jsonb_text(fcinfo);
+}
+
+Datum
+jsonb_name(PG_FUNCTION_ARGS)
+{
+    Jsonb       *in = PG_GETARG_JSONB_P(0);
+    Name        result;
+    JsonbValue    v;
+    StringInfoData jtext;
+    int            len;
+
+    /* Convert scalar null to SQL null */
+    if (JsonbExtractScalar(&in->root, &v) && v.type == jbvNull)
+    {
+        PG_FREE_IF_COPY(in, 0);
+        PG_RETURN_NULL();
+    }
+
+    /* Every other case acts like jsonb_out() */
+    initStringInfo(&jtext);
+    (void) JsonbToCString(&jtext, &in->root, VARSIZE(in));
+
+    PG_FREE_IF_COPY(in, 0);
+
+    /* Truncate oversize input */
+    len = jtext.len;
+    if (len >= NAMEDATALEN)
+        len = pg_mbcliplen(jtext.data, len, NAMEDATALEN - 1);
+
+    /* We use palloc0 here to ensure result is zero-padded */
+    result = (Name) palloc0(NAMEDATALEN);
+    memcpy(NameStr(*result), jtext.data, len);
+
+    PG_RETURN_NAME(result);
+}
+
 /*
  * Convert jsonb to a C-string stripping quotes from scalar strings.
  */
diff --git a/src/include/catalog/pg_cast.dat b/src/include/catalog/pg_cast.dat
index a26ba34e86..1bd40d8fac 100644
--- a/src/include/catalog/pg_cast.dat
+++ b/src/include/catalog/pg_cast.dat
@@ -512,7 +512,7 @@
 { castsource => 'jsonb', casttarget => 'json', castfunc => '0',
   castcontext => 'a', castmethod => 'i' },

-# jsonb to numeric and bool types
+# jsonb to various scalar types
 { castsource => 'jsonb', casttarget => 'bool', castfunc => 'bool(jsonb)',
   castcontext => 'e', castmethod => 'f' },
 { castsource => 'jsonb', casttarget => 'numeric', castfunc => 'numeric(jsonb)',
@@ -527,6 +527,15 @@
   castcontext => 'e', castmethod => 'f' },
 { castsource => 'jsonb', casttarget => 'float8', castfunc => 'float8(jsonb)',
   castcontext => 'e', castmethod => 'f' },
+# these casts replace implicit COERCEVIAIO casts, so must be assignment-level:
+{ castsource => 'jsonb', casttarget => 'text', castfunc => 'text(jsonb)',
+  castcontext => 'a', castmethod => 'f' },
+{ castsource => 'jsonb', casttarget => 'varchar', castfunc => 'varchar(jsonb)',
+  castcontext => 'a', castmethod => 'f' },
+{ castsource => 'jsonb', casttarget => 'bpchar', castfunc => 'bpchar(jsonb)',
+  castcontext => 'a', castmethod => 'f' },
+{ castsource => 'jsonb', casttarget => 'name', castfunc => 'name(jsonb)',
+  castcontext => 'a', castmethod => 'f' },

 # range to multirange
 { castsource => 'int4range', casttarget => 'int4multirange',
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 18560755d2..350ba1f800 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -4705,6 +4705,18 @@
 { oid => '2580', descr => 'convert jsonb to float8',
   proname => 'float8', prorettype => 'float8', proargtypes => 'jsonb',
   prosrc => 'jsonb_float8' },
+{ oid => '8079', descr => 'convert jsonb to text',
+  proname => 'text', prorettype => 'text', proargtypes => 'jsonb',
+  prosrc => 'jsonb_text' },
+{ oid => '8080', descr => 'convert jsonb to varchar',
+  proname => 'varchar', prorettype => 'varchar', proargtypes => 'jsonb',
+  prosrc => 'jsonb_text' },
+{ oid => '8081', descr => 'convert jsonb to char(n)',
+  proname => 'bpchar', prorettype => 'bpchar', proargtypes => 'jsonb',
+  prosrc => 'jsonb_bpchar' },
+{ oid => '8082', descr => 'convert jsonb to name',
+  proname => 'name', prorettype => 'name', proargtypes => 'jsonb',
+  prosrc => 'jsonb_name' },

 # formatting
 { oid => '1770', descr => 'format timestamp with time zone to text',
diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out
index 2baff931bf..d7a3bc299c 100644
--- a/src/test/regress/expected/jsonb.out
+++ b/src/test/regress/expected/jsonb.out
@@ -5781,3 +5781,75 @@ select '12345.0000000000000000000000000000000000000000000005'::jsonb::int8;
  12345
 (1 row)

+select 'true'::jsonb::text;
+ text
+------
+ true
+(1 row)
+
+select '1.0'::jsonb::text;
+ text
+------
+ 1.0
+(1 row)
+
+select '"hello"'::jsonb::text;
+  text
+---------
+ "hello"
+(1 row)
+
+select 'null'::jsonb::text;
+ text
+------
+
+(1 row)
+
+select '[1.0]'::jsonb::text;
+ text
+-------
+ [1.0]
+(1 row)
+
+select '{"a": "b"}'::jsonb::text;
+    text
+------------
+ {"a": "b"}
+(1 row)
+
+select 'true'::jsonb::varchar;
+ varchar
+---------
+ true
+(1 row)
+
+select '1.0'::jsonb::varchar;
+ varchar
+---------
+ 1.0
+(1 row)
+
+select '"hello"'::jsonb::varchar;
+ varchar
+---------
+ "hello"
+(1 row)
+
+select 'null'::jsonb::varchar;
+ varchar
+---------
+
+(1 row)
+
+select '[1.0]'::jsonb::varchar;
+ varchar
+---------
+ [1.0]
+(1 row)
+
+select '{"a": "b"}'::jsonb::varchar;
+  varchar
+------------
+ {"a": "b"}
+(1 row)
+
diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql
index 544bb610e2..b391b1555a 100644
--- a/src/test/regress/sql/jsonb.sql
+++ b/src/test/regress/sql/jsonb.sql
@@ -1572,3 +1572,15 @@ select '12345.0000000000000000000000000000000000000000000005'::jsonb::float8;
 select '12345.0000000000000000000000000000000000000000000005'::jsonb::int2;
 select '12345.0000000000000000000000000000000000000000000005'::jsonb::int4;
 select '12345.0000000000000000000000000000000000000000000005'::jsonb::int8;
+select 'true'::jsonb::text;
+select '1.0'::jsonb::text;
+select '"hello"'::jsonb::text;
+select 'null'::jsonb::text;
+select '[1.0]'::jsonb::text;
+select '{"a": "b"}'::jsonb::text;
+select 'true'::jsonb::varchar;
+select '1.0'::jsonb::varchar;
+select '"hello"'::jsonb::varchar;
+select 'null'::jsonb::varchar;
+select '[1.0]'::jsonb::varchar;
+select '{"a": "b"}'::jsonb::varchar;

Re: Casts from jsonb to other types should cope with json null

От
Maxim Orlov
Дата:


On Fri, 24 Jan 2025 at 22:20, Tom Lane <tgl@sss.pgh.pa.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.