Is it really such a great idea for spi.h to include the world?
executor/spi.h includes far more than it needs, starting with postgres.h
which as a general rule we don't expect any other header file to
include. I think the argument for this was to keep things simple for
SPI-using loadable modules, but I doubt that it's really improving their
lives much. A quick look through the existing files that include spi.h
shows that most of them have to include a pile of other stuff anyway.
I propose changing spi.h to follow the same include-only-what-you-must
rule as every other backend header file. Thoughts?
regards, tom lane
Tom Lane wrote:
executor/spi.h includes far more than it needs, starting with postgres.h
which as a general rule we don't expect any other header file to
include. I think the argument for this was to keep things simple for
SPI-using loadable modules, but I doubt that it's really improving their
lives much. A quick look through the existing files that include spi.h
shows that most of them have to include a pile of other stuff anyway.I propose changing spi.h to follow the same include-only-what-you-must
rule as every other backend header file. Thoughts?
I don't think we ever cleaned out spi.h in the past because we were
worried about 3rd party code using it (I am fine with a cleanup).
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Bruce Momjian wrote:
Tom Lane wrote:
executor/spi.h includes far more than it needs, starting with postgres.h
which as a general rule we don't expect any other header file to
include. I think the argument for this was to keep things simple for
SPI-using loadable modules, but I doubt that it's really improving their
lives much. A quick look through the existing files that include spi.h
shows that most of them have to include a pile of other stuff anyway.I propose changing spi.h to follow the same include-only-what-you-must
rule as every other backend header file. Thoughts?I don't think we ever cleaned out spi.h in the past because we were
worried about 3rd party code using it (I am fine with a cleanup).
I've wondered about spi.h lately too while looking at header cleanup,
and I agree with the proposed solution. The worst that can happen is
that somebody needs to add extra includes in their programs in order for
them to compile with 8.4. We do enough other changes that this one is
really minor. Better late than never anyway.
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera <alvherre@commandprompt.com> writes:
Bruce Momjian wrote:
Tom Lane wrote:
I propose changing spi.h to follow the same include-only-what-you-must
rule as every other backend header file. Thoughts?I don't think we ever cleaned out spi.h in the past because we were
worried about 3rd party code using it (I am fine with a cleanup).
I've wondered about spi.h lately too while looking at header cleanup,
and I agree with the proposed solution. The worst that can happen is
that somebody needs to add extra includes in their programs in order for
them to compile with 8.4. We do enough other changes that this one is
really minor. Better late than never anyway.
Okay, I'll do a trial patch and we can see exactly how much has to be
added (at least among core and contrib) before deciding for sure.
regards, tom lane
I wrote:
Okay, I'll do a trial patch and we can see exactly how much has to be
added (at least among core and contrib) before deciding for sure.
This compiles and passes regression tests. It looks like the main
things there might be an argument for adding back to spi.h would be
pg_type.h and builtins.h, as a very large proportion of the files
using spi.h had to have those added. Comments?
regards, tom lane
Tom Lane wrote:
I wrote:
Okay, I'll do a trial patch and we can see exactly how much has to be
added (at least among core and contrib) before deciding for sure.This compiles and passes regression tests. It looks like the main
things there might be an argument for adding back to spi.h would be
pg_type.h and builtins.h, as a very large proportion of the files
using spi.h had to have those added. Comments?
They are both very lean, so no objections. I guess that the pg_type.h
inclusion is needed due to the predefined type OIDs, and it makes me
wonder whether it would be useful to have them in a separate header.
Not enough concern for the idea to even make it to Bruce's open items
mailbox ...
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera <alvherre@commandprompt.com> writes:
They are both very lean, so no objections. I guess that the pg_type.h
inclusion is needed due to the predefined type OIDs, and it makes me
wonder whether it would be useful to have them in a separate header.
Not enough concern for the idea to even make it to Bruce's open items
mailbox ...
After the header refactoring Zdenek did last year, there's not much
reason to not just #include pg_type.h --- so I'd just as soon keep
those macros together with the associated DATA lines.
regards, tom lane