Horton

Re: [PATCH] Improve geometric types

Thomas Munro <thomas.munro@enterprisedb.com>

View thread View on postgr.es
From: Thomas Munro <thomas.munro@enterprisedb.com>
To: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Cc: Emre Hasegeli <emre@hasegeli.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>
Date: 2018-07-09T21:38:00Z
Lists: pgsql-hackers 
On Tue, Jul 10, 2018 at 7:21 AM, Tomas Vondra
<tomas.vondra@2ndquadrant.com> wrote:
> On 06/05/2018 06:32 PM, Emre Hasegeli wrote:
>>> Those underscore-prefixed names are defined in Microsoft's
>>> <float.h>[3][4].  So now I'm wondering if win32_port.h needs to
>>> #include <float.h> if (_MSC_VER < 1800).
>>
>> I don't have the C experience to decide the correct way.  There are
>> currently many .c files that are including float.h conditionally or
>> unconditionally.  The condition they use is "#ifdef _MSC_VER" without
>> a version.
>>
>> One idea is to include float.h from the new utils/float.h file
>> together with math.h, and remove those includes from the .c files
>> which would include utils/float.h.  We can do this only, or together
>> with what you suggest, or by also keeping the includes on the .c
>> files.  Which way do you think is the proper?
>>
>
> Do we have any solution to the float.h include issues on Windows? I
> don't have any Windows box at hand so I can't verify it, but just using
> "#ifdef _MSC_VER" seems OK to me (and it's used elsewhere). Thomas, why
> do you think the version number restriction is needed here? I don't see
> the version mentioned in the MS docs you linked either.

The version number restriction isn't strictly needed.  I only
suggested it because it'd match the #if that wraps the code that's
actually using those macros, introduced by commit cec8394b5ccd.  That
was presumably done because versions >= 1800 (= Visual Studio 2013)
have their own definitions of isinf() and isnan(), and I guess that
our definitions were probably breaking stuff on that compiler.

> Once this gets resolved, I'd like to get this committed ... so if you
> have other objections, please speak now.

+1, no objections.

-- 
Thomas Munro
http://www.enterprisedb.com

Commits

  1. Improve test coverage of geometric types

  2. Fix problems in handling the line data type

  3. Use the built-in float datatypes to implement geometric types

  4. Remove remaining GEODEBUG references from geo_ops.c

  5. Provide separate header file for built-in float types

  6. Refactor geometric functions and operators

  7. Fix crash in close_ps() for NaN input coordinates.

  8. Fix GiST index build for NaN values in geometric types.

  9. Enable building with Visual Studion 2013.

  10. Suppress -0 in the C field of lines computed by line_construct_pts().