Re: [PATCHES] Headers dependencies cleanup
Zdenek Kotala <[EMAIL PROTECTED]> writes: > Alvaro Herrera napsal(a): >> Not all compilers like (== support) inline macros apparently. > Is it your assumption or do you mean some specific compiler? IIRC, inline is > defined in C99 and my assumption :-) is that it should be supported by all > compilers today. The problem is (1) not all compilers support inline, (2) the ones that do have divergent ideas on its semantics, and (3) the semantics specified by C99 utterly suck (cf tuplesort.c). gcc's traditional semantics for inline are far more usable. We have done "#ifdef gcc" inlines in one or two places where the performance argument for adding such clutter was compelling. We can do that some more, but you'll need equally compelling arguments. Do not bother submitting patches to create a very large number of inlines. regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Headers dependencies cleanup
Zdenek Kotala wrote: Is it your assumption or do you mean some specific compiler? IIRC, inline is defined in C99 and my assumption :-) is that it should be supported by all compilers today. I try to look on buildmachine, There should be some useful configure output. My recollection is that we support C89, which might be a bit out of date, but then we try not to obsolete platforms if possible. cheers andrew -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Headers dependencies cleanup
Zdenek Kotala wrote: > Alvaro Herrera napsal(a): >> Zdenek Kotala wrote: >> >>> PS: Is there any reason to do not start to use inline functions >>> instead of macros in some cases? >> >> Not all compilers like (== support) inline macros apparently. > > Is it your assumption or do you mean some specific compiler? IIRC, inline > is defined in C99 and my assumption :-) is that it should be supported by > all compilers today. I try to look on buildmachine, There should be some > useful configure output. We have some defensive coding for compilers that do not support it, for example on pg_list.h ... I have no idea if there's a compiler in actual use that needs this. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Headers dependencies cleanup
Alvaro Herrera napsal(a): Zdenek Kotala wrote: PS: Is there any reason to do not start to use inline functions instead of macros in some cases? Not all compilers like (== support) inline macros apparently. Is it your assumption or do you mean some specific compiler? IIRC, inline is defined in C99 and my assumption :-) is that it should be supported by all compilers today. I try to look on buildmachine, There should be some useful configure output. Zdenek -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Headers dependencies cleanup
Zdenek Kotala wrote: > PS: Is there any reason to do not start to use inline functions instead > of macros in some cases? Not all compilers like (== support) inline macros apparently. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Headers dependencies cleanup
Tom Lane napsal(a): Bruce Momjian <[EMAIL PROTECTED]> writes: Zdenek Kotala wrote: bufpage.h includes bufmgr.h, but bufpage.h does not require any definition from bufmgr.h. I think this dependency is there for long time. My scripts should have found this issue, see src/tools/pginclude/pgrminclude. Looking over the file more closely, I disagree with Zdenek's claim anyway. Even though the file could physically be scanned without having included bufmgr.h first, many of the macros it defines can't be used without bufmgr.h, and so that file really is a prerequisite. If we removed the include here it would just have to pop up in calling .c files. Anyplace that that was solely because of calling one of the macros defined by bufpage.h, rather than explicitly using anything from bufmgr.h, I claim it'd be wrong. Yeah, I see it now. BufferGetPageSize and BufferGetPage macros uses macros from bufmgr. But I think, They should be moved to the bufmgr.h where is better and more logic place for them. It's probably true that we've let the header inclusions in storage/ and access/ get a bit spaghetti-ish. But I think that if we're going to do something about it, focusing on one or two files is not the way to start. What we need is for someone to go through all those files and propose a clear layering of them. This will very likely involve having to move some declarations around, when we realize something got put in a poorly chosen place. Yes, agree. I need also fix problem with pgxlogreset and design how to track history (multi version) of structures related to upgrade (e.g. page layout ...). I will look on it. Zdenek PS: Is there any reason to do not start to use inline functions instead of macros in some cases? -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Headers dependencies cleanup
Bruce Momjian napsal(a): Zdenek Kotala wrote: Tom Lane napsal(a): Zdenek Kotala <[EMAIL PROTECTED]> writes: During my work I found some header files, which include useless headers and on other way there are some headers which silently assume that requested header will be included by some other headers file. Bruce periodically runs a script that's supposed to catch this type of problem. Maybe we are overdue for another run? Yes, but This problem is there for long time and I think Bruce's script does not catch it. Exactly what problem do you see that he didn't catch? bufpage.h includes bufmgr.h, but bufpage.h does not require any definition from bufmgr.h. I think this dependency is there for long time. My scripts should have found this issue, see src/tools/pginclude/pgrminclude. Hmmm. It is difficult to run it on Solaris. I adjusted scripts but I did not get any change on current head. Zdenek -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Headers dependencies cleanup
Bruce Momjian napsal(a): Zdenek Kotala wrote: Tom Lane napsal(a): Zdenek Kotala <[EMAIL PROTECTED]> writes: During my work I found some header files, which include useless headers and on other way there are some headers which silently assume that requested header will be included by some other headers file. Bruce periodically runs a script that's supposed to catch this type of problem. Maybe we are overdue for another run? Yes, but This problem is there for long time and I think Bruce's script does not catch it. Exactly what problem do you see that he didn't catch? bufpage.h includes bufmgr.h, but bufpage.h does not require any definition from bufmgr.h. I think this dependency is there for long time. My scripts should have found this issue, see src/tools/pginclude/pgrminclude. I'm going to try it and examine output. Zdenek -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Headers dependencies cleanup
Tom Lane wrote: > Bruce Momjian <[EMAIL PROTECTED]> writes: > > I run it every few years rather than every year because it can break > > stuff that we have to manually fix by adding includes. > > > If you want it run now or every year, let me know. > > I'm not in a hurry --- as we found out last time, that script isn't > really bright enough. When the state of the headers gets bad enough > that we're willing to accept collateral damage, then it should be > run, but I don't think it's reliable enough to use routinely. Agreed, that was my analysis too. -- Bruce Momjian <[EMAIL PROTECTED]>http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Headers dependencies cleanup
Bruce Momjian <[EMAIL PROTECTED]> writes: > I run it every few years rather than every year because it can break > stuff that we have to manually fix by adding includes. > If you want it run now or every year, let me know. I'm not in a hurry --- as we found out last time, that script isn't really bright enough. When the state of the headers gets bad enough that we're willing to accept collateral damage, then it should be run, but I don't think it's reliable enough to use routinely. regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Headers dependencies cleanup
Bruce Momjian <[EMAIL PROTECTED]> writes: > Zdenek Kotala wrote: >> bufpage.h includes bufmgr.h, but bufpage.h does not require any definition >> from >> bufmgr.h. I think this dependency is there for long time. > My scripts should have found this issue, see > src/tools/pginclude/pgrminclude. Looking over the file more closely, I disagree with Zdenek's claim anyway. Even though the file could physically be scanned without having included bufmgr.h first, many of the macros it defines can't be used without bufmgr.h, and so that file really is a prerequisite. If we removed the include here it would just have to pop up in calling .c files. Anyplace that that was solely because of calling one of the macros defined by bufpage.h, rather than explicitly using anything from bufmgr.h, I claim it'd be wrong. This example makes me question using scripts for this type of maintenance, though, as it's not clear that a does-it-compile test would understand macro dependencies. It's probably true that we've let the header inclusions in storage/ and access/ get a bit spaghetti-ish. But I think that if we're going to do something about it, focusing on one or two files is not the way to start. What we need is for someone to go through all those files and propose a clear layering of them. This will very likely involve having to move some declarations around, when we realize something got put in a poorly chosen place. regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Headers dependencies cleanup
Zdenek Kotala wrote: > Tom Lane napsal(a): > > Zdenek Kotala <[EMAIL PROTECTED]> writes: > During my work I found some header files, which include useless headers > and on other way there are some headers which silently assume that > requested header will be included by some other headers file. > >>> Bruce periodically runs a script that's supposed to catch this type > >>> of problem. Maybe we are overdue for another run? > > > >> Yes, but This problem is there for long time and I think Bruce's script > >> does not > >> catch it. > > > > Exactly what problem do you see that he didn't catch? > > bufpage.h includes bufmgr.h, but bufpage.h does not require any definition > from > bufmgr.h. I think this dependency is there for long time. My scripts should have found this issue, see src/tools/pginclude/pgrminclude. -- Bruce Momjian <[EMAIL PROTECTED]>http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Headers dependencies cleanup
Tom Lane wrote: > Alvaro Herrera <[EMAIL PROTECTED]> writes: > > Zdenek Kotala wrote: > >> During my work I found some header files, which include useless headers > >> and on other way there are some headers which silently assume that > >> requested header will be included by some other headers file. > >> > >> This patch fixes these dependencies around bufpage.h, bufmgr.h > > > What's the point here, again? > > Bruce periodically runs a script that's supposed to catch this type > of problem. Maybe we are overdue for another run? Last run was July, 2006: revision 1.48 date: 2006/07/13 18:01:01; author: momjian; state: Exp; lines: +1 -2 More include file adjustments. revision 1.47 date: 2006/07/13 17:47:01; author: momjian; state: Exp; lines: +2 -1 More include file adjustments. revision 1.46 date: 2006/07/13 16:49:19; author: momjian; state: Exp; lines: +2 -1 Allow include files to compile own their own. Strip unused include files out unused include files, and add needed includes to C files. I run it every few years rather than every year because it can break stuff that we have to manually fix by adding includes. If you want it run now or every year, let me know. -- Bruce Momjian <[EMAIL PROTECTED]>http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Headers dependencies cleanup
Tom Lane napsal(a): Zdenek Kotala <[EMAIL PROTECTED]> writes: During my work I found some header files, which include useless headers and on other way there are some headers which silently assume that requested header will be included by some other headers file. Bruce periodically runs a script that's supposed to catch this type of problem. Maybe we are overdue for another run? Yes, but This problem is there for long time and I think Bruce's script does not catch it. Exactly what problem do you see that he didn't catch? bufpage.h includes bufmgr.h, but bufpage.h does not require any definition from bufmgr.h. I think this dependency is there for long time. Zdenek -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Headers dependencies cleanup
Zdenek Kotala <[EMAIL PROTECTED]> writes: >>> During my work I found some header files, which include useless headers >>> and on other way there are some headers which silently assume that >>> requested header will be included by some other headers file. >> >> Bruce periodically runs a script that's supposed to catch this type >> of problem. Maybe we are overdue for another run? > Yes, but This problem is there for long time and I think Bruce's script does > not > catch it. Exactly what problem do you see that he didn't catch? regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Headers dependencies cleanup
Tom Lane napsal(a): Alvaro Herrera <[EMAIL PROTECTED]> writes: Zdenek Kotala wrote: During my work I found some header files, which include useless headers and on other way there are some headers which silently assume that requested header will be included by some other headers file. This patch fixes these dependencies around bufpage.h, bufmgr.h What's the point here, again? Bruce periodically runs a script that's supposed to catch this type of problem. Maybe we are overdue for another run? Yes, but This problem is there for long time and I think Bruce's script does not catch it. Zdenek -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Headers dependencies cleanup
Alvaro Herrera napsal(a): Zdenek Kotala wrote: During my work I found some header files, which include useless headers and on other way there are some headers which silently assume that requested header will be included by some other headers file. This patch fixes these dependencies around bufpage.h, bufmgr.h What's the point here, again? Main point there is bufpage.h I have needed include this header in page layout convector but it include bufmgr.h which is useless there and generates a lot of another dependences (e.g. on fmgr.h). Other changes are related to this change. Zdenek -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Headers dependencies cleanup
Alvaro Herrera <[EMAIL PROTECTED]> writes: > Zdenek Kotala wrote: >> During my work I found some header files, which include useless headers >> and on other way there are some headers which silently assume that >> requested header will be included by some other headers file. >> >> This patch fixes these dependencies around bufpage.h, bufmgr.h > What's the point here, again? Bruce periodically runs a script that's supposed to catch this type of problem. Maybe we are overdue for another run? regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Headers dependencies cleanup
Zdenek Kotala wrote: > During my work I found some header files, which include useless headers > and on other way there are some headers which silently assume that > requested header will be included by some other headers file. > > This patch fixes these dependencies around bufpage.h, bufmgr.h What's the point here, again? -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] Headers dependencies cleanup
During my work I found some header files, which include useless headers and on other way there are some headers which silently assume that requested header will be included by some other headers file. This patch fixes these dependencies around bufpage.h, bufmgr.h Zdenek Index: src/backend/access/transam/xlogutils.c === RCS file: /zfs_data/cvs_pgsql/cvsroot/pgsql/src/backend/access/transam/xlogutils.c,v retrieving revision 1.50 diff -c -r1.50 xlogutils.c *** src/backend/access/transam/xlogutils.c 2 May 2007 23:18:03 - 1.50 --- src/backend/access/transam/xlogutils.c 25 Oct 2007 09:16:59 - *** *** 18,23 --- 18,24 #include "postgres.h" #include "access/xlogutils.h" + #include "storage/bufmgr.h" #include "storage/bufpage.h" #include "storage/smgr.h" #include "utils/hsearch.h" Index: src/backend/nodes/print.c === RCS file: /zfs_data/cvs_pgsql/cvsroot/pgsql/src/backend/nodes/print.c,v retrieving revision 1.85 diff -c -r1.85 print.c *** src/backend/nodes/print.c 22 Feb 2007 22:00:23 - 1.85 --- src/backend/nodes/print.c 25 Oct 2007 09:16:59 - *** *** 24,30 #include "optimizer/clauses.h" #include "parser/parsetree.h" #include "utils/lsyscache.h" ! /* * print --- 24,30 #include "optimizer/clauses.h" #include "parser/parsetree.h" #include "utils/lsyscache.h" ! #include "lib/stringinfo.h" /* * print Index: src/backend/nodes/tidbitmap.c === RCS file: /zfs_data/cvs_pgsql/cvsroot/pgsql/src/backend/nodes/tidbitmap.c,v retrieving revision 1.13 diff -c -r1.13 tidbitmap.c *** src/backend/nodes/tidbitmap.c 20 Sep 2007 17:56:31 - 1.13 --- src/backend/nodes/tidbitmap.c 25 Oct 2007 09:16:59 - *** *** 36,41 --- 36,42 #include "nodes/tidbitmap.h" #include "storage/bufpage.h" #include "utils/hsearch.h" + #include "nodes/nodes.h" /* * The maximum number of tuples per page is not large (typically 256 with Index: src/backend/postmaster/postmaster.c === RCS file: /zfs_data/cvs_pgsql/cvsroot/pgsql/src/backend/postmaster/postmaster.c,v retrieving revision 1.542 diff -c -r1.542 postmaster.c *** src/backend/postmaster/postmaster.c 26 Sep 2007 22:36:30 - 1.542 --- src/backend/postmaster/postmaster.c 25 Oct 2007 09:16:59 - *** *** 93,98 --- 93,99 #endif #include "access/transam.h" + #include "access/xlog.h" #include "bootstrap/bootstrap.h" #include "catalog/pg_control.h" #include "lib/dllist.h" Index: src/backend/storage/buffer/bufmgr.c === RCS file: /zfs_data/cvs_pgsql/cvsroot/pgsql/src/backend/storage/buffer/bufmgr.c,v retrieving revision 1.226 diff -c -r1.226 bufmgr.c *** src/backend/storage/buffer/bufmgr.c 25 Sep 2007 22:11:48 - 1.226 --- src/backend/storage/buffer/bufmgr.c 25 Oct 2007 09:16:59 - *** *** 37,42 --- 37,43 #include "postmaster/bgwriter.h" #include "storage/buf_internals.h" #include "storage/bufpage.h" + #include "storage/bufmgr.h" #include "storage/ipc.h" #include "storage/proc.h" #include "storage/smgr.h" Index: src/backend/utils/adt/domains.c === RCS file: /zfs_data/cvs_pgsql/cvsroot/pgsql/src/backend/utils/adt/domains.c,v retrieving revision 1.5 diff -c -r1.5 domains.c *** src/backend/utils/adt/domains.c 5 Jan 2007 22:19:40 - 1.5 --- src/backend/utils/adt/domains.c 25 Oct 2007 09:16:59 - *** *** 35,40 --- 35,41 #include "executor/executor.h" #include "utils/builtins.h" #include "utils/lsyscache.h" + #include "lib/stringinfo.h" /* Index: src/include/access/heapam.h === RCS file: /zfs_data/cvs_pgsql/cvsroot/pgsql/src/include/access/heapam.h,v retrieving revision 1.127 diff -c -r1.127 heapam.h *** src/include/access/heapam.h 20 Sep 2007 17:56:32 - 1.127 --- src/include/access/heapam.h 25 Oct 2007 11:09:29 - *** *** 18,27 --- 18,29 #include "access/relscan.h" #include "access/sdir.h" #include "access/tupmacs.h" + #include "access/xlog.h" #include "access/xlogutils.h" #include "nodes/primnodes.h" #include "storage/block.h" #include "storage/lmgr.h" + #include "storage/bufpage.h" #include "utils/rel.h" #include "utils/tqual.h" Index: src/include/access/htup.h === RCS file: /zfs_data/cvs_pgsql/cvsroot/pgsql/src/include/access/htup.h,v retrieving revision 1.94 diff -c -r1.94 htup.h *** src/include/access/htup.h 20 Sep 2007 17:56:32 - 1.94 --- src/include/access/htup.h 25 Oct
