client_query declared not null, but possible to have null

[ADTC]

In CREATE TABLE audit.logged_actions, the column client_query is declared as not null. This is in conflict with:

IF NOT TG_ARGV[0]::boolean IS DISTINCT FROM 'f'::boolean THEN
    audit_row.client_query = NULL;

In fact, I ran into a problem where the trigger was trying to insert with a null value in client_query, and it failed. I resolved it by changing this column to accept null values.

I also think that most (if not all) columns in logged_actions must accept null values. This will be essential to prevent applications from crashing because the audit trigger failed due to a null insert in a not null column.

[ringerc]

I tend to agree. I won't have time to fix it up in the next few days, but if you're happy to send a pull request I can merge it.

[ADTC]

You're Craig Ringer in SO aren't you? :)

I've made the request. Took me a while to figure out how. I just removed the not null in the fork, but in my local use, I actually made every column nullable (just to be sure this doesn't happen again - I don't want the main application to fail) and there's no value check in action column.

In any case, what is TG_ARGV[0] Is the if check for avoiding the insert of current query for row triggers (only inserted for statement triggers)?

[ringerc]

On 01/27/2014 06:25 PM, ADTC wrote:

You're Craig Ringer in SO aren't you? |:)|

I've made the request. Took me a while to figure out how. I just removed the |not null| in the fork, but in my local use, I actually made every column nullable (just to be sure this doesn't happen again - I don't want the main application to fail).

If there's any circumstance where a table trigger can fire with no relation oid, tble name, or schema name set, then there's something really, really weird going on.

In any case, what is |TG_ARGV[0]| Is the |if| check for avoiding the insert of current query for row triggers (only inserted for statement triggers)?

I don't understand that paragraph.

TG_ARGV[0] ie the first argument in the trigger argument array. (It's zero-indexed, unlike most arrays in PostgreSQL). That argument is a boolean, defaulting to 't', controlling whether this trigger invocation should log the query text.

Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services

[ADTC]

Apologies, I missed a question mark in that paragraph (eats, shoots and leaves!)... I meant to ask: What is TG_ARGV[0]? (first question) followed by: Is the if check for avoiding insert of current_query [...] ? (second question)

You have answered the questions adequately, except what determines whether the query text is logged or not? (that is, what sets the value of TG_ARGV[0]?)

Anyway I should probably look through the code to get the answer. I'll close this issue as it's not important to have the answer. :)

[ringerc]

On 01/28/2014 03:51 PM, ADTC wrote:

Apologies, I missed a question mark in that paragraph /(eats, shoots and leaves!)/... I meant to ask: What is TG_ARGV[0]? (first question) followed by: Is the /if/ check for avoiding insert of current_query [...] ? (second question)

You have answered the questions adequately, except what determines whether the query text is logged or not? (that is, what sets the value of TG_ARGV[0]?)

Anyway I should probably look through the code to get the answer. I'll close this issue as it's not important. |:)|

TG_ARGV[0] is set by the first argument you pass at CREATE TRIGGER time.

https://github.com/2ndQuadrant/audit-trigger/blob/master/audit.sql#L153

which is passed as:

https://github.com/2ndQuadrant/audit-trigger/blob/master/audit.sql#L181

Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services