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

prevent infinite loops #18

Conversation

andriytyurnikov
Copy link
Contributor

via prevent_depth option for backwards compatibility

prevent_depth option for backwards compatibility

prevent_depth option for backwards compatibility
@andriytyurnikov
Copy link
Contributor Author

@jeremyevans do you think some other function definitions could use same option as well?

Copy link
Owner

@jeremyevans jeremyevans left a comment

Choose a reason for hiding this comment

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

I think it would be a good idea to support this for all triggers that could results in additional triggers.

lib/sequel/extensions/pg_triggers.rb Outdated Show resolved Hide resolved
@jeremyevans
Copy link
Owner

Looks good. I'll try to test and merge this today.

@andriytyurnikov
Copy link
Contributor Author

I am not sure I have context for such generalization, so I only altered definitions for counter, sum, and many to many sum.

Also, I am somewhat concerned about depth as a numerical parameter in case of sum and counters, and maybe track_recursive_trigger is a better option or worthy alternative - but this is beyond my depth of understanding at the moment.

@andriytyurnikov
Copy link
Contributor Author

After giving it some thought - I've added guard clause to every other function definition (including to previously missing for many to many summation). It's relevance might vary on case by case basis, but having access via simple parameter seems like a good thing.

@jeremyevans
Copy link
Owner

I squash-merged your first two commits as f32ebdd (I started working on that before your third and fourth commits). I also renamed the option to :trigger_depth_limit and added some minor changes and more tests in d68b631.

I added support for this to some methods, but not all. I only added it to methods that define trigger functions that result in additional triggers. I don't see a reason to support the option if the trigger function defined cannot result in additional triggers.

Please review and let me know if you think we should make further changes.

@jeremyevans jeremyevans closed this Apr 3, 2024
@andriytyurnikov andriytyurnikov deleted the many_to_many_self_referential_counter branch April 3, 2024 20:57
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

Successfully merging this pull request may close these issues.

2 participants