Skip to content

Adds a way to force pgledger_entries to be an append-only relation #26

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.

Sign up for GitHub

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

Open
IvanVas wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Adds a way to force pgledger_entries to be an append-only relation #26

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions pgledger.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,21 @@ CREATE TABLE pgledger_entries (
CREATE INDEX ON pgledger_entries(account_id);
CREATE INDEX ON pgledger_entries(transfer_id);

-- It's recommended to make pgledger_entries an append-only table
-- REVOKE UPDATE, DELETE, INSERT ON pgledger_entries FROM PUBLIC;
Copy link

@Godwottery Godwottery Jun 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit-pick only. For a full hardening, these should never be granted to PUBLIC. They should only be granted as necessary.

-- GRANT INSERT ON pgledger_entries TO app_role; -- change to the role your code uses

-- CREATE OR REPLACE FUNCTION prevent_mutation_on_entries()
-- RETURNS trigger AS $$
-- BEGIN
-- RAISE EXCEPTION
Copy link

@Godwottery Godwottery Jun 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By always raising the exception, you prevent the option of having an admin that can update the row. Perhaps it would be useful to catch a missing permission error, and then raise this exception.

Copy link

@aabbdev aabbdev Jul 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nobody should ever update any entries. If the admin wants to make a change, they must append a new entry instead

-- 'pgledger_entries is immutable – % not allowed', TG_OP;
-- END;
-- $$ LANGUAGE plpgsql;
-- CREATE TRIGGER pgledger_entries_nochange
-- BEFORE UPDATE OR DELETE ON pgledger_entries
-- FOR EACH ROW EXECUTE FUNCTION prevent_mutation_on_entries();
Copy link

@Godwottery Godwottery Jun 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any particular reason why you choose the non-default "FOR EACH ROW" here? In my opinion it should be enough to fire it once per statement. If you try to UPDATE multiple rows in a single statement, you will unnecessarily raise many exceptions instead of only one.


CREATE OR REPLACE FUNCTION pgledger_create_account(
name_param TEXT,
currency_param TEXT,
Expand Down