Re: Ordering of header file inclusion

On Tue, Oct 15, 2019 at 10:57 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> Attached patch contains the fix based on the comments suggested.
> I have added/deleted extra lines in certain places so that the
> readability is better.
>

Hmm, I am not sure if that is better in all cases. It seems to be
making the code look inconsistent at few places. See comments below:

1.
diff --git a/contrib/bloom/blinsert.c b/contrib/bloom/blinsert.c
index 4b2186b..45215ba 100644
--- a/contrib/bloom/blinsert.c
+++ b/contrib/bloom/blinsert.c
@@ -15,6 +15,7 @@
#include "access/genam.h"
#include "access/generic_xlog.h"
#include "access/tableam.h"
+#include "bloom.h"
#include "catalog/index.h"
#include "miscadmin.h"
#include "storage/bufmgr.h"
@@ -23,7 +24,6 @@
#include "utils/memutils.h"
#include "utils/rel.h"

-#include "bloom.h"

PG_MODULE_MAGIC;

diff --git a/contrib/bloom/blscan.c b/contrib/bloom/blscan.c
index 49e364a..4b9a2b8 100644
--- a/contrib/bloom/blscan.c
+++ b/contrib/bloom/blscan.c
@@ -13,14 +13,14 @@
#include "postgres.h"

#include "access/relscan.h"
-#include "pgstat.h"
+#include "bloom.h"
#include "miscadmin.h"
+#include "pgstat.h"
#include "storage/bufmgr.h"
#include "storage/lmgr.h"
#include "utils/memutils.h"
#include "utils/rel.h"

-#include "bloom.h"

/*
* Begin scan of bloom index.

The above changes lead to one extra line between the last header
include and from where the actual code starts.

2.
diff --git a/contrib/intarray/_int_bool.c b/contrib/intarray/_int_bool.c
index 91e2a80..0d2f667 100644
--- a/contrib/intarray/_int_bool.c
+++ b/contrib/intarray/_int_bool.c
@@ -3,11 +3,11 @@
*/
#include "postgres.h"

+#include "_int.h"
+
#include "miscadmin.h"
#include "utils/builtins.h"

-#include "_int.h"
-
PG_FUNCTION_INFO_V1(bqarr_in);
PG_FUNCTION_INFO_V1(bqarr_out);
PG_FUNCTION_INFO_V1(boolop);
diff --git a/contrib/intarray/_int_gin.c b/contrib/intarray/_int_gin.c
index 7aebfec..d6241d4 100644
--- a/contrib/intarray/_int_gin.c
+++ b/contrib/intarray/_int_gin.c
@@ -3,11 +3,11 @@
*/
#include "postgres.h"

+#include "_int.h"
+
#include "access/gin.h"
#include "access/stratnum.h"

Why extra line after inclusion of _int.h?

3.
diff --git a/contrib/intarray/_int_tool.c b/contrib/intarray/_int_tool.c
index fde8d15..75ad04e 100644
--- a/contrib/intarray/_int_tool.c
+++ b/contrib/intarray/_int_tool.c
@@ -5,10 +5,10 @@

#include <limits.h>

-#include "catalog/pg_type.h"
-
#include "_int.h"

+#include "catalog/pg_type.h"
+

Why extra lines after both includes?

4.
diff --git a/contrib/intarray/_intbig_gist.c b/contrib/intarray/_intbig_gist.c
index 2a20abe..87ea86c 100644
--- a/contrib/intarray/_intbig_gist.c
+++ b/contrib/intarray/_intbig_gist.c
@@ -3,12 +3,12 @@
*/
#include "postgres.h"

+#include "_int.h"
+
#include "access/gist.h"
#include "access/stratnum.h"
#include "port/pg_bitutils.h"

-#include "_int.h"
-
#define GETENTRY(vec,pos) ((GISTTYPE *)
DatumGetPointer((vec)->vector[(pos)].key))
/*
** _intbig methods
diff --git a/contrib/isn/isn.c b/contrib/isn/isn.c
index 0c2cac7..36bb582 100644
--- a/contrib/isn/isn.c
+++ b/contrib/isn/isn.c
@@ -15,9 +15,9 @@
#include "postgres.h"

#include "fmgr.h"
+#include "isn.h"
#include "utils/builtins.h"

-#include "isn.h"

Again extra spaces. I am not why you have extra spaces in a few cases.

I haven't reviewed it completely, but generally, the changes seem to
be fine. Please see if you can be consistent in extra space between
includes. Kindly check the same throughout the patch.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com