Skip to content

Add topological sorting for dumped views using TSort#337

Closed
msorc wants to merge 2 commits into
scenic-views:mainfrom
msorc:views-ordering
Closed

Add topological sorting for dumped views using TSort#337
msorc wants to merge 2 commits into
scenic-views:mainfrom
msorc:views-ordering

Conversation

@msorc
Copy link
Copy Markdown

@msorc msorc commented Sep 24, 2021

This introduces topological sorting (https://ruby-doc.org/stdlib-2.6.8/libdoc/tsort/rdoc/TSort.html#method-i-tsort) for views in dumps which will sort views alphabetically and in case of dependencies between views will use a topological sorting.
That provides a consistent views ordering in database dumps and prevents of having occasional diffs in schema dumps.

Based on discussion here #263 and code samples #263 (comment) and #263 (comment)

Comment thread lib/scenic/schema_dumper.rb
@msorc
Copy link
Copy Markdown
Author

msorc commented Sep 24, 2021

Not sure how do you deal with the Hound so made a separate commit to satisfy it

@nedcampion
Copy link
Copy Markdown

This is great, thank you @msorc this one saved us a bunch of time. Any thoughts on merging this one @derekprior @calebhearth, as it relates to #263. Anvyl would be happy to help QA a pre-release version.

Comment on lines +73 to +74
source_v = relation["source_table"]
dependent = relation["dependent_view"]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
source_v = relation["source_table"]
dependent = relation["dependent_view"]
source_v = "#{relation["source_schema"]}.#{relation["source_table"]}"
dependent = "#{relation["dependent_schema"]}.#{relation["dependent_view"]}"

Needs schema here in our app otherwise order is wrong.

Comment on lines +30 to +31
AND source_table.relkind IN ('m', 'v') AND dependent_view.relkind IN ('m', 'v')
ORDER BY dependent_view.relname;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
AND source_table.relkind IN ('m', 'v') AND dependent_view.relkind IN ('m', 'v')
ORDER BY dependent_view.relname;
AND source_table.relkind IN ('m', 'v') AND dependent_view.relkind IN ('m', 'v');

Iterating over result does not need this order so this line can be removed.

@gr8bit
Copy link
Copy Markdown

gr8bit commented Feb 8, 2023

Anyone please merge this to clean up the schema diffs :(

@calebhearth
Copy link
Copy Markdown
Contributor

I've rebased this in #398 (but would be fine closing that if @msorc hasn't gotten fed up with me and wants to rebase this themselves) and plan to test this out now that I have an app that sees this issue, which I'm very excited about.

@msorc
Copy link
Copy Markdown
Author

msorc commented Oct 25, 2023

No, I don't want to rebase it myself :)
@calebhearth please proceed whichever way to finally merge it.

@Unknown-Guy
Copy link
Copy Markdown

Check my comments above - schema_dumper.rb needs schema on lines 73-74. We are using fork with those changes for almost year without issues.

@derekprior
Copy link
Copy Markdown
Contributor

superseded by #398

@derekprior derekprior closed this Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants