palkan / logidze

Database changes log for Rails
MIT License
1.59k stars 74 forks source link

Partition-friendly logging (triggers) #215

Closed bf4 closed 1 year ago

bf4 commented 1 year ago

Logidze triggers are currently "BEFORE UPDATE OR INSERT"

If I were to try to add logidze to a partitioned table I'd get

ActiveRecord::StatementInvalid: PG::WrongObjectType: ERROR: "device_location_events" is a partitioned table DETAIL: Partitioned tables cannot have BEFORE / FOR EACH ROW triggers.

any concerns about me moving logidze to a "AFTER INSERT OR UPDATE" trigger on these tables?

palkan commented 1 year ago

any concerns about me moving logidze to a "AFTER INSERT OR UPDATE" trigger on these tables?

That would require rewriting a bit (a bunch?) of SQL 🙂 We use BEFORE to modify the changeset and let PostgreSQL do the actual update. With AFTER, I believe, we will need to perform UPDATE ... ourselves.

What else? There will be no way to prevent updates/inserts if an exception occurred (and no way to communicate such failures to users except from pg logs).

I think, we can split our logidze_logger one more time and extract the actual NEW generation logic into a non-trigger function, and then provide logidze_logger and logidze_logger_after trigger functions.

bf4 commented 1 year ago

With AFTER, I believe, we will need to perform UPDATE ... ourselves.

huh, going to confirm if I'm doing it wrong in a bunch of places... https://www.postgresql.org/docs/current/sql-createtrigger.html huh, seems you're right.. I just tested my AFTER trigger with without an 'UPDATE', they don't persist the change...

I think, we can split our logidze_logger one more time and extract the actual NEW generation logic into a non-trigger function, and then provide logidze_logger and logidze_logger_after trigger functions.

interesting idea.

prog-supdex commented 1 year ago

Hello @palkan ! :) I have almost completed the task, but I have a question

There is an error Partitioned tables cannot have BEFORE / FOR EACH ROW triggers only for PG 11 and 12 versions, (triggers for partitioned table in PG 10 do not work), so my PR checks postgres version and decides, create AFTER trigger or BEFORE trigger (depending on pg version and partitioned table or not)

Do we need to decide about trigger type (BEFORE/AFTER) depending on pg version and partitioned table or not, or do we need to add an additional option ? For example

bundle exec rails generate logidze:model Post --after_trigger

And the behaviour of creating trigger will be change by depending on this option (--after_trigger), and an user can decide, which type of trigger create

Please tell me how to do it right? :)

Thank you!