Re: Document DateStyle effect on jsonpath string()
On Jul 9, 2024, at 10:45, David E. Wheeler wrote: >> one tiny complaint would be maybe we need `reset datestyle`. > > That’s fair. Done. Here’s a rebase on 5784a49. I also updated the commitfest item[1] to link to a new pull request[2], since I seem to have turned the other one into the tz conversion bug fix. Best, David [1]: https://commitfest.postgresql.org/49/5101/ [2]: https://github.com/theory/postgres/pull/8 v3-0001-Document-impact-of-datestyle-on-jsonpath-string.patch Description: Binary data
Re: Document DateStyle effect on jsonpath string()
On Jul 9, 2024, at 10:35, jian he wrote: > one tiny complaint would be maybe we need `reset datestyle`. That’s fair. Done. D v2-0001-Document-impact-of-datestyle-on-jsonpath-string.patch Description: Binary data
Re: Document DateStyle effect on jsonpath string()
On Thu, Jul 4, 2024 at 10:45 PM David E. Wheeler wrote: > > On Jul 4, 2024, at 04:28, jian he wrote: > > > Do you need to reset the datestyle? > > Wouldn’t hurt but it’s not necessary, no. It’s set only for the execution of > this file, and there are no more calls that rely on it. > > > also the above query is time zone sensitive, maybe the time zone is > > set in another place, but that's not explicit? > > It’s implicit in how PostgreSQL runs its test suite; other tests later change > it. I inserted these two commands into it. show time zone; show datestyle; turns out in the test suite, the default data style is "Postgres, MDY", and the default time zone is "PST8PDT". These two implicit settings weren't mentioned anywhere. your tests look ok to me. one tiny complaint would be maybe we need `reset datestyle`. Because we are in line 600 of src/test/regress/sql/jsonb_jsonpath.sql, We want to make sure the following test is not influenced by guc datestyle. > > > > -String value converted from a JSON boolean, number, string, or > > datetime > > +String value converted from a JSON boolean, number, string, or > > +datetime. Note that the string output of datetimes is determined by > > +the parameter. > > > > imho, your patch has just too many examples. > > I’m confused. There are no examples in my patch, or this bit you cite. > > > for explaining the above sentence, the following example should be enough. > > Are you referring to the tests? I made them comprehensive so that we reliably > demonstrate the behavior of the string() method on all the date/time data > types. They are not examples, not in the documentation sense at least. > I mean tests, sorry for the confusion. added several more tests should be fine. overall looks good to me.
Re: Document DateStyle effect on jsonpath string()
On Jul 4, 2024, at 04:28, jian he wrote: > Do you need to reset the datestyle? Wouldn’t hurt but it’s not necessary, no. It’s set only for the execution of this file, and there are no more calls that rely on it. > also the above query is time zone sensitive, maybe the time zone is > set in another place, but that's not explicit? It’s implicit in how PostgreSQL runs its test suite; other tests later change it. > > -String value converted from a JSON boolean, number, string, or > datetime > +String value converted from a JSON boolean, number, string, or > +datetime. Note that the string output of datetimes is determined by > +the parameter. > > imho, your patch has just too many examples. I’m confused. There are no examples in my patch, or this bit you cite. > for explaining the above sentence, the following example should be enough. Are you referring to the tests? I made them comprehensive so that we reliably demonstrate the behavior of the string() method on all the date/time data types. They are not examples, not in the documentation sense at least. Best, David
Re: Document DateStyle effect on jsonpath string()
On Wed, Jul 3, 2024 at 12:51 AM David E. Wheeler wrote:
>
> Hackers,
>
> In fuzing around trying to work out what’s going on with the formatting of
> timestamptz values cast by the timestamp_tz() jsonpath method[1], I noticed
> that the formatting of the string() method applied to date and time objects
> was not fully tested, or how the output is determined by the DateStyle method.
>
> The attached path aims to rectify this situation by adding tests that chain
> string() after the jsonpath date/time methods, both with the default testing
> “PostreSQL” DateStyle and “ISO”. It also mentions the impact of the DateStyle
> parameter in the string() documentation.
>
> Also available to review as a pull request[2].
>
> Best,
>
> David
>
> [1]:
> https://www.postgresql.org/message-id/7DE080CE-6D8C-4794-9BD1-7D9699172FAB%40justatheory.com
> [2]: https://github.com/theory/postgres/pull/7/files
>
>
+set datestyle = 'ISO';
+select jsonb_path_query_tz('"2023-08-15 12:34:56"',
'$.timestamp_tz().string()'); -- should work
+ jsonb_path_query_tz
+--
+ "2023-08-15 12:34:56-07"
+(1 row)
Do you need to reset the datestyle?
also the above query is time zone sensitive, maybe the time zone is
set in another place, but that's not explicit?
-String value converted from a JSON boolean, number, string, or datetime
+String value converted from a JSON boolean, number, string, or
+datetime. Note that the string output of datetimes is determined by
+the parameter.
imho, your patch has just too many examples.
for explaining the above sentence, the following example should be enough.
begin;
set local time zone +1;
set local datestyle to postgres;
select jsonb_path_query_tz('"2023-08-15 12:34:56"',
'$.timestamp_tz().string()');
set local datestyle to iso;
select jsonb_path_query_tz('"2023-08-15 12:34:56"',
'$.timestamp_tz().string()');
commit;
Document DateStyle effect on jsonpath string()
Hackers, In fuzing around trying to work out what’s going on with the formatting of timestamptz values cast by the timestamp_tz() jsonpath method[1], I noticed that the formatting of the string() method applied to date and time objects was not fully tested, or how the output is determined by the DateStyle method. The attached path aims to rectify this situation by adding tests that chain string() after the jsonpath date/time methods, both with the default testing “PostreSQL” DateStyle and “ISO”. It also mentions the impact of the DateStyle parameter in the string() documentation. Also available to review as a pull request[2]. Best, David [1]: https://www.postgresql.org/message-id/7DE080CE-6D8C-4794-9BD1-7D9699172FAB%40justatheory.com [2]: https://github.com/theory/postgres/pull/7/files v1-0001-Document-impact-of-datestyle-on-jsonpath-string.patch Description: Binary data
