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,60 @@
##########################################################################
#
# pgAdmin 4 - PostgreSQL Tools
#
# Copyright (C) 2013 - 2026, The pgAdmin Development Team
# This software is released under the PostgreSQL Licence
#
##########################################################################

"""Unit tests for parse_nextval_sequence(), covering the schema-qualified
identifier it extracts out of a column's ``nextval(...)`` default, and in
particular the SQL string-literal quote-doubling PostgreSQL applies when
the sequence name itself contains a single quote (#10318).
"""

from pgadmin.browser.server_groups.servers.databases.schemas.tables.\
columns.utils import parse_nextval_sequence
from pgadmin.utils.route import BaseTestGenerator


class TestParseNextvalSequence(BaseTestGenerator):
"""Unit tests for parse_nextval_sequence()."""

scenarios = [
('No default value returns None',
dict(test_method='test_none_defval')),
('A non-nextval default returns None',
dict(test_method='test_non_nextval_defval')),
('A plain schema-qualified sequence name is extracted verbatim',
dict(test_method='test_plain_sequence_name')),
('A sequence name containing a single quote has the doubled '
'quote decoded back to one',
dict(test_method='test_quoted_sequence_name_with_embedded_quote')),
]

def runTest(self):
getattr(self, self.test_method)()

def test_none_defval(self):
self.assertIsNone(parse_nextval_sequence(None))

def test_non_nextval_defval(self):
self.assertIsNone(parse_nextval_sequence('1'))

def test_plain_sequence_name(self):
seq_name = parse_nextval_sequence(
"nextval('public.t_id_seq'::regclass)")
self.assertEqual(seq_name, 'public.t_id_seq')

def test_quoted_sequence_name_with_embedded_quote(self):
# PostgreSQL renders the sequence "id'seq" as the double-quoted
# identifier "id'seq", and then - because the whole thing is the
# argument of a string literal - doubles the embedded single
# quote: nextval('public."id''seq"'::regclass). The extracted
# identifier must have that doubling undone, since it is spliced
# verbatim into CREATE SEQUENCE / ALTER SEQUENCE DDL rather than
# back into a string literal.
seq_name = parse_nextval_sequence(
'nextval(\'public."id\'\'seq"\'::regclass)')
self.assertEqual(seq_name, 'public."id\'seq"')
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,14 @@ def reproject_serial_column(col):
with, so that callers can emit round-trippable DDL. Columns that are
not SERIAL, including ones already reprojected, are left untouched.

The real ``nextval(...)`` expression is kept alongside the emptied
``defval`` (under ``serial_defval``) rather than discarded, because
Schema Diff needs it back verbatim whenever it finds this column
genuinely differs in "serialness" from its counterpart: converting a
column to or from SERIAL means creating or dropping the sequence
behind it, which the pseudo-type's own implied default can't drive by
itself (#10292).

:param col: Column properties, modified in place
:return: The same column
"""
Expand All @@ -280,11 +288,42 @@ def reproject_serial_column(col):
col['displaytypname'] = serial_type
col['cltype'] = serial_type
col['typname'] = serial_type
col['serial_defval'] = col['defval']
col['defval'] = ''

return col


def parse_nextval_sequence(defval):
"""
Extract the schema-qualified sequence name out of a ``nextval(...)``
column default expression, e.g. ``nextval('public.t_id_seq'::regclass)``
yields ``public.t_id_seq``. The identifier is returned exactly as
PostgreSQL would render it as a bare identifier (already quoted if it
needs to be), so callers should use it verbatim rather than
re-quoting it.

PostgreSQL renders the argument to ``::regclass`` as a string literal,
so any single quote that is part of the identifier itself (e.g. a
sequence named ``id'seq``, which the server prints as the
double-quoted identifier ``"id'seq"``) is doubled per standard SQL
string-literal escaping: ``nextval('public."id''seq"'::regclass)``.
That doubling has to be undone before the extracted text is usable
outside of a string literal, i.e. spliced directly into
``CREATE SEQUENCE``/``ALTER SEQUENCE`` DDL, or the doubled quote would
be read back as two literal characters instead of one (#10318).

:param defval: A column's default value expression, or None
:return: The schema-qualified sequence name, or None if it is not a
nextval() default
"""
if not defval:
return None

match = re.match(r"nextval\('(.+)'::regclass\)$", defval)
return match.group(1).replace("''", "'") if match else None


@get_template_path
def get_formatted_columns(conn, tid, data, other_columns,
table_or_type, template_path=None,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,26 @@ ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.table)}}
{% if data.col_type_conversion is defined and data.col_type_conversion == False %} -- {% endif %} ALTER COLUMN {% if data.name %}{{conn|qtTypeIdent(data.name)}}{% else %}{{conn|qtTypeIdent(o_data.name)}}{% endif %} TYPE {{ GET_TYPE.UPDATE_TYPE_SQL(conn, data, o_data) }}{% if data.collspcname and data.collspcname != o_data.collspcname and data.cltype != '"char"' %}
COLLATE {{data.collspcname}}{% elif o_data.collspcname and data.cltype != '"char"' %} COLLATE {{o_data.collspcname}}{% endif %};
{% endif %}
{### Create the sequence a column becoming SERIAL needs, before its default below can reference it (#10292). IF NOT EXISTS is deliberately not used here: it would silently skip an existing, unrelated relation of the same name (without checking it is even a sequence), and the unconditional ALTER SEQUENCE ... OWNED BY below would then reassign ownership of that unrelated object instead of failing loudly (#10318). ###}
{% if data.serial_seq_create is defined %}
CREATE SEQUENCE {{data.serial_seq_create.name}}{% if data.serial_seq_create.cycled %}

CYCLE{% endif %}{% if data.serial_seq_create.increment is not none %}

INCREMENT {{data.serial_seq_create.increment|int}}{% endif %}{% if data.serial_seq_create.start is not none %}

START {{data.serial_seq_create.start|int}}{% endif %}{% if data.serial_seq_create.minimum is not none %}

MINVALUE {{data.serial_seq_create.minimum|int}}{% endif %}{% if data.serial_seq_create.maximum is not none %}

MAXVALUE {{data.serial_seq_create.maximum|int}}{% endif %}{% if data.serial_seq_create.cache is not none %}

CACHE {{data.serial_seq_create.cache|int}}{% endif %};

ALTER SEQUENCE {{data.serial_seq_create.name}}
OWNED BY {{conn|qtIdent(data.schema)}}.{{conn|qtIdent(data.table)}}.{% if data.name %}{{conn|qtIdent(data.name)}}{% else %}{{conn|qtIdent(o_data.name)}}{% endif %};
Comment thread
coderabbitai[bot] marked this conversation as resolved.

{% endif %}
{### Alter column default value ###}
{% if is_view_only and data.defval is defined and data.defval is not none and data.defval != '' and data.defval != o_data.defval %}
ALTER VIEW {{conn|qtIdent(data.schema, data.table)}}
Expand All @@ -35,6 +55,11 @@ ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.table)}}
ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.table)}}
ALTER COLUMN {% if data.name %}{{conn|qtTypeIdent(data.name)}}{% else %}{{conn|qtTypeIdent(o_data.name)}}{% endif %} DROP DEFAULT;

{% endif %}
{### Drop the now-unused sequence a column stops owning by leaving SERIAL; the DEFAULT above must already be gone, or PostgreSQL refuses to drop a sequence still referenced by it (#10292) ###}
{% if data.serial_seq_drop is defined %}
DROP SEQUENCE IF EXISTS {{data.serial_seq_drop}};

{% endif %}
{### Alter column not null value ###}
{% if 'attnotnull' in data and data.attnotnull != o_data.attnotnull %}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,26 @@ ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.table)}}
{% if data.col_type_conversion is defined and data.col_type_conversion == False %} -- {% endif %} ALTER COLUMN {% if data.name %}{{conn|qtTypeIdent(data.name)}}{% else %}{{conn|qtTypeIdent(o_data.name)}}{% endif %} TYPE {{ GET_TYPE.UPDATE_TYPE_SQL(conn, data, o_data) }}{% if data.collspcname and data.collspcname != o_data.collspcname and data.cltype != '"char"' %}
COLLATE {{data.collspcname}}{% elif o_data.collspcname and data.cltype != '"char"' %} COLLATE {{o_data.collspcname}}{% endif %};
{% endif %}
{### Create the sequence a column becoming SERIAL needs, before its default below can reference it (#10292). IF NOT EXISTS is deliberately not used here: it would silently skip an existing, unrelated relation of the same name (without checking it is even a sequence), and the unconditional ALTER SEQUENCE ... OWNED BY below would then reassign ownership of that unrelated object instead of failing loudly (#10318). ###}
{% if data.serial_seq_create is defined %}
CREATE SEQUENCE {{data.serial_seq_create.name}}{% if data.serial_seq_create.cycled %}

CYCLE{% endif %}{% if data.serial_seq_create.increment is not none %}

INCREMENT {{data.serial_seq_create.increment|int}}{% endif %}{% if data.serial_seq_create.start is not none %}

START {{data.serial_seq_create.start|int}}{% endif %}{% if data.serial_seq_create.minimum is not none %}

MINVALUE {{data.serial_seq_create.minimum|int}}{% endif %}{% if data.serial_seq_create.maximum is not none %}

MAXVALUE {{data.serial_seq_create.maximum|int}}{% endif %}{% if data.serial_seq_create.cache is not none %}

CACHE {{data.serial_seq_create.cache|int}}{% endif %};

ALTER SEQUENCE {{data.serial_seq_create.name}}
OWNED BY {{conn|qtIdent(data.schema)}}.{{conn|qtIdent(data.table)}}.{% if data.name %}{{conn|qtIdent(data.name)}}{% else %}{{conn|qtIdent(o_data.name)}}{% endif %};

{% endif %}
{### Alter column default value ###}
{% if is_view_only and data.defval is defined and data.defval is not none and data.defval != '' and data.defval != o_data.defval %}
ALTER VIEW {{conn|qtIdent(data.schema, data.table)}}
Expand All @@ -35,6 +55,11 @@ ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.table)}}
ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.table)}}
ALTER COLUMN {% if data.name %}{{conn|qtTypeIdent(data.name)}}{% else %}{{conn|qtTypeIdent(o_data.name)}}{% endif %} DROP DEFAULT;

{% endif %}
{### Drop the now-unused sequence a column stops owning by leaving SERIAL; the DEFAULT above must already be gone, or PostgreSQL refuses to drop a sequence still referenced by it (#10292) ###}
{% if data.serial_seq_drop is defined %}
DROP SEQUENCE IF EXISTS {{data.serial_seq_drop}};

{% endif %}
{### Alter column not null value ###}
{% if 'attnotnull' in data and data.attnotnull != o_data.attnotnull %}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
##########################################################################
#
# pgAdmin 4 - PostgreSQL Tools
#
# Copyright (C) 2013 - 2026, The pgAdmin Development Team
# This software is released under the PostgreSQL Licence
#
##########################################################################

"""Unit tests for BaseTableView._normalise_serial_column(), covering both
directions of converting a column between a plain integer type and
SERIAL/BIGSERIAL/SMALLSERIAL (#10292), and guarding against the ordinary
(non Schema Diff) column PUT being mistaken for one.
"""

from pgadmin.browser.server_groups.servers.databases.schemas.tables.utils \
import BaseTableView
from pgadmin.utils.route import BaseTestGenerator


class TestNormaliseSerialColumn(BaseTestGenerator):
"""Unit tests for BaseTableView._normalise_serial_column()."""

scenarios = [
('Converting a plain column to SERIAL creates the sequence and '
'restores the default',
dict(test_method='test_becoming_serial')),
('Converting a SERIAL column to plain queues the sequence for '
'dropping',
dict(test_method='test_leaving_serial')),
('A genuine difference on a column that is SERIAL on both sides '
'is unaffected',
dict(test_method='test_both_sides_already_serial')),
('A partial update that never mentions cltype leaves an '
'already-SERIAL column alone',
dict(test_method='test_partial_update_without_cltype_is_ignored')),
]

def runTest(self):
getattr(self, self.test_method)()

def test_becoming_serial(self):
# Schema Diff's source column, reprojected as BIGSERIAL, with the
# real nextval() default preserved under 'serial_defval'.
data = {
'cltype': 'bigserial', 'typname': 'bigserial',
'serial_defval': "nextval('public.t_id_seq'::regclass)",
'seqincrement': 1, 'seqstart': 1, 'seqmin': 1,
'seqmax': 9223372036854775807, 'seqcache': 1, 'seqcycle': False,
}
# The target's current (plain, unreprojected) column.
old_col_data = {
'cltype': 'integer', 'typname': 'integer', 'defval': None,
'seqrelid': None, 'defseqrelid': None, 'attidentity': '',
}

BaseTableView._normalise_serial_column(data, old_col_data)

self.assertEqual(data['cltype'], 'bigint')
self.assertEqual(data['typname'], 'bigint')
self.assertEqual(data['defval'],
"nextval('public.t_id_seq'::regclass)")
self.assertEqual(data['serial_seq_create']['name'], 'public.t_id_seq')
self.assertEqual(data['serial_seq_create']['increment'], 1)
self.assertNotIn('serial_defval', data)
self.assertNotIn('seqincrement', data)

def test_leaving_serial(self):
# Schema Diff's source column: a plain integer, never reprojected.
data = {'cltype': 'integer', 'typname': 'integer', 'defval': None}
# The target's current column genuinely is SERIAL.
old_col_data = {
'cltype': 'integer', 'typname': 'integer',
'defval': "nextval('public.t_id_seq'::regclass)",
'seqrelid': 100, 'defseqrelid': 100, 'attidentity': '',
}

BaseTableView._normalise_serial_column(data, old_col_data)

self.assertEqual(data['serial_seq_drop'], 'public.t_id_seq')
# The type didn't really change; the default is still queued to
# be dropped by the generic template logic (data['defval'] stays
# None/empty and differs from o_data['defval']).
self.assertEqual(data['cltype'], 'integer')

def test_both_sides_already_serial(self):
# Both sides are BIGSERIAL; only some other property (a comment,
# say) differs. The reprojection emptied 'defval' on the source
# side; that must not be read as a request to drop the real one,
# and no sequence should be created or dropped.
data = {
'cltype': 'bigserial', 'typname': 'bigserial',
'serial_defval': "nextval('public.t_id_seq'::regclass)",
'seqincrement': 1,
}
old_col_data = {
'cltype': 'integer', 'typname': 'integer',
'defval': "nextval('public.t_id_seq'::regclass)",
'seqrelid': 100, 'defseqrelid': 100, 'attidentity': '',
}

BaseTableView._normalise_serial_column(data, old_col_data)

self.assertNotIn('defval', data)
self.assertNotIn('serial_seq_create', data)
self.assertNotIn('serial_seq_drop', data)
self.assertNotIn('seqincrement', data)

def test_partial_update_without_cltype_is_ignored(self):
# The ordinary column PUT (not Schema Diff) submits only the
# fields the user actually changed - e.g. a privilege - and omits
# 'cltype' entirely when the type itself wasn't touched, even if
# the column already is SERIAL. This must be a complete no-op.
data = {'attacl': {'added': []}}
old_col_data = {
'cltype': 'integer', 'typname': 'integer',
'defval': "nextval('public.t_id_seq'::regclass)",
'seqrelid': 100, 'defseqrelid': 100, 'attidentity': '',
}

BaseTableView._normalise_serial_column(data, old_col_data)

self.assertEqual(data, {'attacl': {'added': []}})
Loading
Loading