From 944b60c060af8371cef752a7c167f6e9742b3034 Mon Sep 17 00:00:00 2001 From: Dave Page Date: Wed, 19 Aug 2026 11:31:03 +0100 Subject: [PATCH] Schema Diff: reinstate dependency ordering of the generated script 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 #10295. --- .../js/components/SchemaDiffCompare.jsx | 57 +++++++++++++++ .../schema_diff/schema_diff_compare_spec.js | 70 +++++++++++++++++++ 2 files changed, 127 insertions(+) create mode 100644 web/regression/javascript/schema_diff/schema_diff_compare_spec.js diff --git a/web/pgadmin/tools/schema_diff/static/js/components/SchemaDiffCompare.jsx b/web/pgadmin/tools/schema_diff/static/js/components/SchemaDiffCompare.jsx index 310bd068734..0ea44d3b040 100644 --- a/web/pgadmin/tools/schema_diff/static/js/components/SchemaDiffCompare.jsx +++ b/web/pgadmin/tools/schema_diff/static/js/components/SchemaDiffCompare.jsx @@ -44,6 +44,62 @@ function generateFinalScript(script_array, scriptHeader, script_body) { return `${scriptHeader} BEGIN; \n ${script_body} END;`; } +export function computeDependLevels(rows) { + /* Assign each row a dependLevel such that anything another row depends + on always ends up with a strictly higher level than that row, so that + generateFinalScript() (which writes levels highest-first) emits + dependencies before the objects that need them. Rows are matched by + oid, matching how dependenciesOid is resolved elsewhere in this file. + + A row's level is therefore driven by what depends on it (the reverse + of its own "dependencies" list), computed as one more than the + highest level of anything that lists it as a dependency, bottoming + out at 1 for anything nothing else depends on. Circular dependencies + are broken by treating the row currently being resolved as level 1 + for that edge, rather than recursing forever. */ + const oidToRow = new Map(); + rows.forEach((row) => { + if (!_.isUndefined(row.oid) && !_.isNull(row.oid)) { + oidToRow.set(row.oid, row); + } + }); + + const dependents = new Map(); + rows.forEach((row) => { + (row.dependencies || []).forEach((dep) => { + let dependencyRow = oidToRow.get(dep.oid); + if (dependencyRow) { + if (!dependents.has(dependencyRow.id)) dependents.set(dependencyRow.id, []); + dependents.get(dependencyRow.id).push(row); + } + }); + }); + + const levels = new Map(); + const resolving = new Set(); + + function levelOf(row) { + if (levels.has(row.id)) return levels.get(row.id); + if (resolving.has(row.id)) return 1; + resolving.add(row.id); + + let level = 1; + (dependents.get(row.id) || []).forEach((dependent) => { + level = Math.max(level, levelOf(dependent) + 1); + }); + + resolving.delete(row.id); + levels.set(row.id, level); + return level; + } + + rows.forEach((row) => { + row.dependLevel = levelOf(row); + }); + + return rows; +} + function checkAndGetSchemaQuery(data, script_array) { /* Check whether the selected object belongs to source only schema if yes then we will have to add create schema statement before creating any other object.*/ @@ -363,6 +419,7 @@ export function SchemaDiffCompare({ params }) { if (selectedIds.length > 0) { let script_array = { 1: [], 2: [], 3: [], 4: [], 5: [] }, script_body = ''; + computeDependLevels(rows); getGenerateScriptData(rows, selectedIds, script_array, selectedFilters); generatedScript = generateFinalScript(script_array, scriptHeader, script_body); diff --git a/web/regression/javascript/schema_diff/schema_diff_compare_spec.js b/web/regression/javascript/schema_diff/schema_diff_compare_spec.js new file mode 100644 index 00000000000..5adb8ef889e --- /dev/null +++ b/web/regression/javascript/schema_diff/schema_diff_compare_spec.js @@ -0,0 +1,70 @@ +///////////////////////////////////////////////////////////// +// +// pgAdmin 4 - PostgreSQL Tools +// +// Copyright (C) 2013 - 2026, The pgAdmin Development Team +// This software is released under the PostgreSQL Licence +// +////////////////////////////////////////////////////////////// + +import { computeDependLevels } from '../../../pgadmin/tools/schema_diff/static/js/components/SchemaDiffCompare'; + +describe('computeDependLevels', () => { + + it('gives a chain of dependencies strictly increasing levels, deepest first', () => { + // A depends on B, B depends on C. C must end up with a higher level + // than B, and B a higher level than A, so a reverse walk (as + // generateFinalScript performs) writes C, then B, then A. + const rowA = { id: 'a', oid: 1, dependencies: [{ type: 'table', oid: 2 }] }; + const rowB = { id: 'b', oid: 2, dependencies: [{ type: 'type', oid: 3 }] }; + const rowC = { id: 'c', oid: 3, dependencies: [] }; + + computeDependLevels([rowA, rowB, rowC]); + + expect(rowC.dependLevel).toBeGreaterThan(rowB.dependLevel); + expect(rowB.dependLevel).toBeGreaterThan(rowA.dependLevel); + }); + + it('leaves unrelated rows all at the base level', () => { + const rowA = { id: 'a', oid: 1, dependencies: [] }; + const rowB = { id: 'b', oid: 2, dependencies: [] }; + + computeDependLevels([rowA, rowB]); + + expect(rowA.dependLevel).toBe(1); + expect(rowB.dependLevel).toBe(1); + }); + + it('does not loop forever on a circular dependency', () => { + const rowA = { id: 'a', oid: 1, dependencies: [{ type: 'table', oid: 2 }] }; + const rowB = { id: 'b', oid: 2, dependencies: [{ type: 'table', oid: 1 }] }; + + computeDependLevels([rowA, rowB]); + + expect(Number.isFinite(rowA.dependLevel)).toBe(true); + expect(Number.isFinite(rowB.dependLevel)).toBe(true); + }); + + it('ignores dependencies that point at an oid not present in the row set', () => { + const rowA = { id: 'a', oid: 1, dependencies: [{ type: 'extension', oid: 999 }] }; + + computeDependLevels([rowA]); + + expect(rowA.dependLevel).toBe(1); + }); + + it('takes the deepest of several dependency chains converging on the same row', () => { + // Both A and B depend directly on C, but A also depends on D which in + // turn depends on C, so C's level must reflect the longer A -> D -> C + // chain, not just the shorter B -> C one. + const rowA = { id: 'a', oid: 1, dependencies: [{ type: 'table', oid: 3 }, { type: 'table', oid: 4 }] }; + const rowB = { id: 'b', oid: 2, dependencies: [{ type: 'table', oid: 3 }] }; + const rowC = { id: 'c', oid: 3, dependencies: [] }; + const rowD = { id: 'd', oid: 4, dependencies: [{ type: 'table', oid: 3 }] }; + + computeDependLevels([rowA, rowB, rowC, rowD]); + + expect(rowC.dependLevel).toBe(rowD.dependLevel + 1); + expect(rowC.dependLevel).toBeGreaterThan(rowB.dependLevel + 1); + }); +});