Fujii,
* Fujii Masao (masao.fujii@gmail.com) wrote:
> pg_audit uses 1.0.0 as its version number. But, is the third digit really
> required? Why? We usually uses the version number with two digits in
> contrib modules.
I have to admit, I didn't look closely at how we handled versions in
contrib modules and that has been the same since the patch was first
posted, as I recall. No problem changing it to 1.0 and I'll take care
of that soon.
> 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.
> In Makefile, PGFILEDESC should be added.
>
> +# pg_audit/Makefile
>
> should be "# contrib/pg_audit/Makefile" for the consistency.
Good points, will address.
> 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.
Thanks!
Stephen
