Remove utils/acl.h from catalog/objectaddress.h

Started by Peter Eisentrautabout 6 years ago4 messageshackers
Jump to latest
#1Peter Eisentraut
peter_e@gmx.net

I noticed that catalog/objectaddress.h includes utils/acl.h for no
apparent reason. It turns out this used to be needed but not anymore.
So removed it and cleaned up the fallout. Patch attached.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachments:

0001-Remove-utils-acl.h-from-catalog-objectaddress.h.patchtext/plain; charset=UTF-8; name=0001-Remove-utils-acl.h-from-catalog-objectaddress.h.patch; x-mac-creator=0; x-mac-type=0Download+32-2
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#1)
Re: Remove utils/acl.h from catalog/objectaddress.h

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

I noticed that catalog/objectaddress.h includes utils/acl.h for no
apparent reason. It turns out this used to be needed but not anymore.
So removed it and cleaned up the fallout. Patch attached.

Seems reasonable. One thing I noticed is that if you are including
nodes/parsenodes.h explicitly in objectaddress.h, there seems little
point in the #include "nodes/pg_list.h" right beside it.

Sometime we really ought to make an effort to make our header inclusions
less of a mass of spaghetti. But this patch needn't take on that load.

regards, tom lane

#3Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Peter Eisentraut (#1)
Re: Remove utils/acl.h from catalog/objectaddress.h

On 2020-Mar-07, Peter Eisentraut wrote:

I noticed that catalog/objectaddress.h includes utils/acl.h for no apparent
reason. It turns out this used to be needed but not anymore. So removed it
and cleaned up the fallout. Patch attached.

parser/parse_nodes.h already includes nodes/parsenodes.h, so the seeming
redundancy in places such as

diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index c27d255d8d..be63e043c6 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -19,6 +19,7 @@
#include "catalog/pg_statistic.h"
#include "catalog/pg_type.h"
#include "nodes/parsenodes.h"
+#include "parser/parse_node.h"

(and others) is not just apparent; it's also redundant in practice. And
it's not like parse_node.h is ever going to be able not to depend on
parsenodes.h, so I would vote to remove nodes/parsenodes.h from the
headers where you're adding parser/parse_node.h.

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

#4Peter Eisentraut
peter_e@gmx.net
In reply to: Alvaro Herrera (#3)
Re: Remove utils/acl.h from catalog/objectaddress.h

On 2020-03-09 17:07, Alvaro Herrera wrote:

On 2020-Mar-07, Peter Eisentraut wrote:

I noticed that catalog/objectaddress.h includes utils/acl.h for no apparent
reason. It turns out this used to be needed but not anymore. So removed it
and cleaned up the fallout. Patch attached.

parser/parse_nodes.h already includes nodes/parsenodes.h, so the seeming
redundancy in places such as

diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index c27d255d8d..be63e043c6 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -19,6 +19,7 @@
#include "catalog/pg_statistic.h"
#include "catalog/pg_type.h"
#include "nodes/parsenodes.h"
+#include "parser/parse_node.h"

(and others) is not just apparent; it's also redundant in practice. And
it's not like parse_node.h is ever going to be able not to depend on
parsenodes.h, so I would vote to remove nodes/parsenodes.h from the
headers where you're adding parser/parse_node.h.

OK, committed with your and Tom's changes.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services