Re: PostgreSQL Audit Extension

On Sun, Jan 31, 2016 at 5:19 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> Joshua D. Drake wrote:
>> On 01/31/2016 05:07 AM, Alvaro Herrera wrote:
>> >David Steele wrote:
>> >>The attached patch implements audit logging for PostgreSQL as an
>> >>extension.  I believe I have addressed the concerns that were raised at
>> >>the end of the 9.5 development cycle.
>> >
>> >This patch got no feedback at all during the commitfest.  I think there
>> >is some interest on auditing capabilities so it's annoying and
>> >surprising that this has no movement at all.
>> >
>> >If the lack of activity means lack of interest, please can we all keep
>> >what we've been doing in this thread, which is to ignore it, so that we
>> >can just mark this as rejected.  Otherwise, please chime in.  I'm giving
>> >this patch some more time by moving it to next commitfest instead.
>>
>> From my perspective, lack of activity means since it doesn't have a
>> technical requirement to be in -core, it doesn't need to be.
>
> Well, from mine it doesn't mean that.  We kind of assume that "no answer
> means yes"; but here what it really means is that nobody had time to
> look at it and think about it, so it stalled (and so have many other
> patches BTW).  But if you or others think that this patch belongs into
> PGXN, it's good to have that opinion in an email, so that the author can
> think about your perspective and agree or disagree with it.  Just by
> expression that opinion, a thousand other hackers might vote +1 or -1 on
> your proposal -- either way a clear sign.  Silence doesn't let us move
> forward, and we punt the patch to the next CF and let inertia continue.
> That's not good.

OK, I'll bite: I'm worried that this patch will be a maintenance
burden.  It's easy to imagine that changes to core will result in the
necessity or at least desirability of changes to pgaudit, but I'm
definitely not prepared to insist that future authors try to insist
that future patch submitters have to understand this code and update
it as things change.

The set of things that the patch can audit is pretty arbitrary and not
well tied into the core code.  There is a list of string constants in
the code that covers each type of relations plus functions, but not
any other kind of SQL object.  If somebody adds a new relkind, this
would probably need to updated - it would not just work.  If somebody
adds a new type of SQL object, it won't be covered unless the user
takes some explicit action, but there's no obvious guiding principle
to say whether that would be appropriate in any particular case.  In
saying that it's arbitrary, I'm not saying it isn't *useful*.  I'm
saying there could be five extensions like this that make equally
arbitrary decisions about what to do and how to do it, and they could
all be useful to different people.  I don't really want to bless any
given one of those current or hypothetical future solutions.  We have
hooks precisely so that people can write stuff like this and put it up
on PGXN or github or wherever - and this code can be published there,
and people who want to can use it.

It also appears to me that if we did want to do that, it would need
quite a lot of additional cleanup.  I haven't dug in enough to have a
list of specific issues, but it does look to me like there would be
quite a bit.  Maybe that'd be worth doing if there were other
advantages of having this be in core, but I don't see them.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company