Fix logic to properly classify SELECT FOR UPDATE as SELECT.

This logic was submitted in PR #88 but there was some confusion on my part about what it was supposed to do. Since the title was "Suppress logging for internally generated foreign-key queries" I tried to make it do that, and broke SELECT FOR UPDATE logging, which unfortunately had no test.

Reading the PR again, it seems Peter's intention was only to correctly classify SELECT FOR UPDATE as SELECT. In any case that represents an improvement over what we have, even if it does not suppress logging for internally generated foreign-key queries, at least not in the case of SELECT.

So, revert the logic that suppressed the SELECT FOR UPDATE logging and use Peter's logic that correctly classifies them as SELECT.

Also add a SELECT FOR UPDATE test to prevent regressions.

Reported by Sergey Shinderuk (@shinderuk).
Reviewed by Sergey Shinderuk (@shinderuk), Stephen Frost (@sfrost).

expected/pgaudit.out

@@ -546,17 +546,38 @@ UPDATE account
    SET description = 'yada, yada';
 NOTICE:  AUDIT: SESSION,3,1,WRITE,UPDATE,TABLE,public.account,"UPDATE account
    SET description = 'yada, yada';",<not logged>
+--
+-- Object logged because of:
+--     select (password) on account (in the where clause)
+-- Session logged on all tables because log = read and log_relation = on
+SELECT *
+  FROM account
+ WHERE password = 'HASH2'
+   FOR UPDATE;
+NOTICE:  AUDIT: OBJECT,4,1,READ,SELECT,TABLE,public.account,"SELECT *
+  FROM account
+ WHERE password = 'HASH2'
+   FOR UPDATE;",<not logged>
+NOTICE:  AUDIT: SESSION,4,1,READ,SELECT,TABLE,public.account,"SELECT *
+  FROM account
+ WHERE password = 'HASH2'
+   FOR UPDATE;",<not logged>
+ id | name  | password | description 
+----+-------+----------+-------------
+  1 | user1 | HASH2    | yada, yada
+(1 row)
+
 --
 -- Object logged because of:
 -- select (password) on account (in the where clause)
 -- Session logged on all tables because log = read and log_relation = on
 UPDATE account
    SET description = 'yada, yada'
  where password = 'HASH2';
-NOTICE:  AUDIT: OBJECT,4,1,WRITE,UPDATE,TABLE,public.account,"UPDATE account
+NOTICE:  AUDIT: OBJECT,5,1,WRITE,UPDATE,TABLE,public.account,"UPDATE account
    SET description = 'yada, yada'
  where password = 'HASH2';",<not logged>
-NOTICE:  AUDIT: SESSION,4,1,WRITE,UPDATE,TABLE,public.account,"UPDATE account
+NOTICE:  AUDIT: SESSION,5,1,WRITE,UPDATE,TABLE,public.account,"UPDATE account
    SET description = 'yada, yada'
  where password = 'HASH2';",<not logged>
 --
@@ -565,9 +586,9 @@ NOTICE:  AUDIT: SESSION,4,1,WRITE,UPDATE,TABLE,public.account,"UPDATE account
 -- Session logged on all tables because log = read and log_relation = on
 UPDATE account
    SET password = 'HASH2';
-NOTICE:  AUDIT: OBJECT,5,1,WRITE,UPDATE,TABLE,public.account,"UPDATE account
+NOTICE:  AUDIT: OBJECT,6,1,WRITE,UPDATE,TABLE,public.account,"UPDATE account
    SET password = 'HASH2';",<not logged>
-NOTICE:  AUDIT: SESSION,5,1,WRITE,UPDATE,TABLE,public.account,"UPDATE account
+NOTICE:  AUDIT: SESSION,6,1,WRITE,UPDATE,TABLE,public.account,"UPDATE account
    SET password = 'HASH2';",<not logged>
 --
 -- Change configuration of user 1 so that full statements are not logged
@@ -1154,8 +1175,12 @@ NOTICE:  AUDIT: SESSION,66,1,WRITE,INSERT,TABLE,public.aaa,"INSERT INTO aaa VALU
 SET pgaudit.log_parameter TO OFF;
 INSERT INTO bbb VALUES (1);
 NOTICE:  AUDIT: SESSION,67,1,WRITE,INSERT,TABLE,public.bbb,INSERT INTO bbb VALUES (1);,<not logged>
-NOTICE:  AUDIT: OBJECT,67,2,WRITE,UPDATE,TABLE,public.bbb,UPDATE bbb set id = new.id + 1,<not logged>
-NOTICE:  AUDIT: SESSION,67,2,WRITE,UPDATE,TABLE,public.bbb,UPDATE bbb set id = new.id + 1,<not logged>
+NOTICE:  AUDIT: OBJECT,67,2,READ,SELECT,TABLE,public.aaa,"SELECT 1 FROM ONLY ""public"".""aaa"" x WHERE ""id"" OPERATOR(pg_catalog.=) $1 FOR KEY SHARE OF x",<not logged>
+NOTICE:  AUDIT: SESSION,67,2,READ,SELECT,TABLE,public.aaa,"SELECT 1 FROM ONLY ""public"".""aaa"" x WHERE ""id"" OPERATOR(pg_catalog.=) $1 FOR KEY SHARE OF x",<not logged>
+NOTICE:  AUDIT: OBJECT,67,3,WRITE,UPDATE,TABLE,public.bbb,UPDATE bbb set id = new.id + 1,<not logged>
+NOTICE:  AUDIT: SESSION,67,3,WRITE,UPDATE,TABLE,public.bbb,UPDATE bbb set id = new.id + 1,<not logged>
+NOTICE:  AUDIT: OBJECT,67,4,READ,SELECT,TABLE,public.aaa,"SELECT 1 FROM ONLY ""public"".""aaa"" x WHERE ""id"" OPERATOR(pg_catalog.=) $1 FOR KEY SHARE OF x",<not logged>
+NOTICE:  AUDIT: SESSION,67,4,READ,SELECT,TABLE,public.aaa,"SELECT 1 FROM ONLY ""public"".""aaa"" x WHERE ""id"" OPERATOR(pg_catalog.=) $1 FOR KEY SHARE OF x",<not logged>
 SET pgaudit.log_parameter TO ON;
 DROP TABLE bbb;
 DROP TABLE aaa;
@@ -2337,9 +2362,13 @@ INSERT INTO aaa VALUES (generate_series(1,100));
 NOTICE:  AUDIT: SESSION,120,1,WRITE,INSERT,TABLE,public.aaa,"INSERT INTO aaa VALUES (generate_series(1,100));",<none>,100
 SET pgaudit.log_parameter TO OFF;
 INSERT INTO bbb VALUES (1);
-NOTICE:  AUDIT: OBJECT,121,1,WRITE,UPDATE,TABLE,public.bbb,UPDATE bbb set id = new.id + 1,<not logged>,1
-NOTICE:  AUDIT: SESSION,121,1,WRITE,UPDATE,TABLE,public.bbb,UPDATE bbb set id = new.id + 1,<not logged>,1
-NOTICE:  AUDIT: SESSION,121,2,WRITE,INSERT,TABLE,public.bbb,INSERT INTO bbb VALUES (1);,<not logged>,1
+NOTICE:  AUDIT: OBJECT,121,1,READ,SELECT,TABLE,public.aaa,"SELECT 1 FROM ONLY ""public"".""aaa"" x WHERE ""id"" OPERATOR(pg_catalog.=) $1 FOR KEY SHARE OF x",<not logged>,1
+NOTICE:  AUDIT: SESSION,121,1,READ,SELECT,TABLE,public.aaa,"SELECT 1 FROM ONLY ""public"".""aaa"" x WHERE ""id"" OPERATOR(pg_catalog.=) $1 FOR KEY SHARE OF x",<not logged>,1
+NOTICE:  AUDIT: OBJECT,121,2,READ,SELECT,TABLE,public.aaa,"SELECT 1 FROM ONLY ""public"".""aaa"" x WHERE ""id"" OPERATOR(pg_catalog.=) $1 FOR KEY SHARE OF x",<not logged>,1
+NOTICE:  AUDIT: SESSION,121,2,READ,SELECT,TABLE,public.aaa,"SELECT 1 FROM ONLY ""public"".""aaa"" x WHERE ""id"" OPERATOR(pg_catalog.=) $1 FOR KEY SHARE OF x",<not logged>,1
+NOTICE:  AUDIT: OBJECT,121,3,WRITE,UPDATE,TABLE,public.bbb,UPDATE bbb set id = new.id + 1,<not logged>,1
+NOTICE:  AUDIT: SESSION,121,3,WRITE,UPDATE,TABLE,public.bbb,UPDATE bbb set id = new.id + 1,<not logged>,1
+NOTICE:  AUDIT: SESSION,121,4,WRITE,INSERT,TABLE,public.bbb,INSERT INTO bbb VALUES (1);,<not logged>,1
 SET pgaudit.log_parameter TO ON;
 DROP TABLE bbb;
 DROP TABLE aaa;

pgaudit.c

@@ -1012,19 +1012,12 @@ log_select_dml(Oid auditOid, List *rangeTabls)
         /*
          * Don't log if the session user is not a member of the current
          * role.  This prevents contents of security definer functions
-         * from being logged.
+         * from being logged and supresses foreign key queries unless the
+         * session user is the owner of the referenced table.
          */
         if (!is_member_of_role(GetSessionUserId(), GetUserId()))
             return;
 
-        /*
-         * Don't log if this is not a true update UPDATE command, e.g. a
-         * SELECT FOR UPDATE used for foreign key lookups.
-         */
-        if (rte->requiredPerms & ACL_UPDATE &&
-            rte->rellockmode < RowExclusiveLock)
-            continue;
-
         /*
          * If we are not logging all-catalog queries (auditLogCatalog is
          * false) then filter out any system relations here.
@@ -1055,15 +1048,18 @@ log_select_dml(Oid auditOid, List *rangeTabls)
         /*
          * We don't have access to the parsetree here, so we have to generate
          * the node type, object type, and command tag by decoding
-         * rte->requiredPerms and rte->relkind.
+         * rte->requiredPerms and rte->relkind. For updates we also check 
+         * rellockmode so that only true UPDATE commands (not
+         * SELECT FOR UPDATE, etc.) are logged as UPDATE.
          */
         if (rte->requiredPerms & ACL_INSERT)
         {
             auditEventStack->auditEvent.logStmtLevel = LOGSTMT_MOD;
             auditEventStack->auditEvent.commandTag = T_InsertStmt;
             auditEventStack->auditEvent.command = CMDTAG_INSERT;
         }
-        else if (rte->requiredPerms & ACL_UPDATE)
+        else if (rte->requiredPerms & ACL_UPDATE &&
+                 rte->rellockmode >= RowExclusiveLock)
         {
             auditEventStack->auditEvent.logStmtLevel = LOGSTMT_MOD;
             auditEventStack->auditEvent.commandTag = T_UpdateStmt;

sql/pgaudit.sql

@@ -415,6 +415,15 @@ SELECT password
 UPDATE account
    SET description = 'yada, yada';
 
+--
+-- Object logged because of:
+--     select (password) on account (in the where clause)
+-- Session logged on all tables because log = read and log_relation = on
+SELECT *
+  FROM account
+ WHERE password = 'HASH2'
+   FOR UPDATE;
+
 --
 -- Object logged because of:
 -- select (password) on account (in the where clause)