Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: has_trigger_events() #337

Open
AWolf81 opened this issue May 10, 2024 · 6 comments
Open

Feature: has_trigger_events() #337

AWolf81 opened this issue May 10, 2024 · 6 comments

Comments

@AWolf81
Copy link

AWolf81 commented May 10, 2024

I'm pretty new to pgTAP but I think there is no easy way to check if a trigger event is avaliable at a given table.

rules_are is similar but that's not working for triggers.

The line in question of a schema that the function will test is marked in the below example definition:

CREATE TRIGGER test_trigger
AFTER INSERT OR UPDATE ON test_table
FOR EACH ROW
EXECUTE FUNCTION test_trigger_function();

So the idea is to add a has_trigger_events function.

I think the following code is exactly implementing that function:

-- has_trigger_events_test.sql
BEGIN;
SELECT plan(13);
-- SELECT * FROM no_plan();

-- Define a test table
CREATE TABLE test_table (
    id SERIAL PRIMARY KEY,
    name TEXT
);

-- Create a trigger on the test table
CREATE OR REPLACE FUNCTION test_trigger_function()
RETURNS TRIGGER AS $$
BEGIN
    -- Trigger function logic here
    RETURN NEW;
END;
$$ LANGUAGE plpgsql;

CREATE TRIGGER test_trigger
AFTER INSERT OR UPDATE ON test_table
FOR EACH ROW
EXECUTE FUNCTION test_trigger_function();

-- has_trigger_events test function
-- Parameters:
--   p_table_name            - name of table
--   p_trigger_name          - name of trigger function
--   p_expected_events_array - array of events 
--   p_test_description
-- Returns:
--   test_result text
CREATE OR REPLACE FUNCTION has_trigger_events(
    p_table_name text,
    p_trigger_name text,
    p_expected_events_array text[],
    p_test_description text
)
RETURNS TEXT AS $$
DECLARE
    found_events TEXT[];
    extra_events TEXT[];
    missing_events TEXT[];
    result TEXT;
BEGIN
    -- Check if the trigger exists
    IF NOT EXISTS (
        SELECT 1
        FROM information_schema.triggers
        WHERE event_object_table = p_table_name
        AND trigger_name = p_trigger_name
    ) THEN
        RETURN ok(false, 'Trigger ' || p_trigger_name || ' does not exist on table ' || p_table_name);
    END IF;

    -- Fetch trigger events based on provided parameters
    found_events := _get_trigger_events(p_table_name, p_trigger_name);

    -- RAISE NOTICE 'test event: %', ARRAY_TO_STRING(found_events, ', ');

    -- Compare expected events with found events to identify missing and extra events
    missing_events := ARRAY(
        SELECT unnest(p_expected_events_array)
        EXCEPT
        SELECT unnest(found_events)
    );

    extra_events := ARRAY(
        SELECT unnest(found_events)
        EXCEPT
        SELECT unnest(p_expected_events_array)
    );

    -- Generate error message using _are function
    result := _are(
        'events',
        extra_events,
        missing_events,
        p_test_description
    );

    RETURN result;
END;
$$ LANGUAGE plpgsql;

-- Reusable function to fetch trigger events based on provided parameters
-- Parameters:
--   p_table_name - name of table
--   p_triggger_name - name of trigger function
--   p_expected_timing - optional timing e.g. 'AFTER'
CREATE OR REPLACE FUNCTION _get_trigger_events(
    p_table_name text,
    p_trigger_name text,
    p_expected_timing text DEFAULT NULL
)
RETURNS TEXT[] AS $$
DECLARE
    trigger_timing_filter text;
BEGIN
    IF p_expected_timing IS NOT NULL THEN
        trigger_timing_filter := ' AND trigger_timing = ' || quote_literal(p_expected_timing);
    ELSE
        trigger_timing_filter := '';
    END IF;

    RETURN ARRAY(
        SELECT upper(event_manipulation)
        FROM information_schema.triggers
        WHERE event_object_table = p_table_name
        AND trigger_name = p_trigger_name
        || trigger_timing_filter
    );
END;
$$ LANGUAGE plpgsql;

-- Test Case 1: No missing or extra events (Both INSERT and UPDATE are expected).
SELECT * FROM check_test(
    has_trigger_events('test_table', 'test_trigger', ARRAY['INSERT', 'UPDATE'], 'should have no missing or extra events'),
    true,
    'has_trigger_event( tab, trigger, events, description )'
);

-- Test Case 2: Missing event (Only INSERT is expected).
-- counts as 3 tests has_trigger + description correct + diag correct
SELECT * FROM check_test(
    has_trigger_events('test_table', 'test_trigger', ARRAY['INSERT'], 'should fail with extra event UPDATE'),
    false,
    'has_trigger_events( tab, trigger, events, description )',
    'should fail with extra event UPDATE',
    E'    Extra events:\n        "UPDATE"'
);

-- Test Case 3: Extra event (Only UPDATE is expected).
SELECT * FROM check_test(
    has_trigger_events('test_table', 'test_trigger', ARRAY['UPDATE'], 'should fail with extra event INSERT'),
    false,
    'has_trigger_events( tab, trigger, events, description )',
    'should fail with extra event INSERT',
    E'    Extra events:\n        "INSERT"'
);

-- Test Case 4: Both missing and extra events (Only DELETE is expected).
SELECT * FROM check_test(
    has_trigger_events('test_table', 'test_trigger', ARRAY['DELETE'], 'should fail with both missing and extra events'), 
    false,
    'has_trigger_events( tab, trigger, events, description )',
    'should fail with both missing and extra events',
    E'    Extra events:\n        "INSERT"\n        "UPDATE"\n    Missing events:\n        "DELETE"'
);

-- Test Case 5: Trigger does not exist.
SELECT * FROM check_test(
    has_trigger_events('test_table', 'non_existent_trigger', ARRAY['INSERT'], 'should fail - trigger does not exist'),
    false,
    'has_trigger_events( tab, non_existent_trigger, events, description )'
);

-- Test Case 6: Table does not exist.
SELECT * FROM check_test(
    has_trigger_events('non_existent_table', 'test_trigger', ARRAY['INSERT'], 'should fail - table does not exist'),
    false,
    'has_trigger_events( non_existent_table, trigger, events, description )'
);

-- Test Case 7: Empty events.
SELECT * FROM check_test(
    has_trigger_events('test_table', 'test_trigger', ARRAY[]::TEXT[], 'should fail with both missing and extra events'),
    false,
    'has_trigger_events( table, trigger, ARRAY[], description )'
);

SELECT * FROM finish();
ROLLBACK;

-- End of test case

I haven't checked how to add it to pgTAP but if there is a guide on how to contribute, I could create a PR if this feature is interesting for pgTAP.

If this function is not interesting, that's also no problem.

The tests above are covering all cases.

Topics not covered in the function

  • Trigger timing BEFORE / AFTER not added to has_trigger_events, would be possible but makes the has_trigger_events usage more complicated. Maybe this would be better to add as additional test function.
  • Trigger definition e.g. FOR EACH ROW not covered in has_trigger_events - would be better in a separate has_trigger_definition function
  • hasnt_trigger_events not added yet - is it needed? How to add without duplicating the has_trigger_events logic e.g. refactor/rename to check_trigger_events and add an arg. p_check_presence to check has or hasn't based on it.

The mentioned missing parts would be nice to have but I think they're all optional.

@theory
Copy link
Owner

theory commented May 11, 2024

This seems like it would be a useful function to complement triggers_are(), has_trigger(), and trigger_is(). Name it something like trigger_events_are(), which I think fits better with pgTAP's naming conventions (e.g., all the _privs_are() functions). Would also need a variant that takes the schema name. I suspect the signatures would be:

SELECT trigger_events_are ( :schema, :table, :events, :description );
SELECT trigger_events_are ( :schema, :table, :events );
SELECT trigger_events_are ( :table, :events, :description );
SELECT trigger_events_are ( :table, :events );

To add those functions to the project, append them to sql/pgtap.sql.in and to sql/pgtap--1.3.3--1.3.4.sql and put the tests in test/sql/trigger.sql.

@AWolf81
Copy link
Author

AWolf81 commented May 12, 2024

@theory thanks for the advice how to add this.

Yes, you're right, naming trigger_events_are is better.

I'll create a PR once I have this ready.

At the moment, I'm not sure why my tests are failing. I have to check this. Or do you have an idea what I'm doing wrong?
Maybe I have to run the tests with a differnt command?

If I'm running make installcheck I'm getting the following output:

     ... other tests are passing
     ruletap                      ... ok        17680 ms
     resultset                    ... ok         8918 ms
     util                         ... ok         2125 ms
     policy                       ... ok         4164 ms
     trigger                      ... FAILED     3666 ms
     istap                        ... ok         2536 ms

=======================
 1 of 39 tests failed. 
=======================

The differences that caused some tests to fail can be viewed in the
file "/home/alexander/awolf81-fork-pgtap/regression.diffs".  A copy of the test summary that you see
above is saved in the file "/home/alexander/awolf81-fork-pgtap/regression.out".

make: *** [/usr/lib/postgresql/12/lib/pgxs/src/makefiles/pgxs.mk:420: installcheck] Error 1

And the regression.diffs looks like this:

diff -U3 /home/alexander/awolf81-fork-pgtap/test/expected/trigger.out /home/alexander/awolf81-fork-pgtap/results/trigger.out
--- /home/alexander/awolf81-fork-pgtap/test/expected/trigger.out	2024-05-12 21:58:18.365706798 +0200
+++ /home/alexander/awolf81-fork-pgtap/results/trigger.out	2024-05-12 23:56:58.417688814 +0200
@@ -1,5 +1,5 @@
 \unset ECHO
-1..84
+1..97
 ok 1 - has_trigger(schema, table, trigger, desc) should pass
 ok 2 - has_trigger(schema, table, trigger, desc) should have the proper description
 ok 3 - has_trigger(schema, table, trigger, desc) should have the proper diagnostics
@@ -84,3 +84,16 @@
 ok 82 - triggers_are(table, triggers) + extra & missing should fail
 ok 83 - triggers_are(table, triggers) + extra & missing should have the proper description
 ok 84 - triggers_are(table, triggers) + extra & missing should have the proper diagnostics
+ok 85 - trigger_events_are(table, trigger, events, desc) should pass
+ok 86 - trigger_events_are(table, trigger, events, desc) + extra should fail
+ok 87 - trigger_events_are(table, trigger, events, desc) + extra should have the proper description
+ok 88 - trigger_events_are(table, trigger, events, desc) + extra should have the proper diagnostics
+ok 89 - trigger_events_are(table, trigger, events, desc) should fail
+ok 90 - trigger_events_are(table, trigger, events, desc) should have the proper description
+ok 91 - trigger_events_are(table, trigger, events, desc) should have the proper diagnostics
+ok 92 - trigger_events_are(table, trigger, events, desc) + extra & missing should fail
+ok 93 - trigger_events_are(table, trigger, events, desc) + extra & missing should have the proper description
+ok 94 - trigger_events_are(table, trigger, events, desc) + extra & missing should have the proper diagnostics
+ok 95 - trigger_events_are(table, non_existent_trigger, events, desc) should fail
+ok 96 - trigger_events_are(non_existent_table, trigger, events, desc) should fail
+ok 97 - trigger_events_are(table, trigger, ARRAY[], desc) should fail

Running the trigger.sql with pg_prove like following is working:

pg_prove -d regression test/sql/trigger.sql
test/sql/trigger.sql .. ok     
All tests successful.
Files=1, Tests=97,  1 wallclock secs ( 0.19 usr  0.03 sys +  0.35 cusr  0.17 csys =  0.74 CPU)
Result: PASS

@theory
Copy link
Owner

theory commented Jun 6, 2024

Yeah you added new tests so you have to update test/expected/trigger.out. This should fix it:

cp results/trigger.out test/expected/trigger.out

Sorry for the delayed reply, was completely absorbed in writing a couple conference presentations for the last few weeks.

@rodo
Copy link
Contributor

rodo commented Oct 25, 2024

@AWolf81 as you I need a function to check event_trigger, what are your plans regarding what you've already done ? Do you think to push a pull request in the following days/weeks ?
If you need some help I'll be happy to spend some time on it.

@AWolf81
Copy link
Author

AWolf81 commented Oct 25, 2024

@rodo sorry, not much progress here.
I'm busy with other stuff and I'm not sure when I find time to continue working on it. So feel free to take the code from my branch and continue with it - not planning to do a PR.

I've changed the naming to trigger_events_are. My work in progress is pushed to the following branch in my pgtap fork.

Things that are open:

  • Fix the failing test for the schema signature
  • Check if test cases are OK as I changed the trigger_events_are signature
  • Clean pgtap.sql.in and remove commented trigger_events_are, after checking the test
  • Add a documentation, so the usage is clear
  • Merge/Rebase the branch as it is a bit outdated

@rodo
Copy link
Contributor

rodo commented Oct 25, 2024

Ok, thanks for your quick answer @AWolf81 I'll have a look in the following days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants