Re: Support for jsonpath .datetime() method - Mailing list pgsql-hackers

From Nikita Glukhov
Subject Re: Support for jsonpath .datetime() method
Date
Msg-id d6efab15-f3a4-40d6-8ddb-6fd8f64cbc08@postgrespro.ru
Whole thread Raw
In response to Re: Support for jsonpath .datetime() method  (Peter Eisentraut <peter.eisentraut@2ndquadrant.com>)
Responses Re: Support for jsonpath .datetime() method  (Alexander Korotkov <a.korotkov@postgrespro.ru>)
Re: Support for jsonpath .datetime() method  (Peter Eisentraut <peter.eisentraut@2ndquadrant.com>)
Re: Support for jsonpath .datetime() method  (Andrew Dunstan <andrew.dunstan@2ndquadrant.com>)
List pgsql-hackers

On 23.07.2019 16:44, Peter Eisentraut wrote:

I think the best way forward here is to focus first on patch 0002 and
get the additional format templates in, independent of any surrounding
JSON functionality.

In particular, remove parse_datetime() and all the related API changes,
then it becomes much simpler.

The codes FF1..FF6 that you added appear to be correct, but reading the
spec I find there is more missing, specifically

- RRRR and RR
It seems that our YY works like RR should:
SELECT to_date('69', 'YY'); to_date   
------------2069-01-01
(1 row)

SELECT to_date('70', 'YY'); to_date   
------------1970-01-01
(1 row)

But by the standard first two digits of current year should be used in YY.


Oracle follows the standard but its implementation has the different 
rounding algorithm:

SELECT TO_CHAR(TO_DATE('99', 'YY'), 'YYYY') from dual;
2099

SELECT TO_CHAR(TO_DATE('49', 'RR'), 'YYYY') from dual;
2049

SELECT TO_CHAR(TO_DATE('50', 'RR'), 'YYYY') from dual;
1950


So it's unclear what we should do: - implement YY and RR strictly following the standard only in .datetime()- fix YY implementation in to_date()/to_timestamp() and implement RR- use our non-standard templates in .datetime()

- SSSSS (currently only SSSS is supported, but that's not standard)
SSSSS template can be easily added as alias to SSSS. 

Also in some cases we allow timestamps with seven digits of fractional
precision, so perhaps FF7 should be supported as well.  I'm not quite
sure about the details here.  You tests only cover 6 and 9 digits.  It
would be good to cover 7 and perhaps 8 as well, since those are the
boundary cases.
FF7-FF9 weer present in earlier versions of the jsonpath patches, but they
had been removed (see [1]) because they were not completely supported due
to the limited precision of timestamp.

Some concrete pieces of review:

+       <row>
+        <entry><literal>FF1</literal></entry>
+        <entry>decisecond (0-9)</entry>
+       </row>

Let's not use such weird terms as "deciseconds".  We could say
"fractional seconds, 1 digit" etc. or something like that.
And what about "tenths of seconds", "hundredths of seconds"?
+/* Return flags for DCH_from_char() */
+#define DCH_DATED  0x01
+#define DCH_TIMED  0x02
+#define DCH_ZONED  0x04

I think you mean do_to_timestamp() here.  These terms "dated" etc. are
from the SQL standard text, but they should be explained somewhere for
the readers of the code.


--
Nikita Glukhov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

pgsql-hackers by date:

Previous
From: Thomas Munro
Date:
Subject: Re: stress test for parallel workers
Next
From: Justin Pryzby
Date:
Subject: Re: stress test for parallel workers