Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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('<table>_<col>_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
Expand Down Expand Up @@ -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
# ``<table>_<col>_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'):
Comment thread
coderabbitai[bot] marked this conversation as resolved.

serial_type = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading