Hi Robert, Thank you for replying.
On 2/1/16 11:23 PM, Robert Haas wrote: > 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. I agree this is a concern. It's similar to deparse or event triggers in this regard with the notable exception that pgaudit is not in core. However, if it becomes popular enough out of core as everyone insists is preferable then people will still need to maintain it. Just as PostGIS has a close relationship with core, the pgaudit team would need to have the same sort of relationship. Patches would be submitted for review and (hopefully) committed and core developer time would still be spent on pgaudit, ableit indirectly. Core developers would still have to be careful not to break pgaudit if it became popular enough. > The set of things that the patch can audit is pretty arbitrary and not > well tied into the core code. Since the set of what it can audit is every command that can be run by a user in Postgres I don't see how that's arbitrary. The amount of *additional* detail provided for each audit record is also not arbitrary but a function of what information is available through various hooks and event triggers. I was able to improve on the amount of information provided over the original 2Q code (mostly by abandoning support for Postgres < 9.5) but the methodology remains the same. > 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. I think a lot of this could be mitigated by some changes in utility.c. I'm planning patches that will allow mapping command strings back to event tags and a general classifier function that could incidentally be used to improve the granularity of log_statement. > 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. There *could* be five extensions but there are not. To my knowledge there are two and one is just a more evolved version of the other. > 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. People who are interested in audit are also understandably leery of downloading code from an untrusted source. Both PGXN and GitHub are The Wild West as far as conservative auditors are concerned. Your use of the phrase "or wherever" only reinforces the point in my mind. The implication is that it doesn't matter where the pgaudit extension comes from but it does matter, a lot. > 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. I'll be the first to admit that the design is not the prettiest. Trying to figure out what Postgres is doing internally through a couple of hooks is like trying to replicate the script of a play when all you have is the program. However, so far it has been performed well and been reliable in field tests. -- -David da...@pgmasters.net
signature.asc
Description: OpenPGP digital signature