From 229177a7d3630073a1ee1fef374814f7dca7b61d Mon Sep 17 00:00:00 2001 From: agu2347 <94227848+agu2347@users.noreply.github.com> Date: Wed, 12 Aug 2026 15:17:26 +0530 Subject: [PATCH] Schema Diff: ignore defseqrelid to avoid false SERIAL/BIGSERIAL diffs (#10236) get_formatted_columns() reprojects a column as SERIAL/BIGSERIAL/SMALLSERIAL when it owns (pg_depend, deptype='a' -> seqrelid) the same sequence referenced by its own nextval() DEFAULT (pg_attrdef -> defseqrelid, #9896/#10100/#10101). Both seqrelid and defseqrelid survive into the column dict that Schema Diff compares, but they are raw sequence OIDs that are meaningless to compare across two independently-created databases -- two databases can have byte-for-byte identical BIGSERIAL columns while their owned sequences get assigned completely different OIDs. column_keys_to_ignore already excluded seqrelid, but not its sibling defseqrelid. That gap caused: * SchemaDiffTableCompare.compare() / are_dictionaries_identical() to report a table as different purely because of a benign OID mismatch, even when every user-visible column property (cltype included) was identical. * compare_target_cols() to add the column to the 'changed' diff list, which downstream generates SQL via the column update.sql template. Since data.cltype == o_data.cltype ('bigserial' on both sides), this produced the invalid statement reported in the issue: `ALTER COLUMN adl_id TYPE bigserial;` Adding 'defseqrelid' to column_keys_to_ignore fixes both symptoms at the source: the column is no longer considered different at all, so neither the false-positive diff nor the invalid ALTER COLUMN statement is generated. Added a unit test (test_schema_diff_serial_column_ignore.py, mirroring the existing test_serial_detection_unit.py convention) covering: * two BIGSERIAL columns differing only by defseqrelid are not flagged * a genuine difference on an otherwise-identical BIGSERIAL column is still detected (proves the fix doesn't mask real diffs) Fixes #10236 --- .../schemas/tables/schema_diff_table_utils.py | 13 ++- .../test_schema_diff_serial_column_ignore.py | 83 +++++++++++++++++++ 2 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_schema_diff_serial_column_ignore.py diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/schema_diff_table_utils.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/schema_diff_table_utils.py index 7a2b069795c..74b91420f3b 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/schema_diff_table_utils.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/schema_diff_table_utils.py @@ -24,8 +24,19 @@ class SchemaDiffTableCompare(SchemaDiffObjectCompare): 'rows_cnt', 'hastoasttable', 'relhassubclass', 'relacl_str', 'setting'] + # 'seqrelid' (the sequence a column owns) and 'defseqrelid' (the + # sequence referenced by the column's nextval() DEFAULT) are compared + # against EACH OTHER by get_formatted_columns() to reproject a column + # as SERIAL/BIGSERIAL/SMALLSERIAL (see columns/utils.py, #9896/#10100/ + # #10101) -- but the resulting raw sequence OIDs are otherwise + # meaningless across two independently-created databases, even when + # both sides have an identical SERIAL column. Without ignoring + # 'defseqrelid' here too, Schema Diff falsely reports such columns (and + # therefore their whole table) as different, and generates an invalid + # `ALTER COLUMN ... TYPE bigserial` statement despite both sides having + # the exact same reprojected cltype. column_keys_to_ignore = ['atttypid', 'edit_types', 'elemoid', 'seqrelid', - 'indkey', 'seqtypid'] + 'indkey', 'seqtypid', 'defseqrelid'] constraint_keys_to_ignore = ['relname', 'nspname', 'parent_tbl', 'attrelid', 'adrelid', 'fknsp', 'confrelid', diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_schema_diff_serial_column_ignore.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_schema_diff_serial_column_ignore.py new file mode 100644 index 00000000000..cc45297aea4 --- /dev/null +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_schema_diff_serial_column_ignore.py @@ -0,0 +1,83 @@ +########################################################################## +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2026, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## + +"""Unit tests for SchemaDiffTableCompare's column comparison, verifying +that a SERIAL/BIGSERIAL column's raw sequence OID ('defseqrelid') does not +cause Schema Diff to report a false-positive difference (#10236). +""" + +from pgadmin.browser.server_groups.servers.databases.schemas.tables.\ + schema_diff_table_utils import SchemaDiffTableCompare +from pgadmin.utils.route import BaseTestGenerator + + +def _make_bigserial_column(defseqrelid, **overrides): + """A column dict as returned by get_formatted_columns() for a + genuine, already-reprojected BIGSERIAL column.""" + defaults = dict( + name='adl_id', cltype='bigserial', typname='bigserial', + atttypid=20, attlen=8, attnum=1, elemoid=20, seqtypid=20, + indkey=None, seqrelid=defseqrelid, defseqrelid=defseqrelid, + defval='', attnotnull=True, attacl=[], + ) + defaults.update(overrides) + return defaults + + +class TestSchemaDiffSerialColumnIgnore(BaseTestGenerator): + """Unit tests for SchemaDiffTableCompare.compare_target_cols().""" + + scenarios = [ + ('Identical BIGSERIAL columns with differing sequence OIDs are ' + 'not flagged as different', + dict(test_method='test_differing_defseqrelid_not_flagged')), + ('A genuinely different column is still flagged as different', + dict(test_method='test_genuine_difference_still_flagged')), + ] + + def runTest(self): + getattr(self, self.test_method)() + + def test_differing_defseqrelid_not_flagged(self): + # Two independently-created databases will assign different raw + # OIDs to each table's owned sequence, even for structurally + # identical BIGSERIAL columns. That OID difference alone must not + # cause the column (and thus the table) to be reported as + # different, and must not trigger an invalid + # `ALTER COLUMN ... TYPE bigserial` in the generated diff SQL. + source = _make_bigserial_column(defseqrelid=16482) + target_cols = [_make_bigserial_column(defseqrelid=98213)] + + added = [] + updated = [] + SchemaDiffTableCompare.compare_target_cols( + source, target_cols, added, updated) + + self.assertEqual(added, []) + self.assertEqual(updated, []) + # The matching target column must have been consumed. + self.assertEqual(target_cols, []) + + def test_genuine_difference_still_flagged(self): + # A real difference (here, NOT NULL toggled) on an otherwise + # identical BIGSERIAL column must still be detected, proving the + # fix only suppresses the OID noise and doesn't mask real diffs. + source = _make_bigserial_column(defseqrelid=16482, attnotnull=True) + target_cols = [ + _make_bigserial_column(defseqrelid=98213, attnotnull=False) + ] + + added = [] + updated = [] + SchemaDiffTableCompare.compare_target_cols( + source, target_cols, added, updated) + + self.assertEqual(len(updated), 1) + self.assertEqual(updated[0]['name'], 'adl_id') + self.assertEqual(added, [])