On 2014-05-17 00:17:46 +0200, Andres Freund wrote:
> On 2014-05-16 17:48:28 -0400, Tom Lane wrote:
> > This is basically reintroducing a variable that used to exist and was
> > intentionally gotten rid of, on the grounds that it'd fail to track
> > any renaming of the session's current database (cf commit b256f24264).
> > I'm not sure how much effort ought to be put into dealing with that corner
> > case; but let's not reintroduce old bugs just for failure to remember
> > them.
>
> I did look whether there's a race making MyDatabaseName out of date. I
> didn't see any. There's a windows where it could be renamed between
> where I'd done the MyDatabaseName = ... and the LockSharedObject() a few
> lines below. But directly afterwards there's a check that the database name is
> still correct.
> We could move the filling of MyDatabaseName to lessen the amount of
> comments that need to be added.
Moving it also provides the name in bootstrap mode. No idea whether
there'll ever be a use in that but given the variable may later replace
some get_database_name(MyDatabaseId) calls it seems to be a benefit.
I'd briefly changed elog.c to only use MydatabaseName, thinking that
there seems to be little reason to continue using database_name in
elog.c once the variable is introduced. But that's not a good idea:
MyProcPort->database_name exists earlier.
It's imo a bit debatable whether it's a good idea to log database names
in log_line_prefix that don't exist. But that's a separate change.
I've changed it though so that MyDatabaseName is preferred over
MyProc. It's imo easier to see that it has the correct value. And it'll
be correct if we ever support mapping authentication and real database
names or something.
Revised patch attached.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
