Subject: Re: Document DateStyle effect on jsonpath string()

From: Tom Lane <[email-masked]>
Date: Wed, 11 Sep 2024 10:11:05 -0400

Peter Eisentraut <[email-masked]> writes:
> What I'm concerned about is that this makes the behavior of JSON_QUERY 
> non-immutable.  Maybe there are other reasons for it to be 
> non-immutable, in which case this isn't important.  But it might be 
> worth avoiding that?

Fair point, but haven't we already bit that bullet with respect
to timezones?

[ looks... ]  Hmm, it looks like jsonb_path_exists_tz is marked
stable while jsonb_path_exists is claimed to be immutable.
So yeah, there's a problem here.  I'm not 100% convinced that
jsonb_path_exists was truly immutable before, but for sure it
is not now, and that's bad.

regression=# select jsonb_path_query('"2023-08-15 12:34:56"', '$.timestamp().string()');
   jsonb_path_query    
-----------------------
 "2023-08-15 12:34:56"
(1 row)

regression=# set datestyle = postgres;
SET
regression=# select jsonb_path_query('"2023-08-15 12:34:56"', '$.timestamp().string()');
      jsonb_path_query      
----------------------------
 "Tue Aug 15 12:34:56 2023"
(1 row)

			regards, tom lane

---

From: "David E. Wheeler" <[email-masked]>
Date: Wed, 11 Sep 2024 11:00:09 -0400

> regression=# select jsonb_path_query('"2023-08-15 12:34:56"', '$.timestamp().string()');

I wonder, then, whether .string() should be modified to use the ISO format in UTC, and therefore be immutable. That’s the format you get if you omit .string() and let result be stringified from a date/time/timestamp.

FWIW, that’s how my Go port works, since I didn’t bother to replicate the DateStyle GUC (example[1]).

Best,

David

[1]: https://theory.github.io/sqljson/playground/?p=%2524.timestamp%28%29.string%28%29&j=%25222023-08-15%252012%253A34%253A56%2522&a=&o=1

---

From: Tom Lane <[email-masked]>
Date: Wed, 11 Sep 2024 11:11:24 -0400

> I wonder, then, whether .string() should be modified to use the ISO format in UTC, and therefore be immutable. That’s the format you get if you omit .string() and let result be stringified from a d…

What "let result be stringified" behavior are you thinking of,
exactly?  AFAICS there's not sensitivity to timezone unless you
use the _tz variant, otherwise it just regurgitates the input.

I agree that we should force ISO datestyle, but I'm not quite sure
about whether we're in the clear with timezone handling.  We already
had a bunch of specialized rules about timezone handling in the _tz
and not-_tz variants of these functions.  It seems to me that simply
forcing UTC would not be consistent with that pre-existing behavior.
However, I may not have absorbed enough caffeine yet.

			regards, tom lane

---

From: "David E. Wheeler" <[email-masked]>
Date: Wed, 11 Sep 2024 11:52:31 -0400

> use the _tz variant, otherwise it just regurgitates the input.

There is stringification of a time, date, or timestamp value, which has no TZ, but is still affected by DateStyle. Then there is stringification of timetz or timestamptz, which can be created by the .time_tz() and .timstamp_tz() functions, and therefore are impacted by both the DateStyle and TimeZone configs, even when not using the _tz variant:

david=# set timezone = 'America/New_York';
SET
david=# select jsonb_path_query('"2023-08-15 12:34:56-09"', '$.timestamp_tz().string()');
     jsonb_path_query     
--------------------------
 "2023-08-15 17:34:56-04"

david=# set timezone = 'America/Los_Angeles';
SET
david=# select jsonb_path_query('"2023-08-15 12:34:56-09"', '$.timestamp_tz().string()');
     jsonb_path_query     
--------------------------
 "2023-08-15 14:34:56-07"
(1 row)

> However, I may not have absorbed enough caffeine yet.

True, it would not be consistent with the existing behaviors, but I believe these are all new in Postgres 17.

Best,

David

---

From: Tom Lane <[email-masked]>
Date: Wed, 11 Sep 2024 12:06:27 -0400

> There is stringification of a time, date, or timestamp value, which
> has no TZ, but is still affected by DateStyle.

What I understood you to be referencing is what happens without
string(), which AFAICS does not result in any timezone rotation:

regression=# set timezone = 'America/New_York';
SET
regression=# select jsonb_path_query('"2023-08-15 12:34:56-09"', '$.timestamp_tz()');
      jsonb_path_query       
-----------------------------
 "2023-08-15T12:34:56-09:00"
(1 row)

I think I'd be content to have string() duplicate that behavior
--- in fact, it seems like it'd be odd if it doesn't match.

			regards, tom lane

---

From: Tom Lane <[email-masked]>
Date: Wed, 11 Sep 2024 12:26:42 -0400

> --- in fact, it seems like it'd be odd if it doesn't match.

Building on that thought, maybe we could fix it as attached?
This changes the just-committed test cases of course, and I did
not look at whether there are documentation changes to make.

			regards, tom lane

diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index e3ee0093d4..b9c2443b65 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -72,6 +72,7 @@
 #include "utils/datetime.h"
 #include "utils/float.h"
 #include "utils/formatting.h"
+#include "utils/json.h"
 #include "utils/jsonpath.h"
 #include "utils/lsyscache.h"
 #include "utils/memutils.h"
@@ -1629,32 +1630,13 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt, JsonPathItem *jsp,
 						break;
 					case jbvDatetime:
 						{
-							switch (jb->val.datetime.typid)
-							{
-								case DATEOID:
-									tmp = DatumGetCString(DirectFunctionCall1(date_out,
-																			  jb->val.datetime.value));
-									break;
-								case TIMEOID:
-									tmp = DatumGetCString(DirectFunctionCall1(time_out,
-																			  jb->val.datetime.value));
-									break;
-								case TIMETZOID:
-									tmp = DatumGetCString(DirectFunctionCall1(timetz_out,
-																			  jb->val.datetime.value));
-									break;
-								case TIMESTAMPOID:
-									tmp = DatumGetCString(DirectFunctionCall1(timestamp_out,
-																			  jb->val.datetime.value));
-									break;
-								case TIMESTAMPTZOID:
-									tmp = DatumGetCString(DirectFunctionCall1(timestamptz_out,
-																			  jb->val.datetime.value));
-									break;
-								default:
-									elog(ERROR, "unrecognized SQL/JSON datetime type oid: %u",
-										 jb->val.datetime.typid);
-							}
+							char		buf[MAXDATELEN + 1];
+
+							JsonEncodeDateTime(buf,
+											   jb->val.datetime.value,
+											   jb->val.datetime.typid,
+											   &jb->val.datetime.tz);
+							tmp = pstrdup(buf);
 						}
 						break;
 					case jbvNull:

---

From: "David E. Wheeler" <[email-masked]>
Date: Wed, 11 Sep 2024 13:06:00 -0400

> not look at whether there are documentation changes to make.

It looks like that’s what datum_to_json_internal() in json.c does, which IIUC is the default stringification for date and time values.

David

---

From: Tom Lane <[email-masked]>
Date: Wed, 11 Sep 2024 15:08:35 -0400

> It looks like that’s what datum_to_json_internal() in json.c does, which IIUC is the default stringification for date and time values.

Right.  I actually lifted the code from convertJsonbScalar in
jsonb_util.c.

Here's a more fleshed-out patch with docs and regression test
fixes.  I figured we could shorten the tests a bit now that
the point is just to verify that datestyle *doesn't* affect it.

			regards, tom lane

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 1bde4091ca..aa1ac2c4fe 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -18017,16 +18017,15 @@ ERROR:  jsonpath member accessor can only be applied to an object
        </para>
        <para>
         String value converted from a JSON boolean, number, string, or
-        datetime (the output format for datetimes is determined by
-        the <xref linkend="guc-datestyle"/> parameter)
+        datetime
        </para>
        <para>
         <literal>jsonb_path_query_array('[1.23, "xyz", false]', '$[*].string()')</literal>
         <returnvalue>["1.23", "xyz", "false"]</returnvalue>
        </para>
        <para>
-        <literal>jsonb_path_query('"2023-08-15"', '$.datetime().string()')</literal>
-        <returnvalue>"2023-08-15"</returnvalue>
+        <literal>jsonb_path_query('"2023-08-15 12:34:56"', '$.timestamp().string()')</literal>
+        <returnvalue>"2023-08-15T12:34:56"</returnvalue>
        </para></entry>
       </row>
 
diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index e3ee0093d4..b9c2443b65 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -72,6 +72,7 @@
 #include "utils/datetime.h"
 #include "utils/float.h"
 #include "utils/formatting.h"
+#include "utils/json.h"
 #include "utils/jsonpath.h"
 #include "utils/lsyscache.h"
 #include "utils/memutils.h"
@@ -1629,32 +1630,13 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt, JsonPathItem *jsp,
 						break;
 					case jbvDatetime:
 						{
-							switch (jb->val.datetime.typid)
-							{
-								case DATEOID:
-									tmp = DatumGetCString(DirectFunctionCall1(date_out,
-																			  jb->val.datetime.value));
-									break;
-								case TIMEOID:
-									tmp = DatumGetCString(DirectFunctionCall1(time_out,
-																			  jb->val.datetime.value));
-									break;
-								case TIMETZOID:
-									tmp = DatumGetCString(DirectFunctionCall1(timetz_out,
-																			  jb->val.datetime.value));
-									break;
-								case TIMESTAMPOID:
-									tmp = DatumGetCString(DirectFunctionCall1(timestamp_out,
-																			  jb->val.datetime.value));
-									break;
-								case TIMESTAMPTZOID:
-									tmp = DatumGetCString(DirectFunctionCall1(timestamptz_out,
-																			  jb->val.datetime.value));
-									break;
-								default:
-									elog(ERROR, "unrecognized SQL/JSON datetime type oid: %u",
-										 jb->val.datetime.typid);
-							}
+							char		buf[MAXDATELEN + 1];
+
+							JsonEncodeDateTime(buf,
+											   jb->val.datetime.value,
+											   jb->val.datetime.typid,
+											   &jb->val.datetime.tz);
+							tmp = pstrdup(buf);
 						}
 						break;
 					case jbvNull:
diff --git a/src/test/regress/expected/jsonb_jsonpath.out b/src/test/regress/expected/jsonb_jsonpath.out
index 70eeb655a2..acdf7e436f 100644
--- a/src/test/regress/expected/jsonb_jsonpath.out
+++ b/src/test/regress/expected/jsonb_jsonpath.out
@@ -2652,30 +2652,30 @@ select jsonb_path_query('"2023-08-15 12:34:56 +5:30"', '$.timestamp().string()')
 ERROR:  cannot convert value from timestamptz to timestamp without time zone usage
 HINT:  Use *_tz() function for time zone support.
 select jsonb_path_query_tz('"2023-08-15 12:34:56 +5:30"', '$.timestamp().string()'); -- should work
-    jsonb_path_query_tz     
-----------------------------
- "Tue Aug 15 00:04:56 2023"
+  jsonb_path_query_tz  
+-----------------------
+ "2023-08-15T00:04:56"
 (1 row)
 
 select jsonb_path_query('"2023-08-15 12:34:56"', '$.timestamp_tz().string()');
 ERROR:  cannot convert value from timestamp to timestamptz without time zone usage
 HINT:  Use *_tz() function for time zone support.
 select jsonb_path_query_tz('"2023-08-15 12:34:56"', '$.timestamp_tz().string()'); -- should work
-      jsonb_path_query_tz       
---------------------------------
- "Tue Aug 15 12:34:56 2023 PDT"
+     jsonb_path_query_tz     
+-----------------------------
+ "2023-08-15T12:34:56-07:00"
 (1 row)
 
 select jsonb_path_query('"2023-08-15 12:34:56 +5:30"', '$.timestamp_tz().string()');
-        jsonb_path_query        
---------------------------------
- "Tue Aug 15 00:04:56 2023 PDT"
+      jsonb_path_query       
+-----------------------------
+ "2023-08-15T12:34:56+05:30"
 (1 row)
 
 select jsonb_path_query('"2023-08-15 12:34:56"', '$.timestamp().string()');
-      jsonb_path_query      
-----------------------------
- "Tue Aug 15 12:34:56 2023"
+   jsonb_path_query    
+-----------------------
+ "2023-08-15T12:34:56"
 (1 row)
 
 select jsonb_path_query('"12:34:56 +5:30"', '$.time_tz().string()');
@@ -2687,7 +2687,7 @@ select jsonb_path_query('"12:34:56 +5:30"', '$.time_tz().string()');
 select jsonb_path_query_tz('"12:34:56"', '$.time_tz().string()');
  jsonb_path_query_tz 
 ---------------------
- "12:34:56-07"
+ "12:34:56-07:00"
 (1 row)
 
 select jsonb_path_query('"12:34:56"', '$.time().string()');
@@ -2699,53 +2699,26 @@ select jsonb_path_query('"12:34:56"', '$.time().string()');
 select jsonb_path_query('"2023-08-15"', '$.date().string()');
  jsonb_path_query 
 ------------------
- "08-15-2023"
-(1 row)
-
-set datestyle = 'ISO';
-select jsonb_path_query_tz('"2023-08-15 12:34:56"', '$.timestamp_tz().string()');
-   jsonb_path_query_tz    
---------------------------
- "2023-08-15 12:34:56-07"
+ "2023-08-15"
 (1 row)
 
+-- .string() does not react to timezone or datestyle
+begin;
+set local timezone = 'UTC';
+set local datestyle = 'German';
 select jsonb_path_query('"2023-08-15 12:34:56 +5:30"', '$.timestamp_tz().string()');
-     jsonb_path_query     
---------------------------
- "2023-08-15 00:04:56-07"
+      jsonb_path_query       
+-----------------------------
+ "2023-08-15T12:34:56+05:30"
 (1 row)
 
 select jsonb_path_query('"2023-08-15 12:34:56"', '$.timestamp().string()');
    jsonb_path_query    
 -----------------------
- "2023-08-15 12:34:56"
-(1 row)
-
-select jsonb_path_query('"12:34:56 +5:30"', '$.time_tz().string()');
- jsonb_path_query 
-------------------
- "12:34:56+05:30"
-(1 row)
-
-select jsonb_path_query_tz('"12:34:56"', '$.time_tz().string()');
- jsonb_path_query_tz 
----------------------
- "12:34:56-07"
-(1 row)
-
-select jsonb_path_query('"12:34:56"', '$.time().string()');
- jsonb_path_query 
-------------------
- "12:34:56"
-(1 row)
-
-select jsonb_path_query('"2023-08-15"', '$.date().string()');
- jsonb_path_query 
-------------------
- "2023-08-15"
+ "2023-08-15T12:34:56"
 (1 row)
 
-reset datestyle;
+rollback;
 -- Test .time()
 select jsonb_path_query('null', '$.time()');
 ERROR:  jsonpath item method .time() can only be applied to a string
diff --git a/src/test/regress/sql/jsonb_jsonpath.sql b/src/test/regress/sql/jsonb_jsonpath.sql
index 4d57e13eda..da3f7969ca 100644
--- a/src/test/regress/sql/jsonb_jsonpath.sql
+++ b/src/test/regress/sql/jsonb_jsonpath.sql
@@ -611,15 +611,13 @@ select jsonb_path_query_tz('"12:34:56"', '$.time_tz().string()');
 select jsonb_path_query('"12:34:56"', '$.time().string()');
 select jsonb_path_query('"2023-08-15"', '$.date().string()');
 
-set datestyle = 'ISO';
-select jsonb_path_query_tz('"2023-08-15 12:34:56"', '$.timestamp_tz().string()');
+-- .string() does not react to timezone or datestyle
+begin;
+set local timezone = 'UTC';
+set local datestyle = 'German';
 select jsonb_path_query('"2023-08-15 12:34:56 +5:30"', '$.timestamp_tz().string()');
 select jsonb_path_query('"2023-08-15 12:34:56"', '$.timestamp().string()');
-select jsonb_path_query('"12:34:56 +5:30"', '$.time_tz().string()');
-select jsonb_path_query_tz('"12:34:56"', '$.time_tz().string()');
-select jsonb_path_query('"12:34:56"', '$.time().string()');
-select jsonb_path_query('"2023-08-15"', '$.date().string()');
-reset datestyle;
+rollback;
 
 -- Test .time()
 select jsonb_path_query('null', '$.time()');

---

From: "David E. Wheeler" <[email-masked]>
Date: Wed, 11 Sep 2024 15:20:50 -0400

> the point is just to verify that datestyle *doesn't* affect it.

Looks good. Although…

Should it use the database-native stringification standard or the jsonpath stringification standard? In the case of the former, output should omit the “T” time separator and simplify the time zone `07:00` to `07`. But if it’s the latter case, then it’s good as is.

Best,

David

---

From: Tom Lane <[email-masked]>
Date: Wed, 11 Sep 2024 15:43:05 -0400

> Should it use the database-native stringification standard or the jsonpath stringification standard? In the case of the former, output should omit the “T” time separator and simplify the time zone…

Seems to me it should be the jsonpath convention.  If the spec
does require any specific spelling, surely it must be that one.

			regards, tom lane

---

From: "David E. Wheeler" <[email-masked]>
Date: Wed, 11 Sep 2024 15:52:26 -0400

> does require any specific spelling, surely it must be that one.

WFM, though now I’ll have to go change my port 😂.

D

---

From: "David E. Wheeler" <[email-masked]>
Date: Mon, 16 Sep 2024 13:25:17 -0400

> WFM, though now I’ll have to go change my port 😂.

I saw this was committed in cb599b9. Thank you!

BTW, will the back-patch to 17 (cc4fdfa) be included in 17.0 or 17.1?

Best,

David

---

From: Tom Lane <[email-masked]>
Date: Mon, 16 Sep 2024 13:29:55 -0400

> BTW, will the back-patch to 17 (cc4fdfa) be included in 17.0 or 17.1?

17.0.  If we were already past 17.0 I'd have a lot more angst
about changing this behavior.

			regards, tom lane

---

From: "David E. Wheeler" <[email-masked]>
Date: Mon, 16 Sep 2024 14:43:12 -0400

> about changing this behavior.

Great, very glad it made it in.

D