Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

Fix comment in libpqrcv_check_conninfo().
- 629520be5f9d 18.0 cited
Remove useless extern keywords
- 3fb59e789dd9 18.0 cited

jsonpath: Inconsistency of timestamp_tz() Output

David E. Wheeler <david@justatheory.com> — 2024-07-01T15:02:29Z

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]: 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

Re: jsonpath: Inconsistency of timestamp_tz() Output

David E. Wheeler <david@justatheory.com> — 2024-07-02T14:53:24Z

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

Re: jsonpath: Inconsistency of timestamp_tz() Output

David E. Wheeler <david@justatheory.com> — 2024-07-08T17:34:45Z

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] 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], [3].

Best,

David


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

Re: jsonpath: Inconsistency of timestamp_tz() Output

Junwang Zhao <zhjwpku@gmail.com> — 2024-07-09T01:44:00Z

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

Re: jsonpath: Inconsistency of timestamp_tz() Output

David E. Wheeler <david@justatheory.com> — 2024-07-09T14:07:48Z


> 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

Re: jsonpath: Inconsistency of timestamp_tz() Output

David E. Wheeler <david@justatheory.com> — 2024-07-09T14:22:03Z

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

Re: jsonpath: Inconsistency of timestamp_tz() Output

Junwang Zhao <zhjwpku@gmail.com> — 2024-07-09T15:08:52Z

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

Re: jsonpath: Inconsistency of timestamp_tz() Output

David E. Wheeler <david@justatheory.com> — 2024-07-09T15:38:02Z

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.

Best,

David


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

Re: jsonpath: Inconsistency of timestamp_tz() Output

Junwang Zhao <zhjwpku@gmail.com> — 2024-07-10T05:48:17Z

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

Re: jsonpath: Inconsistency of timestamp_tz() Output

David E. Wheeler <david@justatheory.com> — 2024-07-10T14:33:05Z

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

Re: jsonpath: Inconsistency of timestamp_tz() Output

David E. Wheeler <david@justatheory.com> — 2024-07-10T14:35:17Z

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

Re: jsonpath: Inconsistency of timestamp_tz() Output

David E. Wheeler <david@justatheory.com> — 2024-07-10T14:54:27Z

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

Re: jsonpath: Inconsistency of timestamp_tz() Output

David E. Wheeler <david@justatheory.com> — 2024-07-10T15:19:32Z

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

Re: jsonpath: Inconsistency of timestamp_tz() Output

David E. Wheeler <david@justatheory.com> — 2024-07-19T14:05:25Z

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

Re: jsonpath: Inconsistency of timestamp_tz() Output

Jeevan Chalke <jeevan.chalke@enterprisedb.com> — 2024-07-22T07:12:21Z

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>

Re: jsonpath: Inconsistency of timestamp_tz() Output

David E. Wheeler <david@justatheory.com> — 2024-07-22T17:29:02Z

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

Re: jsonpath: Inconsistency of timestamp_tz() Output

Andrew Dunstan <andrew@dunslane.net> — 2024-07-30T11:59:26Z

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

Re: jsonpath: Inconsistency of timestamp_tz() Output

David E. Wheeler <david@justatheory.com> — 2024-07-30T13:42:14Z

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

> I have pushed this.

Thank you, Andrew!

D