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

Wed, 11 Feb 1998 10:22:05 +0100
Jeroen van Vianen <jeroenv@design.nl>
Some cleanups/enhancements
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

Wed, 11 Feb 1998 10:27:59 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] Some cleanups/enhancements
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...:(




Wed, 11 Feb 1998 10:43:00 -0500 (EST)
The Hermit Hacker <scrappy@hub.org>
Re: [HACKERS] Some cleanups/enhancements
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

Wed, 11 Feb 1998 10:52:56 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] Some cleanups/enhancements
> 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


Wed, 11 Feb 1998 16:03:51 +0000
"Thomas G. Lockhart" <lockhart@alumni.caltech.edu>
Re: [HACKERS] Some cleanups/enhancements
> 
> > 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

Wed, 11 Feb 1998 11:18:11 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] Some cleanups/enhancements
-----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-----


11 Feb 1998 12:02:09 -0500
Brian E Gallew <geek+@cmu.edu>
Re: [HACKERS] Some cleanups/enhancements
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

Wed, 11 Feb 1998 18:10:20 +0100
Jeroen van Vianen <jeroenv@design.nl>
Re: [HACKERS] Some cleanups/enhancements
> > > - 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


Wed, 11 Feb 1998 17:19:08 +0000
"Thomas G. Lockhart" <lockhart@alumni.caltech.edu>
Re: [HACKERS] Some cleanups/enhancements
> 
> 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

Wed, 11 Feb 1998 14:12:14 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] Some cleanups/enhancements
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)

Sun, 15 Mar 1998 23:59:32 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] Some cleanups/enhancements
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
Tue, 17 Mar 1998 19:42:11 +0100
Jeroen van Vianen <jeroenv@design.nl>
Re: [HACKERS] Some cleanups/enhancements
> 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)

Tue, 17 Mar 1998 14:07:53 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] Some cleanups/enhancements
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)

Mon, 30 Mar 1998 11:45:01 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] Some cleanups/enhancements