From: | Justin Pryzby <pryzby(at)telsasoft(dot)com> |
---|---|
To: | pgsql-hackers(at)postgresql(dot)org |
Subject: | comment regarding double timestamps; and, infinite timestamps and NaN |
Date: | 2019-12-30 07:47:21 |
Message-ID: | 20191230074721.GM12890@telsasoft.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
selfuncs.c convert_to_scalar() says:
|* The several datatypes representing absolute times are all converted
|* to Timestamp, which is actually a double, and then we just use that
|* double value. Note this will give correct results even for the "special"
|* values of Timestamp, since those are chosen to compare correctly;
|* see timestamp_cmp.
But:
https://www.postgresql.org/docs/10/release-10.html
|Remove support for floating-point timestamps and intervals (Tom Lane)
|This removes configure's --disable-integer-datetimes option. Floating-point timestamps have few advantages and have not been the default since PostgreSQL 8.3.
|b6aa17e De-support floating-point timestamps.
|configure | 18 ++++++------------
|configure.in | 12 ++++++------
|doc/src/sgml/config.sgml | 8 +++-----
|doc/src/sgml/datatype.sgml | 55 +++++++++++--------------------------------------------
|doc/src/sgml/installation.sgml | 22 ----------------------
|src/include/c.h | 7 ++++---
|src/include/pg_config.h.in | 4 ----
|src/include/pg_config.h.win32 | 4 ----
|src/interfaces/ecpg/include/ecpg_config.h.in | 4 ----
|src/interfaces/ecpg/include/pgtypes_interval.h | 2 --
|src/interfaces/ecpg/test/expected/pgtypeslib-dt_test2.c | 6 ++----
|src/interfaces/ecpg/test/expected/pgtypeslib-dt_test2.stdout | 2 ++
|src/interfaces/ecpg/test/pgtypeslib/dt_test2.pgc | 6 ++----
|src/tools/msvc/Solution.pm | 9 ---------
|src/tools/msvc/config_default.pl | 1 -
|15 files changed, 36 insertions(+), 124 deletions(-)
It's true that convert_to_scalar sees doubles:
|static double
|convert_timevalue_to_scalar(Datum value, Oid typid, bool *failure)
|{
| switch (typid)
| {
| case TIMESTAMPOID:
| return DatumGetTimestamp(value);
But:
$ git grep DatumGetTimestamp src/include/
src/include/utils/timestamp.h:#define DatumGetTimestamp(X) ((Timestamp) DatumGetInt64(X))
So I propose it should say something like:
|* The several datatypes representing absolute times are all converted
|* to Timestamp, which is actually an int64, and then we just promote that
|* to double. Note this will give correct results even for the "special"
|* values of Timestamp, since those are chosen to compare correctly;
|* see timestamp_cmp.
That seems to be only used for ineq_histogram_selectivity() interpolation of
histogram bins. It looks to me that at least isn't working for "special
values", and needs to use something other than isnan(). I added debugging code
and tested the attached like:
DROP TABLE t; CREATE TABLE t(t) AS SELECT generate_series(now(), now()+'1 day', '5 minutes');
INSERT INTO t VALUES('-infinity');
ALTER TABLE t ALTER t SET STATISTICS 1;
ANALYZE t;
explain SELECT * FROM t WHERE t>='2010-12-29';
Attachment | Content-Type | Size |
---|---|---|
v1-0001-Correctly-handle-infinite-timestamps-in-ineq_hist.patch | text/x-diff | 2.2 KB |
v1-0002-Repair-comment-regarding-double-timestamps.patch | text/x-diff | 1.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Eisentraut | 2019-12-30 09:08:47 | Re: Windows v readline |
Previous Message | Andrey Borodin | 2019-12-30 06:43:04 | Re: Yet another fast GiST build |