Re: pgsql: Add pg_audit, an auditing extension

From: David Steele <david(at)pgmasters(dot)net>
To: Stephen Frost <sfrost(at)snowman(dot)net>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
Cc: pgsql-committers <pgsql-committers(at)postgresql(dot)org>
Subject: Re: pgsql: Add pg_audit, an auditing extension
Date: 2015-05-15 21:07:00
Message-ID: 55565FF4.9040506@pgmasters.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

On 5/15/15 3:44 PM, Stephen Frost wrote:
> * Fujii Masao (masao(dot)fujii(at)gmail(dot)com) wrote:
>> CREATE EXTENSION pg_audit failed with the following error message
>> when shared_preload_libraries and pg_audit.log are set to pg_audit and
>> ddl, respectively.
>>
>> =# create extension pg_audit ;
>> ERROR: pg_event_trigger_ddl_commands() can only be called in an event
>> trigger function
>> CONTEXT: SQL statement "SELECT UPPER(object_type), object_identity
>> FROM pg_event_trigger_ddl_commands()"
> Interesting. I'm very curious about this error and if it impacts other
> extensions which use event triggers. I'll look into it.

I'm looking into this. There's definitely something strange going on
here, but not sure it's in pg_audit.

>> The categories of some SQL commands are different between log_statement and
>> pg_audit. For example, REINDEX is treated as DDL in pg_audit, but not in
>> log_statement. That's confusing. We should use the same "category-mapping"
>> rule as much as possible.
> David, Simon and I have all considered this at different times and my
> recollection is that we all felt that it made sense as DDL because
> CLUSTER is DDL (which is actually noted in the comments). However, you
> bring up a good point that classifying it as DDL makes it different from
> what the core system does and it'd probably be good to be consistent.
>
> The question here really is- is that the right classification for
> REINDEX to have in core? If so, shouldn't CLUSTER have the same?
> Probably a discussion to have on -hackers rather than here. I'll go
> ahead and change it to match what core does. Then, if core changes to
> make it DDL, pg_audit will automatically pick that up and also treat it
> as DDL.
I admit I was influenced by this comment in core:

case T_ReindexStmt:
lev = LOGSTMT_ALL; /* should this be DDL? */
break;

and the fact that CLUSTER is DDL. I'm not married to REINDEX as DDL,
though. I'm happy to make it consistent with core. If it changes in
core, it would also change in pg_audit.

--
- David Steele
david(at)pgmasters(dot)net

In response to

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2015-05-15 21:38:50 Re: pgsql: Add BRIN infrastructure for "inclusion" opclasses
Previous Message Alvaro Herrera 2015-05-15 21:05:17 pgsql: Add BRIN infrastructure for "inclusion" opclasses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2015-05-15 21:20:38 Re: BRIN range operator class
Previous Message David G. Johnston 2015-05-15 21:04:26 Re: Problems with question marks in operators (JDBC, ECPG, ...)