jsonpath: Inconsistency of timestamp_tz() Output

Started by David E. Wheelerover 1 year ago18 messages
Jump to latest
#1David E. Wheeler
david@kineticode.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@kineticode.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@kineticode.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@kineticode.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@kineticode.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@kineticode.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@kineticode.com
In reply to: Junwang Zhao (#9)
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+10-3
#11David E. Wheeler
david@kineticode.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@kineticode.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@kineticode.com
In reply to: David E. Wheeler (#12)
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+14-3
#14David E. Wheeler
david@kineticode.com
In reply to: David E. Wheeler (#13)
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+14-3
#15Jeevan Chalke
jeevan.chalke@enterprisedb.com
In reply to: David E. Wheeler (#14)
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+27-3
#16David E. Wheeler
david@kineticode.com
In reply to: Jeevan Chalke (#15)
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+27-3
#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@kineticode.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