Re: [COMMITTERS] pgsql-server/src backend/main/main.c backend/p ...

From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org>
Subject: Re: [COMMITTERS] pgsql-server/src backend/main/main.c backend/p ...
Date: 2004-05-19 17:14:58
Message-ID: 200405191714.i4JHEwR13620@candle.pha.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-patches

Tom Lane wrote:
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > Tom Lane wrote:
> >> momjian(at)svr1(dot)postgresql(dot)org (Bruce Momjian) writes:
> >>> Remove elog() calls from find_my_exec; do fprintf(stderr) instead.
> >>
> >> Isn't that a seriously bad idea?
>
> > Yes, it probably is bad, but I am not sure how frequently this will
> > happen, _and_ I need to set my_exec_path very far up in the tree to the
> > timezone stuff knows the location, hence the fact that elog() calls
> > wouldn't work at all anyway, I think.
>
> The more I think about this, the less I like it.
>
> find_my_exec and friends are *not* more critical to the backend than
> elog is, and should not be done sooner. The argument that "we have to
> find postgresql.conf before elog works" is specious --- elog will work
> fine before we have read the config file, indeed had better do so since
> guc.c uses elog to report problems. What will happen is that errors
> will get reported to the compiled-in-default location, which happens to
> be stderr at the moment. All you are accomplishing with this change is
> to hard-wire that default and make it impossible to change in the
> future.

Sure, I didn't know elog() sent to stderr before it was initialized.

The following patch uses elog() when linked to the backend, and stderr
directly when used by client code. The old code was really DEBUG2-level
stuff (it showed how it was searching the PATH mostly), but I have
remove those because for clients that is going to go to stderr, so now
it only does an elog(LOG) when something significant happens.

I did have to re-add the separate compile of exec.c to every place that
used it because clients don't have elog, but that's OK.

> Other concerns: I do not like changing random error reports from elog to
> printf, firstly because it will encourage people to code other error
> reports as printfs, which is something that took us a huge amount of
> work to stamp out, and secondly because the requirement will propagate.
> Are you going to avoid using palloc in find_my_exec, too? What about in
> any subroutines that these things call?
>
> I recommend reverting both this change and the ones to do find_my_exec
> etc in main.c. You're better off doing these things where they were
> done in PostmasterMain, PostgresMain, etc. palloc and elog are simply
> fundamental parts of the backend programming environment; it does not
> make sense to push complex functionality into a part of the code where
> they aren't available.

In the old code, the exec path was only used for dynamic linking for
platforms that needed a full path for dynamic linking. Now the timezone
code and even the $libdir path (used by GUC) is set from my_exec_path,
so it has to be earlier. I can put it much earlier in both postgres and
postmaster, but by having it in main.c, I have it in only one place. It
doesn't do any palloc or anything fancy, because of course it is also
used by client apps.

Patch attached and applied.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Attachment Content-Type Size
unknown_filename text/plain 10.0 KB

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Bruce Momjian 2004-05-19 17:15:21 pgsql-server/src bin/initdb/Makefile bin/pg_du ...
Previous Message Tom Lane 2004-05-19 16:31:07 Re: pgsql-server/src backend/main/main.c backend/p ...

Browse pgsql-patches by date

  From Date Subject
Next Message Tom Lane 2004-05-19 17:27:33 Re: [COMMITTERS] pgsql-server/src backend/main/main.c backend/p ...
Previous Message Bruce Momjian 2004-05-19 16:46:29 Re: new aggregate functions v1