Bug fix for missing years in make_date()

Started by David Fetteralmost 11 years ago9 messages
#1David Fetter
david@fetter.org
1 attachment(s)

Folks,

For reasons unclear, dates before the Common Era are disallowed in
make_date(), even though about 2/3 of the underlying data type's range
up until the present time fits that description.

Please find attached a patch fixing same.

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Attachments:

wider_make_date_001.patchtext/plain; charset=us-asciiDownload
diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index d66f640..8dddd07 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -248,10 +248,15 @@ make_date(PG_FUNCTION_ARGS)
 	tm.tm_mday = PG_GETARG_INT32(2);
 
 	/*
-	 * Note: we'll reject zero or negative year values.  Perhaps negatives
-	 * should be allowed to represent BC years?
+	 * Note: Non-positive years are take to be BCE.
 	 */
-	dterr = ValidateDate(DTK_DATE_M, false, false, false, &tm);
+	if (tm.tm_year >= 0)
+		dterr = ValidateDate(DTK_DATE_M, false, false, false, &tm);
+	else
+	{
+		tm.tm_year = -1 * tm.tm_year;
+		dterr = ValidateDate(DTK_DATE_M, false, false, true, &tm);
+	}
 
 	if (dterr != 0)
 		ereport(ERROR,
diff --git a/src/test/regress/expected/date.out b/src/test/regress/expected/date.out
index 8923f60..73b3062 100644
--- a/src/test/regress/expected/date.out
+++ b/src/test/regress/expected/date.out
@@ -1191,6 +1191,12 @@ select make_date(2013, 7, 15);
  07-15-2013
 (1 row)
 
+select make_date(-44, 3, 15);  -- Non-positive years are BCE
+   make_date   
+---------------
+ 03-15-0044 BC
+(1 row)
+
 select make_time(8, 20, 0.0);
  make_time 
 -----------
@@ -1204,8 +1210,6 @@ select make_date(2013, 13, 1);
 ERROR:  date field value out of range: 2013-13-01
 select make_date(2013, 11, -1);
 ERROR:  date field value out of range: 2013-11--1
-select make_date(-44, 3, 15);  -- perhaps we should allow this sometime?
-ERROR:  date field value out of range: -44-03-15
 select make_time(10, 55, 100.1);
 ERROR:  time field value out of range: 10:55:100.1
 select make_time(24, 0, 2.1);
diff --git a/src/test/regress/sql/date.sql b/src/test/regress/sql/date.sql
index a62e92a..e6bff17 100644
--- a/src/test/regress/sql/date.sql
+++ b/src/test/regress/sql/date.sql
@@ -279,11 +279,11 @@ select isfinite('infinity'::date), isfinite('-infinity'::date), isfinite('today'
 
 -- test constructors
 select make_date(2013, 7, 15);
+select make_date(-44, 3, 15);  -- Non-positive years are BCE
 select make_time(8, 20, 0.0);
 -- should fail
 select make_date(2013, 2, 30);
 select make_date(2013, 13, 1);
 select make_date(2013, 11, -1);
-select make_date(-44, 3, 15);  -- perhaps we should allow this sometime?
 select make_time(10, 55, 100.1);
 select make_time(24, 0, 2.1);
#2Pavel Stehule
pavel.stehule@gmail.com
In reply to: David Fetter (#1)
Re: Bug fix for missing years in make_date()

2015-03-26 23:26 GMT+01:00 David Fetter <david@fetter.org>:

Folks,

For reasons unclear, dates before the Common Era are disallowed in
make_date(), even though about 2/3 of the underlying data type's range
up until the present time fits that description.

Please find attached a patch fixing same.

+1

Pavel

Show quoted text

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Jim Nasby
Jim.Nasby@BlueTreble.com
In reply to: David Fetter (#1)
Re: Bug fix for missing years in make_date()

On 3/26/15 5:26 PM, David Fetter wrote:

+ * Note: Non-positive years are take to be BCE.

s/take/taken/
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4David Fetter
david@fetter.org
In reply to: Jim Nasby (#3)
1 attachment(s)
Re: Bug fix for missing years in make_date()

On Mon, Mar 30, 2015 at 05:35:29PM -0500, Jim Nasby wrote:

On 3/26/15 5:26 PM, David Fetter wrote:

+ * Note: Non-positive years are take to be BCE.

s/take/taken/

Good point. Next patch attached.

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Attachments:

wider_make_date_002.patchtext/plain; charset=us-asciiDownload
diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index d66f640..6102222 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -248,10 +248,15 @@ make_date(PG_FUNCTION_ARGS)
 	tm.tm_mday = PG_GETARG_INT32(2);
 
 	/*
-	 * Note: we'll reject zero or negative year values.  Perhaps negatives
-	 * should be allowed to represent BC years?
+	 * Note: Non-positive years are taken to be BCE.
 	 */
-	dterr = ValidateDate(DTK_DATE_M, false, false, false, &tm);
+	if (tm.tm_year >= 0)
+		dterr = ValidateDate(DTK_DATE_M, false, false, false, &tm);
+	else
+	{
+		tm.tm_year = -1 * tm.tm_year;
+		dterr = ValidateDate(DTK_DATE_M, false, false, true, &tm);
+	}
 
 	if (dterr != 0)
 		ereport(ERROR,
diff --git a/src/test/regress/expected/date.out b/src/test/regress/expected/date.out
index 8923f60..73b3062 100644
--- a/src/test/regress/expected/date.out
+++ b/src/test/regress/expected/date.out
@@ -1191,6 +1191,12 @@ select make_date(2013, 7, 15);
  07-15-2013
 (1 row)
 
+select make_date(-44, 3, 15);  -- Non-positive years are BCE
+   make_date   
+---------------
+ 03-15-0044 BC
+(1 row)
+
 select make_time(8, 20, 0.0);
  make_time 
 -----------
@@ -1204,8 +1210,6 @@ select make_date(2013, 13, 1);
 ERROR:  date field value out of range: 2013-13-01
 select make_date(2013, 11, -1);
 ERROR:  date field value out of range: 2013-11--1
-select make_date(-44, 3, 15);  -- perhaps we should allow this sometime?
-ERROR:  date field value out of range: -44-03-15
 select make_time(10, 55, 100.1);
 ERROR:  time field value out of range: 10:55:100.1
 select make_time(24, 0, 2.1);
diff --git a/src/test/regress/sql/date.sql b/src/test/regress/sql/date.sql
index a62e92a..e6bff17 100644
--- a/src/test/regress/sql/date.sql
+++ b/src/test/regress/sql/date.sql
@@ -279,11 +279,11 @@ select isfinite('infinity'::date), isfinite('-infinity'::date), isfinite('today'
 
 -- test constructors
 select make_date(2013, 7, 15);
+select make_date(-44, 3, 15);  -- Non-positive years are BCE
 select make_time(8, 20, 0.0);
 -- should fail
 select make_date(2013, 2, 30);
 select make_date(2013, 13, 1);
 select make_date(2013, 11, -1);
-select make_date(-44, 3, 15);  -- perhaps we should allow this sometime?
 select make_time(10, 55, 100.1);
 select make_time(24, 0, 2.1);
#5Adam Brightwell
adam.brightwell@crunchydatasolutions.com
In reply to: David Fetter (#4)
Re: Bug fix for missing years in make_date()

Good point. Next patch attached.

  /*
- * Note: we'll reject zero or negative year values.  Perhaps negatives
- * should be allowed to represent BC years?
+ * Note: Non-positive years are taken to be BCE.
  */

Previously, zero was rejected, what does it do now? I'm sure it represents
0 AD/CE, however, is that important enough to note given that it was not
allowed previously?

-Adam

--
Adam Brightwell - adam.brightwell@crunchydatasolutions.com
Database Engineer - www.crunchydatasolutions.com

#6David Fetter
david@fetter.org
In reply to: Adam Brightwell (#5)
Re: Bug fix for missing years in make_date()

On Tue, Mar 31, 2015 at 10:34:45AM -0400, Adam Brightwell wrote:

Good point. Next patch attached.

/*
- * Note: we'll reject zero or negative year values.  Perhaps negatives
- * should be allowed to represent BC years?
+ * Note: Non-positive years are taken to be BCE.
*/

Previously, zero was rejected, what does it do now? I'm sure it represents
0 AD/CE, however, is that important enough to note given that it was not
allowed previously?

Now, it's supposed to take 0 as 1 BCE, -1 as 2 BCE, etc. There should
probably be tests for that. The issue here is that zero was
popularized a very long time after the beginning of the Common Era.

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: David Fetter (#6)
Re: Bug fix for missing years in make_date()

David Fetter <david@fetter.org> writes:

On Tue, Mar 31, 2015 at 10:34:45AM -0400, Adam Brightwell wrote:

Previously, zero was rejected, what does it do now? I'm sure it represents
0 AD/CE, however, is that important enough to note given that it was not
allowed previously?

Now, it's supposed to take 0 as 1 BCE, -1 as 2 BCE, etc. There should
probably be tests for that.

Surely that is *not* what we want? I'd expect any user-facing date
function to reject zero and take -1 as 1 BC, etc. The behavior you
describe is an internal convention, not something we want to expose
to users.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#8David Fetter
david@fetter.org
In reply to: Tom Lane (#7)
Re: Bug fix for missing years in make_date()

On Tue, Mar 31, 2015 at 12:58:27PM -0400, Tom Lane wrote:

David Fetter <david@fetter.org> writes:

On Tue, Mar 31, 2015 at 10:34:45AM -0400, Adam Brightwell wrote:

Previously, zero was rejected, what does it do now? I'm sure it
represents 0 AD/CE, however, is that important enough to note
given that it was not allowed previously?

Now, it's supposed to take 0 as 1 BCE, -1 as 2 BCE, etc. There
should probably be tests for that.

Surely that is *not* what we want?

It is if we're to be consistent with the rest of the system, to wit:

SELECT to_date('YYYY','0000');
to_date
---------------
0001-01-01 BC
(1 row)

I'd expect any user-facing date function to reject zero and take -1
as 1 BC, etc. The behavior you describe is an internal convention,
not something we want to expose to users.

That ship has already sailed.

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#9David Fetter
david@fetter.org
In reply to: David Fetter (#8)
1 attachment(s)
Re: Bug fix for missing years in make_date()

On Tue, Mar 31, 2015 at 12:22:39PM -0700, David Fetter wrote:

On Tue, Mar 31, 2015 at 12:58:27PM -0400, Tom Lane wrote:

David Fetter <david@fetter.org> writes:

On Tue, Mar 31, 2015 at 10:34:45AM -0400, Adam Brightwell wrote:

Previously, zero was rejected, what does it do now? I'm sure it
represents 0 AD/CE, however, is that important enough to note
given that it was not allowed previously?

Now, it's supposed to take 0 as 1 BCE, -1 as 2 BCE, etc. There
should probably be tests for that.

Surely that is *not* what we want?

It is if we're to be consistent with the rest of the system, to wit:

SELECT to_date('YYYY','0000');
to_date
---------------
0001-01-01 BC
(1 row)

Looking at this further, I think that it should be consistent with
cast rather than with to_date().

SELECT date '0000-01-01';
ERROR: date/time field value out of range: "0000-01-01"
LINE 1: SELECT date '0000-01-01';

Please find attached the next revision of the patch.

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Attachments:

wider_make_date_003.patchtext/plain; charset=us-asciiDownload
diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index d66f640..bbf10e9 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -248,10 +248,15 @@ make_date(PG_FUNCTION_ARGS)
 	tm.tm_mday = PG_GETARG_INT32(2);
 
 	/*
-	 * Note: we'll reject zero or negative year values.  Perhaps negatives
-	 * should be allowed to represent BC years?
+	 * Note: Negative years are taken to be BCE.
 	 */
-	dterr = ValidateDate(DTK_DATE_M, false, false, false, &tm);
+	if (tm.tm_year >= 0)
+		dterr = ValidateDate(DTK_DATE_M, false, false, false, &tm);
+	else
+	{
+		tm.tm_year = -1 * tm.tm_year;
+		dterr = ValidateDate(DTK_DATE_M, false, false, true, &tm);
+	}
 
 	if (dterr != 0)
 		ereport(ERROR,
diff --git a/src/test/regress/expected/date.out b/src/test/regress/expected/date.out
index 8923f60..953e262 100644
--- a/src/test/regress/expected/date.out
+++ b/src/test/regress/expected/date.out
@@ -1191,6 +1191,12 @@ select make_date(2013, 7, 15);
  07-15-2013
 (1 row)
 
+select make_date(-44, 3, 15);  -- Negative years are BCE
+   make_date   
+---------------
+ 03-15-0044 BC
+(1 row)
+
 select make_time(8, 20, 0.0);
  make_time 
 -----------
@@ -1198,14 +1204,14 @@ select make_time(8, 20, 0.0);
 (1 row)
 
 -- should fail
+select make_date(0, 1, 1);
+ERROR:  date field value out of range: 0-01-01
 select make_date(2013, 2, 30);
 ERROR:  date field value out of range: 2013-02-30
 select make_date(2013, 13, 1);
 ERROR:  date field value out of range: 2013-13-01
 select make_date(2013, 11, -1);
 ERROR:  date field value out of range: 2013-11--1
-select make_date(-44, 3, 15);  -- perhaps we should allow this sometime?
-ERROR:  date field value out of range: -44-03-15
 select make_time(10, 55, 100.1);
 ERROR:  time field value out of range: 10:55:100.1
 select make_time(24, 0, 2.1);
diff --git a/src/test/regress/sql/date.sql b/src/test/regress/sql/date.sql
index a62e92a..bd9a845 100644
--- a/src/test/regress/sql/date.sql
+++ b/src/test/regress/sql/date.sql
@@ -279,11 +279,12 @@ select isfinite('infinity'::date), isfinite('-infinity'::date), isfinite('today'
 
 -- test constructors
 select make_date(2013, 7, 15);
+select make_date(-44, 3, 15);  -- Negative years are BCE
 select make_time(8, 20, 0.0);
 -- should fail
+select make_date(0, 1, 1);
 select make_date(2013, 2, 30);
 select make_date(2013, 13, 1);
 select make_date(2013, 11, -1);
-select make_date(-44, 3, 15);  -- perhaps we should allow this sometime?
 select make_time(10, 55, 100.1);
 select make_time(24, 0, 2.1);