pgsql: Allow to_timestamp(float8) to convert float infinity to timestam

Started by Tom Laneabout 10 years ago6 messageshackers
Jump to latest
#1Tom Lane
tgl@sss.pgh.pa.us

Allow to_timestamp(float8) to convert float infinity to timestamp infinity.

With the original SQL-function implementation, such cases failed because
we don't support infinite intervals. Converting the function to C lets
us bypass the interval representation, which should be a bit faster as
well as more flexible.

Vitaly Burovoy, reviewed by Anastasia Lubennikova

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/e511d878f3bbc205cd260a79740e646eea3c1cd3

Modified Files
--------------
doc/src/sgml/func.sgml | 50 +++++++++++++-------------
src/backend/utils/adt/timestamp.c | 58 +++++++++++++++++++++++++++++++
src/include/catalog/catversion.h | 2 +-
src/include/catalog/pg_proc.h | 2 +-
src/include/utils/timestamp.h | 1 +
src/test/regress/expected/timestamptz.out | 47 +++++++++++++++++++++++++
src/test/regress/sql/timestamptz.sql | 15 ++++++++
7 files changed, 149 insertions(+), 26 deletions(-)

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

#2Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#1)
Re: [COMMITTERS] pgsql: Allow to_timestamp(float8) to convert float infinity to timestam

On Wed, Mar 30, 2016 at 6:09 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Allow to_timestamp(float8) to convert float infinity to timestamp infinity.

With the original SQL-function implementation, such cases failed because
we don't support infinite intervals. Converting the function to C lets
us bypass the interval representation, which should be a bit faster as
well as more flexible.

+-- The upper boundary differs between integer and float timestamps,
so check the biggest one
+SELECT to_timestamp(185331707078400::float8);  -- error, out of range
+ERROR:  timestamp out of range: "1.85332e+14"
Some of the tests introduced are making MSVC unhappy, because they
depend on the three-digit behavior that Windows is using, leading to
those failures:
  -- The upper boundary differs between integer and float timestamps,
so check the biggest one
  SELECT to_timestamp(185331707078400::float8);  -- error, out of range
! ERROR:  timestamp out of range: "1.85332e+14"
  -- nonfinite values
  SELECT to_timestamp(' Infinity'::float);
   to_timestamp
--- 2338,2344 ----

-- The upper boundary differs between integer and float timestamps,
so check the biggest one
SELECT to_timestamp(185331707078400::float8); -- error, out of range
! ERROR: timestamp out of range: "1.85332e+014"

If the those tests are kept, an alternate output file is necessary (I
can send a patch if needed, I see the failure locally as well).
--
Michael

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

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Michael Paquier (#2)
Re: [COMMITTERS] pgsql: Allow to_timestamp(float8) to convert float infinity to timestam

Michael Paquier <michael.paquier@gmail.com> writes:

On Wed, Mar 30, 2016 at 6:09 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Allow to_timestamp(float8) to convert float infinity to timestamp infinity.

Some of the tests introduced are making MSVC unhappy, because they
depend on the three-digit behavior that Windows is using, leading to
those failures:

Ah, I was wondering about that. The patch as-submitted used "%lf" which
seemed even less likely to be portable, but evidently %g isn't that much
better.

If the those tests are kept, an alternate output file is necessary (I
can send a patch if needed, I see the failure locally as well).

I'm inclined to just drop the out-of-range test cases. They're not that
useful IMO, and alternate expected-files are a real PITA for maintenance.

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

#4Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#3)
Re: [COMMITTERS] pgsql: Allow to_timestamp(float8) to convert float infinity to timestam

On Wed, Mar 30, 2016 at 10:13 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Michael Paquier <michael.paquier@gmail.com> writes:

On Wed, Mar 30, 2016 at 6:09 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Allow to_timestamp(float8) to convert float infinity to timestamp infinity.

Some of the tests introduced are making MSVC unhappy, because they
depend on the three-digit behavior that Windows is using, leading to
those failures:

Ah, I was wondering about that. The patch as-submitted used "%lf" which
seemed even less likely to be portable, but evidently %g isn't that much
better.

Yep.

If the those tests are kept, an alternate output file is necessary (I
can send a patch if needed, I see the failure locally as well).

I'm inclined to just drop the out-of-range test cases. They're not that
useful IMO, and alternate expected-files are a real PITA for maintenance.

Hm. Actually, they are quite useful to check error boundaries, so why
not just simplifying the error message to "timestamp out of range" and
remove the value from it?
--
Michael

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

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Michael Paquier (#4)
Re: [COMMITTERS] pgsql: Allow to_timestamp(float8) to convert float infinity to timestam

Michael Paquier <michael.paquier@gmail.com> writes:

On Wed, Mar 30, 2016 at 10:13 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I'm inclined to just drop the out-of-range test cases. They're not that
useful IMO, and alternate expected-files are a real PITA for maintenance.

Hm. Actually, they are quite useful to check error boundaries, so why
not just simplifying the error message to "timestamp out of range" and
remove the value from it?

Meh. I realize that there are a lot of places where we just say
"timestamp out of range" rather than trying to give a specific value,
but it's really contrary to our message style guidelines to not print
the complained-of value. I think we should leave the ereport calls as-is
and remove the test cases; to do otherwise is putting the regression tests
ahead of users. The upper-boundary test is quite dubiously useful anyway,
because it has to test a value that's very far away from where the
boundary actually is in most builds.

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

#6Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#5)
Re: [COMMITTERS] pgsql: Allow to_timestamp(float8) to convert float infinity to timestam

On Wed, Mar 30, 2016 at 10:47 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Michael Paquier <michael.paquier@gmail.com> writes:

On Wed, Mar 30, 2016 at 10:13 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I'm inclined to just drop the out-of-range test cases. They're not that
useful IMO, and alternate expected-files are a real PITA for maintenance.

Hm. Actually, they are quite useful to check error boundaries, so why
not just simplifying the error message to "timestamp out of range" and
remove the value from it?

Meh. I realize that there are a lot of places where we just say
"timestamp out of range" rather than trying to give a specific value,
but it's really contrary to our message style guidelines to not print
the complained-of value. I think we should leave the ereport calls as-is
and remove the test cases; to do otherwise is putting the regression tests
ahead of users. The upper-boundary test is quite dubiously useful anyway,
because it has to test a value that's very far away from where the
boundary actually is in most builds.

OK, I won't fight more on that.
--
Michael

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