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..5a73903e8fa --- /dev/null +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/tests/test_serial_detection_unit.py @@ -0,0 +1,109 @@ +########################################################################## +# +# 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 feedb5dfb31..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 @@ -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,32 @@ 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. + # + # 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..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,6 +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, + (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