History Logging Trigger Causing Duplication

Question

I'm fairly certain I'm probably just overlooking something simple but here's the situation: I'm trying to create a "reusable" historical logging trigger for a production database. The intent is that, when I attach the trigger to a table in one of the production database schemas, it will verify the existence of (and create if necessary) a historical logging table in the History schema to where any changes (INSERT/UPDATE/DELETE) in the production table will be logged.

I have most of it working, but it seems that something in it is generating duplication on inserted records. Here are the definitions (please, no comments about the mixed-case definitions - this is how our database has been designed):

TABLE DEFINITION

-- Table: Company.Rate
-- DROP TABLE IF EXISTS "Company"."Rate";
CREATE TABLE IF NOT EXISTS "Company"."Rate"
(
    "RateID" integer NOT NULL DEFAULT nextval('"Company"."Rate_seq"'::regclass),
    "TypeCode" character(2) COLLATE pg_catalog."default" NOT NULL,
    "Rate" numeric(9,5) NOT NULL DEFAULT 0,
    "Factor" numeric(7,5) NOT NULL DEFAULT 1,
    "Fee" numeric(6,2) NOT NULL DEFAULT 0,
    "EffectiveDate" date NOT NULL,
    "CancellationDate" date,
    CONSTRAINT rate_pk PRIMARY KEY ("RateID", "TypeCode"),
    CONSTRAINT "Rate_CollateralType_FK" FOREIGN KEY ("TypeCode")
        REFERENCES "Production"."Type" ("TypeCode") MATCH SIMPLE
        ON UPDATE CASCADE
        ON DELETE CASCADE
)
TABLESPACE pg_default;

-- Trigger: LogRateDelete
-- DROP TRIGGER IF EXISTS "LogRateDelete" ON "Company"."Rate";
CREATE OR REPLACE TRIGGER "LogRateDelete"
    BEFORE DELETE
    ON "Company"."Rate"
    FOR EACH ROW
    EXECUTE FUNCTION "History"."LogHistory"('true');

-- Trigger: LogRateInsert
-- DROP TRIGGER IF EXISTS "LogRateInsert" ON "Company"."Rate";
CREATE OR REPLACE TRIGGER "LogRateInsert"
    BEFORE INSERT
    ON "Company"."Rate"
    FOR EACH ROW
    EXECUTE FUNCTION "History"."LogHistory"('false');

-- Trigger: LogRateUpdate
-- DROP TRIGGER IF EXISTS "LogRateUpdate" ON "Company"."Rate";
CREATE OR REPLACE TRIGGER "LogRateUpdate"
    BEFORE UPDATE 
    ON "Company"."Rate"
    FOR EACH ROW
    EXECUTE FUNCTION "History"."LogHistory"('false');

LogHistory TRIGGER FUNCTION FOR HISTORICAL LOGGING

-- FUNCTION: History.LogHistory()
-- DROP FUNCTION IF EXISTS "History"."LogHistory"();
CREATE OR REPLACE FUNCTION "History"."LogHistory"()
    RETURNS trigger
    LANGUAGE 'plpgsql'
    COST 100
    VOLATILE NOT LEAKPROOF
AS $BODY$
DECLARE
    rec record;
    cols text;
    vals text;
    colname text;
    curval text;
    curtype text;
    sqlcmd text;
    recdelete boolean;
    tableexists boolean;
BEGIN
cols := '';
vals := '';
colname := '';
curval := '';
curtype := '';
sqlcmd := '';

sqlcmd := 'SELECT tableexists FROM "History"."VerifyHistoryTableExists"(''' || TG_TABLE_SCHEMA || ''',''' || TG_TABLE_NAME || ''');';
EXECUTE sqlcmd INTO tableexists;

IF NOT tableexists THEN
    RAISE EXCEPTION 'History table for %1 does not exist', '"' || TG_TABLE_SCHEMA || '"."' || TG_TABLE_NAME || '"';
ELSE
    IF TG_ARGV[0] IS NOT NULL THEN
        recdelete := TG_ARGV[0];
    ELSE
        recdelete := FALSE;
    END IF;

    FOR rec IN
        -- ORIGINAL QUERY: Could cause issues if the user doesn't have access to information_schema
        -- SELECT column_name, data_type from information_schema.columns WHERE table_name=TG_TABLE_NAME AND table_schema=TG_TABLE_SCHEMA)

        -- Querying pg_catalog instead of information_schema due to potential issues with user access
        (SELECT a.attname AS column_name,
            pg_catalog.format_type(a.atttypid, a.atttypmod) AS data_type
        FROM pg_catalog.pg_attribute a
        WHERE a.attrelid =
            (SELECT c.oid
            FROM pg_catalog.pg_class c
                LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace
            WHERE c.relname = TG_TABLE_NAME
            AND n.nspname = TG_TABLE_SCHEMA)
        AND a.attnum > 0
        AND NOT a.attisdropped
        ORDER BY a.attnum)
    LOOP
        colname := rec.column_name;
        curtype := lower(rec.data_type);

        EXECUTE format('SELECT $1.%1$I', colname) INTO curval USING NEW;

        IF curval IS NOT NULL  THEN
            CASE WHEN curtype LIKE 'character%' THEN
                curval := '''' || curval || '''';
            WHEN curtype = 'cidr' THEN
                curval := '''' || curval || '''';
            WHEN curtype = 'date' THEN
                curval := '''' || curval || '''';
            WHEN curtype = 'inet' THEN
                curval := '''' || curval || '''';
            WHEN curtype = 'json' THEN
                curval := '''' || curval || '''';
            WHEN curtype = 'macaddr' THEN
                curval := '''' || curval || '''';
            WHEN curtype = 'text' THEN
                curval := '''' || curval || '''';
            WHEN curtype LIKE 'time%' THEN
                curval := '''' || curval || '''';
            WHEN curtype = 'xml' THEN
                curval := '''' || curval || '''';
            ELSE
                curval := curval;
            END CASE;
        ELSE
            curval := 'NULL';
        END IF;

        IF cols = '' THEN
            cols := '"' || colname || '"';
        ELSE
            cols := cols || ', "' || colname || '"';
        END IF;

        IF vals = '' THEN
            vals := curval;
        ELSE
            vals := vals || ', ' || curval;
        END IF;
    END LOOP;

    sqlcmd := 'INSERT INTO "History"."' || TG_TABLE_NAME || 'History" (' || cols || ', "EventUser", "EventOccurred", "RecordDeleted") VALUES (' || vals || ', ''' || SESSION_USER || ''', ''' || CURRENT_TIMESTAMP || ''', ' || recdelete || ');';

    EXECUTE sqlcmd USING NEW;
    RETURN NEW;
END IF;

END;
$BODY$;

VerifyHistoryTableExists FUNCTION

-- FUNCTION: History.VerifyHistoryTableExists(text, text)

-- DROP FUNCTION IF EXISTS "History"."VerifyHistoryTableExists"(text, text);

CREATE OR REPLACE FUNCTION "History"."VerifyHistoryTableExists"(
    prodschemaname text,
    prodtablename text,
    OUT tableexists boolean)
    RETURNS boolean
    LANGUAGE 'plpgsql'
    COST 100
    VOLATILE SECURITY DEFINER PARALLEL UNSAFE
AS $BODY$

DECLARE
    createtable boolean;
    sqlcmd text;
    rec record;
    colname text;
    histcols text;
    prodcols text;
BEGIN

createtable := FALSE;
colname := '';
histcols := '';
prodcols := '';

-- Query pg_catalog to determine if a history table currently exists for the production table calling the function
sqlcmd := 'SELECT NOT EXISTS (SELECT 1 FROM pg_catalog.pg_class c JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace WHERE nspname = ''History'' AND c.relname = ''' || $2 || 'History'');';
EXECUTE sqlcmd INTO createtable;

IF createtable THEN
    -- If the History table for a production table using this trigger function doesn't exist, then create the history table.
    -- Use inheritance to ensure that all of the original table's columns are created and available in the historical table.
    -- Inheritance also helps to keep the tables in sync if the production table structure is modified in some way.
    sqlcmd := 'CREATE TABLE "History"."' || $2 || 'History"
    (
        "EventUser" character varying(60) NOT NULL DEFAULT SESSION_USER,
        "EventOccurred" timestamp without time zone NOT NULL DEFAULT NOW(),
        "RecordDeleted" boolean NOT NULL DEFAULT FALSE
    )
    INHERITS ("' || $1 || '"."' || $2 || '")
    WITH (
        OIDS=FALSE
    );
    ALTER TABLE "History"."' || $2 || 'History"
    OWNER TO "Developers";
    GRANT ALL ON TABLE "History"."' || $2 || 'History" TO "Developers";
    GRANT SELECT ON TABLE "History"."' || $2 || 'History" TO public;';

    EXECUTE sqlcmd;

    FOR rec IN
        -- Querying pg_catalog instead of information_schema due to potential issues with user access
        (SELECT a.attname AS column_name
        FROM pg_catalog.pg_attribute a
        WHERE a.attrelid =
            (SELECT c.oid
            FROM pg_catalog.pg_class c
                LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace
            WHERE c.relname = $2
            AND n.nspname = $1)
        AND a.attnum > 0
        AND NOT a.attisdropped
        ORDER BY a.attnum)
    LOOP
        colname := rec.column_name;

        IF histcols = '' THEN
            histcols := '"' || colname || '"';
        ELSE
            histcols := histcols || ', "' || colname || '"';
        END IF;

        IF prodcols = '' THEN
            prodcols := 'prod."' || colname || '"';
        ELSE
            prodcols := prodcols || ', prod."' || colname || '"';
        END IF;
    END LOOP;

    sqlcmd := 'INSERT INTO "History"."' || $2 || 'History" (' || histcols || ', "EventUser", "EventOccurred", "RecordDeleted")
        SELECT ' || prodcols || ', ''SYSTEM'', NOW(), FALSE FROM ONLY "' || $1 || '"."' || $2 || '" prod;';
    EXECUTE sqlcmd;
END IF;

sqlcmd := 'SELECT EXISTS (SELECT 1 FROM pg_catalog.pg_class c JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace WHERE nspname = ''History'' AND c.relname = ''' || $2 || 'History'');';
EXECUTE sqlcmd INTO tableexists;

END;
$BODY$;

As mentioned, it seems to be working in the fact that, when I inserted my first record into the production Rate table, it automatically created the RateHistory table in the History schema and it inserted a historical entry for that record. However, when I went back to look at the original Rate table, there were two identical records instead of the one that I inserted.

I TRUNCATEd both tables to try and start fresh, but when I INSERTed another new record, I got the same result. The RateHistory table showed a single entry, while the "source" Rate table had two identical rows.

I've gone over and over the functions and the table definition, but I'm just not seeing what would cause it to feed a duplicate record back into the source table - especially because it actually causes a violation the Primary Key constraint. Could someone point out what I'm obviously too dense to pick up on?

Answer 1

Well, there is no problem. The "duplication" has to do with the fact that I'm using inheritance and the default behavior of the database management UI/PostgreSQL. I was so worried about whether or not the trigger and functions were working at all, I forgot to consider how I was retrieving the data once it was inserted. So, yes. I overlooked something very simple and was being completely dense:

By default, the UI issues a simple SELECT * FROM "Rate". However, because the two tables are "linked" together through inheritance, when that statement is sent to PostgreSQL, it's going to return records from both the parent and child tables, giving the appearance of duplication. This will happen with every record and historical transaction, so the "duplication" will get more pronounced as more activity takes place in the table.

In order to view the records in the production table without any of the history, the SQL needs to be SELECT * FROM ONLY "Rate". The ONLY modifier ignores any "linked"/inherited tables. When I run that statement, I do, in fact, get the single record that was inserted.

So, the choice becomes this:

• Make certain that all queries for production data include the ONLY modifier to avoid returning "extra" records or inaccurate results (e.g., SUM(), COUNT(), etc.).
• Remove the inheritance to break the link so that simple FROM clauses without the ONLY modifier work "as expected".

The main problem with the latter is that, if I make changes to the production table, the trigger function won't be able to execute for that table until I make the same changes to the history table. Also, I haven't yet done any testing using the ONLY modifiers in more complex SQL queries (JOIN, GROUP BY, etc.), so there could potentially be some "gotchas" in there.

The obvious problem with the former is that I have to be extra vigilant about including the ONLY modifier - especially when dealing with UPDATE or DELETE queries. Forgetting ONLY could easily result in unnecessary and unwanted destruction of data.

I'm not sure if there's a way to set a flag somewhere or something that will automatically return ONLY results by default, but I'll look into that later. Now that I know what I'm looking for, things should be much easier. (*found it, but it's deprecated - sql_inheritance (boolean))

EDIT

I found this post on the PostgreSQL Wiki - Don't Do This (Don't use table inheritance)

It recommends using foreign keys instead of table inheritance for "linking" tables together. The primary reason for the suggestion is pretty much exactly what I'm dealing with here.

I suppose I could redesign the history table to accommodate that suggestion. The biggest concern I see there is correctly setting up the foreign key to the appropriate primary key - especially since I'm trying to make this a "universal" logging table that can be quickly attached to any production table simply by adding the trigger. Obviously, for "simple" tables with a single-column primary key (e.g., ID), it's no big deal. But I'm not exactly sure how complicated it'd be to handle a multi-column primary key such as what I have defined in the "Rate" table.

The other option I just started investigating a little is the use of event triggers to keep the DDL synced between the historical and production table. I've not dug too deeply into that path yet and I've not tried to build an event trigger before. But, if I could build it similar to what I have now with inheritance - using variables to specify the object names and such - I might still get away with it and achieve the goal of a "universal" historical logging system.