Re: [HACKERS] 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(at)candle(dot)pha(dot)pa(dot)us