Hi,
On 2019-07-31 11:19:08 +0530, vignesh C wrote:
> I noticed that there are many header files being
> included which need not be included.
> I have tried this in a few files and found the
> compilation and regression to be working.
> I have attached the patch for the files that
> I tried.
> I tried this in CentOS, I did not find the header
> files to be platform specific.
> Should we pursue this further and cleanup in
> all the files?
These type of changes imo have a good chance of making things more
fragile. A lot of the includes in header files are purely due to needing
one or two definitions (which often could even be avoided by forward
declarations). If you remove all the includes directly from the c files
that are also included from some .h file, you increase the reliance on
the indirect includes - making it harder to clean up.
If anything, we should *increase* the number of includes, so we don't
rely on indirect includes. But that's also not necessarily the right
choice, because it adds unnecessary dependencies.
> --- a/src/backend/utils/mmgr/freepage.c
> +++ b/src/backend/utils/mmgr/freepage.c
> @@ -56,7 +56,6 @@
> #include "miscadmin.h"
>
> #include "utils/freepage.h"
> -#include "utils/relptr.h"
I don't think it's a good idea to remove this header, for example. A
*lot* of code in freepage.c relies on it. The fact that freepage.h also
includes it here is just due to needing other parts of it
> /* Magic numbers to identify various page types */
> diff --git a/src/backend/utils/mmgr/portalmem.c b/src/backend/utils/mmgr/portalmem.c
> index 334e35b..67268fd 100644
> --- a/src/backend/utils/mmgr/portalmem.c
> +++ b/src/backend/utils/mmgr/portalmem.c
> @@ -26,7 +26,6 @@
> #include "utils/builtins.h"
> #include "utils/memutils.h"
> #include "utils/snapmgr.h"
> -#include "utils/timestamp.h"
Similarly, this file uses timestamp.h functions directly. The fact that
timestamp.h already is included is just due to implementation details of
xact.h that this file shouldn't depend on.
Greetings,
Andres Freund
