Re: jsonpath: Inconsistency of timestamp_tz() Output - Mailing list pgsql-hackers

From Junwang Zhao
Subject Re: jsonpath: Inconsistency of timestamp_tz() Output
Date
Msg-id CAEG8a3LB530nDGiwV+xTchGtGZ2ZE79WX_oKvFKKTxbDDeZScw@mail.gmail.com
Whole thread Raw
In response to Re: jsonpath: Inconsistency of timestamp_tz() Output  ("David E. Wheeler" <david@justatheory.com>)
Responses Re: jsonpath: Inconsistency of timestamp_tz() Output
List pgsql-hackers
On Tue, Jul 9, 2024 at 11:38 PM David E. Wheeler <david@justatheory.com> wrote:
>
> On Jul 9, 2024, at 11:08, Junwang Zhao <zhjwpku@gmail.com> wrote:
>
> > In JsonbValue.val.datatime, there is a tz field, I think that's where
> > the offset stored, it is 18000 in the first example
> >
> > struct
> > {
> > Datum value;
> > Oid typid;
> > int32 typmod;
> > int tz; /* Numeric time zone, in seconds, for
> >          * TimestampTz data type */
> > } datetime;
>
> Oooh, okay, so it’s a jsonb variant of the type. Interesting. Ah, and it’s assigned here[1]:
>
>   jb->val.datetime.tz = tz;
>
> It seems like JSONB timestamptz values want to display the recorded time zone, so I suspect we need to set it when
theconverting from a non-tz to a local tz setting, something like this: 
>
> ``` patch
> diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
> index d79c929822..f63b3b9330 100644
> --- a/src/backend/utils/adt/jsonpath_exec.c
> +++ b/src/backend/utils/adt/jsonpath_exec.c
> @@ -2707,12 +2707,16 @@ executeDateTimeMethod(JsonPathExecContext *cxt, JsonPathItem *jsp,
>                         break;
>                 case jpiTimestampTz:
>                         {
> +                               struct pg_tm *tm;
>                                 /* Convert result type to timestamp with time zone */
>                                 switch (typid)
>                                 {
>                                         case DATEOID:
>                                                 checkTimezoneIsUsedForCast(cxt->useTz,
>                                                                                                    "date",
"timestamptz");
> +                                               if (timestamp2tm(DatumGetTimestamp(value), NULL, tm, NULL, NULL,
NULL)== 0) { 
> +                                                       tz = DetermineTimeZoneOffset(tm, session_timezone);
> +                                               }
>                                                 value = DirectFunctionCall1(date_timestamptz,
>                                                                                                         value);
>                                                 break;
> @@ -2726,6 +2730,9 @@ executeDateTimeMethod(JsonPathExecContext *cxt, JsonPathItem *jsp,
>                                         case TIMESTAMPOID:
>                                                 checkTimezoneIsUsedForCast(cxt->useTz,
>                                                                                                    "timestamp",
"timestamptz");
> +                                               if (timestamp2tm(DatumGetTimestamp(value), NULL, tm, NULL, NULL,
NULL)== 0) { 
> +                                                       tz = DetermineTimeZoneOffset(tm, session_timezone);
> +                                               }
>                                                 value = DirectFunctionCall1(timestamp_timestamptz,
>                                                                                                         value);
>                                                 break;
> ```
>
> Only, you know, doesn’t crash the server.

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

diff --git a/src/backend/utils/adt/jsonpath_exec.c
b/src/backend/utils/adt/jsonpath_exec.c
index d79c9298227..87a695ef633 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -2708,6 +2708,8 @@ executeDateTimeMethod(JsonPathExecContext *cxt,
JsonPathItem *jsp,
                case jpiTimestampTz:
                        {
                                /* Convert result type to timestamp
with time zone */
+                               struct pg_tm tm;
+                               fsec_t fsec;
                                switch (typid)
                                {
                                        case DATEOID:
@@ -2726,6 +2728,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;

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)

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

>
> Best,
>
> David
>
>
> [1]:
https://github.com/postgres/postgres/blob/629520be5f9da9d0192c7f6c8796bfddb4746760/src/backend/utils/adt/jsonpath_exec.c#L2784
>
>


--
Regards
Junwang Zhao



pgsql-hackers by date:

Previous
From: "David G. Johnston"
Date:
Subject: Re: array_in sub function ReadArrayDimensions error message
Next
From: Fujii Masao
Date:
Subject: Re: Add a GUC check hook to ensure summarize_wal cannot be enabled when wal_level is minimal