public inbox for [email protected]  
help / color / mirror / Atom feed
_isnan() on Windows
14+ messages / 7 participants
[nested] [flat]

* _isnan() on Windows
@ 2018-07-10 17:36  Emre Hasegeli <[email protected]>
  0 siblings, 1 reply; 14+ messages in thread

From: Emre Hasegeli @ 2018-07-10 17:36 UTC (permalink / raw)
  To: pgsql-hackers; +Cc: Thomas Munro <[email protected]>; Andres Freund <[email protected]>

isnan() function is evidently not present on <math.h> on Windows
before Visual Studio 2013.  We define it on win32_port.h using
_isnan().  However _isnan() is also not present.  It is on <float.h>.
The patch is attached to include this from win32_port.h.

Thanks to Thomas Munro for point this out to me [1].  It is hard to
notice this issue without testing the changes on Windows.

[1] https://www.postgresql.org/message-id/CAEepm%3D3dE0w%3D_dOcELpPum6suze2NZwc%3DAV4T_xYrDUoHbZJvA%40ma...


Attachments:

  [application/octet-stream] win32_port_float_h_v00.patch (11.6K, ../../CAE2gYzyc0+5uG+Cd9-BSL7NKC8LSHLNg1Aq2=8ubjnUwut4_iw@mail.gmail.com/2-win32_port_float_h_v00.patch)
  download | inline diff:
commit dfdda6d97f72cc34c1bef9b690652941eaa08a7f
Author:     Emre Hasegeli <[email protected]>
AuthorDate: 2018-07-10 17:26:59 +0200
Commit:     Emre Hasegeli <[email protected]>
CommitDate: 2018-07-10 19:28:17 +0200

    Include <float.h> from win32_port.h

diff --git a/src/backend/access/gist/gistproc.c b/src/backend/access/gist/gistproc.c
index 97e6dc9910..0536b318cc 100644
--- a/src/backend/access/gist/gistproc.c
+++ b/src/backend/access/gist/gistproc.c
@@ -10,21 +10,20 @@
  * Portions Copyright (c) 1996-2018, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
  *	src/backend/access/gist/gistproc.c
  *
  *-------------------------------------------------------------------------
  */
 #include "postgres.h"
 
-#include <float.h>
 #include <math.h>
 
 #include "access/gist.h"
 #include "access/stratnum.h"
 #include "utils/builtins.h"
 #include "utils/geo_decls.h"
 
 
 static bool gist_box_leaf_consistent(BOX *key, BOX *query,
 						 StrategyNumber strategy);
diff --git a/src/backend/access/gist/gistutil.c b/src/backend/access/gist/gistutil.c
index 55cccd247a..12804c321c 100644
--- a/src/backend/access/gist/gistutil.c
+++ b/src/backend/access/gist/gistutil.c
@@ -6,21 +6,20 @@
  *
  * Portions Copyright (c) 1996-2018, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
  *			src/backend/access/gist/gistutil.c
  *-------------------------------------------------------------------------
  */
 #include "postgres.h"
 
-#include <float.h>
 #include <math.h>
 
 #include "access/gist_private.h"
 #include "access/htup_details.h"
 #include "access/reloptions.h"
 #include "catalog/pg_opclass.h"
 #include "storage/indexfsm.h"
 #include "storage/lmgr.h"
 #include "utils/builtins.h"
 #include "utils/syscache.h"
diff --git a/src/backend/access/tablesample/bernoulli.c b/src/backend/access/tablesample/bernoulli.c
index 1f2a933935..fba62e7b16 100644
--- a/src/backend/access/tablesample/bernoulli.c
+++ b/src/backend/access/tablesample/bernoulli.c
@@ -17,23 +17,20 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
  *	  src/backend/access/tablesample/bernoulli.c
  *
  *-------------------------------------------------------------------------
  */
 
 #include "postgres.h"
 
-#ifdef _MSC_VER
-#include <float.h>				/* for _isnan */
-#endif
 #include <math.h>
 
 #include "access/hash.h"
 #include "access/tsmapi.h"
 #include "catalog/pg_type.h"
 #include "optimizer/clauses.h"
 #include "optimizer/cost.h"
 #include "utils/builtins.h"
 
 
diff --git a/src/backend/access/tablesample/system.c b/src/backend/access/tablesample/system.c
index f888e04f40..4d937b4258 100644
--- a/src/backend/access/tablesample/system.c
+++ b/src/backend/access/tablesample/system.c
@@ -17,23 +17,20 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
  *	  src/backend/access/tablesample/system.c
  *
  *-------------------------------------------------------------------------
  */
 
 #include "postgres.h"
 
-#ifdef _MSC_VER
-#include <float.h>				/* for _isnan */
-#endif
 #include <math.h>
 
 #include "access/hash.h"
 #include "access/relscan.h"
 #include "access/tsmapi.h"
 #include "catalog/pg_type.h"
 #include "optimizer/clauses.h"
 #include "optimizer/cost.h"
 #include "utils/builtins.h"
 
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index a2a7e0c520..a6811e0338 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -64,23 +64,20 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
  *	  src/backend/optimizer/path/costsize.c
  *
  *-------------------------------------------------------------------------
  */
 
 #include "postgres.h"
 
-#ifdef _MSC_VER
-#include <float.h>				/* for _isnan */
-#endif
 #include <math.h>
 
 #include "access/amapi.h"
 #include "access/htup_details.h"
 #include "access/tsmapi.h"
 #include "executor/executor.h"
 #include "executor/nodeHash.h"
 #include "miscadmin.h"
 #include "nodes/nodeFuncs.h"
 #include "optimizer/clauses.h"
diff --git a/src/backend/utils/adt/arrayfuncs.c b/src/backend/utils/adt/arrayfuncs.c
index 0cbdbe5587..0c6c9da253 100644
--- a/src/backend/utils/adt/arrayfuncs.c
+++ b/src/backend/utils/adt/arrayfuncs.c
@@ -8,23 +8,20 @@
  *
  *
  * IDENTIFICATION
  *	  src/backend/utils/adt/arrayfuncs.c
  *
  *-------------------------------------------------------------------------
  */
 #include "postgres.h"
 
 #include <ctype.h>
-#ifdef _MSC_VER
-#include <float.h>				/* for _isnan */
-#endif
 #include <math.h>
 
 #include "access/hash.h"
 #include "access/htup_details.h"
 #include "catalog/pg_type.h"
 #include "funcapi.h"
 #include "libpq/pqformat.h"
 #include "utils/array.h"
 #include "utils/arrayaccess.h"
 #include "utils/builtins.h"
diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c
index 979f6fd7b2..017cc1a7b1 100644
--- a/src/backend/utils/adt/datetime.c
+++ b/src/backend/utils/adt/datetime.c
@@ -8,21 +8,20 @@
  *
  *
  * IDENTIFICATION
  *	  src/backend/utils/adt/datetime.c
  *
  *-------------------------------------------------------------------------
  */
 #include "postgres.h"
 
 #include <ctype.h>
-#include <float.h>
 #include <limits.h>
 #include <math.h>
 
 #include "access/htup_details.h"
 #include "access/xact.h"
 #include "catalog/pg_type.h"
 #include "common/string.h"
 #include "funcapi.h"
 #include "miscadmin.h"
 #include "nodes/nodeFuncs.h"
diff --git a/src/backend/utils/adt/orderedsetaggs.c b/src/backend/utils/adt/orderedsetaggs.c
index 5867f3df07..be9422dcfb 100644
--- a/src/backend/utils/adt/orderedsetaggs.c
+++ b/src/backend/utils/adt/orderedsetaggs.c
@@ -7,21 +7,20 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  *
  * IDENTIFICATION
  *	  src/backend/utils/adt/orderedsetaggs.c
  *
  *-------------------------------------------------------------------------
  */
 #include "postgres.h"
 
-#include <float.h>
 #include <math.h>
 
 #include "catalog/pg_aggregate.h"
 #include "catalog/pg_operator.h"
 #include "catalog/pg_type.h"
 #include "executor/executor.h"
 #include "miscadmin.h"
 #include "nodes/nodeFuncs.h"
 #include "optimizer/tlist.h"
 #include "utils/array.h"
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index 4b08cdb721..f1c78ffb65 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -91,21 +91,20 @@
  * collation.  Thus, in most cases where we are looking at statistics, we
  * should ignore the actual operator collation and use DEFAULT_COLLATION_OID.
  * We expect that the error induced by doing this is usually not large enough
  * to justify complicating matters.
  *----------
  */
 
 #include "postgres.h"
 
 #include <ctype.h>
-#include <float.h>
 #include <math.h>
 
 #include "access/brin.h"
 #include "access/gin.h"
 #include "access/htup_details.h"
 #include "access/sysattr.h"
 #include "catalog/index.h"
 #include "catalog/pg_am.h"
 #include "catalog/pg_collation.h"
 #include "catalog/pg_operator.h"
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 1d75caebe1..b98036f200 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -10,21 +10,20 @@
  * IDENTIFICATION
  *	  src/backend/utils/adt/timestamp.c
  *
  *-------------------------------------------------------------------------
  */
 
 #include "postgres.h"
 
 #include <ctype.h>
 #include <math.h>
-#include <float.h>
 #include <limits.h>
 #include <sys/time.h>
 
 #include "access/hash.h"
 #include "access/xact.h"
 #include "catalog/pg_type.h"
 #include "common/int128.h"
 #include "funcapi.h"
 #include "libpq/pqformat.h"
 #include "miscadmin.h"
diff --git a/src/backend/utils/misc/help_config.c b/src/backend/utils/misc/help_config.c
index 25f5c82804..871c535756 100644
--- a/src/backend/utils/misc/help_config.c
+++ b/src/backend/utils/misc/help_config.c
@@ -9,21 +9,20 @@
  *
  * Portions Copyright (c) 1996-2018, PostgreSQL Global Development Group
  *
  * IDENTIFICATION
  *	  src/backend/utils/misc/help_config.c
  *
  *-------------------------------------------------------------------------
  */
 #include "postgres.h"
 
-#include <float.h>
 #include <limits.h>
 #include <unistd.h>
 
 #include "utils/guc_tables.h"
 #include "utils/help_config.h"
 
 
 /*
  * This union allows us to mix the numerous different types of structs
  * that we are organizing.
diff --git a/src/include/port/win32_port.h b/src/include/port/win32_port.h
index d31c28f7d4..937d91f927 100644
--- a/src/include/port/win32_port.h
+++ b/src/include/port/win32_port.h
@@ -495,21 +495,23 @@ typedef long ssize_t;
 #else
 typedef __int64 ssize_t;
 #endif
 
 typedef unsigned short mode_t;
 
 #define F_OK 0
 #define W_OK 2
 #define R_OK 4
 
+/* Those functions are not presented on <math.h> on older versions. */
 #if (_MSC_VER < 1800)
+#include <float.h>
 #define isinf(x) ((_fpclass(x) == _FPCLASS_PINF) || (_fpclass(x) == _FPCLASS_NINF))
 #define isnan(x) _isnan(x)
 #endif
 
 /* Pulled from Makefile.port in MinGW */
 #define DLSUFFIX ".dll"
 
 #endif							/* _MSC_VER */
 
 #endif							/* PG_WIN32_PORT_H */
diff --git a/src/interfaces/ecpg/pgtypeslib/datetime.c b/src/interfaces/ecpg/pgtypeslib/datetime.c
index 1e692a5f9e..ed321febf2 100644
--- a/src/interfaces/ecpg/pgtypeslib/datetime.c
+++ b/src/interfaces/ecpg/pgtypeslib/datetime.c
@@ -1,17 +1,16 @@
 /* src/interfaces/ecpg/pgtypeslib/datetime.c */
 
 #include "postgres_fe.h"
 
 #include <time.h>
 #include <ctype.h>
-#include <float.h>
 #include <limits.h>
 
 #include "extern.h"
 #include "dt.h"
 #include "pgtypes_error.h"
 #include "pgtypes_date.h"
 
 date *
 PGTYPESdate_new(void)
 {
diff --git a/src/port/rint.c b/src/port/rint.c
index d27fdfa6b4..d59d9ab774 100644
--- a/src/port/rint.c
+++ b/src/port/rint.c
@@ -5,21 +5,20 @@
  *
  * By Pedro Gimeno Fortea, donated to the public domain
  *
  * IDENTIFICATION
  *	  src/port/rint.c
  *
  *-------------------------------------------------------------------------
  */
 #include "c.h"
 
-#include <float.h>
 #include <math.h>
 
 /*
  * Round to nearest integer, with halfway cases going to the nearest even.
  */
 double
 rint(double x)
 {
 	double		x_orig;
 	double		r;
diff --git a/src/port/snprintf.c b/src/port/snprintf.c
index 8358425980..a184134ee6 100644
--- a/src/port/snprintf.c
+++ b/src/port/snprintf.c
@@ -26,23 +26,20 @@
  * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  *
  * src/port/snprintf.c
  */
 
 #include "c.h"
 
 #include <ctype.h>
-#ifdef _MSC_VER
-#include <float.h>				/* for _isnan */
-#endif
 #include <limits.h>
 #include <math.h>
 #ifndef WIN32
 #include <sys/ioctl.h>
 #endif
 #include <sys/param.h>
 
 #ifndef NL_ARGMAX
 #define NL_ARGMAX 16
 #endif
diff --git a/src/test/regress/regress.c b/src/test/regress/regress.c
index 7060b6fbf3..97a50f30e7 100644
--- a/src/test/regress/regress.c
+++ b/src/test/regress/regress.c
@@ -9,21 +9,20 @@
  * Portions Copyright (c) 1996-2018, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * src/test/regress/regress.c
  *
  *-------------------------------------------------------------------------
  */
 
 #include "postgres.h"
 
-#include <float.h>
 #include <math.h>
 #include <signal.h>
 
 #include "access/htup_details.h"
 #include "access/transam.h"
 #include "access/tuptoaster.h"
 #include "access/xact.h"
 #include "catalog/pg_type.h"
 #include "commands/sequence.h"
 #include "commands/trigger.h"


^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: _isnan() on Windows
@ 2018-07-10 17:51  Alvaro Herrera <[email protected]>
  parent: Emre Hasegeli <[email protected]>
  0 siblings, 1 reply; 14+ messages in thread

From: Alvaro Herrera @ 2018-07-10 17:51 UTC (permalink / raw)
  To: Emre Hasegeli <[email protected]>; +Cc: pgsql-hackers; Thomas Munro <[email protected]>; Andres Freund <[email protected]>

On 2018-Jul-10, Emre Hasegeli wrote:

> isnan() function is evidently not present on <math.h> on Windows
> before Visual Studio 2013.  We define it on win32_port.h using
> _isnan().  However _isnan() is also not present.  It is on <float.h>.
> The patch is attached to include this from win32_port.h.
> 
> Thanks to Thomas Munro for point this out to me [1].  It is hard to
> notice this issue without testing the changes on Windows.

Oh, it looks like commits 33a7101281c6, 8e211f539146, 86dbbf20d849
(probably others) papered over this by the expedient of adding #include
<float.h> to random .c files rather than your patch, which seems the
proper fix.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services




^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: _isnan() on Windows
@ 2018-07-10 19:55  Tom Lane <[email protected]>
  parent: Alvaro Herrera <[email protected]>
  0 siblings, 1 reply; 14+ messages in thread

From: Tom Lane @ 2018-07-10 19:55 UTC (permalink / raw)
  To: Alvaro Herrera <[email protected]>; +Cc: Emre Hasegeli <[email protected]>; pgsql-hackers; Thomas Munro <[email protected]>; Andres Freund <[email protected]>

Alvaro Herrera <[email protected]> writes:
> On 2018-Jul-10, Emre Hasegeli wrote:
>> isnan() function is evidently not present on <math.h> on Windows
>> before Visual Studio 2013.  We define it on win32_port.h using
>> _isnan().  However _isnan() is also not present.  It is on <float.h>.
>> The patch is attached to include this from win32_port.h.
>> 
>> Thanks to Thomas Munro for point this out to me [1].  It is hard to
>> notice this issue without testing the changes on Windows.

> Oh, it looks like commits 33a7101281c6, 8e211f539146, 86dbbf20d849
> (probably others) papered over this by the expedient of adding #include
> <float.h> to random .c files rather than your patch, which seems the
> proper fix.

I disagree --- including <float.h> in c.h, as this would have us do,
seems like a huge expansion of the visibility of that header.  Moreover,
doing that only on Windows seems certain to lead to missing-header
problems in the reverse direction, ie patches developed on Windows
will fail elsewhere.

I think we should stick with the existing coding convention of including
that only in the specific .c files that need it.

			regards, tom lane




^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: _isnan() on Windows
@ 2018-07-10 20:07  Alvaro Herrera <[email protected]>
  parent: Tom Lane <[email protected]>
  0 siblings, 1 reply; 14+ messages in thread

From: Alvaro Herrera @ 2018-07-10 20:07 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Emre Hasegeli <[email protected]>; pgsql-hackers; Thomas Munro <[email protected]>; Andres Freund <[email protected]>

On 2018-Jul-10, Tom Lane wrote:

> Alvaro Herrera <[email protected]> writes:

> > Oh, it looks like commits 33a7101281c6, 8e211f539146, 86dbbf20d849
> > (probably others) papered over this by the expedient of adding #include
> > <float.h> to random .c files rather than your patch, which seems the
> > proper fix.
> 
> I disagree --- including <float.h> in c.h, as this would have us do,
> seems like a huge expansion of the visibility of that header.  Moreover,
> doing that only on Windows seems certain to lead to missing-header
> problems in the reverse direction, ie patches developed on Windows
> will fail elsewhere.

I don't think so, because that's only done for MSVC older than 2013.
Nobody uses that for new development anymore.  Downloading versions
prior to 2017 is difficult enough already.

> I think we should stick with the existing coding convention of including
> that only in the specific .c files that need it.

It seems saner to contain the ugliness to a port-specific file, where it
won't pollute two dozen files for no reason.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services




^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: _isnan() on Windows
@ 2018-07-10 20:23  Tom Lane <[email protected]>
  parent: Alvaro Herrera <[email protected]>
  0 siblings, 1 reply; 14+ messages in thread

From: Tom Lane @ 2018-07-10 20:23 UTC (permalink / raw)
  To: Alvaro Herrera <[email protected]>; +Cc: Emre Hasegeli <[email protected]>; pgsql-hackers; Thomas Munro <[email protected]>; Andres Freund <[email protected]>

Alvaro Herrera <[email protected]> writes:
> On 2018-Jul-10, Tom Lane wrote:
>> I disagree --- including <float.h> in c.h, as this would have us do,
>> seems like a huge expansion of the visibility of that header.  Moreover,
>> doing that only on Windows seems certain to lead to missing-header
>> problems in the reverse direction, ie patches developed on Windows
>> will fail elsewhere.

> I don't think so, because that's only done for MSVC older than 2013.
> Nobody uses that for new development anymore.

Hm.  OK, maybe it's all right given that.  I'm still a bit worried
about downsides, but no doubt the buildfarm will tell us.

			regards, tom lane




^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: _isnan() on Windows
@ 2018-07-11 00:37  Michael Paquier <[email protected]>
  parent: Tom Lane <[email protected]>
  0 siblings, 1 reply; 14+ messages in thread

From: Michael Paquier @ 2018-07-11 00:37 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; Emre Hasegeli <[email protected]>; pgsql-hackers; Thomas Munro <[email protected]>; Andres Freund <[email protected]>

On Tue, Jul 10, 2018 at 04:23:42PM -0400, Tom Lane wrote:
> Alvaro Herrera <[email protected]> writes:
>> On 2018-Jul-10, Tom Lane wrote:
>>> I disagree --- including <float.h> in c.h, as this would have us do,
>>> seems like a huge expansion of the visibility of that header.  Moreover,
>>> doing that only on Windows seems certain to lead to missing-header
>>> problems in the reverse direction, ie patches developed on Windows
>>> will fail elsewhere.
> 
>> I don't think so, because that's only done for MSVC older than 2013.
>> Nobody uses that for new development anymore.
> 
> Hm.  OK, maybe it's all right given that.  I'm still a bit worried
> about downsides, but no doubt the buildfarm will tell us.

It seems to me that this patch is a good idea.  Any objections if I take
care of it?  I have a Windows VM with only MSVC 2015 if I recall
correctly though...
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: _isnan() on Windows
@ 2018-07-11 13:13  Alvaro Herrera <[email protected]>
  parent: Michael Paquier <[email protected]>
  0 siblings, 1 reply; 14+ messages in thread

From: Alvaro Herrera @ 2018-07-11 13:13 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; +Cc: Tom Lane <[email protected]>; Emre Hasegeli <[email protected]>; pgsql-hackers; Thomas Munro <[email protected]>; Andres Freund <[email protected]>

On 2018-Jul-11, Michael Paquier wrote:

> On Tue, Jul 10, 2018 at 04:23:42PM -0400, Tom Lane wrote:
> > Alvaro Herrera <[email protected]> writes:
> >> On 2018-Jul-10, Tom Lane wrote:
> >>> I disagree --- including <float.h> in c.h, as this would have us do,
> >>> seems like a huge expansion of the visibility of that header.  Moreover,
> >>> doing that only on Windows seems certain to lead to missing-header
> >>> problems in the reverse direction, ie patches developed on Windows
> >>> will fail elsewhere.
> > 
> >> I don't think so, because that's only done for MSVC older than 2013.
> >> Nobody uses that for new development anymore.
> > 
> > Hm.  OK, maybe it's all right given that.  I'm still a bit worried
> > about downsides, but no doubt the buildfarm will tell us.
> 
> It seems to me that this patch is a good idea.  Any objections if I take
> care of it?  I have a Windows VM with only MSVC 2015 if I recall
> correctly though...

I just pushed it before seeing your message.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services




^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: _isnan() on Windows
@ 2018-07-12 02:16  Michael Paquier <[email protected]>
  parent: Alvaro Herrera <[email protected]>
  0 siblings, 1 reply; 14+ messages in thread

From: Michael Paquier @ 2018-07-12 02:16 UTC (permalink / raw)
  To: Alvaro Herrera <[email protected]>; +Cc: Tom Lane <[email protected]>; Emre Hasegeli <[email protected]>; pgsql-hackers; Thomas Munro <[email protected]>; Andres Freund <[email protected]>

On Wed, Jul 11, 2018 at 09:13:40AM -0400, Alvaro Herrera wrote:
> I just pushed it before seeing your message.

Fine as well, thanks for picking this up.  The buildfarm shows no
failures about this patch.
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: _isnan() on Windows
@ 2018-07-12 14:20  Tom Lane <[email protected]>
  parent: Michael Paquier <[email protected]>
  0 siblings, 1 reply; 14+ messages in thread

From: Tom Lane @ 2018-07-12 14:20 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; Emre Hasegeli <[email protected]>; pgsql-hackers; Thomas Munro <[email protected]>; Andres Freund <[email protected]>

Michael Paquier <[email protected]> writes:
> On Wed, Jul 11, 2018 at 09:13:40AM -0400, Alvaro Herrera wrote:
>> I just pushed it before seeing your message.

> Fine as well, thanks for picking this up.  The buildfarm shows no
> failures about this patch.

I scraped all the compiler warnings from the buildfarm this morning,
and I see no new ones that could be blamed on this change, so I think
we're good.

bowerbird and hamerkop have some gripes like this:

 bowerbird     | c:\perl64\lib\core\win32.h(218): warning C4005: 'isnan' : macro redefinition (src/pl/plperl/SPI.c) [G:\prog\bf\root\HEAD\pgsql.build\plperl.vcxproj]
 bowerbird     | c:\perl64\lib\core\win32.h(218): warning C4005: 'isnan' : macro redefinition (src/pl/plperl/Util.c) [G:\prog\bf\root\HEAD\pgsql.build\plperl.vcxproj]
 bowerbird     | c:\perl64\lib\core\win32.h(218): warning C4005: 'isnan' : macro redefinition (src/pl/plperl/plperl.c) [G:\prog\bf\root\HEAD\pgsql.build\plperl.vcxproj]
 bowerbird     | c:\perl64\lib\core\win32.h(218): warning C4005: 'isnan' : macro redefinition [G:\prog\bf\root\HEAD\pgsql.build\hstore_plperl.vcxproj]
 bowerbird     | c:\perl64\lib\core\win32.h(218): warning C4005: 'isnan' : macro redefinition [G:\prog\bf\root\HEAD\pgsql.build\jsonb_plperl.vcxproj]
 bowerbird     |   c:\perl64\lib\core\win32.h(218): warning C4005: 'isnan' : macro redefinition (src/pl/plperl/SPI.c) [G:\prog\bf\root\HEAD\pgsql.build\plperl.vcxproj]
 bowerbird     |   c:\perl64\lib\core\win32.h(218): warning C4005: 'isnan' : macro redefinition (src/pl/plperl/Util.c) [G:\prog\bf\root\HEAD\pgsql.build\plperl.vcxproj]
 bowerbird     |   c:\perl64\lib\core\win32.h(218): warning C4005: 'isnan' : macro redefinition (src/pl/plperl/plperl.c) [G:\prog\bf\root\HEAD\pgsql.build\plperl.vcxproj]
 bowerbird     |   c:\perl64\lib\core\win32.h(218): warning C4005: 'isnan' : macro redefinition [G:\prog\bf\root\HEAD\pgsql.build\hstore_plperl.vcxproj]
 bowerbird     |   c:\perl64\lib\core\win32.h(218): warning C4005: 'isnan' : macro redefinition [G:\prog\bf\root\HEAD\pgsql.build\jsonb_plperl.vcxproj]

but those were there before too.  Not sure if there's anything
we can/should try to do about that.

			regards, tom lane




^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: _isnan() on Windows
@ 2018-07-12 14:33  Andrew Dunstan <[email protected]>
  parent: Tom Lane <[email protected]>
  0 siblings, 1 reply; 14+ messages in thread

From: Andrew Dunstan @ 2018-07-12 14:33 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; Michael Paquier <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; Emre Hasegeli <[email protected]>; pgsql-hackers; Thomas Munro <[email protected]>; Andres Freund <[email protected]>



On 07/12/2018 10:20 AM, Tom Lane wrote:
> Michael Paquier <[email protected]> writes:
>> On Wed, Jul 11, 2018 at 09:13:40AM -0400, Alvaro Herrera wrote:
>>> I just pushed it before seeing your message.
>> Fine as well, thanks for picking this up.  The buildfarm shows no
>> failures about this patch.
> I scraped all the compiler warnings from the buildfarm this morning,
> and I see no new ones that could be blamed on this change, so I think
> we're good.
>
> bowerbird and hamerkop have some gripes like this:
>
>   bowerbird     | c:\perl64\lib\core\win32.h(218): warning C4005: 'isnan' : macro redefinition (src/pl/plperl/SPI.c) [G:\prog\bf\root\HEAD\pgsql.build\plperl.vcxproj]
>   bowerbird     | c:\perl64\lib\core\win32.h(218): warning C4005: 'isnan' : macro redefinition (src/pl/plperl/Util.c) [G:\prog\bf\root\HEAD\pgsql.build\plperl.vcxproj]
>   bowerbird     | c:\perl64\lib\core\win32.h(218): warning C4005: 'isnan' : macro redefinition (src/pl/plperl/plperl.c) [G:\prog\bf\root\HEAD\pgsql.build\plperl.vcxproj]
>   bowerbird     | c:\perl64\lib\core\win32.h(218): warning C4005: 'isnan' : macro redefinition [G:\prog\bf\root\HEAD\pgsql.build\hstore_plperl.vcxproj]
>   bowerbird     | c:\perl64\lib\core\win32.h(218): warning C4005: 'isnan' : macro redefinition [G:\prog\bf\root\HEAD\pgsql.build\jsonb_plperl.vcxproj]
>   bowerbird     |   c:\perl64\lib\core\win32.h(218): warning C4005: 'isnan' : macro redefinition (src/pl/plperl/SPI.c) [G:\prog\bf\root\HEAD\pgsql.build\plperl.vcxproj]
>   bowerbird     |   c:\perl64\lib\core\win32.h(218): warning C4005: 'isnan' : macro redefinition (src/pl/plperl/Util.c) [G:\prog\bf\root\HEAD\pgsql.build\plperl.vcxproj]
>   bowerbird     |   c:\perl64\lib\core\win32.h(218): warning C4005: 'isnan' : macro redefinition (src/pl/plperl/plperl.c) [G:\prog\bf\root\HEAD\pgsql.build\plperl.vcxproj]
>   bowerbird     |   c:\perl64\lib\core\win32.h(218): warning C4005: 'isnan' : macro redefinition [G:\prog\bf\root\HEAD\pgsql.build\hstore_plperl.vcxproj]
>   bowerbird     |   c:\perl64\lib\core\win32.h(218): warning C4005: 'isnan' : macro redefinition [G:\prog\bf\root\HEAD\pgsql.build\jsonb_plperl.vcxproj]
>
> but those were there before too.  Not sure if there's anything
> we can/should try to do about that.
>
> 			


We actually undef a bunch of things in plperl.h to keep the compiler 
quiet. Maybe we need to add this to the list?

cheers

andrew

-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services





^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: _isnan() on Windows
@ 2018-07-12 14:38  Tom Lane <[email protected]>
  parent: Andrew Dunstan <[email protected]>
  0 siblings, 1 reply; 14+ messages in thread

From: Tom Lane @ 2018-07-12 14:38 UTC (permalink / raw)
  To: Andrew Dunstan <[email protected]>; +Cc: Michael Paquier <[email protected]>; Alvaro Herrera <[email protected]>; Emre Hasegeli <[email protected]>; pgsql-hackers; Thomas Munro <[email protected]>; Andres Freund <[email protected]>

Andrew Dunstan <[email protected]> writes:
> On 07/12/2018 10:20 AM, Tom Lane wrote:
>> bowerbird and hamerkop have some gripes like this:
>> 
>> bowerbird     | c:\perl64\lib\core\win32.h(218): warning C4005: 'isnan' : macro redefinition (src/pl/plperl/SPI.c) [G:\prog\bf\root\HEAD\pgsql.build\plperl.vcxproj]

> We actually undef a bunch of things in plperl.h to keep the compiler 
> quiet. Maybe we need to add this to the list?

Perhaps.  But how do we tell the platforms where we should do that
from the ones where we shouldn't?

			regards, tom lane




^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: _isnan() on Windows
@ 2018-07-12 15:28  Andrew Dunstan <[email protected]>
  parent: Tom Lane <[email protected]>
  0 siblings, 0 replies; 14+ messages in thread

From: Andrew Dunstan @ 2018-07-12 15:28 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Michael Paquier <[email protected]>; Alvaro Herrera <[email protected]>; Emre Hasegeli <[email protected]>; pgsql-hackers; Thomas Munro <[email protected]>; Andres Freund <[email protected]>



On 07/12/2018 10:38 AM, Tom Lane wrote:
> Andrew Dunstan <[email protected]> writes:
>> On 07/12/2018 10:20 AM, Tom Lane wrote:
>>> bowerbird and hamerkop have some gripes like this:
>>>
>>> bowerbird     | c:\perl64\lib\core\win32.h(218): warning C4005: 'isnan' : macro redefinition (src/pl/plperl/SPI.c) [G:\prog\bf\root\HEAD\pgsql.build\plperl.vcxproj]
>> We actually undef a bunch of things in plperl.h to keep the compiler
>> quiet. Maybe we need to add this to the list?
> Perhaps.  But how do we tell the platforms where we should do that
> from the ones where we shouldn't?
>
> 			

In the _MSCVER section:

#ifdef isnan
#undef isnan
#endif

By inspection the perl header is just defining it to _isnan, for every 
MSC version.

cheers

andrew

-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services





^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* [PATCH v12 11/17] table_scan_bitmap_next_block counts lossy and exact pages
@ 2024-03-22 21:09  Melanie Plageman <[email protected]>
  0 siblings, 0 replies; 14+ messages in thread

From: Melanie Plageman @ 2024-03-22 21:09 UTC (permalink / raw)

Now that the table_scan_bitmap_next_block() callback only returns false
when the bitmap is exhausted, it is simpler to move the management of
the lossy and exact page counters into it. We will eventually remove
this callback and table_scan_bitmap_next_tuple() will update those
counters when a new block is read in.
---
 src/backend/access/heap/heapam_handler.c  |  8 ++++++--
 src/backend/executor/nodeBitmapHeapscan.c |  9 ++-------
 src/include/access/tableam.h              | 21 +++++++++++++--------
 3 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 38c4fd5011c..46f1a374252 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2199,7 +2199,8 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
 
 static bool
 heapam_scan_bitmap_next_block(TableScanDesc scan,
-							  bool *recheck, bool *lossy, BlockNumber *blockno)
+							  bool *recheck, BlockNumber *blockno,
+							  long *lossy_pages, long *exact_pages)
 {
 	HeapScanDesc hscan = (HeapScanDesc) scan;
 	BlockNumber block;
@@ -2349,7 +2350,10 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
 	Assert(ntup <= MaxHeapTuplesPerPage);
 	hscan->rs_ntuples = ntup;
 
-	*lossy = tbmres->ntuples < 0;
+	if (tbmres->ntuples < 0)
+		(*lossy_pages)++;
+	else
+		(*exact_pages)++;
 
 	/*
 	 * Return true to indicate that a valid block was found and the bitmap is
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 8b6f22bc3b6..fb79f57d7a6 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -119,7 +119,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
 {
 	ExprContext *econtext;
 	TableScanDesc scan;
-	bool		lossy;
 	TIDBitmap  *tbm;
 	TupleTableSlot *slot;
 	ParallelBitmapHeapState *pstate = node->pstate;
@@ -298,14 +297,10 @@ new_page:
 
 		BitmapAdjustPrefetchIterator(node);
 
-		if (!table_scan_bitmap_next_block(scan, &node->recheck, &lossy, &node->blockno))
+		if (!table_scan_bitmap_next_block(scan, &node->recheck, &node->blockno,
+										  &node->lossy_pages, &node->exact_pages))
 			break;
 
-		if (lossy)
-			node->lossy_pages++;
-		else
-			node->exact_pages++;
-
 		/*
 		 * If serial, we can error out if the the prefetch block doesn't stay
 		 * ahead of the current block.
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index 592a8421bdf..a06ed271eb6 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -804,8 +804,8 @@ typedef struct TableAmRoutine
 	 * work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might
 	 * make sense to perform tuple visibility checks at this time).
 	 *
-	 * lossy indicates whether or not the block's representation in the bitmap
-	 * is lossy or exact.
+	 * lossy_pages is incremented if the block's representation in the bitmap
+	 * is lossy, otherwise, exact_pages is incremented.
 	 *
 	 * XXX: Currently this may only be implemented if the AM uses md.c as its
 	 * storage manager, and uses ItemPointer->ip_blkid in a manner that maps
@@ -822,8 +822,10 @@ typedef struct TableAmRoutine
 	 * scan_bitmap_next_tuple need to exist, or neither.
 	 */
 	bool		(*scan_bitmap_next_block) (TableScanDesc scan,
-										   bool *recheck, bool *lossy,
-										   BlockNumber *blockno);
+										   bool *recheck,
+										   BlockNumber *blockno,
+										   long *lossy_pages,
+										   long *exact_pages);
 
 	/*
 	 * Fetch the next tuple of a bitmap table scan into `slot` and return true
@@ -2019,15 +2021,17 @@ table_relation_estimate_size(Relation rel, int32 *attr_widths,
 /*
  * Prepare to fetch / check / return tuples as part of a bitmap table scan.
  * `scan` needs to have been started via table_beginscan_bm(). Returns false if
- * there are no more blocks in the bitmap, true otherwise. lossy is set to true
- * if bitmap is lossy for the selected block and false otherwise.
+ * there are no more blocks in the bitmap, true otherwise. lossy_pages is
+ * incremented if bitmap is lossy for the selected block and exact_pages is
+ * incremented otherwise.
  *
  * Note, this is an optionally implemented function, therefore should only be
  * used after verifying the presence (at plan time or such).
  */
 static inline bool
 table_scan_bitmap_next_block(TableScanDesc scan,
-							 bool *recheck, bool *lossy, BlockNumber *blockno)
+							 bool *recheck, BlockNumber *blockno,
+							 long *lossy_pages, long *exact_pages)
 {
 	/*
 	 * We don't expect direct calls to table_scan_bitmap_next_block with valid
@@ -2038,7 +2042,8 @@ table_scan_bitmap_next_block(TableScanDesc scan,
 		elog(ERROR, "unexpected table_scan_bitmap_next_block call during logical decoding");
 
 	return scan->rs_rd->rd_tableam->scan_bitmap_next_block(scan, recheck,
-														   lossy, blockno);
+														   blockno, lossy_pages,
+														   exact_pages);
 }
 
 /*
-- 
2.40.1


--6jpz2j246qmht4bt
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v12-0012-Hard-code-TBMIterateResult-offsets-array-size.patch"



^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* [PATCH v3] Introduce ternary reloptions
@ 2026-01-16 15:04  Álvaro Herrera <[email protected]>
  0 siblings, 0 replies; 14+ messages in thread

From: Álvaro Herrera @ 2026-01-16 15:04 UTC (permalink / raw)

Introduce ternary reloption as a replacement for current
`vacuum_truncate` implementation. Remove the `vacuum_truncate_set`
separate flag and use TERNARY_UNSET instead.

This could also be used for other options such as `vacuum_index_cleanup`
and `buffering`, but lets get the scaffolding in first.

Discussion: https://postgr.es/m/3474141.usfYGdeWWP@thinkpad-pgpro
---
 src/backend/access/common/reloptions.c   | 137 ++++++++++++++++++-----
 src/backend/commands/vacuum.c            |   4 +-
 src/include/access/reloptions.h          |  27 ++---
 src/include/postgres.h                   |  15 +++
 src/include/utils/rel.h                  |   3 +-
 src/test/regress/expected/reloptions.out |  36 ++++++
 src/test/regress/sql/reloptions.sql      |  21 ++++
 src/tools/pgindent/typedefs.list         |   2 +
 8 files changed, 202 insertions(+), 43 deletions(-)

diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 0b83f98ed5f..6f1f577581a 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -40,9 +40,9 @@
  *
  * To add an option:
  *
- * (i) decide on a type (bool, integer, real, enum, string), name, default
- * value, upper and lower bounds (if applicable); for strings, consider a
- * validation routine.
+ * (i) decide on a type (bool, ternary, integer, real, enum, string), name,
+ * default value, upper and lower bounds (if applicable); for strings,
+ * consider a validation routine.
  * (ii) add a record below (or use add_<type>_reloption).
  * (iii) add it to the appropriate options struct (perhaps StdRdOptions)
  * (iv) add it to the appropriate handling routine (perhaps
@@ -147,15 +147,6 @@ static relopt_bool boolRelOpts[] =
 		},
 		false
 	},
-	{
-		{
-			"vacuum_truncate",
-			"Enables vacuum to truncate empty pages at the end of this table",
-			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST,
-			ShareUpdateExclusiveLock
-		},
-		true
-	},
 	{
 		{
 			"deduplicate_items",
@@ -170,6 +161,25 @@ static relopt_bool boolRelOpts[] =
 	{{NULL}}
 };
 
+static relopt_ternary ternaryRelOpts[] =
+{
+	{
+		{
+			"vacuum_truncate",
+			"Enables vacuum to truncate empty pages at the end of this table",
+			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST,
+			ShareUpdateExclusiveLock
+		},
+		TERNARY_UNSET
+	},
+	/* list terminator */
+	{
+		{
+			NULL
+		}
+	}
+};
+
 static relopt_int intRelOpts[] =
 {
 	{
@@ -609,6 +619,13 @@ initialize_reloptions(void)
 								   boolRelOpts[i].gen.lockmode));
 		j++;
 	}
+	for (i = 0; ternaryRelOpts[i].gen.name; i++)
+	{
+		Assert(DoLockModesConflict(ternaryRelOpts[i].gen.lockmode,
+								   ternaryRelOpts[i].gen.lockmode));
+		j++;
+	}
+
 	for (i = 0; intRelOpts[i].gen.name; i++)
 	{
 		Assert(DoLockModesConflict(intRelOpts[i].gen.lockmode,
@@ -649,6 +666,14 @@ initialize_reloptions(void)
 		j++;
 	}
 
+	for (i = 0; ternaryRelOpts[i].gen.name; i++)
+	{
+		relOpts[j] = &ternaryRelOpts[i].gen;
+		relOpts[j]->type = RELOPT_TYPE_TERNARY;
+		relOpts[j]->namelen = strlen(relOpts[j]->name);
+		j++;
+	}
+
 	for (i = 0; intRelOpts[i].gen.name; i++)
 	{
 		relOpts[j] = &intRelOpts[i].gen;
@@ -809,6 +834,9 @@ allocate_reloption(bits32 kinds, int type, const char *name, const char *desc,
 		case RELOPT_TYPE_BOOL:
 			size = sizeof(relopt_bool);
 			break;
+		case RELOPT_TYPE_TERNARY:
+			size = sizeof(relopt_ternary);
+			break;
 		case RELOPT_TYPE_INT:
 			size = sizeof(relopt_int);
 			break;
@@ -892,6 +920,57 @@ add_local_bool_reloption(local_relopts *relopts, const char *name,
 	add_local_reloption(relopts, (relopt_gen *) newoption, offset);
 }
 
+/*
+ * init_ternary_reloption
+ *		Allocate and initialize a new ternary reloption
+ */
+static relopt_ternary *
+init_ternary_reloption(bits32 kinds, const char *name, const char *desc,
+					   pg_ternary default_val, LOCKMODE lockmode)
+{
+	relopt_ternary *newoption;
+
+	newoption = (relopt_ternary *)
+		allocate_reloption(kinds, RELOPT_TYPE_TERNARY, name, desc, lockmode);
+	newoption->default_val = default_val;
+
+	return newoption;
+}
+
+/*
+ * add_ternary_reloption
+ *		Add a new ternary reloption
+ */
+void
+add_ternary_reloption(bits32 kinds, const char *name, const char *desc,
+					  pg_ternary default_val, LOCKMODE lockmode)
+{
+	relopt_ternary *newoption;
+
+	newoption =
+		init_ternary_reloption(kinds, name, desc, default_val, lockmode);
+
+	add_reloption((relopt_gen *) newoption);
+}
+
+/*
+ * add_local_ternary_reloption
+ *		Add a new ternary local reloption
+ *
+ * 'offset' is offset of ternary-typed field.
+ */
+void
+add_local_ternary_reloption(local_relopts *relopts, const char *name,
+							const char *desc, pg_ternary default_val,
+							int offset)
+{
+	relopt_ternary *newoption;
+
+	newoption =
+		init_ternary_reloption(RELOPT_KIND_LOCAL, name, desc, default_val, 0);
+
+	add_local_reloption(relopts, (relopt_gen *) newoption, offset);
+}
 
 /*
  * init_real_reloption
@@ -1626,6 +1705,19 @@ parse_one_reloption(relopt_value *option, char *text_str, int text_len,
 									option->gen->name, value)));
 			}
 			break;
+		case RELOPT_TYPE_TERNARY:
+			{
+				bool		b;
+
+				parsed = parse_bool(value, &b);
+				option->values.ternary_val = b ? TERNARY_TRUE : TERNARY_FALSE;
+				if (validate && !parsed)
+					ereport(ERROR,
+							(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+							 errmsg("invalid value for ternary option \"%s\": %s",
+									option->gen->name, value)));
+			}
+			break;
 		case RELOPT_TYPE_INT:
 			{
 				relopt_int *optint = (relopt_int *) option->gen;
@@ -1789,17 +1881,6 @@ fillRelOptions(void *rdopts, Size basesize,
 				char	   *itempos = ((char *) rdopts) + elems[j].offset;
 				char	   *string_val;
 
-				/*
-				 * If isset_offset is provided, store whether the reloption is
-				 * set there.
-				 */
-				if (elems[j].isset_offset > 0)
-				{
-					char	   *setpos = ((char *) rdopts) + elems[j].isset_offset;
-
-					*(bool *) setpos = options[i].isset;
-				}
-
 				switch (options[i].gen->type)
 				{
 					case RELOPT_TYPE_BOOL:
@@ -1807,6 +1888,11 @@ fillRelOptions(void *rdopts, Size basesize,
 							options[i].values.bool_val :
 							((relopt_bool *) options[i].gen)->default_val;
 						break;
+					case RELOPT_TYPE_TERNARY:
+						*(pg_ternary *) itempos = options[i].isset ?
+							options[i].values.ternary_val :
+							((relopt_ternary *) options[i].gen)->default_val;
+						break;
 					case RELOPT_TYPE_INT:
 						*(int *) itempos = options[i].isset ?
 							options[i].values.int_val :
@@ -1923,8 +2009,8 @@ default_reloptions(Datum reloptions, bool validate, relopt_kind kind)
 		offsetof(StdRdOptions, parallel_workers)},
 		{"vacuum_index_cleanup", RELOPT_TYPE_ENUM,
 		offsetof(StdRdOptions, vacuum_index_cleanup)},
-		{"vacuum_truncate", RELOPT_TYPE_BOOL,
-		offsetof(StdRdOptions, vacuum_truncate), offsetof(StdRdOptions, vacuum_truncate_set)},
+		{"vacuum_truncate", RELOPT_TYPE_TERNARY,
+		offsetof(StdRdOptions, vacuum_truncate)},
 		{"vacuum_max_eager_freeze_failure_rate", RELOPT_TYPE_REAL,
 		offsetof(StdRdOptions, vacuum_max_eager_freeze_failure_rate)}
 	};
@@ -2004,7 +2090,6 @@ build_local_reloptions(local_relopts *relopts, Datum options, bool validate)
 		elems[i].optname = opt->option->name;
 		elems[i].opttype = opt->option->type;
 		elems[i].offset = opt->offset;
-		elems[i].isset_offset = 0;	/* not supported for local relopts yet */
 
 		i++;
 	}
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index aa4fbec143f..696eab9bd97 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -2224,9 +2224,9 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams params,
 	{
 		StdRdOptions *opts = (StdRdOptions *) rel->rd_options;
 
-		if (opts && opts->vacuum_truncate_set)
+		if (opts && opts->vacuum_truncate != TERNARY_UNSET)
 		{
-			if (opts->vacuum_truncate)
+			if (opts->vacuum_truncate == TERNARY_TRUE)
 				params.truncate = VACOPTVALUE_ENABLED;
 			else
 				params.truncate = VACOPTVALUE_DISABLED;
diff --git a/src/include/access/reloptions.h b/src/include/access/reloptions.h
index 2f08e1b0cf0..dfbef2babf2 100644
--- a/src/include/access/reloptions.h
+++ b/src/include/access/reloptions.h
@@ -29,6 +29,7 @@
 typedef enum relopt_type
 {
 	RELOPT_TYPE_BOOL,
+	RELOPT_TYPE_TERNARY,		/* on, off, unset */
 	RELOPT_TYPE_INT,
 	RELOPT_TYPE_REAL,
 	RELOPT_TYPE_ENUM,
@@ -80,6 +81,7 @@ typedef struct relopt_value
 	union
 	{
 		bool		bool_val;
+		pg_ternary	ternary_val;
 		int			int_val;
 		double		real_val;
 		int			enum_val;
@@ -94,6 +96,12 @@ typedef struct relopt_bool
 	bool		default_val;
 } relopt_bool;
 
+typedef struct relopt_ternary
+{
+	relopt_gen	gen;
+	int			default_val;
+} relopt_ternary;
+
 typedef struct relopt_int
 {
 	relopt_gen	gen;
@@ -152,19 +160,6 @@ typedef struct
 	const char *optname;		/* option's name */
 	relopt_type opttype;		/* option's datatype */
 	int			offset;			/* offset of field in result struct */
-
-	/*
-	 * isset_offset is an optional offset of a field in the result struct that
-	 * stores whether the option is explicitly set for the relation or if it
-	 * just picked up the default value.  In most cases, this can be
-	 * accomplished by giving the reloption a special out-of-range default
-	 * value (e.g., some integer reloptions use -2), but this isn't always
-	 * possible.  For example, a Boolean reloption cannot be given an
-	 * out-of-range default, so we need another way to discover the source of
-	 * its value.  This offset is only used if given a value greater than
-	 * zero.
-	 */
-	int			isset_offset;
 } relopt_parse_elt;
 
 /* Local reloption definition */
@@ -195,6 +190,9 @@ typedef struct local_relopts
 extern relopt_kind add_reloption_kind(void);
 extern void add_bool_reloption(bits32 kinds, const char *name, const char *desc,
 							   bool default_val, LOCKMODE lockmode);
+extern void add_ternary_reloption(bits32 kinds, const char *name,
+								  const char *desc, pg_ternary default_val,
+								  LOCKMODE lockmode);
 extern void add_int_reloption(bits32 kinds, const char *name, const char *desc,
 							  int default_val, int min_val, int max_val,
 							  LOCKMODE lockmode);
@@ -214,6 +212,9 @@ extern void register_reloptions_validator(local_relopts *relopts,
 extern void add_local_bool_reloption(local_relopts *relopts, const char *name,
 									 const char *desc, bool default_val,
 									 int offset);
+extern void add_local_ternary_reloption(local_relopts *relopts,
+										const char *name, const char *desc,
+										pg_ternary default_val, int offset);
 extern void add_local_int_reloption(local_relopts *relopts, const char *name,
 									const char *desc, int default_val,
 									int min_val, int max_val, int offset);
diff --git a/src/include/postgres.h b/src/include/postgres.h
index 7d93fbce709..47c88780776 100644
--- a/src/include/postgres.h
+++ b/src/include/postgres.h
@@ -543,6 +543,21 @@ Float8GetDatum(float8 X)
  * ----------------------------------------------------------------
  */
 
+/*
+ * pg_ternary
+ *		Boolean value with an extra "unset" value
+ *
+ * This enum can be used for values that want to distinguish between true,
+ * false, and unset.
+*/
+
+typedef enum pg_ternary
+{
+	TERNARY_FALSE = 0,
+	TERNARY_TRUE = 1,
+	TERNARY_UNSET = -1
+} pg_ternary;
+
 /*
  * NON_EXEC_STATIC: It's sometimes useful to define a variable or function
  * that is normally static but extern when using EXEC_BACKEND (see
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index d03ab247788..5feb18a5373 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -347,8 +347,7 @@ typedef struct StdRdOptions
 	bool		user_catalog_table; /* use as an additional catalog relation */
 	int			parallel_workers;	/* max number of parallel workers */
 	StdRdOptIndexCleanup vacuum_index_cleanup;	/* controls index vacuuming */
-	bool		vacuum_truncate;	/* enables vacuum to truncate a relation */
-	bool		vacuum_truncate_set;	/* whether vacuum_truncate is set */
+	pg_ternary		vacuum_truncate;	/* enables vacuum to truncate a relation */
 
 	/*
 	 * Fraction of pages in a relation that vacuum can eagerly scan and fail
diff --git a/src/test/regress/expected/reloptions.out b/src/test/regress/expected/reloptions.out
index 9de19b4e3f1..1c99f79ab01 100644
--- a/src/test/regress/expected/reloptions.out
+++ b/src/test/regress/expected/reloptions.out
@@ -98,6 +98,42 @@ SELECT reloptions FROM pg_class WHERE oid = 'reloptions_test'::regclass;
  {fillfactor=13,autovacuum_enabled=false}
 (1 row)
 
+-- Tests for future (FIXME) ternary options
+-- behave as boolean option: accept unassigned name and truncated value
+DROP TABLE reloptions_test;
+CREATE TABLE reloptions_test(i INT) WITH (vacuum_truncate);
+SELECT reloptions FROM pg_class WHERE oid = 'reloptions_test'::regclass;
+       reloptions       
+------------------------
+ {vacuum_truncate=true}
+(1 row)
+
+DROP TABLE reloptions_test;
+CREATE TABLE reloptions_test(i INT) WITH (vacuum_truncate=FaLS);
+SELECT reloptions FROM pg_class WHERE oid = 'reloptions_test'::regclass;
+       reloptions       
+------------------------
+ {vacuum_truncate=fals}
+(1 row)
+
+-- preferred "true" alias is used when storing
+DROP TABLE reloptions_test;
+CREATE TABLE reloptions_test(i INT) WITH (vacuum_index_cleanup=on);
+SELECT reloptions FROM pg_class WHERE oid = 'reloptions_test'::regclass;
+        reloptions         
+---------------------------
+ {vacuum_index_cleanup=on}
+(1 row)
+
+-- custom "third" value is available
+DROP TABLE reloptions_test;
+CREATE TABLE reloptions_test(i INT) WITH (vacuum_index_cleanup=auto);
+SELECT reloptions FROM pg_class WHERE oid = 'reloptions_test'::regclass;
+         reloptions          
+-----------------------------
+ {vacuum_index_cleanup=auto}
+(1 row)
+
 -- Test vacuum_truncate option
 DROP TABLE reloptions_test;
 CREATE TEMP TABLE reloptions_test(i INT NOT NULL, j text)
diff --git a/src/test/regress/sql/reloptions.sql b/src/test/regress/sql/reloptions.sql
index 24fbe0b478d..f5980dafcbc 100644
--- a/src/test/regress/sql/reloptions.sql
+++ b/src/test/regress/sql/reloptions.sql
@@ -59,6 +59,27 @@ UPDATE pg_class
 ALTER TABLE reloptions_test RESET (illegal_option);
 SELECT reloptions FROM pg_class WHERE oid = 'reloptions_test'::regclass;
 
+-- Tests for future (FIXME) ternary options
+
+-- behave as boolean option: accept unassigned name and truncated value
+DROP TABLE reloptions_test;
+CREATE TABLE reloptions_test(i INT) WITH (vacuum_truncate);
+SELECT reloptions FROM pg_class WHERE oid = 'reloptions_test'::regclass;
+
+DROP TABLE reloptions_test;
+CREATE TABLE reloptions_test(i INT) WITH (vacuum_truncate=FaLS);
+SELECT reloptions FROM pg_class WHERE oid = 'reloptions_test'::regclass;
+
+-- preferred "true" alias is used when storing
+DROP TABLE reloptions_test;
+CREATE TABLE reloptions_test(i INT) WITH (vacuum_index_cleanup=on);
+SELECT reloptions FROM pg_class WHERE oid = 'reloptions_test'::regclass;
+
+-- custom "third" value is available
+DROP TABLE reloptions_test;
+CREATE TABLE reloptions_test(i INT) WITH (vacuum_index_cleanup=auto);
+SELECT reloptions FROM pg_class WHERE oid = 'reloptions_test'::regclass;
+
 -- Test vacuum_truncate option
 DROP TABLE reloptions_test;
 
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 3f3a888fd0e..1c8610fd46c 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -3952,6 +3952,7 @@ pg_sha512_ctx
 pg_snapshot
 pg_special_case
 pg_stack_base_t
+pg_ternary
 pg_time_t
 pg_time_usec_t
 pg_tz
@@ -4079,6 +4080,7 @@ relopt_kind
 relopt_parse_elt
 relopt_real
 relopt_string
+relopt_ternary
 relopt_type
 relopt_value
 relopts_validator
-- 
2.47.3


--c36t3fndxhkzbl5z--






^ permalink  raw  reply  [nested|flat] 14+ messages in thread


end of thread, other threads:[~2026-01-16 15:04 UTC | newest]

Thread overview: 14+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2018-07-10 17:36 _isnan() on Windows Emre Hasegeli <[email protected]>
2018-07-10 17:51 ` Alvaro Herrera <[email protected]>
2018-07-10 19:55   ` Tom Lane <[email protected]>
2018-07-10 20:07     ` Alvaro Herrera <[email protected]>
2018-07-10 20:23       ` Tom Lane <[email protected]>
2018-07-11 00:37         ` Michael Paquier <[email protected]>
2018-07-11 13:13           ` Alvaro Herrera <[email protected]>
2018-07-12 02:16             ` Michael Paquier <[email protected]>
2018-07-12 14:20               ` Tom Lane <[email protected]>
2018-07-12 14:33                 ` Andrew Dunstan <[email protected]>
2018-07-12 14:38                   ` Tom Lane <[email protected]>
2018-07-12 15:28                     ` Andrew Dunstan <[email protected]>
2024-03-22 21:09 [PATCH v12 11/17] table_scan_bitmap_next_block counts lossy and exact pages Melanie Plageman <[email protected]>
2026-01-16 15:04 [PATCH v3] Introduce ternary reloptions Álvaro Herrera <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox