public inbox for [email protected]  
help / color / mirror / Atom feed
From: David E. Wheeler <[email protected]>
To: Junwang Zhao <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: jsonpath: Inconsistency of timestamp_tz() Output
Date: Wed, 10 Jul 2024 10:33:05 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAEG8a3LB530nDGiwV+xTchGtGZ2ZE79WX_oKvFKKTxbDDeZScw@mail.gmail.com>
References: <[email protected]>
	<CAEG8a3J2rBZ-S2VpwJCsWoLSmdSqWfnd0QH9kO_YE4ZJFSqGXQ@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<CAEG8a3+QHYaLk+g8_LDnpbPMEC-Ht7-mOV2OsvTqqCmbWWJhew@mail.gmail.com>
	<[email protected]>
	<CAEG8a3LB530nDGiwV+xTchGtGZ2ZE79WX_oKvFKKTxbDDeZScw@mail.gmail.com>

On Jul 10, 2024, at 01:48, Junwang Zhao <[email protected]> wrote:

> I apply your patch with some minor change(to make the server not crash):

Oh, thank you! Kicking myself for not catching the obvious.

> It now gives the local tz:
> 
> [local] postgres@postgres:5432-54960=# set time zone 'America/New_York';
> SET
> Time: 2.894 ms
> [local] postgres@postgres:5432-54960=# select
> jsonb_path_query_tz('"2024-08-15 12:34:56"', '$.timestamp_tz()');
> ┌─────────────────────────────┐
> │     jsonb_path_query_tz     │
> ├─────────────────────────────┤
> │ "2024-08-15T12:34:56-04:00" │
> └─────────────────────────────┘
> (1 row)
> 
> Time: 293813.022 ms (04:53.813)

Yes, and I think that’s what we want, since it preserves and displays the offset for strings that contain them:

    david=# set time zone 'America/New_York';
    SET
    david=# select jsonb_path_query_tz('"2024-08-15 12:34:56+10"', '$.timestamp_tz()');
        jsonb_path_query_tz
    -----------------------------
    "2024-08-15T12:34:56+10:00"

> I'm not sure whether the SQL/JSON standard mentioned this, I searched a
> little bit, but found no clue :(

Yeah I don’t know either, but now at least it’s consistent. I’ve attached a patch to fix it.

Ideally, I think, we wouldn’t convert the value and determine the offset twice, but teach date_timestamptz and timestamp_timestamptz (or date2timestamptz and timestamp2timestamptz?) how to return the offset, or create alternate functions that do so. Not sure what calling style should be adopted here, but this at least addresses the issue. Happy to resubmit something more efficient upon function design feedback.

Best,

David







Attachments:

  [application/octet-stream] v1-0001-Preserve-tz-when-converting-to-jsonb-timestamptz.patch (4.0K, ../[email protected]/2-v1-0001-Preserve-tz-when-converting-to-jsonb-timestamptz.patch)
  download | inline diff:
From 31fea30552dc4e6a05c969adf61cdb010cfb9fbe Mon Sep 17 00:00:00 2001
From: "David E. Wheeler" <[email protected]>
Date: Wed, 10 Jul 2024 10:23:24 -0400
Subject: [PATCH v1] Preserve tz when converting to jsonb timestamptz

The JSONB jbvDatetime type has a field for offset, and displays the time
in that offset. For example, when the time zone GUC is set to
America/New_York, the jsonpath `timestamp_tz()` method returns a value
that displays a parsed value with its offset, not the local offset:

    david=# set time zone 'America/New_York';
    SET
    david=# select jsonb_path_query_tz('"2024-08-15 12:34:56+10"', '$.timestamp_tz()');
        jsonb_path_query_tz
    -----------------------------
    "2024-08-15T12:34:56+10:00"

This was not true for values parsed by `timestamp_tz()` that lacked an
offset. It correctly assumes the local time zone, but displays it in
UTC:

    david=# select jsonb_path_query_tz('"2024-08-15 12:34:56"', '$.timestamp_tz()');
        jsonb_path_query_tz
    -----------------------------
    "2024-08-15T16:34:56+00:00"

To fix this inconsistent behavior, determine the offset for values being
cast from `DATEOID` and `DATEOID` types to `jpiTimestampTz` and store it
in the resulting jbvDatetime value. With this change, the result now
preserves the offset just as it does when converting from offset-aware
values:

    david=# select jsonb_path_query_tz('"2024-08-15 12:34:56"', '$.timestamp_tz()');
        jsonb_path_query_tz
    -----------------------------
    "2023-08-15T12:34:56-04:00"

Author: David Wheeler
Reviewed-by: Junwang Zhao
Discussion: https://postgr.es/m/7DE080CE-6D8C-4794-9BD1-7D9699172FAB%40justatheory.com
---
 src/backend/utils/adt/jsonpath_exec.c        | 8 ++++++++
 src/test/regress/expected/jsonb_jsonpath.out | 4 ++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index d79c929822..f7d255746b 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -2707,12 +2707,17 @@ executeDateTimeMethod(JsonPathExecContext *cxt, JsonPathItem *jsp,
 			break;
 		case jpiTimestampTz:
 			{
+				struct pg_tm tm;
+				fsec_t		fsec;
 				/* Convert result type to timestamp with time zone */
 				switch (typid)
 				{
 					case DATEOID:
 						checkTimezoneIsUsedForCast(cxt->useTz,
 												   "date", "timestamptz");
+						if (timestamp2tm(DatumGetTimestamp(value), NULL, &tm, &fsec, NULL, NULL) == 0) {
+							tz = DetermineTimeZoneOffset(&tm, session_timezone);
+						}
 						value = DirectFunctionCall1(date_timestamptz,
 													value);
 						break;
@@ -2726,6 +2731,9 @@ executeDateTimeMethod(JsonPathExecContext *cxt, JsonPathItem *jsp,
 					case TIMESTAMPOID:
 						checkTimezoneIsUsedForCast(cxt->useTz,
 												   "timestamp", "timestamptz");
+						if (timestamp2tm(DatumGetTimestamp(value), NULL, &tm, &fsec, NULL, NULL) == 0) {
+							tz = DetermineTimeZoneOffset(&tm, session_timezone);
+						}
 						value = DirectFunctionCall1(timestamp_timestamptz,
 													value);
 						break;
diff --git a/src/test/regress/expected/jsonb_jsonpath.out b/src/test/regress/expected/jsonb_jsonpath.out
index a6112e86fa..62cc25f159 100644
--- a/src/test/regress/expected/jsonb_jsonpath.out
+++ b/src/test/regress/expected/jsonb_jsonpath.out
@@ -2914,7 +2914,7 @@ HINT:  Use *_tz() function for time zone support.
 select jsonb_path_query_tz('"2023-08-15"', '$.timestamp_tz()'); -- should work
      jsonb_path_query_tz     
 -----------------------------
- "2023-08-15T07:00:00+00:00"
+ "2023-08-14T23:00:00-08:00"
 (1 row)
 
 select jsonb_path_query('"12:34:56"', '$.timestamp_tz()');
@@ -3101,7 +3101,7 @@ HINT:  Use *_tz() function for time zone support.
 select jsonb_path_query_tz('"2023-08-15 12:34:56"', '$.timestamp_tz()'); -- should work
      jsonb_path_query_tz     
 -----------------------------
- "2023-08-15T02:34:56+00:00"
+ "2023-08-15T12:34:56+10:00"
 (1 row)
 
 select jsonb_path_query('"2023-08-15 12:34:56 +05:30"', '$.timestamp_tz()');
-- 
2.45.2



view thread (10+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: jsonpath: Inconsistency of timestamp_tz() Output
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox