On 2/26/16, Shulgin, Oleksandr <oleksandr.shul...@zalando.de> wrote:
> On Fri, Feb 26, 2016 at 3:24 PM, Ivan Kartyshov
> <i.kartys...@postgrespro.ru>
> wrote:
>
>> The following review has been posted through the commitfest application:
>
>> make installcheck-world: tested, failed
>> Implements feature: tested, failed
>> Spec compliant: tested, failed
>> Documentation: tested, failed
>>
>> Applied this patch, it works well, make what it expected correctly, code
>> style is maintained. Regression tests passed OK. No documentation or
>> comments.
>>
>
> Why does it say "tested, failed" for all points above there? ;-)
I hope Ivan misunderstood that "tested" and "passed" are two different
options, not a single "tested, passed" ;-)
Unfortunately, it seems there should be a discussion about meaning of
"YYYY": it seems authors made it as ISO8601-compliant (but there are
still several bugs), but there is no proofs in the documentation[1].
On the one hand there is only "year" mentioned for "YYYY", "YYY",
etc., and "ISO 8601 ... year" is "week-numbering", i.e. "IYYY", "IYY",
etc. only for using with "IDDD", "ID" and "IW".
On the other hand "extract" has two different options: "year" and
"isoyear" and only the second one is ISO8601-compliant (moreover it is
week-numbering at the same time):
postgres=# SELECT y src, EXTRACT(year FROM y) AS year, EXTRACT(isoyear
FROM y) AS isoyear
postgres-# FROM unnest(ARRAY[
postgres(# '4713-01-01 BC','4714-12-31 BC','4714-12-29 BC','4714-12-28
BC']::date[]) y;
src | year | isoyear
---------------+-------+---------
4713-01-01 BC | -4713 | -4712
4714-12-31 BC | -4714 | -4712
4714-12-29 BC | -4714 | -4712
4714-12-28 BC | -4714 | -4713
(4 rows)
So there is lack of consistency: something should be changed: either
"extract(year..." (and the documentation[2], but there is "Keep in
mind there is no 0 AD, ..." for a long time, so it is a change which
breaks compatibility; and the patch will be completely different), or
the patch (it has an influence on "IYYY", so it is obviously wrong).
Now (after the letter[3] from Thomas Munro) I know the patch is not
ready for committing even with minimal changes. But I'm waiting for a
discussion: what part should be changed?
I would change behavior of "to_date" and "to_timestamp" to match with
extract options "year"/"isoyear", but I think getting decision of the
community before modifying the patch is a better idea.
[1]http://www.postgresql.org/docs/devel/static/functions-formatting.html
[2]http://www.postgresql.org/docs/devel/static/functions-datetime.html
[3]http://www.postgresql.org/message-id/CAEepm=0aznvy+frtyni04imdw4tlgzaelljqmhmcjbre+ln...@mail.gmail.com
--
Best regards,
Vitaly Burovoy
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
