BUG #16419: wrong parsing BC year in to_date() function

On Wed, May 6, 2020 at 2:58 PM PG Bug reporting form <noreply@postgresql.org>
wrote:

The following bug has been logged on the website:

Bug reference: 16419
Logged by: Saeed Hubaishan
Email address: dar_alathar@hotmail.com
PostgreSQL version: 12.2
Operating system: Windows 10x64
Description:

select to_date('-1-01-01','yyyy-mm-dd');
will get
0002-01-01 BC

Yep...

select to_date('1','YYYY')::text; // Year 1 AD
select to_date('0','YYYY')::text; // Year 1 BC (there is no year zero)
select to_date('-1','YYYY')::text; // Year 2 BC

to_date tries very hard to not error - if you need to use it make sure your
data conforms to the format you specify.

David J.

Any one suppose that these functions return the same:
make_date(-1,1,1)
to_date('-1-01-01','yyyy-mm-dd')
But make_date will give 0001-01-01 BC
And to_date will give 0002-01-01 BC

If you think this is right behavior I think this must be documented

من: David G. Johnston <david.g.johnston@gmail.com>
‏‏تم الإرسال: Thursday, May 7, 2020 1:45:14 AM
إلى: dar_alathar@hotmail.com <dar_alathar@hotmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
‏‏الموضوع: Re: BUG #16419: wrong parsing BC year in to_date() function

On Wed, May 6, 2020 at 2:58 PM PG Bug reporting form <noreply@postgresql.org<mailto:noreply@postgresql.org>> wrote:
The following bug has been logged on the website:

Bug reference: 16419
Logged by: Saeed Hubaishan
Email address: dar_alathar@hotmail.com<mailto:dar_alathar@hotmail.com>
PostgreSQL version: 12.2
Operating system: Windows 10x64
Description:

select to_date('-1-01-01','yyyy-mm-dd');
will get
0002-01-01 BC

Yep...

select to_date('1','YYYY')::text; // Year 1 AD
select to_date('0','YYYY')::text; // Year 1 BC (there is no year zero)
select to_date('-1','YYYY')::text; // Year 2 BC

to_date tries very hard to not error - if you need to use it make sure your data conforms to the format you specify.

David J.

من: David G. Johnston<mailto:david.g.johnston@gmail.com>
إرسال: ‏الخميس,‏ ‏14 ‏رمضان,‏ ‏1441 ‏01:45 ‏ص
الموضوع: Re: BUG #16419: wrong parsing BC year in to_date() function

On Wed, May 6, 2020 at 2:58 PM PG Bug reporting form <noreply@postgresql.org<mailto:noreply@postgresql.org>> wrote:
The following bug has been logged on the website:

Bug reference: 16419
Logged by: Saeed Hubaishan
Email address: dar_alathar@hotmail.com<mailto:dar_alathar@hotmail.com>
PostgreSQL version: 12.2
Operating system: Windows 10x64
Description:

select to_date('-1-01-01','yyyy-mm-dd');
will get
0002-01-01 BC

Yep...

select to_date('1','YYYY')::text; // Year 1 AD
select to_date('0','YYYY')::text; // Year 1 BC (there is no year zero)
select to_date('-1','YYYY')::text; // Year 2 BC

to_date tries very hard to not error - if you need to use it make sure your data conforms to the format you specify.

David J.

‪On Wed, May 6, 2020 at 6:31 PM ‫دار الآثار للنشر والتوزيع-صنعاء Dar
Alathar-Yemen‬‎ <dar_alathar@hotmail.com> wrote:‬

Any one suppose that these functions return the same:
make_date(-1,1,1)
to_date('-1-01-01','yyyy-mm-dd')

But make_date will give 0001-01-01 BC

And to_date will give 0002-01-01 BC

Interesting...and a fair point.

What seems to be happening here is that to_date is trying to be helpful by
doing:

select to_date('0000','YYYY'); // 0001-01-01 BC

It does this seemingly by subtracting one from the year, making it
positive, then (I infer) appending "BC" to the result. Thus for the year
"-1" it yields "0002-01-01 BC"

make_date just chooses to reject the year 0 and treat the negative as an
alternative to specifying BC

There seems to be zero tests for to_date involving negative years, and the
documentation doesn't talk of them.

I'll let the -hackers speak up as to how they want to go about handling
to_date (research how it behaves in the other database it tries to emulate
and either document or possibly change the behavior in v14) but do suggest
that a simple explicit description of how to_date works in the presence of
negative years be back-patched. A bullet in the usage notes section
probably suffices:

"If a YYYY format string captures a negative year, or 0000, it will treat
it as a BC year after decreasing the value by one. So 0000 maps to 1 BC
and -1 maps to 2 BC and so on."

So, no, make_date and to_date do not agree on this point; and they do not
have to. There is no way to specify "BC" in make_date function so using
negative there makes sense. You can specify BC in the input string for
to_date and indeed that is the only supported (documented) way to do so.

David J.

On Wed, May 6, 2020 at 8:12 PM David G. Johnston <david.g.johnston@gmail.com>
wrote:

It does this seemingly by subtracting one from the year, making it
positive, then (I infer) appending "BC" to the result. Thus for the year
"-1" it yields "0002-01-01 BC"

Specifically:

https://github.com/postgres/postgres/blob/fb544735f11480a697fcab791c058adc166be1fa/src/backend/utils/adt/formatting.c#L236

/*
* There is no 0 AD. Years go from 1 BC to 1 AD, so we make it
* positive and map year == -1 to year zero, and shift all negative
* years up one. For interval years, we just return the year.
*/
#define ADJUST_YEAR(year, is_interval) ((is_interval) ? (year) : ((year) <=
0 ? -((year) - 1) : (year)))

The code comment took me a bit to process - seems like the following would
be better (if its right - I don't know why interval is a pure no-op while
non-interval normalizes to a positive integer).

Years go from 1 BC to 1 AD, so we adjust the year zero, and all negative
years, by shifting them away one year, We then return the positive value
of the result because the caller tracks the BC/AD aspect of the year
separately and only deals with positive year values coming out of this
macro. Intervals denote the distance away from 0 a year is so we can
simply take the supplied value and return it. Interval processing code
expects a negative result for intervals going into BC.

David J.

To make "to_date" work as "make_date" with negative years these llines:
https://github.com/postgres/postgres/blob/fb544735f11480a697fcab791c058adc166be1fa/src/backend/utils/adt/formatting.c#L4559-L4560 :
if (tmfc.bc && tm->tm_year > 0)
tm->tm_year = -(tm->tm_year - 1);
must be changed to:
if (tmfc.bc && tm->tm_year > 0)
{
tm->tm_year = -(tm->tm_year - 1);
}
else if (tm->tm_year < 0) {
tm->tm_year ++;
}

research how it behaves in the other database it tries to emulate and either document or possibly change the behavior in v14
As in https://stackoverflow.com/questions/6779521/how-do-i-insert-a-bc-date-into-oracle and http://rwijk.blogspot.com/2008/10/year-zero.html
In Oracle

to_date('-4700/01/01','syyyy/mm/dd')
returns

01/01/4700 BC
In documents https://docs.oracle.com/cd/B28359_01/olap.111/b28126/dml_commands_1029.htm#OLADM780

YEAR

SYEAR

Year, spelled out; S prefixes BC dates with a minus sign (-).

YYYY

SYYYY

4-digit year; S prefixes BC dates with a minus sign.

Redirecting to -hackers for visibility. I feel there needs to be something
done here, even if just documentation (a bullet in the usage notes section
- and a code comment update for the macro) pointing this out and not
changing any behavior.

David J.

On Wed, May 6, 2020 at 8:12 PM David G. Johnston <david.g.johnston@gmail.com>
wrote:

‪On Wed, May 6, 2020 at 6:31 PM ‫دار الآثار للنشر والتوزيع-صنعاء Dar
Alathar-Yemen‬‎ <dar_alathar@hotmail.com> wrote:‬

Any one suppose that these functions return the same:
make_date(-1,1,1)
to_date('-1-01-01','yyyy-mm-dd')

But make_date will give 0001-01-01 BC

And to_date will give 0002-01-01 BC

Interesting...and a fair point.

What seems to be happening here is that to_date is trying to be helpful by
doing:

select to_date('0000','YYYY'); // 0001-01-01 BC

It does this seemingly by subtracting one from the year, making it
positive, then (I infer) appending "BC" to the result. Thus for the year
"-1" it yields "0002-01-01 BC"

make_date just chooses to reject the year 0 and treat the negative as an
alternative to specifying BC

There seems to be zero tests for to_date involving negative years, and the
documentation doesn't talk of them.

I'll let the -hackers speak up as to how they want to go about handling
to_date (research how it behaves in the other database it tries to emulate
and either document or possibly change the behavior in v14) but do suggest
that a simple explicit description of how to_date works in the presence of
negative years be back-patched. A bullet in the usage notes section
probably suffices:

"If a YYYY format string captures a negative year, or 0000, it will treat
it as a BC year after decreasing the value by one. So 0000 maps to 1 BC
and -1 maps to 2 BC and so on."

So, no, make_date and to_date do not agree on this point; and they do not
have to. There is no way to specify "BC" in make_date function so using
negative there makes sense. You can specify BC in the input string for
to_date and indeed that is the only supported (documented) way to do so.

[and the next email]

On Tue, 2020-05-12 at 18:09 -0700, David G. Johnston wrote:

Redirecting to -hackers for visibility. I feel there needs to be something done here, even if just documentation (a bullet in the usage notes section - and a code comment update for the macro)
pointing this out and not changing any behavior.

David J.

On Wed, May 6, 2020 at 8:12 PM David G. Johnston <david.g.johnston@gmail.com> wrote:

‪On Wed, May 6, 2020 at 6:31 PM ‫دار الآثار للنشر والتوزيع-صنعاء Dar Alathar-Yemen‬‎ <dar_alathar@hotmail.com> wrote:‬

Any one suppose that these functions return the same:
make_date(-1,1,1)
to_date('-1-01-01','yyyy-mm-dd')

But make_date will give 0001-01-01 BC

And to_date will give 0002-01-01 BC

Interesting...and a fair point.

What seems to be happening here is that to_date is trying to be helpful by doing:

select to_date('0000','YYYY'); // 0001-01-01 BC

It does this seemingly by subtracting one from the year, making it positive, then (I infer) appending "BC" to the result. Thus for the year "-1" it yields "0002-01-01 BC"

make_date just chooses to reject the year 0 and treat the negative as an alternative to specifying BC

There seems to be zero tests for to_date involving negative years, and the documentation doesn't talk of them.

I'll let the -hackers speak up as to how they want to go about handling to_date (research how it behaves in the other database it tries to emulate and either document or possibly change the
behavior in v14) but do suggest that a simple explicit description of how to_date works in the presence of negative years be back-patched. A bullet in the usage notes section probably suffices:

"If a YYYY format string captures a negative year, or 0000, it will treat it as a BC year after decreasing the value by one. So 0000 maps to 1 BC and -1 maps to 2 BC and so on."

So, no, make_date and to_date do not agree on this point; and they do not have to. There is no way to specify "BC" in make_date function so using negative there makes sense. You can specify BC
in the input string for to_date and indeed that is the only supported (documented) way to do so.

[and the next email]

Specifically:

https://github.com/postgres/postgres/blob/fb544735f11480a697fcab791c058adc166be1fa/src/backend/utils/adt/formatting.c#L236

/*
* There is no 0 AD. Years go from 1 BC to 1 AD, so we make it
* positive and map year == -1 to year zero, and shift all negative
* years up one. For interval years, we just return the year.
*/
#define ADJUST_YEAR(year, is_interval) ((is_interval) ? (year) : ((year) <= 0 ? -((year) - 1) : (year)))

The code comment took me a bit to process - seems like the following would be better (if its right - I don't know why interval is a pure no-op while non-interval normalizes to a positive integer).

Years go from 1 BC to 1 AD, so we adjust the year zero, and all negative years, by shifting them away one year, We then return the positive value of the result because the caller tracks the BC/AD
aspect of the year separately and only deals with positive year values coming out of this macro. Intervals denote the distance away from 0 a year is so we can simply take the supplied value and
return it. Interval processing code expects a negative result for intervals going into BC.

David J.

Since "to_date" is an Oracle compatibility function, here is what Oracle 18.4 has to say to that:

SQL> SELECT to_date('0000', 'YYYY') FROM dual;
SELECT to_date('0000', 'YYYY') FROM dual
*
ERROR at line 1:
ORA-01841: (full) year must be between -4713 and +9999, and not be 0

SQL> SELECT to_date('-0001', 'YYYY') FROM dual;
SELECT to_date('-0001', 'YYYY') FROM dual
*
ERROR at line 1:
ORA-01841: (full) year must be between -4713 and +9999, and not be 0

SQL> SELECT to_date('-0001', 'SYYYY') FROM dual;

TO_DATE('-0001','SYYYY
----------------------
0001-05-01 00:00:00 BC

Yours,
Laurenz Albe

On Tue, May 12, 2020 at 8:56 PM Laurenz Albe <laurenz.albe@cybertec.at>
wrote:

On Tue, 2020-05-12 at 18:09 -0700, David G. Johnston wrote:

Redirecting to -hackers for visibility. I feel there needs to be

something done here, even if just documentation (a bullet in the usage
notes section - and a code comment update for the macro)

pointing this out and not changing any behavior.

Since "to_date" is an Oracle compatibility function, here is what Oracle
18.4 has to say to that:

SQL> SELECT to_date('0000', 'YYYY') FROM dual;
SELECT to_date('0000', 'YYYY') FROM dual
*
ERROR at line 1:
ORA-01841: (full) year must be between -4713 and +9999, and not be 0

Attached is a concrete patch (back-patchable hopefully) documenting the
current reality.

As noted in the patch commit message (commentary really):

make_timestamp not agreeing with make_date on how to handle negative years
should probably just be fixed - but that is for someone else to handle.

Whether to actually change the behavior of to_date is up for debate though
I would presume it would not be back-patched.

David J.

On Wed, Jul 15, 2020 at 09:26:53AM -0700, David G. Johnston wrote:

On Tue, May 12, 2020 at 8:56 PM Laurenz Albe <laurenz.albe@cybertec.at> wrote:

On Tue, 2020-05-12 at 18:09 -0700, David G. Johnston wrote:

Redirecting to -hackers for visibility.� I feel there needs to be

something done here, even if just documentation (a bullet in the usage
notes section - and a code comment update for the macro)

pointing this out and not changing any behavior.

Since "to_date" is an Oracle compatibility function, here is what Oracle
18.4 has to say to that:

SQL> SELECT to_date('0000', 'YYYY') FROM dual;
SELECT to_date('0000', 'YYYY') FROM dual
� � � � � � � �*
ERROR at line 1:
ORA-01841: (full) year must be between -4713 and +9999, and not be 0

Attached is a concrete patch (back-patchable hopefully) documenting the current
reality.

As noted in the patch commit message (commentary really):

make_timestamp not agreeing with make_date on how to handle negative years
should probably just be fixed - but that is for someone else to handle.

Whether to actually change the behavior of to_date is up for debate though I
would presume it would not be back-patched.

OK, so, looking at this thread, we have to_date() treating -1 as -2 BC,
make_date() treating -1 as 1 BC, and we have Oracle, which to_date() is
supposed to match, making -1 as 1 BC.

Because we already have the to_date/make_date inconsistency, and the -1
to -2 BC mapping is confusing, and doesn't match Oracle, I think the
clean solution is to change PG 14 to treat -1 as 1 BC, and document the
incompatibility in the release notes.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

The usefulness of a cup is in its emptiness, Bruce Lee

On Thu, Sep 3, 2020 at 6:21 PM Bruce Momjian <bruce@momjian.us> wrote:

On Wed, Jul 15, 2020 at 09:26:53AM -0700, David G. Johnston wrote:

Whether to actually change the behavior of to_date is up for debate

though I

would presume it would not be back-patched.

OK, so, looking at this thread, we have to_date() treating -1 as -2 BC,
make_date() treating -1 as 1 BC, and we have Oracle, which to_date() is
supposed to match, making -1 as 1 BC.

Because we already have the to_date/make_date inconsistency, and the -1
to -2 BC mapping is confusing, and doesn't match Oracle, I think the
clean solution is to change PG 14 to treat -1 as 1 BC, and document the
incompatibility in the release notes.

I agree that someone else should write another patch to fix the behavior
for v14. Still suggest committing the proposed patch to master and all
supported versions to document the existing behavior correctly. The fix
patch can work from that.

David J.

On Fri, Sep 4, 2020 at 12:45:36PM -0700, David G. Johnston wrote:

On Thu, Sep 3, 2020 at 6:21 PM Bruce Momjian <bruce@momjian.us> wrote:

On Wed, Jul 15, 2020 at 09:26:53AM -0700, David G. Johnston wrote:

Whether to actually change the behavior of to_date is up for debate

though I

would presume it would not be back-patched.

OK, so, looking at this thread, we have to_date() treating -1 as -2 BC,
make_date() treating -1 as 1 BC, and we have Oracle, which to_date() is
supposed to match, making -1 as 1 BC.

Because we already have the to_date/make_date inconsistency, and the -1
to -2 BC mapping is confusing, and doesn't match Oracle, I think the
clean solution is to change PG 14 to treat -1 as 1 BC, and document the
incompatibility in the release notes.

I agree that someone else should write another patch to fix the behavior for
v14.� Still suggest committing the proposed patch to master and all supported
versions to document the existing behavior correctly.� The fix patch can work
from that.

I think we need to apply the patches to all branches at the same time.
I am not sure we want to document a behavior we know will change in PG
14.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

The usefulness of a cup is in its emptiness, Bruce Lee

On 2020-09-04 21:45, David G. Johnston wrote:

On Thu, Sep 3, 2020 at 6:21 PM Bruce Momjian <bruce@momjian.us
<mailto:bruce@momjian.us>> wrote:

On Wed, Jul 15, 2020 at 09:26:53AM -0700, David G. Johnston wrote:

Whether to actually change the behavior of to_date is up for

debate though I

would presume it would not be back-patched.

OK, so, looking at this thread, we have to_date() treating -1 as -2 BC,
make_date() treating -1 as 1 BC, and we have Oracle, which to_date() is
supposed to match, making -1 as 1 BC.

Because we already have the to_date/make_date inconsistency, and the -1
to -2 BC mapping is confusing, and doesn't match Oracle, I think the
clean solution is to change PG 14 to treat -1 as 1 BC, and document the
incompatibility in the release notes.

I agree that someone else should write another patch to fix the behavior
for v14.  Still suggest committing the proposed patch to master and all
supported versions to document the existing behavior correctly.  The fix
patch can work from that.

Adding support for negative years in make_timestamp seems pretty
straightforward; see attached patch.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation: not tested

Patch looks good to me.

The new status of this patch is: Ready for Committer

Bruce Momjian <bruce@momjian.us> writes:

On Fri, Sep 4, 2020 at 12:45:36PM -0700, David G. Johnston wrote:

Because we already have the to_date/make_date inconsistency, and the -1
to -2 BC mapping is confusing, and doesn't match Oracle, I think the
clean solution is to change PG 14 to treat -1 as 1 BC, and document the
incompatibility in the release notes.

I agree that someone else should write another patch to fix the behavior for
v14.  Still suggest committing the proposed patch to master and all supported
versions to document the existing behavior correctly.  The fix patch can work
from that.

I think we need to apply the patches to all branches at the same time.
I am not sure we want to document a behavior we know will change in PG
14.

I think this is nuts. The current behavior is obviously broken;
we should just treat it as a bug and fix it, including back-patching.
I do not think there is a compatibility problem of any significance.
Who out there is going to have an application that is relying on the
ability to insert BC dates in this way?

regards, tom lane

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

Adding support for negative years in make_timestamp seems pretty
straightforward; see attached patch.

In hopes of moving things along, I pushed that, along with documentation
additions. I couldn't quite convince myself that it was a bug fix
though, so no back-patch. (I don't think we really need any doc
changes about it in the back branches, either.)

regards, tom lane

I wrote:

I think this is nuts. The current behavior is obviously broken;
we should just treat it as a bug and fix it, including back-patching.
I do not think there is a compatibility problem of any significance.
Who out there is going to have an application that is relying on the
ability to insert BC dates in this way?

Concretely, I propose the attached. This adjusts Dar Alathar-Yemen's
patch (it didn't do the right thing IMO for the combination of bc
and year < 0) and adds test cases and docs.

Oracle would have us throw an error for year zero, but our historical
behavior has been to read it as 1 BC. That's not so obviously wrong
that I'd want to change it in the back branches. Maybe it could be
done as a follow-up change in HEAD.

regards, tom lane

On Tue, Sep 29, 2020 at 01:26:29PM -0400, Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

On Fri, Sep 4, 2020 at 12:45:36PM -0700, David G. Johnston wrote:

Because we already have the to_date/make_date inconsistency, and the -1
to -2 BC mapping is confusing, and doesn't match Oracle, I think the
clean solution is to change PG 14 to treat -1 as 1 BC, and document the
incompatibility in the release notes.

I agree that someone else should write another patch to fix the behavior for
v14.� Still suggest committing the proposed patch to master and all supported
versions to document the existing behavior correctly.� The fix patch can work
from that.

I think we need to apply the patches to all branches at the same time.
I am not sure we want to document a behavior we know will change in PG
14.

I think this is nuts. The current behavior is obviously broken;
we should just treat it as a bug and fix it, including back-patching.
I do not think there is a compatibility problem of any significance.
Who out there is going to have an application that is relying on the
ability to insert BC dates in this way?

You are agreeing with what I am suggesting then?

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

The usefulness of a cup is in its emptiness, Bruce Lee

Bruce Momjian <bruce@momjian.us> writes:

On Tue, Sep 29, 2020 at 01:26:29PM -0400, Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

On Fri, Sep 4, 2020 at 12:45:36PM -0700, David G. Johnston wrote:

Because we already have the to_date/make_date inconsistency, and the -1
to -2 BC mapping is confusing, and doesn't match Oracle, I think the
clean solution is to change PG 14 to treat -1 as 1 BC, and document the
incompatibility in the release notes.

I agree that someone else should write another patch to fix the behavior for
v14.  Still suggest committing the proposed patch to master and all supported
versions to document the existing behavior correctly.  The fix patch can work
from that.

I think this is nuts. The current behavior is obviously broken;
we should just treat it as a bug and fix it, including back-patching.
I do not think there is a compatibility problem of any significance.
Who out there is going to have an application that is relying on the
ability to insert BC dates in this way?

You are agreeing with what I am suggesting then?

Hm, I read your reference to "the release notes" as suggesting that
we should change it only in a major release, ie HEAD only (and it
looks like David read it the same). If you meant minor release notes,
then we're on the same page.

regards, tom lane