Thread

1. Some cleanups/enhancements

   Jeroen van Vianen <jeroenv@design.nl> — 1998-02-11T09:22:05Z

   Hi,
   
   I'm running PostgreSQL 6.3 on Linux 2.1.85 with gcc 2.8.0 and libc5. So
   far no problems, however I noted some cleanups / enhancements which I
   would like to do. Before I send you a bunch of patches I thought I'll
   tell you what I'm planning to do. Please comment on my list and indicate
   whether I should go ahead.
   
   - Fix all Makefiles so 'make dep' and 'make depend' work
   - Fix all Makefiles so 'make clean' throws away the depend file
   - Some other Makefile cleanups
   - gcc 2.8.0 issues some additional warnings which are very easy to fix:
     - register i --> register int i
     - Ambiguous else --> add braces:
       if (cond1)
         if (cond2)
           ...
         else
           ...
     - etc.
   - Add a template for linux-elf-586 with (optimized) code for a Pentium
   (gcc 2.8.0 not only supports -m486 but also -mpentium and -mpentiumpro).
   Why not use template names similar to the output of config.guess (maybe
   with some symbolic links)?
   - Fix for tools/find_static: add two (in my opinion funny) indices to
   improve speed tremendously:
   
   Nested Loop  (cost=6.05 size=1 width=42)
     ->   Index Scan on debug  (cost=2.05 size=2 width=12)
     ->   Index Scan on debug2  (cost=2.00 size=2 width=30)
   
   rather than
   
   Hash Join  (cost=993.76 size=1 width=42)
     ->   Seq Scan on debug  (cost=495.81 size=2 width=12)
     ->   Hash  (cost=0.00 size=0 width=0)
       ->     Seq Scan on debug2  (cost=495.81 size=2 width=30)
   
   - Cleanup of some code that uses heap_formtuple to allow a NULL value
   for the nulls parameter, indicating there are no null columns (comes in
   handy for catalog/pg_*.c), among others.
   - Why is there some code to change the case of the procedural language
   to lower case except for 'C' (in fact it's there twice)? Why not use
   strcasecmp and remove these pices of code?
   - Add pcalloc(n,s) to allocate n*s bytes and set them to zero and use
   them where appropriate (I won't touch all code :-))
   - Shouldn't same_tuple in executor/nodeGroup.c use the equality operator
   for the type concerned to test for equality of the attributes rather
   than print them to a buffer and use strcmp? Shouldn't the pointers for
   these functions be looked up once in ExecInitGroup and stored somewhere?
   Shouldn't this function go to heaptuple.c and be renamed heap_sametuple?
   - Lump heaptuple.c and heapvalid.c together
   - I also saw quite some #ifdef NOT_USED and other similar stuff. I don't
   want to touch these now, but shouldn't some of these be removed soon?
   - Add a pg_version function that returns a string like 'PostgreSQL 6.3'
   to indicate the version of PostgreSQL a user is using (with 'select
   pg_version()'). Might be handy to include in the bug reports.
   
   These are all the things that I found after browsing through the code
   one night (primarily in backend/access, backend/catalog and
   backend/executor).
   
   Let me know what you think of the above list and I will proceed. If you
   have any hints on how I might proceed (especially with same_tuple)
   please don't hesitate. Expect the changes to be available somewhere
   after the weekend.
   
   Cheers,
   
   Jeroen van Vianen
   
   

2. Re: [HACKERS] Some cleanups/enhancements

   Bruce Momjian <maillist@candle.pha.pa.us> — 1998-02-11T15:27:59Z

   These all sound good to me.  
   
   The NOT_USED stuff was left because we thought some day we may need
   them.  If you see stuff that clearly doesn't do anything valuable, get
   rid of it.
   
   > 
   > Hi,
   > 
   > I'm running PostgreSQL 6.3 on Linux 2.1.85 with gcc 2.8.0 and libc5. So
   > far no problems, however I noted some cleanups / enhancements which I
   > would like to do. Before I send you a bunch of patches I thought I'll
   > tell you what I'm planning to do. Please comment on my list and indicate
   > whether I should go ahead.
   > 
   > - Fix all Makefiles so 'make dep' and 'make depend' work
   > - Fix all Makefiles so 'make clean' throws away the depend file
   > - Some other Makefile cleanups
   > - gcc 2.8.0 issues some additional warnings which are very easy to fix:
   >   - register i --> register int i
   >   - Ambiguous else --> add braces:
   >     if (cond1)
   >       if (cond2)
   >         ...
   >       else
   >         ...
   >   - etc.
   > - Add a template for linux-elf-586 with (optimized) code for a Pentium
   > (gcc 2.8.0 not only supports -m486 but also -mpentium and -mpentiumpro).
   > Why not use template names similar to the output of config.guess (maybe
   > with some symbolic links)?
   > - Fix for tools/find_static: add two (in my opinion funny) indices to
   > improve speed tremendously:
   > 
   > Nested Loop  (cost=6.05 size=1 width=42)
   >   ->   Index Scan on debug  (cost=2.05 size=2 width=12)
   >   ->   Index Scan on debug2  (cost=2.00 size=2 width=30)
   > 
   > rather than
   > 
   > Hash Join  (cost=993.76 size=1 width=42)
   >   ->   Seq Scan on debug  (cost=495.81 size=2 width=12)
   >   ->   Hash  (cost=0.00 size=0 width=0)
   >     ->     Seq Scan on debug2  (cost=495.81 size=2 width=30)
   > 
   > - Cleanup of some code that uses heap_formtuple to allow a NULL value
   > for the nulls parameter, indicating there are no null columns (comes in
   > handy for catalog/pg_*.c), among others.
   > - Why is there some code to change the case of the procedural language
   > to lower case except for 'C' (in fact it's there twice)? Why not use
   > strcasecmp and remove these pices of code?
   > - Add pcalloc(n,s) to allocate n*s bytes and set them to zero and use
   > them where appropriate (I won't touch all code :-))
   > - Shouldn't same_tuple in executor/nodeGroup.c use the equality operator
   > for the type concerned to test for equality of the attributes rather
   > than print them to a buffer and use strcmp? Shouldn't the pointers for
   > these functions be looked up once in ExecInitGroup and stored somewhere?
   > Shouldn't this function go to heaptuple.c and be renamed heap_sametuple?
   > - Lump heaptuple.c and heapvalid.c together
   > - I also saw quite some #ifdef NOT_USED and other similar stuff. I don't
   > want to touch these now, but shouldn't some of these be removed soon?
   > - Add a pg_version function that returns a string like 'PostgreSQL 6.3'
   > to indicate the version of PostgreSQL a user is using (with 'select
   > pg_version()'). Might be handy to include in the bug reports.
   > 
   > These are all the things that I found after browsing through the code
   > one night (primarily in backend/access, backend/catalog and
   > backend/executor).
   > 
   > Let me know what you think of the above list and I will proceed. If you
   > have any hints on how I might proceed (especially with same_tuple)
   > please don't hesitate. Expect the changes to be available somewhere
   > after the weekend.
   > 
   > Cheers,
   > 
   > Jeroen van Vianen
   > 
   > 
   
   
   -- 
   Bruce Momjian
   maillist@candle.pha.pa.us
   
   

3. Re: [HACKERS] Some cleanups/enhancements

   Marc G. Fournier <scrappy@hub.org> — 1998-02-11T15:43:00Z

   On Wed, 11 Feb 1998, Jeroen van Vianen wrote:
   
   > Hi,
   > 
   > I'm running PostgreSQL 6.3 on Linux 2.1.85 with gcc 2.8.0 and libc5. So
   > far no problems, however I noted some cleanups / enhancements which I
   > would like to do. Before I send you a bunch of patches I thought I'll
   > tell you what I'm planning to do. Please comment on my list and indicate
   > whether I should go ahead.
   > 
   > - Fix all Makefiles so 'make dep' and 'make depend' work
   
   	Can someone explain what, exactly, 'make depend' accomplishes?  We
   don't use it right now, so I'm wondering why (if?) we need it now?
   
   > - Some other Makefile cleanups
   > - gcc 2.8.0 issues some additional warnings which are very easy to fix:
   >   - register i --> register int i
   >   - Ambiguous else --> add braces:
   >     if (cond1)
   >       if (cond2)
   >         ...
   >       else
   >         ...
   >   - etc.
   
   	Sounds great...
   
   > - Add a template for linux-elf-586 with (optimized) code for a Pentium
   > (gcc 2.8.0 not only supports -m486 but also -mpentium and -mpentiumpro).
   > Why not use template names similar to the output of config.guess (maybe
   > with some symbolic links)?
   
   	Erk...I think 'templates' are getting a little out of hand here,
   no? 
   
   > - Why is there some code to change the case of the procedural language
   > to lower case except for 'C' (in fact it's there twice)? Why not use
   > strcasecmp and remove these pices of code?
   
   	I question this in *alot* of places...like why pg_dlopen is
   defined as just 'dlopen()' in some ports *shrug*  Why not just call it
   directly? *raised eyebrow*
   
   > These are all the things that I found after browsing through the code
   > one night (primarily in backend/access, backend/catalog and
   > backend/executor).
   > 
   > Let me know what you think of the above list and I will proceed. If you
   > have any hints on how I might proceed (especially with same_tuple)
   > please don't hesitate. Expect the changes to be available somewhere
   > after the weekend.
   
   	The only thing I ask is that you submit these in such a way that
   they can be easily reviewed before committing them...we are in a beta mode
   right now, and altho some of this makes for nice cleanups, some of this
   should most likely be gingerly added...
   
   	If at all possible, a seperate patch for each point above would be
   really good, with an explanation of each.  If it weren't for beta-status,
   I wouldn't care, since we could debug after, but with only 2/2.5 weeks
   till release, we are getting tight for debugging...:(
   
   
   
   
   

4. Re: [HACKERS] Some cleanups/enhancements

   Bruce Momjian <maillist@candle.pha.pa.us> — 1998-02-11T15:52:56Z

   I recommend running the regression test before/after the changes, to
   make sure something didn't get broken.
   
   > 
   > On Wed, 11 Feb 1998, Jeroen van Vianen wrote:
   > 
   > > Hi,
   > > 
   > > I'm running PostgreSQL 6.3 on Linux 2.1.85 with gcc 2.8.0 and libc5. So
   > > far no problems, however I noted some cleanups / enhancements which I
   > > would like to do. Before I send you a bunch of patches I thought I'll
   > > tell you what I'm planning to do. Please comment on my list and indicate
   > > whether I should go ahead.
   > > 
   > > - Fix all Makefiles so 'make dep' and 'make depend' work
   > 
   > 	Can someone explain what, exactly, 'make depend' accomplishes?  We
   > don't use it right now, so I'm wondering why (if?) we need it now?
   > 
   > > - Some other Makefile cleanups
   > > - gcc 2.8.0 issues some additional warnings which are very easy to fix:
   > >   - register i --> register int i
   > >   - Ambiguous else --> add braces:
   > >     if (cond1)
   > >       if (cond2)
   > >         ...
   > >       else
   > >         ...
   > >   - etc.
   > 
   > 	Sounds great...
   > 
   > > - Add a template for linux-elf-586 with (optimized) code for a Pentium
   > > (gcc 2.8.0 not only supports -m486 but also -mpentium and -mpentiumpro).
   > > Why not use template names similar to the output of config.guess (maybe
   > > with some symbolic links)?
   > 
   > 	Erk...I think 'templates' are getting a little out of hand here,
   > no? 
   > 
   > > - Why is there some code to change the case of the procedural language
   > > to lower case except for 'C' (in fact it's there twice)? Why not use
   > > strcasecmp and remove these pices of code?
   > 
   > 	I question this in *alot* of places...like why pg_dlopen is
   > defined as just 'dlopen()' in some ports *shrug*  Why not just call it
   > directly? *raised eyebrow*
   > 
   > > These are all the things that I found after browsing through the code
   > > one night (primarily in backend/access, backend/catalog and
   > > backend/executor).
   > > 
   > > Let me know what you think of the above list and I will proceed. If you
   > > have any hints on how I might proceed (especially with same_tuple)
   > > please don't hesitate. Expect the changes to be available somewhere
   > > after the weekend.
   > 
   > 	The only thing I ask is that you submit these in such a way that
   > they can be easily reviewed before committing them...we are in a beta mode
   > right now, and altho some of this makes for nice cleanups, some of this
   > should most likely be gingerly added...
   > 
   > 	If at all possible, a seperate patch for each point above would be
   > really good, with an explanation of each.  If it weren't for beta-status,
   > I wouldn't care, since we could debug after, but with only 2/2.5 weeks
   > till release, we are getting tight for debugging...:(
   > 
   > 
   > 
   > 
   > 
   
   
   -- 
   Bruce Momjian
   maillist@candle.pha.pa.us
   
   

5. Re: [HACKERS] Some cleanups/enhancements

   Thomas Lockhart <lockhart@alumni.caltech.edu> — 1998-02-11T16:03:51Z

   > I'm running PostgreSQL 6.3 on Linux 2.1.85 with gcc 2.8.0 and libc5. So
   > far no problems, however I noted some cleanups / enhancements which I
   > would like to do. Before I send you a bunch of patches I thought I'll
   > tell you what I'm planning to do. Please comment on my list and indicate
   > whether I should go ahead.
   >
   > - Fix all Makefiles so 'make dep' and 'make depend' work
   > - Fix all Makefiles so 'make clean' throws away the depend file
   > - Some other Makefile cleanups
   
   These all sound good. If there is a possibility of large breakage, wait
   until after v6.3.
   
   > - gcc 2.8.0 issues some additional warnings which are very easy to fix:
   >   - register i --> register int i
   
   I'm sure there will be differing opinions :-/, but imho all register
   declarations should be removed. Modern compilers do a good job of
   optimization, and register declarations can get in the way by forcing the
   compiler to burn a register to accomodate the declared item.
   
   >   - Ambiguous else --> add braces:
   >     if (cond1)
   >       if (cond2)
   >         ...
   >       else
   >         ...
   
   Sure.
   
   > - Add a template for linux-elf-586 with (optimized) code for a Pentium
   > (gcc 2.8.0 not only supports -m486 but also -mpentium and -mpentiumpro).
   > Why not use template names similar to the output of config.guess (maybe
   > with some symbolic links)?
   
   Does gcc 2.7.x support the -mpentium and -mpentiumpro switches? If not,
   then the template should be more explicit in name (e.g.
   "linux-elf-586-gcc2.8") or we should update the FAQ or include comments in
   linux-elf with some suggestions. It was only in the last release or two
   that the -m486 was added, and I worried about causing trouble for _all_ of
   those 386 users :)
   
   > - Shouldn't same_tuple in executor/nodeGroup.c use the equality operator
   > for the type concerned to test for equality of the attributes rather
   > than print them to a buffer and use strcmp? Shouldn't the pointers for
   > these functions be looked up once in ExecInitGroup and stored somewhere?
   > Shouldn't this function go to heaptuple.c and be renamed heap_sametuple?
   
   Yes, I would think so. The only downside to this is that, since two items
   which fail the equality test may look identical when formatted (e.g.
   floating point numbers with the lsb differing) the results may look a bit
   funny and be difficult to track down. Still, I think this is the right way
   to go...
   
   > - Lump heaptuple.c and heapvalid.c together
   > - I also saw quite some #ifdef NOT_USED and other similar stuff. I don't
   > want to touch these now, but shouldn't some of these be removed soon?
   
   Only when the module is completely understood. So, don't remove blindly,
   but if it is clear that it is obsolete code which is not providing hints on
   what should be done in the future, then it is OK to remove.
   
   > - Add a pg_version function that returns a string like 'PostgreSQL 6.3'
   > to indicate the version of PostgreSQL a user is using (with 'select
   > pg_version()'). Might be handy to include in the bug reports.
   
   Good idea.
   
   Some or all of these changes might not be appropriate for v6.3, since we
   are in beta testing and since they do not affect the current functionality.
   For those cases, how about submitting patches based on the final v6.3
   release?
   
                                                                 - Tom
   
   
   

6. Re: [HACKERS] Some cleanups/enhancements

   Bruce Momjian <maillist@candle.pha.pa.us> — 1998-02-11T16:18:11Z

   > 
   > > I'm running PostgreSQL 6.3 on Linux 2.1.85 with gcc 2.8.0 and libc5. So
   > > far no problems, however I noted some cleanups / enhancements which I
   > > would like to do. Before I send you a bunch of patches I thought I'll
   > > tell you what I'm planning to do. Please comment on my list and indicate
   > > whether I should go ahead.
   > >
   > > - Fix all Makefiles so 'make dep' and 'make depend' work
   > > - Fix all Makefiles so 'make clean' throws away the depend file
   > > - Some other Makefile cleanups
   > 
   > These all sound good. If there is a possibility of large breakage, wait
   > until after v6.3.
   > 
   > > - gcc 2.8.0 issues some additional warnings which are very easy to fix:
   > >   - register i --> register int i
   > 
   > I'm sure there will be differing opinions :-/, but imho all register
   > declarations should be removed. Modern compilers do a good job of
   > optimization, and register declarations can get in the way by forcing the
   > compiler to burn a register to accomodate the declared item.
   
   Totally agree.  Get rid of all register's competely.  I don't think this
   will affect Vadim, as most of them are in utility directories.
   
   I agree with your other points too.
   
   
   > 
   > >   - Ambiguous else --> add braces:
   > >     if (cond1)
   > >       if (cond2)
   > >         ...
   > >       else
   > >         ...
   > 
   > Sure.
   > 
   > > - Add a template for linux-elf-586 with (optimized) code for a Pentium
   > > (gcc 2.8.0 not only supports -m486 but also -mpentium and -mpentiumpro).
   > > Why not use template names similar to the output of config.guess (maybe
   > > with some symbolic links)?
   > 
   > Does gcc 2.7.x support the -mpentium and -mpentiumpro switches? If not,
   > then the template should be more explicit in name (e.g.
   > "linux-elf-586-gcc2.8") or we should update the FAQ or include comments in
   > linux-elf with some suggestions. It was only in the last release or two
   > that the -m486 was added, and I worried about causing trouble for _all_ of
   > those 386 users :)
   > 
   > > - Shouldn't same_tuple in executor/nodeGroup.c use the equality operator
   > > for the type concerned to test for equality of the attributes rather
   > > than print them to a buffer and use strcmp? Shouldn't the pointers for
   > > these functions be looked up once in ExecInitGroup and stored somewhere?
   > > Shouldn't this function go to heaptuple.c and be renamed heap_sametuple?
   > 
   > Yes, I would think so. The only downside to this is that, since two items
   > which fail the equality test may look identical when formatted (e.g.
   > floating point numbers with the lsb differing) the results may look a bit
   > funny and be difficult to track down. Still, I think this is the right way
   > to go...
   > 
   > > - Lump heaptuple.c and heapvalid.c together
   > > - I also saw quite some #ifdef NOT_USED and other similar stuff. I don't
   > > want to touch these now, but shouldn't some of these be removed soon?
   > 
   > Only when the module is completely understood. So, don't remove blindly,
   > but if it is clear that it is obsolete code which is not providing hints on
   > what should be done in the future, then it is OK to remove.
   > 
   > > - Add a pg_version function that returns a string like 'PostgreSQL 6.3'
   > > to indicate the version of PostgreSQL a user is using (with 'select
   > > pg_version()'). Might be handy to include in the bug reports.
   > 
   > Good idea.
   > 
   > Some or all of these changes might not be appropriate for v6.3, since we
   > are in beta testing and since they do not affect the current functionality.
   > For those cases, how about submitting patches based on the final v6.3
   > release?
   > 
   >                                                               - Tom
   > 
   > 
   > 
   
   
   -- 
   Bruce Momjian
   maillist@candle.pha.pa.us
   
   

7. Re: [HACKERS] Some cleanups/enhancements

   Brian E Gallew <geek+@cmu.edu> — 1998-02-11T17:02:09Z

   -----BEGIN PGP SIGNED MESSAGE-----
   
   Then <lockhart@alumni.caltech.edu> spoke up and said:
   > > - Add a template for linux-elf-586 with (optimized) code for a Pentium
   > > (gcc 2.8.0 not only supports -m486 but also -mpentium and -mpentiumpro).
   > > Why not use template names similar to the output of config.guess (maybe
   > > with some symbolic links)?
   
   IMHO, none of the gcc-specific optimization switches belong in any
   templates.  If you're using gcc on a pentium, and your version
   supportes -mpentium, then it should be in the SPECS file in your gcc
   installation.  Admittedly, this presumes a certain clue level on the
   part of the gcc installer/maintainer, but it reduces the clutter level
   with templates somewhat.
   
   - -- 
   
   
   =====================================================================
   | "If you're all through trying to burn the field down, will you    |
   | kindly get up and tell me why you're sitting in a fruit field,    |
   | stark naked, frying peaches?"                                     |
   =====================================================================
   | Finger geek@andrew.cmu.edu for my public key.                     |
   =====================================================================
   
   -----BEGIN PGP SIGNATURE-----
   Version: 2.6.2
   
   iQBVAwUBNOHZkIdzVnzma+gdAQEg7QH+MOviZ+pcq5wdpE1d7tK3yB2Ai/03qGti
   7cBxzMQLq82g1/5wT+lXm9Rh3plbyvTBCUpU48kpXTYAYjeAvVIzzQ==
   =C0DN
   -----END PGP SIGNATURE-----
   
   
   

8. Re: [HACKERS] Some cleanups/enhancements

   Jeroen van Vianen <jeroenv@design.nl> — 1998-02-11T17:10:20Z

   Thomas G. Lockhart wrote:
   > 
   > > I'm running PostgreSQL 6.3 on Linux 2.1.85 with gcc 2.8.0 and libc5. So
   > > far no problems, however I noted some cleanups / enhancements which I
   > > would like to do. Before I send you a bunch of patches I thought I'll
   > > tell you what I'm planning to do. Please comment on my list and indicate
   > > whether I should go ahead.
   > >
   > > - Fix all Makefiles so 'make dep' and 'make depend' work
   > > - Fix all Makefiles so 'make clean' throws away the depend file
   > > - Some other Makefile cleanups
   > 
   > These all sound good. If there is a possibility of large breakage, wait
   > until after v6.3.
   
   > Some or all of these changes might not be appropriate for v6.3, since we
   > are in beta testing and since they do not affect the current functionality.
   > For those cases, how about submitting patches based on the final v6.3
   > release?
   
   After the messages I've read so far, I'll wait until after the final
   release of 6.3 and try to do the patches one at a time, so there'll be
   plenty of time :-) to review them.
   
   > [snip]
   
   > > - Add a template for linux-elf-586 with (optimized) code for a Pentium
   > > (gcc 2.8.0 not only supports -m486 but also -mpentium and -mpentiumpro).
   > > Why not use template names similar to the output of config.guess (maybe
   > > with some symbolic links)?
   > 
   > Does gcc 2.7.x support the -mpentium and -mpentiumpro switches? If not,
   > then the template should be more explicit in name (e.g.
   > "linux-elf-586-gcc2.8") or we should update the FAQ or include comments in
   > linux-elf with some suggestions. It was only in the last release or two
   > that the -m486 was added, and I worried about causing trouble for _all_ of
   > those 386 users :)
   
   No, it doesn't. linux-elf-586-gcc2.8 sounds OK to me.
   
   > [snip]
   
   
   The Hermit Hacker wrote:
   > > - Fix all Makefiles so 'make dep' and 'make depend' work
   > 
   >         Can someone explain what, exactly, 'make depend' accomplishes?  We
   > don't use it right now, so I'm wondering why (if?) we need it now?
   
   It makes sure that your C files get compiled if you change a header
   file. Your C compiler should be able to find out which files are
   included and create lines which can be included in the Makefile (man gcc
   :-) )
   
   > > - Some other Makefile cleanups
   > > - gcc 2.8.0 issues some additional warnings which are very easy to fix:
   > >   - register i --> register int i
   > >   - Ambiguous else --> add braces:
   > >     if (cond1)
   > >       if (cond2)
   > >         ...
   > >       else
   > >         ...
   > >   - etc.
   > 
   >         Sounds great...
   
   If I find something like this, I'll remove the register as well.
   
   > > - Add a template for linux-elf-586 with (optimized) code for a Pentium
   > > (gcc 2.8.0 not only supports -m486 but also -mpentium and -mpentiumpro).
   > > Why not use template names similar to the output of config.guess (maybe
   > > with some symbolic links)?
   > 
   >         Erk...I think 'templates' are getting a little out of hand here,
   > no?
   > 
   > > - Why is there some code to change the case of the procedural language
   > > to lower case except for 'C' (in fact it's there twice)? Why not use
   > > strcasecmp and remove these pices of code?
   > 
   >         I question this in *alot* of places...like why pg_dlopen is
   > defined as just 'dlopen()' in some ports *shrug*  Why not just call it
   > directly? *raised eyebrow*
   > 
   
   [snip]
   
   Bruce Momjian wrote:
   > 
   > I recommend running the regression test before/after the changes, to
   > make sure something didn't get broken.
   
   Sure.
   
   > [snip]
   
   See you in a 2-2.5 weeks :-)
   
   Jeroen van Vianen
   
   

9. Re: [HACKERS] Some cleanups/enhancements

   Thomas Lockhart <lockhart@alumni.caltech.edu> — 1998-02-11T17:19:08Z

   > > > - Add a template for linux-elf-586 with (optimized) code for a Pentium
   > > > (gcc 2.8.0 not only supports -m486 but also -mpentium and -mpentiumpro).
   > > > Why not use template names similar to the output of config.guess (maybe
   > > > with some symbolic links)?
   >
   > IMHO, none of the gcc-specific optimization switches belong in any
   > templates.  If you're using gcc on a pentium, and your version
   > supportes -mpentium, then it should be in the SPECS file in your gcc
   > installation.  Admittedly, this presumes a certain clue level on the
   > part of the gcc installer/maintainer, but it reduces the clutter level
   > with templates somewhat.
   
   Great! Want to write it up for the docs? Need a cookbook and an explanation;
   I'll add it in...
   
   If it requires a new install of gcc, then the other option might be to include
   it in Makefile.custom as
   
     CFLAGS+= -mpentium
   
                                                  - Tom
   
   
   

10. Re: [HACKERS] Some cleanups/enhancements

    Bruce Momjian <maillist@candle.pha.pa.us> — 1998-02-11T19:12:14Z

    > 
    > Thomas G. Lockhart wrote:
    > > 
    > > > I'm running PostgreSQL 6.3 on Linux 2.1.85 with gcc 2.8.0 and libc5. So
    > > > far no problems, however I noted some cleanups / enhancements which I
    > > > would like to do. Before I send you a bunch of patches I thought I'll
    > > > tell you what I'm planning to do. Please comment on my list and indicate
    > > > whether I should go ahead.
    > > >
    > > > - Fix all Makefiles so 'make dep' and 'make depend' work
    > > > - Fix all Makefiles so 'make clean' throws away the depend file
    > > > - Some other Makefile cleanups
    > > 
    > > These all sound good. If there is a possibility of large breakage, wait
    > > until after v6.3.
    > 
    > > Some or all of these changes might not be appropriate for v6.3, since we
    > > are in beta testing and since they do not affect the current functionality.
    > > For those cases, how about submitting patches based on the final v6.3
    > > release?
    > 
    > After the messages I've read so far, I'll wait until after the final
    > release of 6.3 and try to do the patches one at a time, so there'll be
    > plenty of time :-) to review them.
    > 
    > > [snip]
    > 
    > > > - Add a template for linux-elf-586 with (optimized) code for a Pentium
    > > > (gcc 2.8.0 not only supports -m486 but also -mpentium and -mpentiumpro).
    > > > Why not use template names similar to the output of config.guess (maybe
    > > > with some symbolic links)?
    > > 
    > > Does gcc 2.7.x support the -mpentium and -mpentiumpro switches? If not,
    > > then the template should be more explicit in name (e.g.
    > > "linux-elf-586-gcc2.8") or we should update the FAQ or include comments in
    > > linux-elf with some suggestions. It was only in the last release or two
    > > that the -m486 was added, and I worried about causing trouble for _all_ of
    > > those 386 users :)
    > 
    > No, it doesn't. linux-elf-586-gcc2.8 sounds OK to me.
    > > > - Some other Makefile cleanups
    > > > - gcc 2.8.0 issues some additional warnings which are very easy to fix:
    > > >   - register i --> register int i
    > > >   - Ambiguous else --> add braces:
    > > >     if (cond1)
    > > >       if (cond2)
    > > >         ...
    > > >       else
    > > >         ...
    > > >   - etc.
    > > 
    > >         Sounds great...
    > 
    > If I find something like this, I'll remove the register as well.
    
    Register is gone.  Just removed them.
    
    -- 
    Bruce Momjian
    maillist@candle.pha.pa.us
    
    

11. Re: [HACKERS] Some cleanups/enhancements

    Bruce Momjian <maillist@candle.pha.pa.us> — 1998-03-16T04:59:32Z

    jeroenv@design.nl, can you send in patches for these?
    
    > 
    > > I'm running PostgreSQL 6.3 on Linux 2.1.85 with gcc 2.8.0 and libc5. So
    > > far no problems, however I noted some cleanups / enhancements which I
    > > would like to do. Before I send you a bunch of patches I thought I'll
    > > tell you what I'm planning to do. Please comment on my list and indicate
    > > whether I should go ahead.
    > >
    > > - Fix all Makefiles so 'make dep' and 'make depend' work
    > > - Fix all Makefiles so 'make clean' throws away the depend file
    > > - Some other Makefile cleanups
    > 
    > These all sound good. If there is a possibility of large breakage, wait
    > until after v6.3.
    > 
    > > - gcc 2.8.0 issues some additional warnings which are very easy to fix:
    > >   - register i --> register int i
    > 
    > I'm sure there will be differing opinions :-/, but imho all register
    > declarations should be removed. Modern compilers do a good job of
    > optimization, and register declarations can get in the way by forcing the
    > compiler to burn a register to accomodate the declared item.
    > 
    > >   - Ambiguous else --> add braces:
    > >     if (cond1)
    > >       if (cond2)
    > >         ...
    > >       else
    > >         ...
    > 
    > Sure.
    > 
    > > - Add a template for linux-elf-586 with (optimized) code for a Pentium
    > > (gcc 2.8.0 not only supports -m486 but also -mpentium and -mpentiumpro).
    > > Why not use template names similar to the output of config.guess (maybe
    > > with some symbolic links)?
    > 
    > Does gcc 2.7.x support the -mpentium and -mpentiumpro switches? If not,
    > then the template should be more explicit in name (e.g.
    > "linux-elf-586-gcc2.8") or we should update the FAQ or include comments in
    > linux-elf with some suggestions. It was only in the last release or two
    > that the -m486 was added, and I worried about causing trouble for _all_ of
    > those 386 users :)
    > 
    > > - Shouldn't same_tuple in executor/nodeGroup.c use the equality operator
    > > for the type concerned to test for equality of the attributes rather
    > > than print them to a buffer and use strcmp? Shouldn't the pointers for
    > > these functions be looked up once in ExecInitGroup and stored somewhere?
    > > Shouldn't this function go to heaptuple.c and be renamed heap_sametuple?
    > 
    > Yes, I would think so. The only downside to this is that, since two items
    > which fail the equality test may look identical when formatted (e.g.
    > floating point numbers with the lsb differing) the results may look a bit
    > funny and be difficult to track down. Still, I think this is the right way
    > to go...
    > 
    > > - Lump heaptuple.c and heapvalid.c together
    > > - I also saw quite some #ifdef NOT_USED and other similar stuff. I don't
    > > want to touch these now, but shouldn't some of these be removed soon?
    > 
    > Only when the module is completely understood. So, don't remove blindly,
    > but if it is clear that it is obsolete code which is not providing hints on
    > what should be done in the future, then it is OK to remove.
    > 
    > > - Add a pg_version function that returns a string like 'PostgreSQL 6.3'
    > > to indicate the version of PostgreSQL a user is using (with 'select
    > > pg_version()'). Might be handy to include in the bug reports.
    > 
    > Good idea.
    > 
    > Some or all of these changes might not be appropriate for v6.3, since we
    > are in beta testing and since they do not affect the current functionality.
    > For those cases, how about submitting patches based on the final v6.3
    > release?
    > 
    >                                                               - Tom
    > 
    > 
    > 
    
    
    -- 
    Bruce Momjian                          |  830 Blythe Avenue
    maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
      +  If your life is a hard drive,     |  (610) 353-9879(w)
      +  Christ can be your backup.        |  (610) 853-3000(h)
    
    

12. Re: [HACKERS] Some cleanups/enhancements

    Jeroen van Vianen <jeroenv@design.nl> — 1998-03-17T18:42:11Z

    Bruce Momjian wrote:
    > 
    > jeroenv@design.nl, can you send in patches for these?
    > 
    > >
    > > > I'm running PostgreSQL 6.3 on Linux 2.1.85 with gcc 2.8.0 and libc5. So
    > > > far no problems, however I noted some cleanups / enhancements which I
    > > > would like to do. Before I send you a bunch of patches I thought I'll
    > > > tell you what I'm planning to do. Please comment on my list and indicate
    > > > whether I should go ahead.
    > > >
    > > > - Fix all Makefiles so 'make dep' and 'make depend' work
    > > > - Fix all Makefiles so 'make clean' throws away the depend file
    > > > - Some other Makefile cleanups
    > >
    > > These all sound good. If there is a possibility of large breakage, wait
    > > until after v6.3.
    > >
    > > > - gcc 2.8.0 issues some additional warnings which are very easy to fix:
    > > >   - register i --> register int i
    
    There's a patch attached to fix gcc 2.8.x warnings, except for the
    yyerror ones from bison. It also includes a few 'enhancements' to the C
    programming style (which are, of course, personal).
    
    The other patch removes the compilation of backend/lib/qsort.c, as
    qsort() is a standard function in stdlib.h and can be used any where
    else (and it is). It was only used in
    backend/optimizer/geqo/geqo_pool.c, backend/optimizer/path/predmig.c,
    and backend/storage/page/bufpage.c
    
    > > Some or all of these changes might not be appropriate for v6.3, since we
    > > are in beta testing and since they do not affect the current functionality.
    > > For those cases, how about submitting patches based on the final v6.3
    > > release?
    
    There's more to come. Please review these patches. I ran the regression
    tests and they only failed where this was expected (random, geo, etc).
    
    Cheers,
    
    Jeroen
    

13. Re: [HACKERS] Some cleanups/enhancements

    Bruce Momjian <maillist@candle.pha.pa.us> — 1998-03-17T19:07:53Z

    > There's a patch attached to fix gcc 2.8.x warnings, except for the
    > yyerror ones from bison. It also includes a few 'enhancements' to the C
    > programming style (which are, of course, personal).
    > 
    > The other patch removes the compilation of backend/lib/qsort.c, as
    > qsort() is a standard function in stdlib.h and can be used any where
    > else (and it is). It was only used in
    > backend/optimizer/geqo/geqo_pool.c, backend/optimizer/path/predmig.c,
    > and backend/storage/page/bufpage.c
    > 
    
    Woh, this is some pretty serious code.  Nice.
    
    -- 
    Bruce Momjian                          |  830 Blythe Avenue
    maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
      +  If your life is a hard drive,     |  (610) 353-9879(w)
      +  Christ can be your backup.        |  (610) 853-3000(h)
    
    

14. Re: [HACKERS] Some cleanups/enhancements

    Bruce Momjian <maillist@candle.pha.pa.us> — 1998-03-30T16:45:01Z

    Applied.  Looked good, and qsort.c removed as you suggested.
    
    
    > There's a patch attached to fix gcc 2.8.x warnings, except for the
    > yyerror ones from bison. It also includes a few 'enhancements' to the C
    > programming style (which are, of course, personal).
    > 
    > The other patch removes the compilation of backend/lib/qsort.c, as
    > qsort() is a standard function in stdlib.h and can be used any where
    > else (and it is). It was only used in
    > backend/optimizer/geqo/geqo_pool.c, backend/optimizer/path/predmig.c,
    > and backend/storage/page/bufpage.c
    > 
    > > > Some or all of these changes might not be appropriate for v6.3, since we
    > > > are in beta testing and since they do not affect the current functionality.
    > > > For those cases, how about submitting patches based on the final v6.3
    > > > release?
    > 
    > There's more to come. Please review these patches. I ran the regression
    > tests and they only failed where this was expected (random, geo, etc).
    > 
    > Cheers,
    > 
    > Jeroen
    
    
    -- 
    Bruce Momjian                          |  830 Blythe Avenue
    maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
      +  If your life is a hard drive,     |  (610) 353-9879(w)
      +  Christ can be your backup.        |  (610) 853-3000(h)