Schema Diff: reinstate dependency ordering of the generated script - #10319
Schema Diff: reinstate dependency ordering of the generated script#10319dpage wants to merge 1 commit into
Conversation
dependLevel drove the order in which generateFinalScript() writes objects, with higher levels emitted first so dependencies land before whatever needs them, but nothing has assigned it since the React port deleted schema_diff_dependency.js: every row got Object.keys' default undefined bucket, so the script came out in whatever order the rows happened to be in. computeDependLevels() rebuilds the levels for the full result set from each row's dependencies, walking the reverse graph so a row that other rows depend on always ends up with the highest of their levels plus one, with a cycle guard for the circular dependencies the script header already warns about. Fixes pgadmin-org#10295.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 0 remain after this review. WalkthroughSchema Diff now computes dependency levels for compared objects before generating a script. The calculation handles dependency chains, unrelated objects, missing targets, and circular dependencies. Regression tests cover these cases and verify deepest-chain selection. ChangesSchema Diff dependency ordering
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change restores dependency-based ordering for generated scripts and includes focused tests; no actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
What this is
dependLeveldrives the order in whichgenerateFinalScript()writes objects into the generated script, with higher levels emitted first so dependencies land before whatever needs them. Nothing has assigned it since the React port deletedschema_diff_dependency.jswholesale: every row gotObject.keys' defaultundefinedbucket, so the script came out in whatever order the rows happened to be in. In practice this means a generated script can fail outright when an object appears before something it depends on.The fix
computeDependLevels()rebuilds the levels for the full result set from each row'sdependencies, walking the reverse graph so that a row which other rows depend on always ends up with the highest of their levels plus one, with a cycle guard for the circular dependencies the script header already warns about. It's called once, over the full row set, before the script is assembled.Testing
Added Jest unit tests for
computeDependLevels()covering a dependency chain, unrelated rows, a circular dependency, a dependency pointing at an object outside the row set, and converging dependency chains of different depths. Existingschema_diffJest specs and eslint both pass clean.Fixes #10295.
Summary by CodeRabbit