From da9395cc7070055923dbfc9b119917dada32bf51 Mon Sep 17 00:00:00 2001 From: Kundan Sable Date: Wed, 22 Jul 2026 14:24:13 +0530 Subject: [PATCH 1/4] fix: use pg_depend ownership instead of name guessing for SERIAL detection get_formatted_columns() guessed a column's owned sequence via make_object_name(table, col, 'seq') and rewrote nextval(...) defaults to SERIAL only when that literal name matched. This breaks in two ways: - Renaming a table/column changes the guessed name but not the sequence's actual name, so the column stops being recognized as SERIAL and the reverse-engineered script emits a broken nextval('old_seq'::regclass) that fails on a fresh database. - A column with a nextval() default that happens to reference an unrelated, coincidentally-named sequence gets silently rewritten as SERIAL, orphaning the real sequence. The properties template already exposes the column's actual owned sequence (seqrelid) via a LEFT JOIN on pg_depend. Use that instead of guessing, so SERIAL is only emitted when a genuine ownership dependency exists, independent of naming. Fixes #10100, #10101 --- .../databases/schemas/tables/columns/utils.py | 46 +++++++++++-------- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/utils.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/utils.py index feedb5dfb31..2f014c72e32 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/utils.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/utils.py @@ -17,8 +17,6 @@ import DataTypeReader from pgadmin.browser.server_groups.servers.utils import parse_priv_from_db, \ parse_priv_to_db -from pgadmin.browser.server_groups.servers.databases.utils \ - import make_object_name from functools import wraps import re @@ -234,14 +232,18 @@ def get_formatted_columns(conn, tid, data, other_columns, This function will iterate and return formatted data for all the columns. - Serial-column detection is on by default: a column whose default - is ``nextval('__seq'...)`` with an integer/smallint/ - bigint type is the reverse-engineered form of a SERIAL declaration, - and we reproject it back to ``serial`` / ``smallserial`` / - ``bigserial`` so callers can emit valid round-trippable DDL. Pass - ``with_serial=False`` only if you genuinely need the raw libpq - representation (e.g. a low-level introspection caller that handles - the sequence itself). Issue #9896. + Serial-column detection is on by default: a column that owns the + sequence referenced by its ``nextval(...)`` default (with an + integer/smallint/bigint type) is the reverse-engineered form of a + SERIAL declaration, and we reproject it back to ``serial`` / + ``smallserial`` / ``bigserial`` so callers can emit valid + round-trippable DDL. Ownership is determined from ``pg_depend`` (the + ``seqrelid`` exposed by properties.sql), not by guessing the + sequence name, so it stays correct across renames and never rewrites + an unrelated column. Pass ``with_serial=False`` only if you genuinely + need the raw libpq representation (e.g. a low-level introspection + caller that handles the sequence itself). Issues #9896, #10100, + #10101. :param conn: Connection Object :param tid: Table ID @@ -269,14 +271,22 @@ def get_formatted_columns(conn, tid, data, other_columns, other_col['inheritedfrom'] if with_serial: - # Here we assume if a column is serial - serial_seq_name = make_object_name( - data['name'], col['name'], 'seq') - # replace the escaped quotes for comparison - defval = (col.get('defval', '') or '').replace("''", "'").\ - replace('""', '"') - - if serial_seq_name in defval and defval.startswith("nextval('")\ + # A column is SERIAL only when it genuinely owns the sequence + # referenced by its DEFAULT. properties.sql LEFT JOINs + # pg_depend (a sequence's pg_class depending on this column's + # pg_attribute) and surfaces the owned sequence oid as + # ``seqrelid`` - that dependency is the authoritative + # ownership signal. Relying on it (instead of guessing the + # ``
__seq`` name) keeps detection correct after a + # table/column/sequence rename and never rewrites an + # unrelated column whose default happens to match the guessed + # name (#10100, #10101). Identity columns carry an internal + # dependency too, but never a nextval() default, and are + # additionally excluded via attidentity. + defval = col.get('defval', '') or '' + + if col.get('seqrelid') and defval.startswith("nextval('") \ + and not col.get('attidentity') \ and col['typname'] in ('integer', 'smallint', 'bigint'): serial_type = { From da072dc9f32f3c9f9e635ab8e52eceb43573dbc7 Mon Sep 17 00:00:00 2001 From: Kundan Sable Date: Wed, 22 Jul 2026 14:57:53 +0530 Subject: [PATCH 2/4] fix: require default's own sequence dependency to match owned sequence Ownership and default are independent dependencies in PostgreSQL: a column can own one sequence (pg_depend deptype='a', the existing seqrelid check) while its DEFAULT's nextval() call names a completely different one (pg_depend deptype='n' from the pg_attrdef entry). The previous check only verified ownership, so a column in this split ownership/default state was still wrongly reprojected as SERIAL, discarding its real, distinct nextval() default. Expose the default's own sequence dependency as defseqrelid in properties.sql and require it to equal seqrelid before treating a column as SERIAL, so split ownership/default columns keep their original explicit nextval() default. Add a mock-based regression test covering the split case, a genuine SERIAL column, and identity columns, following the pattern already used for shared-server unit tests since BaseTestGenerator's setUp() needs a live server connection. Verified against a real PostgreSQL 13 instance: a column owning seq_owned but defaulting to nextval('seq_used') round-trips as DEFAULT nextval('seq_used'::regclass), not SERIAL. --- .../tests/test_serial_detection_unit.py | 108 ++++++++++++++++++ .../databases/schemas/tables/columns/utils.py | 10 ++ .../columns/sql/default/properties.sql | 2 + 3 files changed, 120 insertions(+) create mode 100644 web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/tests/test_serial_detection_unit.py diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/tests/test_serial_detection_unit.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/tests/test_serial_detection_unit.py new file mode 100644 index 00000000000..f3402379973 --- /dev/null +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/tests/test_serial_detection_unit.py @@ -0,0 +1,108 @@ +########################################################################## +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2026, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## + +"""Unit tests for get_formatted_columns()'s SERIAL-column detection using +mocks. + +These verify the ownership-vs-default-sequence comparison (#10100, +#10101) without requiring a running PostgreSQL server. +""" + +from unittest.mock import MagicMock, patch +from pgadmin.utils.route import BaseTestGenerator + +UTILS_MODULE = ('pgadmin.browser.server_groups.servers.databases.schemas.' + 'tables.columns.utils') + + +def _make_column(**overrides): + """A properties.sql result row, as returned for a single column.""" + defaults = dict( + name='id', atttypid=23, attlen=4, attnum=1, attndims=0, + atttypmod=-1, attacl=None, attnotnull=False, attoptions=None, + attfdwoptions=None, attstattarget=-1, attstorage='p', + attidentity='', defval=None, typname='integer', + displaytypname='integer', cltype='integer', + inheritedfrom=None, inheritedid=None, elemoid=23, typnspname='pg_catalog', + defaultstorage='p', description=None, indkey=None, isdup=False, + collspcname='', is_fk=False, seclabels=None, is_sys_column=False, + colconstype='n', genexpr=None, relname='t', is_view_only=False, + attcompression=None, seqrelid=None, defseqrelid=None, + ) + defaults.update(overrides) + return defaults + + +class TestSerialColumnDetection(BaseTestGenerator): + """Unit tests for ServerModule.get_formatted_columns() SERIAL + detection using mock rows.""" + + scenarios = [ + ('Split ownership/default keeps explicit nextval default', + dict(test_method='test_split_ownership_default_not_serial')), + ('Genuine SERIAL column is reprojected', + dict(test_method='test_genuine_serial_is_detected')), + ('Identity column is never reprojected', + dict(test_method='test_identity_column_not_serial')), + ] + + @patch(UTILS_MODULE + '.render_template', return_value='SELECT 1;') + def runTest(self, mock_render): + getattr(self, self.test_method)() + + def _run(self, column_row): + from pgadmin.browser.server_groups.servers.databases.schemas.\ + tables.columns.utils import get_formatted_columns + + conn = MagicMock() + conn.execute_dict.return_value = (True, {'rows': [column_row]}) + conn.execute_2darray.return_value = (True, {'rows': []}) + + with patch(UTILS_MODULE + '.column_formatter'): + data = get_formatted_columns( + conn, tid=1, data={}, other_columns=[], + table_or_type='table', template_path='columns/sql/default') + + return data['columns'][0] + + def test_split_ownership_default_not_serial(self): + # Column owns sequence 100 (pg_depend deptype='a'), but its + # DEFAULT's nextval() call references a different sequence, 200 + # (pg_depend deptype='n' from the pg_attrdef entry). SERIAL must + # not be applied - ownership and default disagree. + col = _make_column( + defval="nextval('seq_used'::regclass)", + seqrelid=100, defseqrelid=200, + ) + result = self._run(col) + self.assertEqual(result['typname'], 'integer') + self.assertEqual(result['cltype'], 'integer') + self.assertEqual(result['defval'], "nextval('seq_used'::regclass)") + + def test_genuine_serial_is_detected(self): + # Ownership and default both point at the same sequence (100) - + # this is what a real SERIAL column looks like. + col = _make_column( + defval="nextval('t_id_seq'::regclass)", + seqrelid=100, defseqrelid=100, + ) + result = self._run(col) + self.assertEqual(result['typname'], 'serial') + self.assertEqual(result['cltype'], 'serial') + self.assertEqual(result['defval'], '') + + def test_identity_column_not_serial(self): + # Identity columns can carry an internal sequence dependency on + # both sides, but must never be reprojected as SERIAL. + col = _make_column( + defval=None, seqrelid=100, defseqrelid=100, attidentity='a', + ) + result = self._run(col) + self.assertEqual(result['typname'], 'integer') + self.assertEqual(result['defval'], None) diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/utils.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/utils.py index 2f014c72e32..be604d8e52c 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/utils.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/utils.py @@ -283,10 +283,20 @@ def get_formatted_columns(conn, tid, data, other_columns, # name (#10100, #10101). Identity columns carry an internal # dependency too, but never a nextval() default, and are # additionally excluded via attidentity. + # + # Ownership and default are independent dependencies in + # PostgreSQL: a column can own one sequence (pg_depend + # deptype='a', -> ``seqrelid``) while its DEFAULT's nextval() + # call names a completely different one (pg_depend deptype='n' + # from the pg_attrdef entry, -> ``defseqrelid``). SERIAL only + # applies when both dependencies point at the same sequence, so + # this split-ownership/default case keeps its original, + # explicit nextval() default instead of being rewritten. defval = col.get('defval', '') or '' if col.get('seqrelid') and defval.startswith("nextval('") \ and not col.get('attidentity') \ + and col.get('seqrelid') == col.get('defseqrelid') \ and col['typname'] in ('integer', 'smallint', 'bigint'): serial_type = { diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/columns/sql/default/properties.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/columns/sql/default/properties.sql index fe6d0376392..f561fefcf03 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/columns/sql/default/properties.sql +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/columns/sql/default/properties.sql @@ -37,12 +37,14 @@ SELECT DISTINCT ON (att.attnum) att.attname as name, att.atttypid, att.attlen, a (CASE WHEN (att.attgenerated in ('s')) THEN pg_catalog.pg_get_expr(def.adbin, def.adrelid) END) AS genexpr, tab.relname as relname, (CASE WHEN tab.relkind = 'v' THEN true ELSE false END) AS is_view_only, (CASE WHEN att.attcompression = 'p' THEN 'pglz' WHEN att.attcompression = 'l' THEN 'lz4' END) AS attcompression, + defseqdep.refobjid AS defseqrelid, seq.* FROM pg_catalog.pg_attribute att JOIN pg_catalog.pg_type ty ON ty.oid=atttypid LEFT OUTER JOIN pg_catalog.pg_attrdef def ON adrelid=att.attrelid AND adnum=att.attnum LEFT OUTER JOIN pg_catalog.pg_description des ON (des.objoid=att.attrelid AND des.objsubid=att.attnum AND des.classoid='pg_class'::regclass) LEFT OUTER JOIN (pg_catalog.pg_depend dep JOIN pg_catalog.pg_class cs ON dep.classid='pg_class'::regclass AND dep.objid=cs.oid AND cs.relkind='S') ON dep.refobjid=att.attrelid AND dep.refobjsubid=att.attnum + LEFT OUTER JOIN pg_catalog.pg_depend defseqdep ON defseqdep.classid='pg_catalog.pg_attrdef'::regclass AND defseqdep.objid=def.oid AND defseqdep.deptype='n' AND defseqdep.refclassid='pg_catalog.pg_class'::regclass LEFT OUTER JOIN pg_catalog.pg_index pi ON pi.indrelid=att.attrelid AND indisprimary LEFT OUTER JOIN pg_catalog.pg_collation coll ON att.attcollation=coll.oid LEFT OUTER JOIN pg_catalog.pg_namespace nspc ON coll.collnamespace=nspc.oid From 8f1ca94e6aa2af8d2e39b043901662a60490dc22 Mon Sep 17 00:00:00 2001 From: Kundan Sable Date: Wed, 22 Jul 2026 15:01:36 +0530 Subject: [PATCH 3/4] style: fix pycodestyle violations in serial-detection unit test E127 continuation-line indent and E501 line-length (82 > 79) on the UTILS_MODULE string and the _make_column() defaults dict. --- .../tables/columns/tests/test_serial_detection_unit.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/tests/test_serial_detection_unit.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/tests/test_serial_detection_unit.py index f3402379973..5a73903e8fa 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/tests/test_serial_detection_unit.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/tests/test_serial_detection_unit.py @@ -18,7 +18,7 @@ from pgadmin.utils.route import BaseTestGenerator UTILS_MODULE = ('pgadmin.browser.server_groups.servers.databases.schemas.' - 'tables.columns.utils') + 'tables.columns.utils') def _make_column(**overrides): @@ -29,7 +29,8 @@ def _make_column(**overrides): attfdwoptions=None, attstattarget=-1, attstorage='p', attidentity='', defval=None, typname='integer', displaytypname='integer', cltype='integer', - inheritedfrom=None, inheritedid=None, elemoid=23, typnspname='pg_catalog', + inheritedfrom=None, inheritedid=None, elemoid=23, + typnspname='pg_catalog', defaultstorage='p', description=None, indkey=None, isdup=False, collspcname='', is_fk=False, seclabels=None, is_sys_column=False, colconstype='n', genexpr=None, relname='t', is_view_only=False, From 55d5cd51182c4e654cbfc08d2c704e4f1c06f8e1 Mon Sep 17 00:00:00 2001 From: Kundan Sable Date: Wed, 22 Jul 2026 15:14:27 +0530 Subject: [PATCH 4/4] fix: restrict defseqrelid to sequence relations and collapse to a scalar The defseqdep join filtered the default's own pg_depend row down to classid='pg_attrdef' and deptype='n', but never checked that the referenced object (refobjid) is actually a sequence. A default expression can carry other deptype='n' dependencies on non-sequence pg_class objects too - e.g. a composite type referenced via a cast inside the expression - which would fan the LEFT JOIN out to multiple rows for the same column and let SELECT DISTINCT ON (att.attnum) pick an arbitrary one, occasionally resolving defseqrelid to something other than the intended sequence. Verified against a real server: a default expression referencing both a sequence and a composite type produces two deptype='n' pg_depend rows, one per object. Join through pg_class with relkind='S' so only sequence dependencies are considered, and collapse the lookup into a scalar subquery (the same pattern already used for typnspname/isdup elsewhere in this template) with an ORDER BY ... LIMIT 1 tie-breaker, so it can never fan out the outer query regardless of how many dependencies exist. Existing single-dependency behavior (the common case) is unchanged. --- .../templates/columns/sql/default/properties.sql | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/columns/sql/default/properties.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/columns/sql/default/properties.sql index f561fefcf03..ee94614ff96 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/columns/sql/default/properties.sql +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/columns/sql/default/properties.sql @@ -37,14 +37,20 @@ SELECT DISTINCT ON (att.attnum) att.attname as name, att.atttypid, att.attlen, a (CASE WHEN (att.attgenerated in ('s')) THEN pg_catalog.pg_get_expr(def.adbin, def.adrelid) END) AS genexpr, tab.relname as relname, (CASE WHEN tab.relkind = 'v' THEN true ELSE false END) AS is_view_only, (CASE WHEN att.attcompression = 'p' THEN 'pglz' WHEN att.attcompression = 'l' THEN 'lz4' END) AS attcompression, - defseqdep.refobjid AS defseqrelid, + (SELECT dsd.refobjid + FROM pg_catalog.pg_depend dsd + JOIN pg_catalog.pg_class dsc ON dsd.refclassid='pg_catalog.pg_class'::regclass + AND dsd.refobjid=dsc.oid AND dsc.relkind='S' + WHERE dsd.classid='pg_catalog.pg_attrdef'::regclass + AND dsd.objid=def.oid AND dsd.deptype='n' + ORDER BY dsd.refobjid + LIMIT 1) AS defseqrelid, seq.* FROM pg_catalog.pg_attribute att JOIN pg_catalog.pg_type ty ON ty.oid=atttypid LEFT OUTER JOIN pg_catalog.pg_attrdef def ON adrelid=att.attrelid AND adnum=att.attnum LEFT OUTER JOIN pg_catalog.pg_description des ON (des.objoid=att.attrelid AND des.objsubid=att.attnum AND des.classoid='pg_class'::regclass) LEFT OUTER JOIN (pg_catalog.pg_depend dep JOIN pg_catalog.pg_class cs ON dep.classid='pg_class'::regclass AND dep.objid=cs.oid AND cs.relkind='S') ON dep.refobjid=att.attrelid AND dep.refobjsubid=att.attnum - LEFT OUTER JOIN pg_catalog.pg_depend defseqdep ON defseqdep.classid='pg_catalog.pg_attrdef'::regclass AND defseqdep.objid=def.oid AND defseqdep.deptype='n' AND defseqdep.refclassid='pg_catalog.pg_class'::regclass LEFT OUTER JOIN pg_catalog.pg_index pi ON pi.indrelid=att.attrelid AND indisprimary LEFT OUTER JOIN pg_catalog.pg_collation coll ON att.attcollation=coll.oid LEFT OUTER JOIN pg_catalog.pg_namespace nspc ON coll.collnamespace=nspc.oid