teoljungberg / fx

Versioned database functions and triggers for Rails
MIT License
762 stars 76 forks source link

Sort functions and triggers by their name when dumping them to the schema #139

Closed agrobbin closed 11 months ago

agrobbin commented 11 months ago

This is my first contribution to F(x), so please let me know if I missed anything!

Active Record's schema dumping approach is to output all elements in a deterministic order, sorted by the name of the element (enum, table, etc.).

F(x) has been fetching functions and triggers by insertion order (oid), and while that can often work fine, a situation we've run into is when we restore our database from a dump (say, from staging / production). pg_dump will output the functions and triggers in their alphabetical order rather than based on creation order.

This causes a lot of churn in db/schema.rb if you're consistently leaning on a dumped database to do local development:

Since the order of the functions themselves doesn't actually matter (you can reference functions that don't yet exist in the body of a function), following the established name-ordered pattern of db/schema.rb eliminates this churn in db/schema.rb, regardless of whether you work off of a production (or production-like) database dump, or not.

teoljungberg commented 11 months ago

Since the order of the functions themselves doesn't actually matter (you can reference functions that don't yet exist in the body of a function), following the established name-ordered pattern of db/schema.rb eliminates this churn in db/schema.rb, regardless of whether you work off of a production (or production-like) database dump, or not.

I've always thought this was the case, as that is why I have implemented them using oid sorting.

I've followed the same philosophy as Scenic as does (https://github.com/scenic-views/scenic/issues/263) and would instead want to implement topological sorting more than alphabetical.

As for creating a diff because of dumping/loading a production database will cause schema changes with a lot of tools (and fx is not the only one) - I see it as an antipattern and am not interested in supporting that.

However, thank you for the PR and contribution!

teoljungberg commented 11 months ago

You can override https://github.com/teoljungberg/fx/blob/c71a17583ffdb8102fd3adc284a3b45e31021284/lib/fx/adapters/postgres.rb#L49-L51 to sort them by name in an initializer in your application.

Since the code is public api, and I don't change FX that often - you should be safe doing this.

agrobbin commented 11 months ago

I understand the anti-pattern of relying on a production dump, but we also ran into this when working off of a template database locally with fixture data, and creating a new database more quickly via createdb -T.

Would you be open to a configuration option to change the dumped order? That way the default can stay as is, someone could choose alphabetical, or someone could eventually choose topological (if support were added in the future)?

teoljungberg commented 11 months ago

... Would you be open to a configuration option to change the dumped order? ...

Not right now, as this is the first time hearing about this feature request. Feel free to apply a patch locally in your application to change the sorting order for now

salidux commented 4 weeks ago

Hi @teoljungberg,

I also ran into the same need that @agrobbin described, where ordering functions and triggers alphabetically in the schema dump would help reduce unnecessary changes in db/schema.rb. I forked the repo and implemented this as an optional configuration, so it’s disabled by default, preserving the gem’s current behavior. Additionally, I applied the sorting at the SchemaDumper level, ensuring compatibility across all database adapters.

Here’s the link to my change for reference: Commit link.

Would you reconsider a PR with this approach? I’d be glad to open one if you think it’s a good fit.

Thanks for considering it!