comment regarding double timestamps; and, infinite timestamps and NaN

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

Responses

Browse pgsql-hackers by date

  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