Re: Add support for logging the current role

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Add support for logging the current role
Date: 2011-01-12 17:59:57
Message-ID: 20110112175957.GK4933@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

* Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
> Logging the OID seems to be of questionable value.

I certainly disagree about this, not being able to figure out what's
causing a 'permissions denied' error because you don't know which role
the log is coming from is *very* annoying. Having to go look up the
role from the OID in the log is also annoying, but less so, imv. :)

> I thought of the
> update-the-variable-when-it-changes approach too, but besides being a
> bit expensive if it's changing frequently, it's not necessarily safe
> to do the syscache lookup there either - see the comments for
> GetUserIdAndSecContext (which are really for SetUserIdAndSecContext,
> but they're in an odd place).

Alright, here's a patch which adds the ability to log the current role's
OID and which calls GetUserIdAndSecContext() directly and handles the
possibility that CurrentUserId isn't valid. Perhaps we should just grab
CurrentUserId directly rather than going through
GetUserIdAndSecContext()? I could certainly do that instead.

Also includes those additional comments in elog.c.

Thanks,

Stephen

commit d9a7acd5ea1f5214b44875b6d257c5c59590167c
Author: Stephen Frost <sfrost(at)snowman(dot)net>
Date: Wed Jan 12 12:53:50 2011 -0500

Use GetUserIdAndSecContext to get role OID in elog

We can't be sure that GetUserId() will be called when current_user
is a valid Oid, per the comments in GetUserIdAndSecContext, when
called from elog.c, so instead call GetUserIdAndSecContext directly
and handle the possible invalid Oid ourselves.

commit 605497b062298ea195d8999f8cefca10968ae22f
Author: Stephen Frost <sfrost(at)snowman(dot)net>
Date: Wed Jan 12 12:29:44 2011 -0500

Change to logging role's OID instead of name

Remove the SysCache lookup from elog.c/log_line_prefix by logging
the role's OID instead, this addresses a concern where a SysCache
lookup could malfunction badly due to logging from a failed
transaction. Note that using SysCache from the elog routines could
also be a performance hit, though this would only be the case if a
user chose to enable that logging.

Attachment Content-Type Size
log_role_option.patch text/x-diff 2.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2011-01-12 18:06:59 Re: pg_depend explained
Previous Message Bruce Momjian 2011-01-12 17:46:40 Re: libpq documentation cleanups (repost 3)