MIME-Version: 1.0
References: <157644931740.18138.16968232991166819127@wrigleys.postgresql.org>
In-Reply-To: <157644931740.18138.16968232991166819127@wrigleys.postgresql.org>
From: Pavel Stehule <pavel.stehule@gmail.com>
Date: Mon, 16 Dec 2019 12:25:34 +0100
Message-ID: 
 <CAFj8pRCnPmD8j2v+Sh5zsqvR2ef+tNqSLecJXeZOr8hCgrCb9g@mail.gmail.com>
Subject: Re: Example 42.4. A PL/pgSQL Trigger Function for Auditing
To: simon.dallaway@datacom.co.nz, pgsql-docs@lists.postgresql.org
Content-Type: multipart/alternative; boundary="00000000000081fc0e0599d07a1d"
Precedence: bulk

--00000000000081fc0e0599d07a1d
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

po 16. 12. 2019 v 12:12 odes=C3=ADlatel PG Doc comments form <
noreply@postgresql.org> napsal:

> The following documentation comment has been logged on the website:
>
> Page: https://www.postgresql.org/docs/12/plpgsql-trigger.html
> Description:
>
> I'm wondering if it would be worthwhile to put a totally generic auditing
> function into the documentation e.g.
>
> CREATE OR REPLACE FUNCTION zz_audit_() RETURNS TRIGGER AS $nothing$
>
> -- This function is intended to be used by a delete/insert/update trigger
> for any table.
> -- It relies on the existence of a table named zz_audit_XXX (where XXX is
> the table being audited) that contains the
> -- same columns as the table XXX except that two additional columns must
> exist prior to the columns from XXX
> --    operation character(1) NOT NULL,
> --    tstamp    timestamp with time zone NOT NULL,
> --    ...       remaining columns per table XXX
>
>     DECLARE audit_table_name NAME :=3D CONCAT('zz_audit_', TG_TABLE_NAME)=
;
>     BEGIN
>
>         IF (TG_OP =3D 'DELETE') THEN
>             EXECUTE 'INSERT INTO ' || audit_table_name || ' SELECT ''D'',
> now(), ' || ' $1.*' USING OLD;
>         ELSIF (TG_OP =3D 'UPDATE') THEN
>                         EXECUTE 'INSERT INTO ' || audit_table_name || '
> SELECT ''U'', now(), ' ||
> ' $1.*' USING NEW;
>         ELSIF (TG_OP =3D 'INSERT') THEN
>                         EXECUTE 'INSERT INTO ' || audit_table_name || '
> SELECT ''I'', now(), ' ||
> ' $1.*' USING NEW;
>         END IF;
>
>         RETURN NULL; -- result is ignored since this is an AFTER trigger
>     END;
> $nothing$ LANGUAGE plpgsql;
>

Just few points to this code

1. bad, useless brackets in IF .. ELSIF expressions - plpgsql is not C or
Java
2. unescaped identifiers in dynamic SQL - EXECUTE
3. there is not reason for INSERT SELECT.

Regards

Pavel

--00000000000081fc0e0599d07a1d
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr"><br></div><br><div class=3D"gmail_quote">=
<div dir=3D"ltr" class=3D"gmail_attr">po 16. 12. 2019 v=C2=A012:12 odes=C3=
=ADlatel PG Doc comments form &lt;<a href=3D"mailto:noreply@postgresql.org"=
>noreply@postgresql.org</a>&gt; napsal:<br></div><blockquote class=3D"gmail=
_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204=
,204);padding-left:1ex">The following documentation comment has been logged=
 on the website:<br>
<br>
Page: <a href=3D"https://www.postgresql.org/docs/12/plpgsql-trigger.html" r=
el=3D"noreferrer" target=3D"_blank">https://www.postgresql.org/docs/12/plpg=
sql-trigger.html</a><br>
Description:<br>
<br>
I&#39;m wondering if it would be worthwhile to put a totally generic auditi=
ng<br>
function into the documentation e.g.<br>
<br>
CREATE OR REPLACE FUNCTION zz_audit_() RETURNS TRIGGER AS $nothing$<br>
<br>
-- This function is intended to be used by a delete/insert/update trigger<b=
r>
for any table.<br>
-- It relies on the existence of a table named zz_audit_XXX (where XXX is<b=
r>
the table being audited) that contains the<br>
-- same columns as the table XXX except that two additional columns must<br=
>
exist prior to the columns from XXX<br>
--=C2=A0 =C2=A0 operation character(1) NOT NULL,<br>
--=C2=A0 =C2=A0 tstamp=C2=A0 =C2=A0 timestamp with time zone NOT NULL,<br>
--=C2=A0 =C2=A0 ...=C2=A0 =C2=A0 =C2=A0 =C2=A0remaining columns per table X=
XX<br>
<br>
=C2=A0 =C2=A0 DECLARE audit_table_name NAME :=3D CONCAT(&#39;zz_audit_&#39;=
, TG_TABLE_NAME);<br>
=C2=A0 =C2=A0 BEGIN<br>
<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 IF (TG_OP =3D &#39;DELETE&#39;) THEN<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 EXECUTE &#39;INSERT INTO &#39; ||=
 audit_table_name || &#39; SELECT &#39;&#39;D&#39;&#39;,<br>
now(), &#39; || &#39; $1.*&#39; USING OLD;<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 ELSIF (TG_OP =3D &#39;UPDATE&#39;) THEN<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 EXECUTE &#39;INSERT INTO &#39; || audit_table_name || &#39; SELE=
CT &#39;&#39;U&#39;&#39;, now(), &#39; ||<br>
&#39; $1.*&#39; USING NEW;<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 ELSIF (TG_OP =3D &#39;INSERT&#39;) THEN<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 EXECUTE &#39;INSERT INTO &#39; || audit_table_name || &#39; SELE=
CT &#39;&#39;I&#39;&#39;, now(), &#39; ||<br>
&#39; $1.*&#39; USING NEW;<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 END IF;<br>
<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 RETURN NULL; -- result is ignored since this is=
 an AFTER trigger<br>
=C2=A0 =C2=A0 END;<br>
$nothing$ LANGUAGE plpgsql;<br></blockquote><div><br></div><div>Just few po=
ints to this code</div><div><br></div><div>1. bad, useless brackets in IF .=
. ELSIF expressions - plpgsql is not C or Java</div><div>2. unescaped ident=
ifiers in dynamic SQL - EXECUTE</div><div>3. there is not reason for INSERT=
 SELECT.</div><div><br></div><div>Regards</div><div><br></div><div>Pavel<br=
></div></div></div>

--00000000000081fc0e0599d07a1d--