jsonpath: Inconsistency of timestamp_tz() Output

Started by David E. Wheelerover 1 year ago18 messages
#1David E. Wheeler
david@justatheory.com

Hackers,

There’s an odd difference in the behavior of timestamp_tz() outputs. Running with America/New_York as my TZ, it looks fine for a full timestamptz, identical to how casting the types directly works:

david=# set time zone 'America/New_York';
SET

david=# select '2024-08-15 12:34:56-04'::timestamptz;
timestamptz
------------------------
2024-08-15 12:34:56-04
(1 row)

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

Both show the time in America/New_York, which is great. But when casting from a date, the behavior differs. Casting directly:

david=# select '2024-08-15'::date::timestamptz;
timestamptz
------------------------
2024-08-15 00:00:00-04

It stringifies to the current zone setting again, as expected. But look at the output from a path query:

david=# select jsonb_path_query_tz('"2023-08-15"', '$.timestamp_tz()');
jsonb_path_query_tz
-----------------------------
"2023-08-15T04:00:00+00:00"

It’s using UTC for the display output! Shouldn’t it be using America/New_York?

Note that I’m comparing a cast from date to timestamptz because that’s how the jsonpath parsing works[1]https://github.com/postgres/postgres/blob/3497c87/src/backend/utils/adt/jsonpath_exec.c#L2708-L2718: it ultimately uses date2timestamptz_opt_overflow()[2]https://github.com/postgres/postgres/blob/3497c87/src/backend/utils/adt/date.c#L613-L698 to make the conversion, which appears to set the offset from the time zone GUC, so I’m not sure where it’s converted to UTC before stringifying.

Maybe an argument is missing from the stringification path?

FWIW, explicitly calling the string() jsonpath method does produce a result in the current TZ:

david=# select jsonb_path_query_tz('"2023-08-15"', '$.timestamp_tz().string()');
jsonb_path_query_tz
--------------------------
"2023-08-15 00:00:00-04"

That bit uses timestamptz_out to format the output, but JSONB has its own stringification[4]https://github.com/postgres/postgres/blob/3fb59e789dd9f21610101d1ec106ad58095e24f3/src/backend/utils/adt/json.c#L369-L407 (called here[5]https://github.com/postgres/postgres/blob/3fb59e7/src/backend/utils/adt/jsonb.c#L743-L748), but I can’t tell what might be different between a timestamptz cast from a date and one not cast from a date.

Note the same divergency in behavior occurs when the source value is a timestamp, too. Compare:

david=# select '2024-08-15 12:34:56'::timestamp::timestamptz;
timestamptz
------------------------
2024-08-15 12:34:56-04
(1 row)

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

Anyway, should the output of timestamptz JSONB values be made more consistent? I’m happy to make a patch to do so, but could use a hand figuring out where the behavior varies.

Best,

David

[1]: https://github.com/postgres/postgres/blob/3497c87/src/backend/utils/adt/jsonpath_exec.c#L2708-L2718
[2]: https://github.com/postgres/postgres/blob/3497c87/src/backend/utils/adt/date.c#L613-L698
[3]: https://github.com/postgres/postgres/blob/3fb59e789dd9f21610101d1ec106ad58095e24f3/src/backend/utils/adt/jsonpath_exec.c#L1650-L1653
[4]: https://github.com/postgres/postgres/blob/3fb59e789dd9f21610101d1ec106ad58095e24f3/src/backend/utils/adt/json.c#L369-L407
[5]: https://github.com/postgres/postgres/blob/3fb59e7/src/backend/utils/adt/jsonb.c#L743-L748

#2David E. Wheeler
david@justatheory.com
In reply to: David E. Wheeler (#1)
Re: jsonpath: Inconsistency of timestamp_tz() Output

On Jul 1, 2024, at 11:02, David E. Wheeler <david@justatheory.com> wrote:

Anyway, should the output of timestamptz JSONB values be made more consistent? I’m happy to make a patch to do so, but could use a hand figuring out where the behavior varies.

I think if the formatting was more consistent, the test output would be:

``` patch
--- 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-15T00:00:00-07:00"
 (1 row)
 select jsonb_path_query('"12:34:56"', '$.timestamp_tz()');
@@ -3003,7 +3003,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-15T12:34:56+00:00"
+ "2023-08-15T12:34:56+10:00"
 (1 row)

select jsonb_path_query('"10-03-2017 12:34"', '$.datetime("dd-mm-yyyy HH24:MI")');
```

That second example is a bit different than I noticed up-thread, not just a formatting issue but the offset is never applied!. That test run under tz +10, and this is how it works with the non-JSONB data types:

david=# set time zone '+10';
SET
Time: 0.689 ms
david=# select '2023-08-15 12:34:56'::timestamptz;
timestamptz
------------------------
2023-08-15 12:34:56+10
(1 row)

Time: 0.491 ms
david=# select '2023-08-15 12:34:56'::timestamp::timestamptz;
timestamptz
------------------------
2023-08-15 12:34:56+10
(1 row)

Best,

David

#3David E. Wheeler
david@justatheory.com
In reply to: David E. Wheeler (#2)
Re: jsonpath: Inconsistency of timestamp_tz() Output

On Jul 2, 2024, at 10:53, David E. Wheeler <david@justatheory.com> wrote:

``` patch
--- 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-15T00:00:00-07:00"
(1 row)
select jsonb_path_query('"12:34:56"', '$.timestamp_tz()');
@@ -3003,7 +3003,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-15T12:34:56+00:00"
+ "2023-08-15T12:34:56+10:00"
(1 row)

select jsonb_path_query('"10-03-2017 12:34"', '$.datetime("dd-mm-yyyy HH24:MI")');
```

FWIW I fixed this issue in my jsonpath port, which I released over the weekend.[1]https://justatheory.com/2024/07/go-sqljson-path/ You can see what I think should be the proper output for these two examples in these Playground links, where the response will use your browser’s time zone: [2]https://theory.github.io/sqljson/playground/?p=%2524.timestamp_tz%28%29&amp;j=%25222023-08-15%2522&amp;a=&amp;o=49, [3]https://theory.github.io/sqljson/playground/?p=%2524.timestamp_tz%28%29&amp;j=%25222023-08-15%252012%253A34%253A56%2522&amp;a=&amp;o=49.

Best,

David

[1]: https://justatheory.com/2024/07/go-sqljson-path/
[2]: https://theory.github.io/sqljson/playground/?p=%2524.timestamp_tz%28%29&amp;j=%25222023-08-15%2522&amp;a=&amp;o=49
[3]: https://theory.github.io/sqljson/playground/?p=%2524.timestamp_tz%28%29&amp;j=%25222023-08-15%252012%253A34%253A56%2522&amp;a=&amp;o=49

#4Junwang Zhao
zhjwpku@gmail.com
In reply to: David E. Wheeler (#1)
Re: jsonpath: Inconsistency of timestamp_tz() Output

On Mon, Jul 1, 2024 at 11:02 PM David E. Wheeler <david@justatheory.com> wrote:

Hackers,

There’s an odd difference in the behavior of timestamp_tz() outputs. Running with America/New_York as my TZ, it looks fine for a full timestamptz, identical to how casting the types directly works:

david=# set time zone 'America/New_York';
SET

david=# select '2024-08-15 12:34:56-04'::timestamptz;
timestamptz
------------------------
2024-08-15 12:34:56-04
(1 row)

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

# select jsonb_path_query_tz('"2024-08-15 12:34:56-05"', '$.timestamp_tz()');

Do you also expect this to show the time in America/New_York?

This is what I get:

[local] postgres@postgres:5432-28176=# select
jsonb_path_query_tz('"2024-08-15 12:34:56-05"', '$.timestamp_tz()');
┌─────────────────────────────┐
│ jsonb_path_query_tz │
├─────────────────────────────┤
│ "2024-08-15T12:34:56-05:00" │
└─────────────────────────────┘
(1 row)

The logic in executeDateTimeMethod seems to convert the input to a UTC
timestamp base on the session TZ,
the output seems not cast based on the TZ.

Both show the time in America/New_York, which is great. But when casting from a date, the behavior differs. Casting directly:

david=# select '2024-08-15'::date::timestamptz;
timestamptz
------------------------
2024-08-15 00:00:00-04

It stringifies to the current zone setting again, as expected. But look at the output from a path query:

david=# select jsonb_path_query_tz('"2023-08-15"', '$.timestamp_tz()');
jsonb_path_query_tz
-----------------------------
"2023-08-15T04:00:00+00:00"

It’s using UTC for the display output! Shouldn’t it be using America/New_York?

Note that I’m comparing a cast from date to timestamptz because that’s how the jsonpath parsing works[1]: it ultimately uses date2timestamptz_opt_overflow()[2] to make the conversion, which appears to set the offset from the time zone GUC, so I’m not sure where it’s converted to UTC before stringifying.

Maybe an argument is missing from the stringification path?

FWIW, explicitly calling the string() jsonpath method does produce a result in the current TZ:

david=# select jsonb_path_query_tz('"2023-08-15"', '$.timestamp_tz().string()');
jsonb_path_query_tz
--------------------------
"2023-08-15 00:00:00-04"

That bit uses timestamptz_out to format the output, but JSONB has its own stringification[4] (called here[5]), but I can’t tell what might be different between a timestamptz cast from a date and one not cast from a date.

Note the same divergency in behavior occurs when the source value is a timestamp, too. Compare:

david=# select '2024-08-15 12:34:56'::timestamp::timestamptz;
timestamptz
------------------------
2024-08-15 12:34:56-04
(1 row)

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

Anyway, should the output of timestamptz JSONB values be made more consistent? I’m happy to make a patch to do so, but could use a hand figuring out where the behavior varies.

Best,

David

[1]: https://github.com/postgres/postgres/blob/3497c87/src/backend/utils/adt/jsonpath_exec.c#L2708-L2718
[2]: https://github.com/postgres/postgres/blob/3497c87/src/backend/utils/adt/date.c#L613-L698
[3]: https://github.com/postgres/postgres/blob/3fb59e789dd9f21610101d1ec106ad58095e24f3/src/backend/utils/adt/jsonpath_exec.c#L1650-L1653
[4]: https://github.com/postgres/postgres/blob/3fb59e789dd9f21610101d1ec106ad58095e24f3/src/backend/utils/adt/json.c#L369-L407
[5]: https://github.com/postgres/postgres/blob/3fb59e7/src/backend/utils/adt/jsonb.c#L743-L748

--
Regards
Junwang Zhao

#5David E. Wheeler
david@justatheory.com
In reply to: Junwang Zhao (#4)
Re: jsonpath: Inconsistency of timestamp_tz() Output

On Jul 8, 2024, at 21:44, Junwang Zhao <zhjwpku@gmail.com> wrote:

# select jsonb_path_query_tz('"2024-08-15 12:34:56-05"', '$.timestamp_tz()');

Do you also expect this to show the time in America/New_York?

This is what I get:

[local] postgres@postgres:5432-28176=# select
jsonb_path_query_tz('"2024-08-15 12:34:56-05"', '$.timestamp_tz()');
┌─────────────────────────────┐
│ jsonb_path_query_tz │
├─────────────────────────────┤
│ "2024-08-15T12:34:56-05:00" │
└─────────────────────────────┘
(1 row)

The logic in executeDateTimeMethod seems to convert the input to a UTC
timestamp base on the session TZ,
the output seems not cast based on the TZ.

Right, which now that I think about it seems odd, because timestamptz does not actually store an offset. As you say, it converts the time to UTC and stores only that, then displays the offset relative to the current time zone setting.

So in plain SQL it always displays relative to the current TZ offset:

david=# set time zone 'America/New_York';
SET
david=# select '2023-08-15 12:34:56-05'::timestamptz;
timestamptz
------------------------
2023-08-15 13:34:56-04
(1 row)

david=# select '2023-08-15 12:34:56'::timestamptz;
timestamptz
------------------------
2023-08-15 12:34:56-04
(1 row)

In jsopath expressions, however, it does not, as your example demonstrates:

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

How is it retaining the offset? Should it?

The display is properly adjusted when using string():

david=# select jsonb_path_query_tz('"2024-08-15 12:34:56-05"', '$.timestamp_tz().string()');
jsonb_path_query_tz
--------------------------
"2024-08-15 13:34:56-04"
(1 row)

So perhaps I had things reversed before. Maybe it’s actually doing the right then when it converts a timestamp to a timestamptz, but not when it the input contains an offset, as in your example.

D

#6David E. Wheeler
david@justatheory.com
In reply to: David E. Wheeler (#5)
Re: jsonpath: Inconsistency of timestamp_tz() Output

On Jul 9, 2024, at 10:07, David E. Wheeler <david@justatheory.com> wrote:

So perhaps I had things reversed before. Maybe it’s actually doing the right then when it converts a timestamp to a timestamptz, but not when it the input contains an offset, as in your example.

To clarify, there’s an inconsistency in the output of timestamp_tz() depending on whether the input has an offset or not. With offset:

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

And without:

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"

I suspect the latter is correct, given that the timestamptz type appears to be an int64, presumably always in UTC. I don’t understand where the first example stores the offset.

Best,

David

#7Junwang Zhao
zhjwpku@gmail.com
In reply to: David E. Wheeler (#6)
Re: jsonpath: Inconsistency of timestamp_tz() Output

On Tue, Jul 9, 2024 at 10:22 PM David E. Wheeler <david@justatheory.com> wrote:

On Jul 9, 2024, at 10:07, David E. Wheeler <david@justatheory.com> wrote:

So perhaps I had things reversed before. Maybe it’s actually doing the right then when it converts a timestamp to a timestamptz, but not when it the input contains an offset, as in your example.

To clarify, there’s an inconsistency in the output of timestamp_tz() depending on whether the input has an offset or not. With offset:

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

And without:

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"

I suspect the latter is correct, given that the timestamptz type appears to be an int64, presumably always in UTC. I don’t understand where the first example stores the offset.

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;

Best,

David

--
Regards
Junwang Zhao

#8David E. Wheeler
david@justatheory.com
In reply to: Junwang Zhao (#7)
Re: jsonpath: Inconsistency of timestamp_tz() Output

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]https://github.com/postgres/postgres/blob/629520be5f9da9d0192c7f6c8796bfddb4746760/src/backend/utils/adt/jsonpath_exec.c#L2784:

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 the converting 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.

Best,

David

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

#9Junwang Zhao
zhjwpku@gmail.com
In reply to: David E. Wheeler (#8)
Re: jsonpath: Inconsistency of timestamp_tz() Output

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 the converting 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

#10David E. Wheeler
david@justatheory.com
In reply to: Junwang Zhao (#9)
1 attachment(s)
Re: jsonpath: Inconsistency of timestamp_tz() Output

On Jul 10, 2024, at 01:48, Junwang Zhao <zhjwpku@gmail.com> 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:

v1-0001-Preserve-tz-when-converting-to-jsonb-timestamptz.patchapplication/octet-stream; name=v1-0001-Preserve-tz-when-converting-to-jsonb-timestamptz.patch; x-unix-mode=0644Download
From 31fea30552dc4e6a05c969adf61cdb010cfb9fbe Mon Sep 17 00:00:00 2001
From: "David E. Wheeler" <david@justatheory.com>
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

#11David E. Wheeler
david@justatheory.com
In reply to: David E. Wheeler (#10)
Re: jsonpath: Inconsistency of timestamp_tz() Output

On Jul 10, 2024, at 10:33, David E. Wheeler <david@justatheory.com> wrote:

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.

Here’s a September CommitFest item, though I think it should be fixed before the next beta.

https://commitfest.postgresql.org/49/5119/

Best,

David

#12David E. Wheeler
david@justatheory.com
In reply to: David E. Wheeler (#10)
Re: jsonpath: Inconsistency of timestamp_tz() Output

On Jul 10, 2024, at 10:33, David E. Wheeler <david@justatheory.com> wrote:

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

Actually I think there’s a subtlety still missing here:

@@ -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)

This test runs while the time zone is set to “PST8PDT”, but it’s got the PST offset when it should be PDT:

david=# set time zone 'PST8PDT';
SET
david=# select '2023-08-15'::timestamptz;
timestamptz
------------------------
2023-08-15 00:00:00-07

So it should be -7, not -8. Not sure where to tell it to pay proper attention to daylight savings time.

Best,

David

#13David E. Wheeler
david@justatheory.com
In reply to: David E. Wheeler (#12)
1 attachment(s)
Re: jsonpath: Inconsistency of timestamp_tz() Output

On Jul 10, 2024, at 10:54, David E. Wheeler <david@justatheory.com> wrote:

So it should be -7, not -8. Not sure where to tell it to pay proper attention to daylight savings time.

Oh, and the time and date were wrong, too, because I blindly used the same conversion for dates as for timestamps. Fixed in v2.

PR: https://github.com/theory/postgres/pull/7
CF: https://commitfest.postgresql.org/49/5119/

Best,

David

Attachments:

v2-0001-Preserve-tz-when-converting-to-jsonb-timestamptz.patchapplication/octet-stream; name=v2-0001-Preserve-tz-when-converting-to-jsonb-timestamptz.patch; x-unix-mode=0644Download
From 6188d9b42df1d12c728cb09cbcda206475d36732 Mon Sep 17 00:00:00 2001
From: "David E. Wheeler" <david@justatheory.com>
Date: Wed, 10 Jul 2024 11:17:54 -0400
Subject: [PATCH v2] 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        | 12 ++++++++++++
 src/test/regress/expected/jsonb_jsonpath.out |  4 ++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index d79c929822..ac174bbaa6 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -2707,12 +2707,21 @@ 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");
+						DateADT dateVal = DatumGetDateADT(value);
+						j2date(dateVal + POSTGRES_EPOCH_JDATE,
+							&(tm.tm_year), &(tm.tm_mon), &(tm.tm_mday));
+						tm.tm_hour = 0;
+						tm.tm_min = 0;
+						tm.tm_sec = 0;
+						tz = DetermineTimeZoneOffset(&tm, session_timezone);
 						value = DirectFunctionCall1(date_timestamptz,
 													value);
 						break;
@@ -2726,6 +2735,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..90e705ff14 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-15T00:00:00-07: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

#14David E. Wheeler
david@justatheory.com
In reply to: David E. Wheeler (#13)
1 attachment(s)
Re: jsonpath: Inconsistency of timestamp_tz() Output

On Jul 10, 2024, at 11:19, David E. Wheeler <david@justatheory.com> wrote:

Oh, and the time and date were wrong, too, because I blindly used the same conversion for dates as for timestamps. Fixed in v2.

PR: https://github.com/theory/postgres/pull/7
CF: https://commitfest.postgresql.org/49/5119/

Rebase on 5784a49. No other changes. I would consider this a bug in features added for 17.

Best,

David

Attachments:

v3-0001-Preserve-tz-when-converting-to-jsonb-timestamptz.patchapplication/octet-stream; name=v3-0001-Preserve-tz-when-converting-to-jsonb-timestamptz.patch; x-unix-mode=0644Download
From 7f037618a85a1ac5363ba6485d0f51bb4c22137b Mon Sep 17 00:00:00 2001
From: "David E. Wheeler" <david@justatheory.com>
Date: Wed, 10 Jul 2024 11:17:54 -0400
Subject: [PATCH v3] 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        | 12 ++++++++++++
 src/test/regress/expected/jsonb_jsonpath.out |  4 ++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index d79c929822..ac174bbaa6 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -2707,12 +2707,21 @@ 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");
+						DateADT dateVal = DatumGetDateADT(value);
+						j2date(dateVal + POSTGRES_EPOCH_JDATE,
+							&(tm.tm_year), &(tm.tm_mon), &(tm.tm_mday));
+						tm.tm_hour = 0;
+						tm.tm_min = 0;
+						tm.tm_sec = 0;
+						tz = DetermineTimeZoneOffset(&tm, session_timezone);
 						value = DirectFunctionCall1(date_timestamptz,
 													value);
 						break;
@@ -2726,6 +2735,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 7bb4eb1bc2..02abaac689 100644
--- a/src/test/regress/expected/jsonb_jsonpath.out
+++ b/src/test/regress/expected/jsonb_jsonpath.out
@@ -2964,7 +2964,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-15T00:00:00-07:00"
 (1 row)
 
 select jsonb_path_query('"12:34:56"', '$.timestamp_tz()');
@@ -3151,7 +3151,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

#15Jeevan Chalke
jeevan.chalke@enterprisedb.com
In reply to: David E. Wheeler (#14)
1 attachment(s)
Re: jsonpath: Inconsistency of timestamp_tz() Output

On Fri, Jul 19, 2024 at 7:35 PM David E. Wheeler <david@justatheory.com>
wrote:

On Jul 10, 2024, at 11:19, David E. Wheeler <david@justatheory.com> wrote:

Oh, and the time and date were wrong, too, because I blindly used the

same conversion for dates as for timestamps. Fixed in v2.

PR: https://github.com/theory/postgres/pull/7
CF: https://commitfest.postgresql.org/49/5119/

Rebase on 5784a49. No other changes. I would consider this a bug in
features added for 17.

I agree with David that we need to set the tz explicitly as the JsonbValue
struct maintains that separately.

However, in the attached version, I have added some comments and also,
fixed some indentation.

Thanks

Best,

David

--

*Jeevan Chalke*

*Principal, ManagerProduct Development*

enterprisedb.com <https://www.enterprisedb.com&gt;

Attachments:

v4-0001-Preserve-tz-when-converting-to-jsonb-timestamptz.patchapplication/octet-stream; name=v4-0001-Preserve-tz-when-converting-to-jsonb-timestamptz.patchDownload
From 7bb9b78dc5dbb57d4fd932a3e02953a702d01ffb Mon Sep 17 00:00:00 2001
From: Jeevan Chalke <jeevan.chalke@enterprisedb.com>
Date: Mon, 22 Jul 2024 11:33:20 +0530
Subject: [PATCH v4] 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 `TIMESTAMPOID` 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 and Jeevan Chalke
Discussion: https://postgr.es/m/7DE080CE-6D8C-4794-9BD1-7D9699172FAB%40justatheory.com
---
 src/backend/utils/adt/jsonpath_exec.c        | 25 +++++++++++++++++++++++++
 src/test/regress/expected/jsonb_jsonpath.out |  4 ++--
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index d79c929..c47221b 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -2707,12 +2707,27 @@ 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");
+
+						/*
+						 * Get the timezone value explicitly since JsonbValue
+						 * keeps that separate.
+						 */
+						j2date(DatumGetDateADT(value) + POSTGRES_EPOCH_JDATE,
+							   &(tm.tm_year), &(tm.tm_mon), &(tm.tm_mday));
+						tm.tm_hour = 0;
+						tm.tm_min = 0;
+						tm.tm_sec = 0;
+						tz = DetermineTimeZoneOffset(&tm, session_timezone);
+
 						value = DirectFunctionCall1(date_timestamptz,
 													value);
 						break;
@@ -2726,6 +2741,16 @@ executeDateTimeMethod(JsonPathExecContext *cxt, JsonPathItem *jsp,
 					case TIMESTAMPOID:
 						checkTimezoneIsUsedForCast(cxt->useTz,
 												   "timestamp", "timestamptz");
+
+						/*
+						 * Get the timezone value explicitly since JsonbValue
+						 * keeps that separate.
+						 */
+						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 7bb4eb1..02abaac 100644
--- a/src/test/regress/expected/jsonb_jsonpath.out
+++ b/src/test/regress/expected/jsonb_jsonpath.out
@@ -2964,7 +2964,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-15T00:00:00-07:00"
 (1 row)
 
 select jsonb_path_query('"12:34:56"', '$.timestamp_tz()');
@@ -3151,7 +3151,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()');
-- 
1.8.3.1

#16David E. Wheeler
david@justatheory.com
In reply to: Jeevan Chalke (#15)
1 attachment(s)
Re: jsonpath: Inconsistency of timestamp_tz() Output

On Jul 22, 2024, at 03:12, Jeevan Chalke <jeevan.chalke@enterprisedb.com> wrote:

I agree with David that we need to set the tz explicitly as the JsonbValue struct maintains that separately.

However, in the attached version, I have added some comments and also, fixed some indentation.

Thank you for the review. I changed a single word in your comments (which are welcome). Thank you!

Just to reiterate, this is not an ideal fix, as the `date_timestamptz` and `timestamp_timestamptz` perform the same calculations. It would be nice to do it only once.

Best,

David

Attachments:

v5-0001-Preserve-tz-when-converting-to-jsonb-timestamptz.patchapplication/octet-stream; name=v5-0001-Preserve-tz-when-converting-to-jsonb-timestamptz.patch; x-unix-mode=0644Download
From 11c8c3ca4d9fd6fb38a077d0fd27bddfebf578c1 Mon Sep 17 00:00:00 2001
From: "David E. Wheeler" <david@justatheory.com>
Date: Mon, 22 Jul 2024 13:27:10 -0400
Subject: [PATCH v5] 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 `TIMESTAMPOID` 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 and Jeevan Chalke
Discussion: https://postgr.es/m/7DE080CE-6D8C-4794-9BD1-7D9699172FAB%40justatheory.com
---
 src/backend/utils/adt/jsonpath_exec.c        | 25 ++++++++++++++++++++
 src/test/regress/expected/jsonb_jsonpath.out |  4 ++--
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index d79c929822..4232ec419b 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -2707,12 +2707,27 @@ 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");
+
+						/*
+						 * Get the timezone value explicitly since JsonbValue
+						 * keeps it separate.
+						 */
+						j2date(DatumGetDateADT(value) + POSTGRES_EPOCH_JDATE,
+							   &(tm.tm_year), &(tm.tm_mon), &(tm.tm_mday));
+						tm.tm_hour = 0;
+						tm.tm_min = 0;
+						tm.tm_sec = 0;
+						tz = DetermineTimeZoneOffset(&tm, session_timezone);
+
 						value = DirectFunctionCall1(date_timestamptz,
 													value);
 						break;
@@ -2726,6 +2741,16 @@ executeDateTimeMethod(JsonPathExecContext *cxt, JsonPathItem *jsp,
 					case TIMESTAMPOID:
 						checkTimezoneIsUsedForCast(cxt->useTz,
 												   "timestamp", "timestamptz");
+
+						/*
+						 * Get the timezone value explicitly since JsonbValue
+						 * keeps it separate.
+						 */
+						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 7bb4eb1bc2..02abaac689 100644
--- a/src/test/regress/expected/jsonb_jsonpath.out
+++ b/src/test/regress/expected/jsonb_jsonpath.out
@@ -2964,7 +2964,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-15T00:00:00-07:00"
 (1 row)
 
 select jsonb_path_query('"12:34:56"', '$.timestamp_tz()');
@@ -3151,7 +3151,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

#17Andrew Dunstan
andrew@dunslane.net
In reply to: Jeevan Chalke (#15)
Re: jsonpath: Inconsistency of timestamp_tz() Output

On 2024-07-22 Mo 3:12 AM, Jeevan Chalke wrote:

On Fri, Jul 19, 2024 at 7:35 PM David E. Wheeler
<david@justatheory.com> wrote:

On Jul 10, 2024, at 11:19, David E. Wheeler
<david@justatheory.com> wrote:

Oh, and the time and date were wrong, too, because I blindly

used the same conversion for dates as for timestamps. Fixed in v2.

PR: https://github.com/theory/postgres/pull/7
CF: https://commitfest.postgresql.org/49/5119/

Rebase on 5784a49. No other changes. I would consider this a bug
in features added for 17.

I agree with David that we need to set the tz explicitly as the
JsonbValue struct maintains that separately.

However, in the attached version, I have added some comments and also,
fixed some indentation.

I have pushed this.

cheers

andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

#18David E. Wheeler
david@justatheory.com
In reply to: Andrew Dunstan (#17)
Re: jsonpath: Inconsistency of timestamp_tz() Output

On Jul 30, 2024, at 07:59, Andrew Dunstan <andrew@dunslane.net> wrote:

I have pushed this.

Thank you, Andrew!

D