Adds a way to force pgledger_entries to be an append-only relation#26
Adds a way to force pgledger_entries to be an append-only relation#26IvanVas wants to merge 1 commit intopgr0ss:mainfrom
Conversation
|
This is a cool idea. I'm wondering about the best way to include it, though. We can leave it as a comment, but then there's no guarantee it's correct. We could uncomment it and make it work with the default postgres from docker compose. But then it's a bit more work to port the sql to your own project. Just brainstorming, but one idea would be to put it in its own file. That way, we can run it and write tests against it. But it would still be optional for someone using |
True. I've tested it manually, but that's a one-time.
Yup, I like it! I was thinking about the same. |
Sure, let's try it and see how it goes. Thanks! |
Godwottery
left a comment
Godwottery
left a comment
There was a problem hiding this comment.
Generally good idea. Some minor comments.
| -- $$ LANGUAGE plpgsql; | ||
| -- CREATE TRIGGER pgledger_entries_nochange | ||
| -- BEFORE UPDATE OR DELETE ON pgledger_entries | ||
| -- FOR EACH ROW EXECUTE FUNCTION prevent_mutation_on_entries(); |
There was a problem hiding this comment.
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 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; |
There was a problem hiding this comment.
Minor nit-pick only. For a full hardening, these should never be granted to PUBLIC. They should only be granted as necessary.
| -- CREATE OR REPLACE FUNCTION prevent_mutation_on_entries() | ||
| -- RETURNS trigger AS $$ | ||
| -- BEGIN | ||
| -- RAISE EXCEPTION |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Nobody should ever update any entries. If the admin wants to make a change, they must append a new entry instead
|
It's good idea and mandatory for a ledger |
4 participants
No description provided.