Обсуждение: pgsql: Make the order of the header file includes consistent in non-bac
Make the order of the header file includes consistent in non-backend modules.
Similar to commit 7e735035f2, this commit makes the order of header file
inclusion consistent for non-backend modules.
In passing, fix the case where we were using angle brackets (<>) for the
local module includes instead of quotes ("").
Author: Vignesh C
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/CALDaNm2Sznv8RR6Ex-iJO6xAdsxgWhCoETkaYX=+9DW3q0QCfA@mail.gmail.com
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/dddf4cdc3300073ec04b2c3e482a4c1fa4b8191b
Modified Files
--------------
src/bin/pg_archivecleanup/pg_archivecleanup.c | 6 ++----
src/bin/pg_basebackup/pg_basebackup.c | 2 +-
src/bin/pg_basebackup/pg_receivewal.c | 6 ++----
src/bin/pg_basebackup/pg_recvlogical.c | 7 ++-----
src/bin/pg_basebackup/receivelog.c | 9 +++------
src/bin/pg_basebackup/streamutil.c | 6 ++----
src/bin/pg_basebackup/walmethods.c | 3 +--
src/bin/pg_config/pg_config.c | 2 +-
src/bin/pg_controldata/pg_controldata.c | 3 +--
src/bin/pg_dump/common.c | 8 +++-----
src/bin/pg_dump/parallel.c | 3 +--
src/bin/pg_dump/pg_backup_archiver.c | 7 +++----
src/bin/pg_dump/pg_backup_custom.c | 3 +--
src/bin/pg_dump/pg_backup_db.c | 16 +++++++---------
src/bin/pg_dump/pg_backup_directory.c | 8 ++++----
src/bin/pg_dump/pg_backup_null.c | 5 ++---
src/bin/pg_dump/pg_backup_tar.c | 14 +++++++-------
src/bin/pg_dump/pg_dump.c | 13 +++++--------
src/bin/pg_dump/pg_dump_sort.c | 3 +--
src/bin/pg_dump/pg_dumpall.c | 7 +++----
src/bin/pg_dump/pg_restore.c | 4 +---
src/bin/pg_resetwal/pg_resetwal.c | 7 +++----
src/bin/pg_rewind/datapagemap.c | 3 +--
src/bin/pg_rewind/fetch.c | 2 +-
src/bin/pg_rewind/filemap.c | 5 ++---
src/bin/pg_rewind/libpq_fetch.c | 7 +++----
src/bin/pg_rewind/parsexlog.c | 6 ++----
src/bin/pg_rewind/pg_rewind.c | 9 ++++-----
src/bin/pg_rewind/timeline.c | 3 +--
src/bin/pg_test_fsync/pg_test_fsync.c | 3 +--
src/bin/pg_upgrade/check.c | 1 -
src/bin/pg_upgrade/controldata.c | 4 ++--
src/bin/pg_upgrade/dump.c | 4 +---
src/bin/pg_upgrade/file.c | 13 ++++++-------
src/bin/pg_upgrade/function.c | 4 +---
src/bin/pg_upgrade/info.c | 4 +---
src/bin/pg_upgrade/option.c | 6 ++----
src/bin/pg_upgrade/parallel.c | 1 -
src/bin/pg_upgrade/pg_upgrade.c | 10 +++++-----
src/bin/pg_upgrade/relfilenode.c | 7 +++----
src/bin/pg_upgrade/server.c | 1 -
src/bin/pg_upgrade/util.c | 5 ++---
src/bin/pg_upgrade/version.c | 5 +----
src/bin/pg_waldump/compat.c | 2 +-
src/bin/pg_waldump/pg_waldump.c | 3 +--
src/bin/pgbench/pgbench.c | 15 ++++++++-------
src/bin/psql/common.c | 10 ++++------
src/bin/psql/copy.c | 10 ++++------
src/bin/psql/crosstabview.c | 3 +--
src/bin/psql/describe.c | 7 ++-----
src/bin/psql/help.c | 3 +--
src/bin/psql/input.c | 5 ++---
src/bin/psql/large_obj.c | 5 ++---
src/bin/psql/mainloop.c | 8 +++-----
src/bin/psql/prompt.c | 3 +--
src/bin/psql/startup.c | 10 +++-------
src/bin/psql/variables.c | 4 +---
src/common/f2s.c | 3 +--
src/fe_utils/print.c | 4 +---
src/fe_utils/recovery_gen.c | 3 +--
src/fe_utils/string_utils.c | 4 +---
src/interfaces/ecpg/compatlib/informix.c | 16 ++++++++--------
src/interfaces/ecpg/ecpglib/connect.c | 4 ++--
src/interfaces/ecpg/ecpglib/data.c | 10 +++++-----
src/interfaces/ecpg/ecpglib/descriptor.c | 5 ++---
src/interfaces/ecpg/ecpglib/error.c | 2 +-
src/interfaces/ecpg/ecpglib/execute.c | 17 ++++++++---------
src/interfaces/ecpg/ecpglib/memory.c | 4 ++--
src/interfaces/ecpg/ecpglib/misc.c | 11 ++++++-----
src/interfaces/ecpg/ecpglib/prepare.c | 4 ++--
src/interfaces/ecpg/ecpglib/sqlda.c | 9 ++++-----
src/interfaces/ecpg/ecpglib/typename.c | 5 ++---
src/interfaces/ecpg/pgtypeslib/common.c | 2 +-
src/interfaces/ecpg/pgtypeslib/datetime.c | 4 ++--
src/interfaces/ecpg/pgtypeslib/dt_common.c | 2 +-
src/interfaces/ecpg/pgtypeslib/interval.c | 4 ++--
src/interfaces/ecpg/pgtypeslib/numeric.c | 5 +++--
src/interfaces/ecpg/pgtypeslib/timestamp.c | 5 ++---
src/interfaces/ecpg/preproc/c_keywords.c | 5 ++---
src/interfaces/ecpg/preproc/ecpg_keywords.c | 5 ++---
src/interfaces/libpq/fe-auth.c | 3 +--
src/interfaces/libpq/fe-connect.c | 16 +++++++---------
src/interfaces/libpq/fe-exec.c | 9 ++++-----
src/interfaces/libpq/fe-misc.c | 3 +--
src/interfaces/libpq/fe-protocol2.c | 8 +++-----
src/interfaces/libpq/fe-protocol3.c | 10 ++++------
src/interfaces/libpq/fe-secure-gssapi.c | 2 +-
src/interfaces/libpq/fe-secure.c | 8 ++++----
src/pl/plpgsql/src/pl_comp.c | 4 +---
src/pl/plpgsql/src/pl_exec.c | 4 +---
src/pl/plpgsql/src/pl_funcs.c | 4 +---
src/pl/plpgsql/src/pl_handler.c | 4 +---
src/port/pg_crc32c_armv8.c | 4 ++--
src/port/pg_crc32c_sse42.c | 4 ++--
src/port/tar.c | 4 +++-
src/test/isolation/isolationtester.c | 5 ++---
.../modules/test_ginpostinglist/test_ginpostinglist.c | 2 +-
src/test/modules/test_integerset/test_integerset.c | 6 +++---
src/test/modules/test_rls_hooks/test_rls_hooks.c | 7 ++-----
src/test/modules/test_shm_mq/setup.c | 3 +--
src/test/regress/pg_regress.c | 3 +--
src/test/regress/regress.c | 3 +--
102 files changed, 237 insertions(+), 345 deletions(-)
Hi Amit,
On Fri, Oct 25, 2019 at 02:24:50AM +0000, Amit Kapila wrote:
> Make the order of the header file includes consistent in non-backend modules.
>
> Similar to commit 7e735035f2, this commit makes the order of header file
> inclusion consistent for non-backend modules.
>
> In passing, fix the case where we were using angle brackets (<>) for the
> local module includes instead of quotes ("").
locust has just reported a build failure here:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=locust&dt=2019-10-25%2002%3A58%3A55
And the cause is visibly this portion of the commit:
--- a/src/interfaces/ecpg/pgtypeslib/dt_common.c
+++ b/src/interfaces/ecpg/pgtypeslib/dt_common.c
@@ -7,9 +7,9 @@
#include <math.h>
#include "common/string.h"
-#include "pgtypeslib_extern.h"
#include "dt.h"
#include "pgtypes_timestamp.h"
+#include "pgtypeslib_extern.h"
--
Michael
Вложения
On Fri, Oct 25, 2019 at 8:47 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> Hi Amit,
>
> On Fri, Oct 25, 2019 at 02:24:50AM +0000, Amit Kapila wrote:
> > Make the order of the header file includes consistent in non-backend modules.
> >
> > Similar to commit 7e735035f2, this commit makes the order of header file
> > inclusion consistent for non-backend modules.
> >
> > In passing, fix the case where we were using angle brackets (<>) for the
> > local module includes instead of quotes ("").
>
> locust has just reported a build failure here:
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=locust&dt=2019-10-25%2002%3A58%3A55
>
Yeah, I have noticed that and working on it.
> And the cause is visibly this portion of the commit:
> --- a/src/interfaces/ecpg/pgtypeslib/dt_common.c
> +++ b/src/interfaces/ecpg/pgtypeslib/dt_common.c
> @@ -7,9 +7,9 @@
> #include <math.h>
>
> #include "common/string.h"
> -#include "pgtypeslib_extern.h"
> #include "dt.h"
> #include "pgtypes_timestamp.h"
> +#include "pgtypeslib_extern.h"
Right, but trying to see why it hasn't failed on other machines and in
my local build.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
On Fri, Oct 25, 2019 at 8:54 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Fri, Oct 25, 2019 at 8:47 AM Michael Paquier <michael@paquier.xyz> wrote:
> >
> > Hi Amit,
> >
> > On Fri, Oct 25, 2019 at 02:24:50AM +0000, Amit Kapila wrote:
> > > Make the order of the header file includes consistent in non-backend modules.
> > >
> > > Similar to commit 7e735035f2, this commit makes the order of header file
> > > inclusion consistent for non-backend modules.
> > >
> > > In passing, fix the case where we were using angle brackets (<>) for the
> > > local module includes instead of quotes ("").
> >
> > locust has just reported a build failure here:
> > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=locust&dt=2019-10-25%2002%3A58%3A55
> >
>
> Yeah, I have noticed that and working on it.
>
> > And the cause is visibly this portion of the commit:
> > --- a/src/interfaces/ecpg/pgtypeslib/dt_common.c
> > +++ b/src/interfaces/ecpg/pgtypeslib/dt_common.c
> > @@ -7,9 +7,9 @@
> > #include <math.h>
> >
> > #include "common/string.h"
> > -#include "pgtypeslib_extern.h"
> > #include "dt.h"
> > #include "pgtypes_timestamp.h"
> > +#include "pgtypeslib_extern.h"
>
> Right, but trying to see why it hasn't failed on other machines and in
> my local build.
>
One possible reason could be that pgtypeslib_extern.h has define like below:
#ifndef bool
#define bool char
#endif /* ndef bool */
It is possible that all the functions that have bool has a parameter
in dt_common.c treat it as 'char' where as the declaration of same
functions in dt.h doesn't consider bool as char because now
pgtypeslib_extern.h is included after dt.h. It seems that the
platforms where it failed doesn't have bool defined. What do you
think? I think we can revert the change in that file.
BTW, prairiedog is also show similar failure and both seems to have
similar OS except for versions.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
On Fri, Oct 25, 2019 at 9:32 AM Amit Kapila <amit.kapila16@gmail.com> wrote: > > On Fri, Oct 25, 2019 at 8:54 AM Amit Kapila <amit.kapila16@gmail.com> wrote: > > > > > And the cause is visibly this portion of the commit: > > > --- a/src/interfaces/ecpg/pgtypeslib/dt_common.c > > > +++ b/src/interfaces/ecpg/pgtypeslib/dt_common.c > > > @@ -7,9 +7,9 @@ > > > #include <math.h> > > > > > > #include "common/string.h" > > > -#include "pgtypeslib_extern.h" > > > #include "dt.h" > > > #include "pgtypes_timestamp.h" > > > +#include "pgtypeslib_extern.h" > > > > Right, but trying to see why it hasn't failed on other machines and in > > my local build. > > > > One possible reason could be that pgtypeslib_extern.h has define like below: > #ifndef bool > #define bool char > #endif /* ndef bool */ > > It is possible that all the functions that have bool has a parameter > in dt_common.c treat it as 'char' where as the declaration of same > functions in dt.h doesn't consider bool as char because now > pgtypeslib_extern.h is included after dt.h. It seems that the > platforms where it failed doesn't have bool defined. What do you > think? I think we can revert the change in that file. > I am planning to revert that part of the change as attached. We can think of even moving the above definition of bool but I am not sure what is the right place for the same and I am not sure if it is a better idea. I think that can be explored later. It is better to revert that change for now. Do you think we should add some comment like: "We need to include pgtypeslib_extern.h before dt.h as it defines bool which is used in dt.h."? I have not added as I couldn't experiment any thing related to that. I will apply the attached in one hour or so unless you or someone has a better idea. In the meantime, we can see if there is any other problem with buildfarm. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Вложения
On Fri, Oct 25, 2019 at 09:32:29AM +0530, Amit Kapila wrote: > One possible reason could be that pgtypeslib_extern.h has define like below: > #ifndef bool > #define bool char > #endif /* ndef bool */ Possible. I have not looked closely. > It is possible that all the functions that have bool has a parameter > in dt_common.c treat it as 'char' where as the declaration of same > functions in dt.h doesn't consider bool as char because now > pgtypeslib_extern.h is included after dt.h. It seems that the > platforms where it failed doesn't have bool defined. What do you > think? I think we can revert the change in that file. > > BTW, prairiedog is also show similar failure and both seems to have > similar OS except for versions. Remembering the recent efforts that went behind stdbool.h (see 9a95a77), I think that we should be able to cleanup this header more. Reverting partially the change looks better for now to put that the buildfarm back to green. -- Michael
Вложения
>>>>> "Amit" == Amit Kapila <amit.kapila16@gmail.com> writes: Amit> BTW, prairiedog is also show similar failure and both seems to Amit> have similar OS except for versions. Be aware that prairiedog and locust (unlike the rest of the buildfarm) have compilers whose "bool" type is not 1 byte long; PG can't use that, so we don't include <stdbool.h> on those platforms. Looking at c.h, the problem seems to be that when we define our own bool in the absence of a usable stdbool.h, we do so as a typedef and not a macro. So pgtypeslib_extern.h ends up redefining it in that case, and (this bit may be my fault: see d26a810eb) uses a different type to c.h (char vs. unsigned char). (stdbool.h is required by spec to do #define bool _Bool rather than a typedef, hence the difference in behavior) I'm no expert on the ECPG internals, but this doesn't look like an exposed interface, so maybe it just shouldn't be trying to #define bool at all and just rely on c.h (from postgres_fe.h) to declare the type? -- Andrew (irc:RhodiumToad)
On Fri, Oct 25, 2019 at 1:30 PM Andrew Gierth <andrew@tao11.riddles.org.uk> wrote: > > >>>>> "Amit" == Amit Kapila <amit.kapila16@gmail.com> writes: > > Amit> BTW, prairiedog is also show similar failure and both seems to > Amit> have similar OS except for versions. > > Be aware that prairiedog and locust (unlike the rest of the buildfarm) > have compilers whose "bool" type is not 1 byte long; PG can't use that, > so we don't include <stdbool.h> on those platforms. > > Looking at c.h, the problem seems to be that when we define our own bool > in the absence of a usable stdbool.h, we do so as a typedef and not a > macro. So pgtypeslib_extern.h ends up redefining it in that case, and > (this bit may be my fault: see d26a810eb) uses a different type to c.h > (char vs. unsigned char). > > (stdbool.h is required by spec to do #define bool _Bool rather than a > typedef, hence the difference in behavior) > > I'm no expert on the ECPG internals, but this doesn't look like an > exposed interface, so maybe it just shouldn't be trying to #define bool > at all and just rely on c.h (from postgres_fe.h) to declare the type? > +1. I think we can discuss this on hackers, so started a thread [1]. [1] - https://www.postgresql.org/message-id/CAA4eK1LmaKO7Du9M9Lo%3DkxGU8sB6aL8fa3sF6z6d5yYYVe3BuQ%40mail.gmail.com -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
On Fri, Oct 25, 2019 at 11:47 AM Michael Paquier <michael@paquier.xyz> wrote: > > On Fri, Oct 25, 2019 at 09:32:29AM +0530, Amit Kapila wrote: > > One possible reason could be that pgtypeslib_extern.h has define like below: > > #ifndef bool > > #define bool char > > #endif /* ndef bool */ > > Possible. I have not looked closely. > > > It is possible that all the functions that have bool has a parameter > > in dt_common.c treat it as 'char' where as the declaration of same > > functions in dt.h doesn't consider bool as char because now > > pgtypeslib_extern.h is included after dt.h. It seems that the > > platforms where it failed doesn't have bool defined. What do you > > think? I think we can revert the change in that file. > > > > BTW, prairiedog is also show similar failure and both seems to have > > similar OS except for versions. > > Remembering the recent efforts that went behind stdbool.h (see > 9a95a77), I think that we should be able to cleanup this header more. > Reverting partially the change looks better for now to put that the > buildfarm back to green. > Reverted and the buildfarm is green again. Thanks! -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
On Fri, Oct 25, 2019 at 03:21:13PM +0530, Amit Kapila wrote: > Reverted and the buildfarm is green again. Thanks! Thanks for fixing, and the follow-up thread. (Just saw the rest as well.) -- Michael