From b40790f31d8ed6c973e69be5d8dd92edd41cf65c Mon Sep 17 00:00:00 2001 From: VIBVEL47 <161462689+VIBVEL47@users.noreply.github.com> Date: Thu, 22 Oct 2026 20:42:06 +0530 Subject: [PATCH 1/3] Fix changing the inherited tables of an existing table. #10657 Removing a parent from 'Inherited from' removed its columns from the Columns tab, but only NO INHERIT was run, which keeps them on the table as local columns, so they came back on reopening. Drop them as well, unless a remaining parent still provides them. Adding a parent to an existing table failed with "child table is missing column", as the parent's columns were skipped and INHERIT requires the table to have them. Add the parent's missing columns and CHECK constraints, using their definitions from the catalog, before INHERIT. The tree icon returned after saving was based on the table's properties from before the update, so it showed the previous inheritance. Use the table's state after the update instead. Co-Authored-By: Claude Opus 5.5 --- .../templates/tables/sql/15_plus/update.sql | 11 + .../default/get_missing_inherited_checks.sql | 17 ++ .../default/get_missing_inherited_columns.sql | 26 ++ .../templates/tables/sql/default/update.sql | 11 + .../tests/test_table_inherits_columns.py | 239 ++++++++++++++++++ .../servers/databases/schemas/tables/utils.py | 75 +++++- 6 files changed, 372 insertions(+), 7 deletions(-) create mode 100644 web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_missing_inherited_checks.sql create mode 100644 web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_missing_inherited_columns.sql create mode 100644 web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_table_inherits_columns.py diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/15_plus/update.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/15_plus/update.sql index b825bf20f75..6a21f2291c6 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/15_plus/update.sql +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/15_plus/update.sql @@ -37,6 +37,17 @@ ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} {## Update Inherits table definition ##} {#####################################################} {% if data.coll_inherits_added|length > 0 %} +{### A table must have the parent's columns and CHECK constraints before it can inherit from it ###} +{% for col in inherit_columns %} +ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} + ADD COLUMN {{conn|qtIdent(col.name)}} {{col.displaytypname}}{% if col.collspcname %} COLLATE {{col.collspcname}}{% endif %}{% if col.attnotnull %} NOT NULL{% endif %}; + +{% endfor %} +{% for chk in inherit_checks %} +ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} + ADD CONSTRAINT {{conn|qtIdent(chk.name)}} {{chk.definition}}; + +{% endfor %} {% for val in data.coll_inherits_added %} ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} INHERIT {{val}}; diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_missing_inherited_checks.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_missing_inherited_checks.sql new file mode 100644 index 00000000000..34e6333aef7 --- /dev/null +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_missing_inherited_checks.sql @@ -0,0 +1,17 @@ +{### Inheritable CHECK constraints of the given parents that the table does not have yet ###} +SELECT name, definition FROM ( + SELECT DISTINCT ON (c.conname) c.conname AS name, + pg_catalog.pg_get_constraintdef(c.oid, true) AS definition, p.ord + FROM + pg_catalog.unnest(ARRAY[{% for val in parents %}{{val|qtLiteral(conn)}}::regclass::oid{% if not loop.last %}, {% endif %}{% endfor %}]) + WITH ORDINALITY AS p(oid, ord) + JOIN + pg_catalog.pg_constraint c ON c.conrelid = p.oid + WHERE c.contype = 'c' AND NOT c.connoinherit + AND NOT EXISTS ( + SELECT 1 FROM pg_catalog.pg_constraint cc + WHERE cc.conrelid = {{tid}}::oid AND cc.contype = 'c' + AND cc.conname = c.conname) + ORDER BY c.conname, p.ord +) checks +ORDER BY ord, name diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_missing_inherited_columns.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_missing_inherited_columns.sql new file mode 100644 index 00000000000..58ca4c6cbba --- /dev/null +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_missing_inherited_columns.sql @@ -0,0 +1,26 @@ +{### Columns of the given parents that the table does not have yet ###} +SELECT name, displaytypname, collspcname, attnotnull FROM ( + SELECT DISTINCT ON (a.attname) a.attname AS name, + pg_catalog.format_type(a.atttypid, a.atttypmod) AS displaytypname, + CASE WHEN a.attcollation <> ty.typcollation THEN + pg_catalog.quote_ident(cn.nspname)||'.'||pg_catalog.quote_ident(co.collname) + END AS collspcname, + a.attnotnull, p.ord, a.attnum + FROM + pg_catalog.unnest(ARRAY[{% for val in parents %}{{val|qtLiteral(conn)}}::regclass::oid{% if not loop.last %}, {% endif %}{% endfor %}]) + WITH ORDINALITY AS p(oid, ord) + JOIN + pg_catalog.pg_attribute a ON a.attrelid = p.oid AND a.attnum > 0 AND NOT a.attisdropped + JOIN + pg_catalog.pg_type ty ON ty.oid = a.atttypid + LEFT OUTER JOIN + pg_catalog.pg_collation co ON co.oid = a.attcollation + LEFT OUTER JOIN + pg_catalog.pg_namespace cn ON cn.oid = co.collnamespace + WHERE NOT EXISTS ( + SELECT 1 FROM pg_catalog.pg_attribute ca + WHERE ca.attrelid = {{tid}}::oid AND ca.attname = a.attname + AND ca.attnum > 0 AND NOT ca.attisdropped) + ORDER BY a.attname, p.ord, a.attnum +) cols +ORDER BY ord, attnum diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/update.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/update.sql index e720c171efa..85f029e6e74 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/update.sql +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/update.sql @@ -29,6 +29,17 @@ ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} {## Update Inherits table definition ##} {#####################################################} {% if data.coll_inherits_added|length > 0 %} +{### A table must have the parent's columns and CHECK constraints before it can inherit from it ###} +{% for col in inherit_columns %} +ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} + ADD COLUMN {{conn|qtIdent(col.name)}} {{col.displaytypname}}{% if col.collspcname %} COLLATE {{col.collspcname}}{% endif %}{% if col.attnotnull %} NOT NULL{% endif %}; + +{% endfor %} +{% for chk in inherit_checks %} +ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} + ADD CONSTRAINT {{conn|qtIdent(chk.name)}} {{chk.definition}}; + +{% endfor %} {% for val in data.coll_inherits_added %} ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} INHERIT {{val}}; diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_table_inherits_columns.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_table_inherits_columns.py new file mode 100644 index 00000000000..3ea3cbde81f --- /dev/null +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_table_inherits_columns.py @@ -0,0 +1,239 @@ +########################################################################## +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2026, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## + +"""Changing 'Inherited from' on an existing table (#10657). + +Removing a parent removes its columns from the table dialog's Columns tab, +but NO INHERIT keeps them on the child as local columns, so saving must +drop them too, except a column that a remaining parent still provides. + +Adding a parent must first give the table the parent's columns and CHECK +constraints that it does not have yet, as INHERIT fails without them. + +Either way, the tree icon returned by the save must show the table's +inheritance after the change, not before it. +""" + +import json +import uuid + +from pgadmin.browser.server_groups.servers.databases.schemas.tests import \ + utils as schema_utils +from pgadmin.browser.server_groups.servers.databases.tests import utils as \ + database_utils +from pgadmin.utils.route import BaseTestGenerator +from regression import parent_node_dict +from regression.python_test_utils import test_utils as utils +from . import utils as tables_utils + + +class InheritsColumnsTestMixin: + """Create the parents and the child table, and query the database.""" + + def setUp(self): + self.db_name = parent_node_dict["database"][-1]["db_name"] + schema_info = parent_node_dict["schema"][-1] + self.server_id = schema_info["server_id"] + self.db_id = schema_info["db_id"] + db_con = database_utils.connect_database(self, utils.SERVER_GROUP, + self.server_id, self.db_id) + if not db_con['data']["connected"]: + raise Exception("Could not connect to database to add a table.") + + self.schema_id = schema_info["schema_id"] + self.schema_name = schema_info["schema_name"] + if not schema_utils.verify_schemas(self.server, self.db_name, + self.schema_name): + raise Exception("Could not find the schema to add a table.") + + suffix = str(uuid.uuid4())[1:8] + self.parent_names = [] + for idx, definition in enumerate(self.parents): + parent_name = "test_inh_parent_%s_%s" % (idx, suffix) + self._query("CREATE TABLE {0}.{1} ({2})".format( + self.schema_name, parent_name, definition)) + self.parent_names.append(parent_name) + + self.table_name = "test_inh_child_%s" % suffix + self._query("CREATE TABLE {0}.{1} ({2}){3}".format( + self.schema_name, self.table_name, self.child, + self._inherits_clause())) + self.table_id = self._query( + "SELECT '{0}.{1}'::regclass::oid".format( + self.schema_name, self.table_name))[0][0] + + def _inherits_clause(self): + return '' + + def _query(self, sql): + connection = utils.get_db_connection(self.db_name, + self.server['username'], + self.server['db_password'], + self.server['host'], + self.server['port'], + self.server['sslmode']) + try: + pg_cursor = connection.cursor() + pg_cursor.execute(sql) + result = pg_cursor.fetchall() if pg_cursor.description else None + connection.commit() + return result + finally: + connection.close() + + def _child_columns(self): + return [row[0] for row in self._query( + "SELECT attname FROM pg_catalog.pg_attribute " + "WHERE attrelid = {0}::oid AND attnum > 0 " + "AND NOT attisdropped ORDER BY attname".format(self.table_id))] + + def _qualified(self, name): + return '{0}.{1}'.format(self.schema_name, name) + + def _get_msql(self, data): + self.url = '/browser/table/msql/' + response = tables_utils.api_get_msql( + self, {k: json.dumps(v) for k, v in data.items()}) + self.assertEqual(response.status_code, 200) + return json.loads(response.data.decode('utf-8'))['data'] + + def _put(self, data): + self.url = '/browser/table/obj/' + self.data = dict(data, oid=self.table_id) + response = tables_utils.api_put(self) + self.assertEqual(response.status_code, 200) + return json.loads(response.data.decode('utf-8'))['node'] + + def tearDown(self): + database_utils.disconnect_database(self, self.server_id, self.db_id) + + +class TableRemoveInheritsColumnsTestCase(InheritsColumnsTestMixin, + BaseTestGenerator): + """Remove the first parent of a table, sending the columns that the + table dialog removes from its Columns tab as deleted.""" + + scenarios = [ + ('Columns of the only parent are dropped', + dict(parents=['id integer, a_name text'], + child='own_col text', + expected_sql=['NO INHERIT', 'DROP COLUMN IF EXISTS id;', + 'DROP COLUMN IF EXISTS a_name;'], + unexpected_sql=['own_col'], + expected_columns=['own_col'], + expected_icon='icon-table')), + ('A column still provided by another parent is kept', + dict(parents=['id integer, a_name text', + 'id integer, b_name text'], + child='own_col text', + expected_sql=['NO INHERIT', 'DROP COLUMN IF EXISTS a_name;'], + unexpected_sql=['DROP COLUMN IF EXISTS id;', 'b_name', + 'own_col'], + expected_columns=['id', 'b_name', 'own_col'], + expected_icon='icon-table-inherits')), + ] + + def _inherits_clause(self): + return ' INHERITS ({0})'.format( + ', '.join(self._qualified(p) for p in self.parent_names)) + + def runTest(self): + removed_parent_id = self._query( + "SELECT '{0}'::regclass::oid".format( + self._qualified(self.parent_names[0])))[0][0] + + self.url = '/browser/table/obj/' + response = tables_utils.api_get(self) + self.assertEqual(response.status_code, 200) + properties = json.loads(response.data.decode('utf-8')) + + # What the table dialog sends after the first parent is removed + # from 'Inherited from'. + deleted = [c for c in properties['columns'] + if c.get('inheritedid') == removed_parent_id] + self.assertTrue(deleted) + data = { + 'coll_inherits': properties['coll_inherits'][1:], + 'columns': {'deleted': deleted} + } + + sql = self._get_msql(data) + for text in self.expected_sql: + self.assertIn(text, sql) + for text in self.unexpected_sql: + self.assertNotIn(text, sql) + + node = self._put(data) + self.assertEqual(self._child_columns(), + sorted(self.expected_columns)) + # The tree icon follows the inheritance after the save. + self.assertEqual(node['icon'], self.expected_icon) + + +class TableAddInheritsColumnsTestCase(InheritsColumnsTestMixin, + BaseTestGenerator): + """Add a parent to an existing table, sending the parent's columns as + the table dialog adds them to its Columns tab.""" + + scenarios = [ + ('Missing columns and CHECK constraints are added before INHERIT', + dict(parents=['id varchar(10) NOT NULL, code text COLLATE "C", ' + 'CONSTRAINT id_not_empty CHECK (id <> \'\')'], + child='cid bigint', + expected_sql=['ADD COLUMN id character varying(10) NOT NULL;', + 'ADD COLUMN code text COLLATE pg_catalog."C";', + "ADD CONSTRAINT id_not_empty " + "CHECK (id::text <> ''::text);"], + unexpected_sql=['cid'], + expected_columns=['cid', 'code', 'id'])), + ('Columns the table already has are not added again', + dict(parents=['id bigint, name text'], + child='id bigint, cid bigint', + expected_sql=['ADD COLUMN name text;'], + unexpected_sql=['ADD COLUMN id', 'cid', 'ADD CONSTRAINT'], + expected_columns=['cid', 'id', 'name'])), + ] + + def runTest(self): + parent = self._qualified(self.parent_names[0]) + parent_id = self._query( + "SELECT '{0}'::regclass::oid".format(parent))[0][0] + parent_columns = self._query( + "SELECT attname, pg_catalog.format_type(atttypid, NULL) " + "FROM pg_catalog.pg_attribute WHERE attrelid = {0}::oid " + "AND attnum > 0 AND NOT attisdropped ORDER BY attnum".format( + parent_id)) + existing = self._child_columns() + + # What the table dialog sends after the parent is added to + # 'Inherited from'. + data = { + 'coll_inherits': [parent], + 'columns': {'added': [ + {'name': name, 'cltype': cltype, 'inheritedfrom': parent, + 'inheritedid': parent_id} + for name, cltype in parent_columns if name not in existing + ]} + } + + sql = self._get_msql(data) + for text in self.expected_sql: + self.assertIn(text, sql) + self.assertLess(sql.index(text), sql.index('INHERIT ' + parent)) + for text in self.unexpected_sql: + self.assertNotIn(text, sql) + + node = self._put(data) + self.assertEqual(self._child_columns(), + sorted(self.expected_columns)) + self.assertEqual(node['icon'], 'icon-table-inherits') + self.assertEqual(self._query( + "SELECT inhparent FROM pg_catalog.pg_inherits " + "WHERE inhrelid = {0}::oid".format(self.table_id)), + [(parent_id,)]) diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py index eb094ab4cdf..ffe297404d2 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py @@ -46,6 +46,7 @@ tables.row_security_policies import \ utils as row_security_policies_utils from pgadmin.utils.preferences import Preferences +from pgadmin.utils.exception import ExecuteError from pgadmin.browser.server_groups.servers.databases.schemas.utils \ import VacuumSettings from pgadmin.tools.schema_diff.node_registry import SchemaDiffRegistry @@ -1286,14 +1287,59 @@ def _filter_new_tables(data, old_data): set(data['coll_inherits']) ) - def _check_for_column_delete(self, columns, data, column_sql): + def _get_still_inherited_columns(self, scid, data): + """ + Return the names of the columns still provided by the parents left + in 'Inherited from', or None if no parent is being removed. + """ + if not data.get('coll_inherits_removed'): + return None + + other_columns = [] + if data.get('coll_inherits'): + is_error, errmsg = self._get_inherited_tables(scid, data, + other_columns) + if is_error: + raise ExecuteError(errmsg) + + return {c['name'] for c in other_columns} + + def _get_inherit_prerequisites(self, tid, data): + """ + Return the columns and CHECK constraints of the parents being added + to 'Inherited from' that the table does not have yet, as INHERIT + fails unless the table already has them. + """ + if not data.get('coll_inherits_added'): + return [], [] + + result = [] + for template in ['get_missing_inherited_columns.sql', + 'get_missing_inherited_checks.sql']: + sql = render_template( + "/".join([self.table_template_path, template]), + tid=tid, parents=data['coll_inherits_added'], + conn=self.conn) + status, res = self.conn.execute_dict(sql) + if not status: + raise ExecuteError(res) + result.append(res['rows']) + + return result + + def _check_for_column_delete(self, columns, data, column_sql, + still_inherited=None): # If column(s) is/are deleted if 'deleted' in columns: for c in columns['deleted']: c['schema'] = data['schema'] c['table'] = data['name'] - # Sql for drop column - if c.get('inheritedfrom', None) is None: + # Sql for drop column. NO INHERIT keeps the columns of a + # removed parent as local columns, so drop them as well, + # unless a remaining parent still provides them. + if c.get('inheritedfrom', None) is None or \ + (still_inherited is not None and + c['name'] not in still_inherited): column_sql += render_template("/".join( [self.column_template_path, self._DELETE_SQL]), data=c, conn=self.conn).strip('\n') + \ @@ -1640,9 +1686,14 @@ def get_sql(self, did, scid, tid, data, res, add_not_exists_clause=False, if self._unlogged_unsupported(old_data, self.manager.version): data.pop('relpersistence', None) + inherit_columns, inherit_checks = \ + self._get_inherit_prerequisites(tid, data) + sql = render_template( "/".join([self.table_template_path, self._UPDATE_SQL]), - o_data=old_data, data=data, conn=self.conn + o_data=old_data, data=data, conn=self.conn, + inherit_columns=inherit_columns, + inherit_checks=inherit_checks ) # Removes training new lines sql = sql.strip('\n') + self.double_newline @@ -1656,8 +1707,9 @@ def get_sql(self, did, scid, tid, data, res, add_not_exists_clause=False, column_sql = '\n' # If column(s) is/are deleted - column_sql = self._check_for_column_delete(columns, data, - column_sql) + column_sql = self._check_for_column_delete( + columns, data, column_sql, + self._get_still_inherited_columns(scid, data)) # If column(s) is/are changed column_sql = self._check_for_column_update(columns, data, @@ -1781,7 +1833,16 @@ def update(self, gid, sid, did, scid, tid, **kwargs): res['rows'][0]['is_sub_partitioned'] = is_partitioned icon = self.get_partition_icon_css_class(res['rows'][0]) else: - icon = self.get_icon_css_class(res['rows'][0]) + # res holds the properties from before the update, which + # may have added or removed parents, so use the table's + # current inheritance for its icon. + sql = render_template( + "/".join([self.table_template_path, self._NODES_SQL]), + scid=rest['rows'][0]['scid'], tid=tid) + status, node_res = self.conn.execute_2darray(sql) + if not status: + return internal_server_error(errormsg=node_res) + icon = self.get_icon_css_class(node_res['rows'][0]) other_node_info = {} if 'description' in data: From 681f2e55810e190b14a7ed008cdd595d684ae5a8 Mon Sep 17 00:00:00 2001 From: VIBVEL47 <161462689+VIBVEL47@users.noreply.github.com> Date: Thu, 22 Oct 2026 21:48:05 +0530 Subject: [PATCH 2/3] Handle generated columns, NOT NULL and inherited CHECK constraints when changing a table's parents. #10657 Adding a parent created its generated columns as ordinary columns and left a column the table already has nullable when the parent's is NOT NULL, and PostgreSQL rejects INHERIT in both cases. Add generated columns with their expression, after the columns it uses, and set NOT NULL on such existing columns before INHERIT. Removing a parent left its CHECK constraints on the table, as NO INHERIT keeps them as local constraints. Drop the ones that no remaining parent also has, as is done for the parent's columns. Co-Authored-By: Claude Opus 5.5 --- .../templates/tables/sql/15_plus/update.sql | 13 +++- ...ql => get_inherit_prerequisite_checks.sql} | 0 .../get_inherit_prerequisite_columns.sql | 35 +++++++++++ .../default/get_missing_inherited_columns.sql | 26 -------- .../sql/default/get_uninherit_checks.sql | 13 ++++ .../templates/tables/sql/default/update.sql | 13 +++- .../tests/test_table_inherits_columns.py | 62 +++++++++++++++---- .../servers/databases/schemas/tables/utils.py | 47 ++++++++------ 8 files changed, 151 insertions(+), 58 deletions(-) rename web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/{get_missing_inherited_checks.sql => get_inherit_prerequisite_checks.sql} (100%) create mode 100644 web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_inherit_prerequisite_columns.sql delete mode 100644 web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_missing_inherited_columns.sql create mode 100644 web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_uninherit_checks.sql diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/15_plus/update.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/15_plus/update.sql index 6a21f2291c6..17b2c002165 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/15_plus/update.sql +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/15_plus/update.sql @@ -39,8 +39,13 @@ ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} {% if data.coll_inherits_added|length > 0 %} {### A table must have the parent's columns and CHECK constraints before it can inherit from it ###} {% for col in inherit_columns %} +{% if col.is_missing %} ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} - ADD COLUMN {{conn|qtIdent(col.name)}} {{col.displaytypname}}{% if col.collspcname %} COLLATE {{col.collspcname}}{% endif %}{% if col.attnotnull %} NOT NULL{% endif %}; + ADD COLUMN {{conn|qtIdent(col.name)}} {{col.displaytypname}}{% if col.collspcname %} COLLATE {{col.collspcname}}{% endif %}{% if col.attnotnull %} NOT NULL{% endif %}{% if col.genexpr %} GENERATED ALWAYS AS ({{col.genexpr}}) STORED{% endif %}; +{% else %} +ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} + ALTER COLUMN {{conn|qtIdent(col.name)}} SET NOT NULL; +{% endif %} {% endfor %} {% for chk in inherit_checks %} @@ -59,6 +64,12 @@ ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} NO INHERIT {{val}}; +{% endfor %} +{### NO INHERIT keeps the removed parents' CHECK constraints on the table ###} +{% for chk in uninherit_checks %} +ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} + DROP CONSTRAINT IF EXISTS {{conn|qtIdent(chk.name)}}; + {% endfor %} {% endif %} {#####################################################} diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_missing_inherited_checks.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_inherit_prerequisite_checks.sql similarity index 100% rename from web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_missing_inherited_checks.sql rename to web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_inherit_prerequisite_checks.sql diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_inherit_prerequisite_columns.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_inherit_prerequisite_columns.sql new file mode 100644 index 00000000000..3d8c4c3de0d --- /dev/null +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_inherit_prerequisite_columns.sql @@ -0,0 +1,35 @@ +{### Columns of the given parents that the table must have before it can inherit from them: ###} +{### the ones it does not have yet, and the ones it has without the NOT NULL a parent requires ###} +SELECT name, displaytypname, collspcname, attnotnull, genexpr, is_missing FROM ( + SELECT DISTINCT ON (a.attname) a.attname AS name, + pg_catalog.format_type(a.atttypid, a.atttypmod) AS displaytypname, + CASE WHEN a.attcollation <> ty.typcollation THEN + pg_catalog.quote_ident(cn.nspname)||'.'||pg_catalog.quote_ident(co.collname) + END AS collspcname, + pg_catalog.bool_or(a.attnotnull) OVER (PARTITION BY a.attname) AS attnotnull, + CASE WHEN a.attgenerated = 's' THEN + pg_catalog.pg_get_expr(def.adbin, def.adrelid) + END AS genexpr, + ca.attname IS NULL AS is_missing, ca.attnotnull AS child_attnotnull, + p.ord, a.attnum + FROM + pg_catalog.unnest(ARRAY[{% for val in parents %}{{val|qtLiteral(conn)}}::regclass::oid{% if not loop.last %}, {% endif %}{% endfor %}]) + WITH ORDINALITY AS p(oid, ord) + JOIN + pg_catalog.pg_attribute a ON a.attrelid = p.oid AND a.attnum > 0 AND NOT a.attisdropped + JOIN + pg_catalog.pg_type ty ON ty.oid = a.atttypid + LEFT OUTER JOIN + pg_catalog.pg_collation co ON co.oid = a.attcollation + LEFT OUTER JOIN + pg_catalog.pg_namespace cn ON cn.oid = co.collnamespace + LEFT OUTER JOIN + pg_catalog.pg_attrdef def ON def.adrelid = a.attrelid AND def.adnum = a.attnum + LEFT OUTER JOIN + pg_catalog.pg_attribute ca ON ca.attrelid = {{tid}}::oid AND ca.attname = a.attname + AND ca.attnum > 0 AND NOT ca.attisdropped + ORDER BY a.attname, p.ord, a.attnum +) cols +WHERE is_missing OR (attnotnull AND NOT child_attnotnull) +{### Generated columns go last, as their expressions use the other columns ###} +ORDER BY genexpr IS NOT NULL, ord, attnum diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_missing_inherited_columns.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_missing_inherited_columns.sql deleted file mode 100644 index 58ca4c6cbba..00000000000 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_missing_inherited_columns.sql +++ /dev/null @@ -1,26 +0,0 @@ -{### Columns of the given parents that the table does not have yet ###} -SELECT name, displaytypname, collspcname, attnotnull FROM ( - SELECT DISTINCT ON (a.attname) a.attname AS name, - pg_catalog.format_type(a.atttypid, a.atttypmod) AS displaytypname, - CASE WHEN a.attcollation <> ty.typcollation THEN - pg_catalog.quote_ident(cn.nspname)||'.'||pg_catalog.quote_ident(co.collname) - END AS collspcname, - a.attnotnull, p.ord, a.attnum - FROM - pg_catalog.unnest(ARRAY[{% for val in parents %}{{val|qtLiteral(conn)}}::regclass::oid{% if not loop.last %}, {% endif %}{% endfor %}]) - WITH ORDINALITY AS p(oid, ord) - JOIN - pg_catalog.pg_attribute a ON a.attrelid = p.oid AND a.attnum > 0 AND NOT a.attisdropped - JOIN - pg_catalog.pg_type ty ON ty.oid = a.atttypid - LEFT OUTER JOIN - pg_catalog.pg_collation co ON co.oid = a.attcollation - LEFT OUTER JOIN - pg_catalog.pg_namespace cn ON cn.oid = co.collnamespace - WHERE NOT EXISTS ( - SELECT 1 FROM pg_catalog.pg_attribute ca - WHERE ca.attrelid = {{tid}}::oid AND ca.attname = a.attname - AND ca.attnum > 0 AND NOT ca.attisdropped) - ORDER BY a.attname, p.ord, a.attnum -) cols -ORDER BY ord, attnum diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_uninherit_checks.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_uninherit_checks.sql new file mode 100644 index 00000000000..9262bf26f5d --- /dev/null +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_uninherit_checks.sql @@ -0,0 +1,13 @@ +{### CHECK constraints the table inherits from the given removed parents and from none of the remaining ones, ###} +{### which NO INHERIT would leave on the table as local constraints ###} +{% macro PARENT_CHECK(tables) %} + SELECT 1 FROM pg_catalog.pg_constraint pc + WHERE pc.conrelid = ANY(ARRAY[{% for val in tables %}{{val|qtLiteral(conn)}}::regclass::oid{% if not loop.last %}, {% endif %}{% endfor %}]::pg_catalog.oid[]) + AND pc.contype = 'c' AND NOT pc.connoinherit AND pc.conname = c.conname +{%- endmacro %} +SELECT c.conname AS name +FROM pg_catalog.pg_constraint c +WHERE c.conrelid = {{tid}}::oid AND c.contype = 'c' AND c.coninhcount > 0 + AND EXISTS ({{ PARENT_CHECK(removed) }}) + AND NOT EXISTS ({{ PARENT_CHECK(remaining) }}) +ORDER BY c.conname diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/update.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/update.sql index 85f029e6e74..f117253f9c0 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/update.sql +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/update.sql @@ -31,8 +31,13 @@ ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} {% if data.coll_inherits_added|length > 0 %} {### A table must have the parent's columns and CHECK constraints before it can inherit from it ###} {% for col in inherit_columns %} +{% if col.is_missing %} ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} - ADD COLUMN {{conn|qtIdent(col.name)}} {{col.displaytypname}}{% if col.collspcname %} COLLATE {{col.collspcname}}{% endif %}{% if col.attnotnull %} NOT NULL{% endif %}; + ADD COLUMN {{conn|qtIdent(col.name)}} {{col.displaytypname}}{% if col.collspcname %} COLLATE {{col.collspcname}}{% endif %}{% if col.attnotnull %} NOT NULL{% endif %}{% if col.genexpr %} GENERATED ALWAYS AS ({{col.genexpr}}) STORED{% endif %}; +{% else %} +ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} + ALTER COLUMN {{conn|qtIdent(col.name)}} SET NOT NULL; +{% endif %} {% endfor %} {% for chk in inherit_checks %} @@ -51,6 +56,12 @@ ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} NO INHERIT {{val}}; +{% endfor %} +{### NO INHERIT keeps the removed parents' CHECK constraints on the table ###} +{% for chk in uninherit_checks %} +ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} + DROP CONSTRAINT IF EXISTS {{conn|qtIdent(chk.name)}}; + {% endfor %} {% endif %} {#####################################################} diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_table_inherits_columns.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_table_inherits_columns.py index 3ea3cbde81f..528f9e0241b 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_table_inherits_columns.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_table_inherits_columns.py @@ -10,11 +10,13 @@ """Changing 'Inherited from' on an existing table (#10657). Removing a parent removes its columns from the table dialog's Columns tab, -but NO INHERIT keeps them on the child as local columns, so saving must -drop them too, except a column that a remaining parent still provides. +but NO INHERIT keeps them, and the parent's CHECK constraints, on the child +as local ones, so saving must drop them too, except the ones that a +remaining parent still provides. -Adding a parent must first give the table the parent's columns and CHECK -constraints that it does not have yet, as INHERIT fails without them. +Adding a parent must first give the table the parent's columns (including +generated ones), NOT NULL markings and CHECK constraints that it does not +have yet, as INHERIT fails without them. Either way, the tree icon returned by the save must show the table's inheritance after the change, not before it. @@ -93,6 +95,12 @@ def _child_columns(self): "WHERE attrelid = {0}::oid AND attnum > 0 " "AND NOT attisdropped ORDER BY attname".format(self.table_id))] + def _child_checks(self): + return [row[0] for row in self._query( + "SELECT conname FROM pg_catalog.pg_constraint " + "WHERE conrelid = {0}::oid AND contype = 'c' " + "ORDER BY conname".format(self.table_id))] + def _qualified(self, name): return '{0}.{1}'.format(self.schema_name, name) @@ -127,16 +135,32 @@ class TableRemoveInheritsColumnsTestCase(InheritsColumnsTestMixin, 'DROP COLUMN IF EXISTS a_name;'], unexpected_sql=['own_col'], expected_columns=['own_col'], + expected_checks=[], expected_icon='icon-table')), ('A column still provided by another parent is kept', - dict(parents=['id integer, a_name text', - 'id integer, b_name text'], + dict(parents=['id integer, a_name text, ' + 'CONSTRAINT id_pos CHECK (id > 0), ' + 'CONSTRAINT id_set CHECK (id <> 0)', + 'id integer, b_name text, ' + 'CONSTRAINT id_set CHECK (id <> 0)'], child='own_col text', - expected_sql=['NO INHERIT', 'DROP COLUMN IF EXISTS a_name;'], + expected_sql=['NO INHERIT', 'DROP COLUMN IF EXISTS a_name;', + 'DROP CONSTRAINT IF EXISTS id_pos;'], unexpected_sql=['DROP COLUMN IF EXISTS id;', 'b_name', - 'own_col'], + 'own_col', 'id_set'], expected_columns=['id', 'b_name', 'own_col'], + expected_checks=['id_set'], expected_icon='icon-table-inherits')), + ('A CHECK constraint also defined on the table is dropped', + dict(parents=['id integer, CONSTRAINT id_pos CHECK (id > 0)'], + child='id integer, own_col text, ' + 'CONSTRAINT id_pos CHECK (id > 0)', + expected_sql=['NO INHERIT', 'DROP COLUMN IF EXISTS id;', + 'DROP CONSTRAINT IF EXISTS id_pos;'], + unexpected_sql=['own_col'], + expected_columns=['own_col'], + expected_checks=[], + expected_icon='icon-table')), ] def _inherits_clause(self): @@ -172,6 +196,7 @@ def runTest(self): node = self._put(data) self.assertEqual(self._child_columns(), sorted(self.expected_columns)) + self.assertEqual(self._child_checks(), self.expected_checks) # The tree icon follows the inheritance after the save. self.assertEqual(node['icon'], self.expected_icon) @@ -184,20 +209,28 @@ class TableAddInheritsColumnsTestCase(InheritsColumnsTestMixin, scenarios = [ ('Missing columns and CHECK constraints are added before INHERIT', dict(parents=['id varchar(10) NOT NULL, code text COLLATE "C", ' + 'g integer GENERATED ALWAYS AS (n * 2) STORED, ' + 'n integer, ' 'CONSTRAINT id_not_empty CHECK (id <> \'\')'], child='cid bigint', expected_sql=['ADD COLUMN id character varying(10) NOT NULL;', 'ADD COLUMN code text COLLATE pg_catalog."C";', + 'ADD COLUMN n integer;', + 'ADD COLUMN g integer ' + 'GENERATED ALWAYS AS ((n * 2)) STORED;', "ADD CONSTRAINT id_not_empty " "CHECK (id::text <> ''::text);"], unexpected_sql=['cid'], - expected_columns=['cid', 'code', 'id'])), + expected_columns=['cid', 'code', 'g', 'id', 'n'], + expected_checks=['id_not_empty'])), ('Columns the table already has are not added again', - dict(parents=['id bigint, name text'], + dict(parents=['id bigint NOT NULL, name text'], child='id bigint, cid bigint', - expected_sql=['ADD COLUMN name text;'], + expected_sql=['ALTER COLUMN id SET NOT NULL;', + 'ADD COLUMN name text;'], unexpected_sql=['ADD COLUMN id', 'cid', 'ADD CONSTRAINT'], - expected_columns=['cid', 'id', 'name'])), + expected_columns=['cid', 'id', 'name'], + expected_checks=[])), ] def runTest(self): @@ -228,10 +261,15 @@ def runTest(self): self.assertLess(sql.index(text), sql.index('INHERIT ' + parent)) for text in self.unexpected_sql: self.assertNotIn(text, sql) + # A generated column comes after the columns its expression uses. + if 'ADD COLUMN g ' in sql: + self.assertLess(sql.index('ADD COLUMN n '), + sql.index('ADD COLUMN g ')) node = self._put(data) self.assertEqual(self._child_columns(), sorted(self.expected_columns)) + self.assertEqual(self._child_checks(), self.expected_checks) self.assertEqual(node['icon'], 'icon-table-inherits') self.assertEqual(self._query( "SELECT inhparent FROM pg_catalog.pg_inherits " diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py index ffe297404d2..93feecade2c 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py @@ -1304,28 +1304,43 @@ def _get_still_inherited_columns(self, scid, data): return {c['name'] for c in other_columns} - def _get_inherit_prerequisites(self, tid, data): + def _get_inherit_changes(self, tid, data): """ - Return the columns and CHECK constraints of the parents being added - to 'Inherited from' that the table does not have yet, as INHERIT - fails unless the table already has them. + Return what the table needs alongside INHERIT and NO INHERIT for the + parents added to and removed from 'Inherited from': + - inherit_columns, inherit_checks: the columns (or their NOT NULL) + and CHECK constraints of the added parents that the table does + not have yet, as INHERIT fails unless it has them. + - uninherit_checks: the CHECK constraints the table only has from + the removed parents, which NO INHERIT keeps on it. """ - if not data.get('coll_inherits_added'): - return [], [] + changes = {'inherit_columns': [], 'inherit_checks': [], + 'uninherit_checks': []} - result = [] - for template in ['get_missing_inherited_columns.sql', - 'get_missing_inherited_checks.sql']: + def _fetch(template, **kwargs): sql = render_template( "/".join([self.table_template_path, template]), - tid=tid, parents=data['coll_inherits_added'], - conn=self.conn) + tid=tid, conn=self.conn, **kwargs) status, res = self.conn.execute_dict(sql) if not status: raise ExecuteError(res) - result.append(res['rows']) + return res['rows'] - return result + if data.get('coll_inherits_added'): + changes['inherit_columns'] = _fetch( + 'get_inherit_prerequisite_columns.sql', + parents=data['coll_inherits_added']) + changes['inherit_checks'] = _fetch( + 'get_inherit_prerequisite_checks.sql', + parents=data['coll_inherits_added']) + + if data.get('coll_inherits_removed'): + changes['uninherit_checks'] = _fetch( + 'get_uninherit_checks.sql', + removed=data['coll_inherits_removed'], + remaining=data.get('coll_inherits', [])) + + return changes def _check_for_column_delete(self, columns, data, column_sql, still_inherited=None): @@ -1686,14 +1701,10 @@ def get_sql(self, did, scid, tid, data, res, add_not_exists_clause=False, if self._unlogged_unsupported(old_data, self.manager.version): data.pop('relpersistence', None) - inherit_columns, inherit_checks = \ - self._get_inherit_prerequisites(tid, data) - sql = render_template( "/".join([self.table_template_path, self._UPDATE_SQL]), o_data=old_data, data=data, conn=self.conn, - inherit_columns=inherit_columns, - inherit_checks=inherit_checks + **self._get_inherit_changes(tid, data) ) # Removes training new lines sql = sql.strip('\n') + self.double_newline From e49b82f1ea8df8ea6c3ef229c9d5b2ee5570e9fe Mon Sep 17 00:00:00 2001 From: VIBVEL47 <161462689+VIBVEL47@users.noreply.github.com> Date: Fri, 23 Oct 2026 11:34:32 +0530 Subject: [PATCH 3/3] Copy parent defaults and drop leftover NOT NULL when changing a table's parents. #10657 Adding a parent with a NOT NULL column to a table that already has rows failed, as the added column had no value for them. Add the parent's columns with their defaults, as CREATE TABLE ... INHERITS does, so the existing rows are filled in. Removing a parent left NOT NULL on a column that a remaining parent still provides but allows NULLs in, as NO INHERIT keeps it. Drop it, except on primary key and replica identity columns. Co-Authored-By: Claude Opus 5.5 --- .../templates/tables/sql/15_plus/update.sql | 9 +++- .../get_inherit_prerequisite_columns.sql | 5 ++- .../sql/default/get_uninherit_not_nulls.sql | 19 +++++++++ .../templates/tables/sql/default/update.sql | 9 +++- .../tests/test_table_inherits_columns.py | 42 ++++++++++++++++--- .../servers/databases/schemas/tables/utils.py | 17 ++++---- 6 files changed, 84 insertions(+), 17 deletions(-) create mode 100644 web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_uninherit_not_nulls.sql diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/15_plus/update.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/15_plus/update.sql index 17b2c002165..eedc15b9766 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/15_plus/update.sql +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/15_plus/update.sql @@ -41,7 +41,7 @@ ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} {% for col in inherit_columns %} {% if col.is_missing %} ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} - ADD COLUMN {{conn|qtIdent(col.name)}} {{col.displaytypname}}{% if col.collspcname %} COLLATE {{col.collspcname}}{% endif %}{% if col.attnotnull %} NOT NULL{% endif %}{% if col.genexpr %} GENERATED ALWAYS AS ({{col.genexpr}}) STORED{% endif %}; + ADD COLUMN {{conn|qtIdent(col.name)}} {{col.displaytypname}}{% if col.collspcname %} COLLATE {{col.collspcname}}{% endif %}{% if col.attnotnull %} NOT NULL{% endif %}{% if col.defval %} DEFAULT {{col.defval}}{% endif %}{% if col.genexpr %} GENERATED ALWAYS AS ({{col.genexpr}}) STORED{% endif %}; {% else %} ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} ALTER COLUMN {{conn|qtIdent(col.name)}} SET NOT NULL; @@ -65,11 +65,16 @@ ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} NO INHERIT {{val}}; {% endfor %} -{### NO INHERIT keeps the removed parents' CHECK constraints on the table ###} +{### NO INHERIT keeps the removed parents' CHECK constraints and NOT NULL on the table ###} {% for chk in uninherit_checks %} ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} DROP CONSTRAINT IF EXISTS {{conn|qtIdent(chk.name)}}; +{% endfor %} +{% for col in uninherit_not_nulls %} +ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} + ALTER COLUMN {{conn|qtIdent(col.name)}} DROP NOT NULL; + {% endfor %} {% endif %} {#####################################################} diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_inherit_prerequisite_columns.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_inherit_prerequisite_columns.sql index 3d8c4c3de0d..3750e8ee24e 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_inherit_prerequisite_columns.sql +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_inherit_prerequisite_columns.sql @@ -1,12 +1,15 @@ {### Columns of the given parents that the table must have before it can inherit from them: ###} {### the ones it does not have yet, and the ones it has without the NOT NULL a parent requires ###} -SELECT name, displaytypname, collspcname, attnotnull, genexpr, is_missing FROM ( +SELECT name, displaytypname, collspcname, attnotnull, defval, genexpr, is_missing FROM ( SELECT DISTINCT ON (a.attname) a.attname AS name, pg_catalog.format_type(a.atttypid, a.atttypmod) AS displaytypname, CASE WHEN a.attcollation <> ty.typcollation THEN pg_catalog.quote_ident(cn.nspname)||'.'||pg_catalog.quote_ident(co.collname) END AS collspcname, pg_catalog.bool_or(a.attnotnull) OVER (PARTITION BY a.attname) AS attnotnull, + CASE WHEN a.attgenerated = '' THEN + pg_catalog.pg_get_expr(def.adbin, def.adrelid) + END AS defval, CASE WHEN a.attgenerated = 's' THEN pg_catalog.pg_get_expr(def.adbin, def.adrelid) END AS genexpr, diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_uninherit_not_nulls.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_uninherit_not_nulls.sql new file mode 100644 index 00000000000..4b38437b778 --- /dev/null +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/get_uninherit_not_nulls.sql @@ -0,0 +1,19 @@ +{### Columns kept through a remaining parent that are NOT NULL only because of the given removed parents, ###} +{### which NO INHERIT would leave NOT NULL. A primary key or replica identity column must stay NOT NULL. ###} +{% macro PARENT_COLUMN(tables, not_null) %} + SELECT 1 FROM pg_catalog.pg_attribute pa + WHERE pa.attrelid = ANY(ARRAY[{% for val in tables %}{{val|qtLiteral(conn)}}::regclass::oid{% if not loop.last %}, {% endif %}{% endfor %}]::pg_catalog.oid[]) + AND pa.attname = a.attname AND pa.attnum > 0 AND NOT pa.attisdropped + {% if not_null %}AND pa.attnotnull{% endif %} +{%- endmacro %} +SELECT a.attname AS name +FROM pg_catalog.pg_attribute a +WHERE a.attrelid = {{tid}}::oid AND a.attnum > 0 AND NOT a.attisdropped AND a.attnotnull + AND EXISTS ({{ PARENT_COLUMN(removed, true) }}) + AND EXISTS ({{ PARENT_COLUMN(remaining, false) }}) + AND NOT EXISTS ({{ PARENT_COLUMN(remaining, true) }}) + AND NOT EXISTS ( + SELECT 1 FROM pg_catalog.pg_index i + WHERE i.indrelid = a.attrelid AND (i.indisprimary OR i.indisreplident) + AND a.attnum = ANY(i.indkey)) +ORDER BY a.attnum diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/update.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/update.sql index f117253f9c0..b0164dca05e 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/update.sql +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/update.sql @@ -33,7 +33,7 @@ ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} {% for col in inherit_columns %} {% if col.is_missing %} ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} - ADD COLUMN {{conn|qtIdent(col.name)}} {{col.displaytypname}}{% if col.collspcname %} COLLATE {{col.collspcname}}{% endif %}{% if col.attnotnull %} NOT NULL{% endif %}{% if col.genexpr %} GENERATED ALWAYS AS ({{col.genexpr}}) STORED{% endif %}; + ADD COLUMN {{conn|qtIdent(col.name)}} {{col.displaytypname}}{% if col.collspcname %} COLLATE {{col.collspcname}}{% endif %}{% if col.attnotnull %} NOT NULL{% endif %}{% if col.defval %} DEFAULT {{col.defval}}{% endif %}{% if col.genexpr %} GENERATED ALWAYS AS ({{col.genexpr}}) STORED{% endif %}; {% else %} ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} ALTER COLUMN {{conn|qtIdent(col.name)}} SET NOT NULL; @@ -57,11 +57,16 @@ ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} NO INHERIT {{val}}; {% endfor %} -{### NO INHERIT keeps the removed parents' CHECK constraints on the table ###} +{### NO INHERIT keeps the removed parents' CHECK constraints and NOT NULL on the table ###} {% for chk in uninherit_checks %} ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} DROP CONSTRAINT IF EXISTS {{conn|qtIdent(chk.name)}}; +{% endfor %} +{% for col in uninherit_not_nulls %} +ALTER TABLE IF EXISTS {{conn|qtIdent(data.schema, data.name)}} + ALTER COLUMN {{conn|qtIdent(col.name)}} DROP NOT NULL; + {% endfor %} {% endif %} {#####################################################} diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_table_inherits_columns.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_table_inherits_columns.py index 528f9e0241b..aacc45632bc 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_table_inherits_columns.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_table_inherits_columns.py @@ -10,13 +10,14 @@ """Changing 'Inherited from' on an existing table (#10657). Removing a parent removes its columns from the table dialog's Columns tab, -but NO INHERIT keeps them, and the parent's CHECK constraints, on the child -as local ones, so saving must drop them too, except the ones that a -remaining parent still provides. +but NO INHERIT keeps them, and the parent's CHECK constraints and NOT NULL +markings, on the child as local ones, so saving must drop them too, except +the ones that a remaining parent still provides. Adding a parent must first give the table the parent's columns (including -generated ones), NOT NULL markings and CHECK constraints that it does not -have yet, as INHERIT fails without them. +generated ones, and with their defaults so existing rows get a value), +NOT NULL markings and CHECK constraints that it does not have yet, as +INHERIT fails without them. Either way, the tree icon returned by the save must show the table's inheritance after the change, not before it. @@ -69,6 +70,9 @@ def setUp(self): self.table_id = self._query( "SELECT '{0}.{1}'::regclass::oid".format( self.schema_name, self.table_name))[0][0] + if getattr(self, 'child_rows', None): + self._query("INSERT INTO {0}.{1} VALUES {2}".format( + self.schema_name, self.table_name, self.child_rows)) def _inherits_clause(self): return '' @@ -101,6 +105,12 @@ def _child_checks(self): "WHERE conrelid = {0}::oid AND contype = 'c' " "ORDER BY conname".format(self.table_id))] + def _child_not_nulls(self): + return [row[0] for row in self._query( + "SELECT attname FROM pg_catalog.pg_attribute " + "WHERE attrelid = {0}::oid AND attnum > 0 AND attnotnull " + "AND NOT attisdropped ORDER BY attname".format(self.table_id))] + def _qualified(self, name): return '{0}.{1}'.format(self.schema_name, name) @@ -161,6 +171,16 @@ class TableRemoveInheritsColumnsTestCase(InheritsColumnsTestMixin, expected_columns=['own_col'], expected_checks=[], expected_icon='icon-table')), + ('NOT NULL only required by the removed parent is dropped', + dict(parents=['id integer NOT NULL, code text NOT NULL', + 'id integer, code text NOT NULL'], + child='own_col text', + expected_sql=['NO INHERIT', 'ALTER COLUMN id DROP NOT NULL;'], + unexpected_sql=['DROP COLUMN', 'ALTER COLUMN code'], + expected_columns=['code', 'id', 'own_col'], + expected_not_nulls=['code'], + expected_checks=[], + expected_icon='icon-table-inherits')), ] def _inherits_clause(self): @@ -197,6 +217,9 @@ def runTest(self): self.assertEqual(self._child_columns(), sorted(self.expected_columns)) self.assertEqual(self._child_checks(), self.expected_checks) + if hasattr(self, 'expected_not_nulls'): + self.assertEqual(self._child_not_nulls(), + self.expected_not_nulls) # The tree icon follows the inheritance after the save. self.assertEqual(node['icon'], self.expected_icon) @@ -231,6 +254,15 @@ class TableAddInheritsColumnsTestCase(InheritsColumnsTestMixin, unexpected_sql=['ADD COLUMN id', 'cid', 'ADD CONSTRAINT'], expected_columns=['cid', 'id', 'name'], expected_checks=[])), + ('A NOT NULL column gets its default for the existing rows', + dict(parents=['id integer NOT NULL DEFAULT 5, name text'], + child='cid bigint', + child_rows='(1), (2)', + expected_sql=['ADD COLUMN id integer NOT NULL DEFAULT 5;', + 'ADD COLUMN name text;'], + unexpected_sql=['cid', 'ADD CONSTRAINT'], + expected_columns=['cid', 'id', 'name'], + expected_checks=[])), ] def runTest(self): diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py index 93feecade2c..6caba3d65b4 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py @@ -1311,11 +1311,12 @@ def _get_inherit_changes(self, tid, data): - inherit_columns, inherit_checks: the columns (or their NOT NULL) and CHECK constraints of the added parents that the table does not have yet, as INHERIT fails unless it has them. - - uninherit_checks: the CHECK constraints the table only has from - the removed parents, which NO INHERIT keeps on it. + - uninherit_checks, uninherit_not_nulls: the CHECK constraints and + NOT NULL markings the table only has from the removed parents, + which NO INHERIT keeps on it. """ changes = {'inherit_columns': [], 'inherit_checks': [], - 'uninherit_checks': []} + 'uninherit_checks': [], 'uninherit_not_nulls': []} def _fetch(template, **kwargs): sql = render_template( @@ -1335,10 +1336,12 @@ def _fetch(template, **kwargs): parents=data['coll_inherits_added']) if data.get('coll_inherits_removed'): - changes['uninherit_checks'] = _fetch( - 'get_uninherit_checks.sql', - removed=data['coll_inherits_removed'], - remaining=data.get('coll_inherits', [])) + for key, template in [ + ('uninherit_checks', 'get_uninherit_checks.sql'), + ('uninherit_not_nulls', 'get_uninherit_not_nulls.sql')]: + changes[key] = _fetch( + template, removed=data['coll_inherits_removed'], + remaining=data.get('coll_inherits', [])) return changes