Schema Diff: make the regression test assert its generated script, and fix what that found

web/pgadmin/browser/server_groups/servers/databases/casts/templates/casts/sql/default/nodes.sql

@@ -24,6 +24,6 @@
     {% endif %}
     {% if schema_diff %}
         AND CASE WHEN (SELECT COUNT(*) FROM pg_catalog.pg_depend
-            WHERE objid = ca.oid AND deptype = 'e') > 0 THEN FALSE ELSE TRUE END
+            WHERE objid = ca.oid AND deptype IN ('e', 'i')) > 0 THEN FALSE ELSE TRUE END
     {% endif %}
     ORDER BY st.typname, tt.typname

web/pgadmin/browser/server_groups/servers/databases/schemas/foreign_tables/__init__.py

@@ -1194,6 +1194,8 @@ def get_sql(self, **kwargs):
                 # Parse the data coming from client
                 data = column_utils.parse_format_columns(data, mode='edit')
 
+                ForeignTableView._normalise_column_collation(data['columns'])
+
                 columns = data['columns']
                 column_sql = '\n'
 
@@ -1229,6 +1231,31 @@ def get_sql(self, **kwargs):
                                   conn=self.conn)
             return sql, data['name']
 
+    @staticmethod
+    def _normalise_column_collation(columns):
+        """
+        Give a column's collation the name the column templates expect.
+
+        The dialog calls a column's collation ``collspcname``, and that is
+        what foreign_table_columns' create and update templates render,
+        whilst get_columns.sql calls it ``collname``, which is what Schema
+        Diff hands us. Without reconciling the two, a column added or
+        retyped by Schema Diff silently loses its collation and the two
+        databases stay different however many times the script is applied
+        (#10300).
+
+        :param columns: The column difference, modified in place
+        """
+        # A create carries a plain list of columns; only an update carries
+        # the added/changed/deleted difference this applies to.
+        if not isinstance(columns, dict):
+            return
+
+        for action in ('added', 'changed'):
+            for column in columns.get(action) or []:
+                if not column.get('collspcname') and column.get('collname'):
+                    column['collspcname'] = column['collname']
+
     def _check_for_column_delete(self, columns, data, column_sql):
         # If column(s) is/are deleted
         if 'deleted' in columns:
@@ -1826,15 +1853,31 @@ def _modify_column_data(data, tmp_columns):
         :param data: Data for columns.
         :param tmp_columns: tmp_columns list.
         """
+        def index_of(name):
+            for index, column in enumerate(tmp_columns):
+                if column.get('name') == name:
+                    return index
+            return None
+
         if 'added' in data['columns']:
             for item in data['columns']['added']:
                 tmp_columns.append(item)
         if 'changed' in data['columns']:
+            # tmp_columns holds the table as it stands, so a changed column
+            # is already in it in its old form and has to be replaced;
+            # appending it would declare the column twice and PostgreSQL
+            # would reject the recreated table outright (#10297).
             for item in data['columns']['changed']:
-                tmp_columns.append(item)
+                index = index_of(item.get('name'))
+                if index is None:
+                    tmp_columns.append(item)
+                else:
+                    tmp_columns[index] = item
         if 'deleted' in data['columns']:
             for item in data['columns']['deleted']:
-                tmp_columns.remove(item)
+                index = index_of(item.get('name'))
+                if index is not None:
+                    tmp_columns.pop(index)
 
     @staticmethod
     def _modify_constraints_data(data):

web/pgadmin/browser/server_groups/servers/databases/schemas/functions/templates/functions/pg/sql/default/node.sql

@@ -19,7 +19,7 @@ WHERE
 {% endif %}
 {% if schema_diff %}
     AND CASE WHEN (SELECT COUNT(*) FROM pg_catalog.pg_depend
-        WHERE objid = pr.oid AND deptype = 'e') > 0 THEN FALSE ELSE TRUE END
+        WHERE objid = pr.oid AND deptype IN ('e', 'i')) > 0 THEN FALSE ELSE TRUE END
 {% endif %}
     AND typname NOT IN ('trigger', 'event_trigger')
 ORDER BY

web/pgadmin/browser/server_groups/servers/databases/schemas/functions/templates/functions/ppas/sql/default/node.sql

@@ -20,7 +20,7 @@ WHERE
 {% endif %}
 {% if schema_diff %}
     AND CASE WHEN (SELECT COUNT(*) FROM pg_catalog.pg_depend
-        WHERE objid = pr.oid AND deptype = 'e') > 0 THEN FALSE ELSE TRUE END
+        WHERE objid = pr.oid AND deptype IN ('e', 'i')) > 0 THEN FALSE ELSE TRUE END
 {% endif %}
     AND typname NOT IN ('trigger', 'event_trigger')
 ORDER BY

web/pgadmin/browser/server_groups/servers/databases/schemas/sequences/__init__.py

@@ -628,6 +628,50 @@ def msql(self, gid, sid, did, scid, seid=None):
             status=200
         )
 
+    def _add_restart_for_new_bounds(self, data, old_data):
+        """
+        Ask for the sequence to be repositioned when the bounds being set
+        would leave it outside them.
+
+        PostgreSQL will not raise a sequence's MINVALUE above, or lower its
+        MAXVALUE below, the value the sequence currently sits at: it
+        rejects the whole statement with "RESTART value (n) cannot be less
+        than MINVALUE (m)", so every other change in it is lost too.
+        Repositioning onto the nearest value the new bounds allow is the
+        only way such a change can be applied, so ask for it rather than
+        generating a statement that cannot run (#10298). A sequence already
+        within its new bounds is left where it is, because handing out
+        values that have been used already would be worse than either.
+
+        :param data: The change being applied, modified in place
+        :param old_data: The sequence as it stands
+        """
+        minimum = data.get('minimum')
+        maximum = data.get('maximum')
+
+        if minimum is None and maximum is None:
+            return
+
+        current = data.get('current_value')
+        if current is None:
+            sql = render_template(
+                "/".join([self.template_path, 'get_def.sql']),
+                data=old_data, conn=self.conn
+            )
+            status, res = self.conn.execute_dict(sql)
+            if not status or not res['rows']:
+                return
+
+            current = res['rows'][0]['last_value']
+
+        if current is None:
+            return
+
+        if minimum is not None and int(minimum) > int(current):
+            data['restart'] = int(minimum)
+        elif maximum is not None and int(maximum) < int(current):
+            data['restart'] = int(maximum)
+
     def get_SQL(self, gid, sid, did, data, scid, seid=None,
                 add_not_exists_clause=False):
         """
@@ -667,6 +711,9 @@ def get_SQL(self, gid, sid, did, data, scid, seid=None,
             for arg in required_args:
                 if arg not in data:
                     data[arg] = old_data[arg]
+
+            self._add_restart_for_new_bounds(data, old_data)
+
             sql = render_template(
                 "/".join([self.template_path, self._UPDATE_SQL]),
                 data=data, o_data=old_data, conn=self.conn

web/pgadmin/browser/server_groups/servers/databases/schemas/sequences/templates/sequences/sql/15_plus/update.sql

@@ -41,6 +41,9 @@ ALTER SEQUENCE IF EXISTS {{ conn|qtIdent(o_data.schema, data.name) }}
 {% if data.maximum is defined %}
 {% set defquery = defquery+'\n    MAXVALUE '+data.maximum|string %}
 {% endif %}
+{% if data.restart is defined %}
+{% set defquery = defquery+'\n    RESTART '+data.restart|string %}
+{% endif %}
 {% if data.cache is defined %}
 {% set defquery = defquery+'\n    CACHE '+data.cache|string %}
 {% endif %}

web/pgadmin/browser/server_groups/servers/databases/schemas/sequences/templates/sequences/sql/default/update.sql

@@ -36,6 +36,9 @@ SELECT setval({{ seqname|qtLiteral(conn) }}, {{ data.current_value }}, false);
 {% if data.maximum is defined %}
 {% set defquery = defquery+'\n    MAXVALUE '+data.maximum|string %}
 {% endif %}
+{% if data.restart is defined %}
+{% set defquery = defquery+'\n    RESTART '+data.restart|string %}
+{% endif %}
 {% if data.cache is defined %}
 {% set defquery = defquery+'\n    CACHE '+data.cache|string %}
 {% endif %}

web/pgadmin/browser/server_groups/servers/databases/schemas/types/__init__.py

@@ -224,6 +224,18 @@ class TypeView(PGChildNodeView, DataTypeReader, SchemaDiffObjectCompare):
                       'schema', 'oid-2', 'type_acl', 'rngcollation', 'attnum',
                       'typowner']
 
+    # A range type carries its subtype, collation and support functions as
+    # plain values, which types of every other kind have no equivalent of at
+    # all. directory_diff() silently drops a value that only one side has,
+    # so comparing a range against a type of another kind would lose the
+    # subtype and leave nothing to render but `CREATE TYPE ... AS RANGE ()`
+    # when the type has to be dropped and recreated. Defining the keys on
+    # both sides keeps them in the difference (#10304).
+    range_keys_to_normalise = ['rngsubtype', 'typname', 'rngmultirangetype',
+                               'collname', 'rngsubopc', 'opcname',
+                               'rngcanonical', 'rngsubdiff_proc',
+                               'rngsubdiff']
+
     def check_precondition(f):
         """
         This function will behave as a decorator which will checks
@@ -1587,6 +1599,17 @@ def fetch_objects_to_compare(self, sid, did, scid):
         for row in rset['rows']:
             status, data = self._fetch_properties(scid, row['oid'])
             if status:
+                # The catalogue writes '-' where a type has no support
+                # function, which is not something that can be handed back
+                # to CREATE TYPE; the reverse-engineered SQL path drops it
+                # the same way before rendering (#10304).
+                for key, value in data.items():
+                    if value == '-':
+                        data[key] = None
+
+                for key in self.range_keys_to_normalise:
+                    data.setdefault(key, None)
+
                 res[row['name']] = data
 
         return res

web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/types/pg/sql/default/nodes.sql

@@ -14,6 +14,6 @@ WHERE t.typtype != 'd' AND t.typname NOT LIKE E'\\_%' AND t.typnamespace = {{sci
 {% endif %}
 {% if schema_diff %}
     AND CASE WHEN (SELECT COUNT(*) FROM pg_catalog.pg_depend
-        WHERE objid = t.oid AND deptype = 'e') > 0 THEN FALSE ELSE TRUE END
+        WHERE objid = t.oid AND deptype IN ('e', 'i')) > 0 THEN FALSE ELSE TRUE END
 {% endif %}
 ORDER BY t.typname;

web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/types/ppas/sql/default/nodes.sql

@@ -14,6 +14,6 @@ WHERE t.typtype != 'd' AND t.typname NOT LIKE E'\\_%' AND t.typnamespace = {{sci
 {% endif %}
 {% if schema_diff %}
     AND CASE WHEN (SELECT COUNT(*) FROM pg_catalog.pg_depend
-        WHERE objid = t.oid AND deptype = 'e') > 0 THEN FALSE ELSE TRUE END
+        WHERE objid = t.oid AND deptype IN ('e', 'i')) > 0 THEN FALSE ELSE TRUE END
 {% endif %}
 ORDER BY t.typname;

web/pgadmin/tools/schema_diff/__init__.py

@@ -637,8 +637,12 @@ def compare_database(params):
 
     except Exception as e:
         app.logger.exception(e)
+        # Reporting success as well would hand the client a comparison
+        # that stopped part way through as though it were complete
+        # (#10303).
         socketio.emit('compare_database_failed', str(e),
                       namespace=SOCKETIO_NAMESPACE, to=request.sid)
+        return
 
     socketio.emit('compare_database_success', comparison_result,
                   namespace=SOCKETIO_NAMESPACE, to=request.sid)
@@ -702,8 +706,12 @@ def compare_schema(params):
 
     except Exception as e:
         app.logger.exception(e)
+        # As above: a partial comparison must not be reported as a
+        # successful one (#10303).
         socketio.emit('compare_schema_failed', str(e),
                       namespace=SOCKETIO_NAMESPACE, to=request.sid)
+        return
+
     socketio.emit('compare_schema_success', comparison_result,
                   namespace=SOCKETIO_NAMESPACE, to=request.sid)