Fixed an issue where schema diff marks an identical table as different. Fixes #5350

Fixed an issue where schema diff showing changes in the identical domain constraints. Fixes #5315 Ignore the keys from the source and target list and also sort both the lists.
2025-02-25 18:55:31 -06:00 · 2020-04-20 18:04:16 +05:30 · 2020-04-20 18:04:16 +05:30 · 4692b21bcf
commit 4692b21bcf
parent 36a3d4e53b
19 changed files with 290 additions and 28 deletions
--- a/docs/en_US/release_notes_4_21.rst
+++ b/docs/en_US/release_notes_4_21.rst
@ -56,6 +56,8 @@ Bug fixes
 | `Issue #5275 <https://redmine.postgresql.org/issues/5275>`_ -  Fixed tab key navigation issue for parameters in table dialog.
 | `Issue #5302 <https://redmine.postgresql.org/issues/5302>`_ -  Fixed an issue where difference SQL is not seen in the schema diff tool for Types.
 | `Issue #5314 <https://redmine.postgresql.org/issues/5314>`_ -  Ensure that switch cell is in sync with switch control for accessibility.
+| `Issue #5315 <https://redmine.postgresql.org/issues/5315>`_ -  Fixed an issue where schema diff showing changes in the identical domain constraints.
+| `Issue #5350 <https://redmine.postgresql.org/issues/5350>`_ -  Fixed an issue where schema diff marks an identical table as different.
 | `Issue #5351 <https://redmine.postgresql.org/issues/5351>`_ -  Fixed compilation warnings while building pgAdmin.
 | `Issue #5361 <https://redmine.postgresql.org/issues/5361>`_ -  Fixes an issue where pgAdmin4 GUI does not display properly in IE 11.
 | `Issue #5362 <https://redmine.postgresql.org/issues/5362>`_ -  Fixed an issue where the identical packages and sequences visible as different in the schema diff tool.
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/foreign_tables/init.py
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/foreign_tables/init.py
@ -1477,7 +1477,7 @@ class ForeignTableView(PGChildNodeView, DataTypeReader,
            if 'deleted' in data['columns']:
                for item in data['columns']['deleted']:
                    tmp_columns.remove(item)
-            data['columns'] = tmp_columns
+        data['columns'] = tmp_columns

        tmp_constraints = []
        if 'constraints' in data:
@ -1502,7 +1502,7 @@ class ForeignTableView(PGChildNodeView, DataTypeReader,
            if 'deleted' in data['ftoptions']:
                for item in data['ftoptions']['deleted']:
                    tmp_ftoptions.remove(item)
-            data['ftoptions'] = tmp_ftoptions
+        data['ftoptions'] = tmp_ftoptions


 SchemaDiffRegistry(blueprint.node_type, ForeignTableView)
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/templates/mviews/pg/9.3_plus/sql/delete.sql
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/templates/mviews/pg/9.3_plus/sql/delete.sql
@ -9,5 +9,5 @@ LEFT JOIN pg_namespace nsp ON c.relnamespace = nsp.oid
 WHERE
    c.relfilenode = {{ vid }};
 {% elif (name and nspname) %}
-DROP MATERIALIZED VIEW {{ conn|qtIdent(nspname, name) }} {% if cascade %} CASCADE {% endif %}
+DROP MATERIALIZED VIEW {{ conn|qtIdent(nspname, name) }} {% if cascade %} CASCADE {% endif %};
 {% endif %}
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/templates/mviews/pg/9.4_plus/sql/delete.sql
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/templates/mviews/pg/9.4_plus/sql/delete.sql
@ -9,5 +9,5 @@ LEFT JOIN pg_namespace nsp ON c.relnamespace = nsp.oid
 WHERE
    c.relfilenode = {{ vid }};
 {% elif (name and nspname) %}
-DROP MATERIALIZED VIEW {{ conn|qtIdent(nspname, name) }} {% if cascade %} CASCADE {% endif %}
+DROP MATERIALIZED VIEW {{ conn|qtIdent(nspname, name) }} {% if cascade %} CASCADE {% endif %};
 {% endif %}
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/templates/mviews/ppas/9.3_plus/sql/delete.sql
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/templates/mviews/ppas/9.3_plus/sql/delete.sql
@ -9,5 +9,5 @@ LEFT JOIN pg_namespace nsp ON c.relnamespace = nsp.oid
 WHERE
    c.relfilenode = {{ vid }};
 {% elif (name and nspname) %}
-DROP MATERIALIZED VIEW {{ conn|qtIdent(nspname, name) }} {% if cascade %} CASCADE {% endif %}
+DROP MATERIALIZED VIEW {{ conn|qtIdent(nspname, name) }} {% if cascade %} CASCADE {% endif %};
 {% endif %}
--- a/web/pgadmin/static/vendor/backgrid/backgrid-select-all.js
+++ b/web/pgadmin/static/vendor/backgrid/backgrid-select-all.js
@ -8,7 +8,7 @@
 (function (root, factory) {
  if (typeof define === 'function' && define.amd) {
    // AMD. Register as an anonymous module.
-    define(["backbone", "backgrid", "underscore"], factory);
+    define(["backbone", "backgrid", "underscore", "sources/gettext"], factory);
  } else if (typeof exports == "object") {
    // CommonJS
    module.exports = factory(require("backbone"), require("backgrid"), require("underscore"));
@ -16,7 +16,7 @@
  // Browser
  else factory(root.Backbone, root.Backgrid, root._);

-}(this, function (Backbone, Backgrid, _) {
+}(this, function (Backbone, Backgrid, _, gettext) {

  "use strict";

--- a/web/pgadmin/tools/schema_diff/directory_compare.py
+++ b/web/pgadmin/tools/schema_diff/directory_compare.py
@ -14,6 +14,9 @@ from pgadmin.tools.schema_diff.model import SchemaDiffModel

 count = 1

+list_keys_array = ['name', 'colname', 'argid', 'token', 'option', 'conname',
+                   'member_name', 'label', 'attname']
+

 def compare_dictionaries(view_object, source_params, target_params,
                         target_schema, source_dict, target_dict, node,
@ -275,6 +278,11 @@ def are_dictionaries_identical(source_dict, target_dict, ignore_keys):
                                              target_dict[key], ignore_keys):
                return False
        elif type(source_dict[key]) is list:
+            # Sort the source and target list on the basis of
+            # list key array.
+            source_dict[key], target_dict[key] = sort_list(source_dict[key],
+                                                           target_dict[key])
+            # Compare the source and target lists
            if not are_lists_identical(source_dict[key], target_dict[key],
                                       ignore_keys):
                return False
@ -339,29 +347,13 @@ def directory_diff(source_dict, target_dict, ignore_keys=[], difference={}):
                        # TODO
                        pass
                    elif type(source) is dict:
-                        tmp_key_array = ['name', 'colname', 'argid', 'token',
-                                         'option', 'conname', 'member_name',
-                                         'label', 'attname']
                        # Check the above keys are exist in the dictionary
-                        tmp_key = is_key_exists(tmp_key_array, source)
+                        tmp_key = is_key_exists(list_keys_array, source)
                        if tmp_key is not None:
-                            if type(target_dict[key]) is list and \
-                                    len(target_dict[key]) > 0:
-                                tmp = None
-                                for item in tmp_target:
-                                    if tmp_key in item and \
-                                            item[tmp_key] == \
-                                            source[tmp_key]:
-                                        tmp = copy.deepcopy(item)
-                                if tmp and source != tmp:
-                                    updated.append(copy.deepcopy(source))
-                                    tmp_target.remove(tmp)
-                                elif tmp and source == tmp:
-                                    tmp_target.remove(tmp)
-                                elif tmp is None:
-                                    added.append(source)
-                            else:
-                                added.append(source)
+                            # Compare the two list by ignoring the keys.
+                            compare_list_by_ignoring_keys(source, tmp_target,
+                                                          added, updated,
+                                                          tmp_key, ignore_keys)

                        difference[key] = {}
                        if len(added) > 0:
@ -468,3 +460,67 @@ def parse_acl(source, target, diff_dict):
        diff_dict.update({key: diff})
    elif key in diff_dict:
        diff_dict.pop(key)
+
+
+def sort_list(source, target):
+    """
+    This function is used to sort the source and target list on the
+    basis of key found in the source and target list.
+    :param source:
+    :param target:
+    :return:
+    """
+    # Check the above keys are exist in the dictionary
+    if len(source) > 0:
+        tmp_key = is_key_exists(list_keys_array, source[0])
+        if tmp_key is not None:
+            source = sorted(source, key=lambda k: k[tmp_key])
+
+    # Check the above keys are exist in the dictionary
+    if len(target) > 0:
+        tmp_key = is_key_exists(list_keys_array, target[0])
+        if tmp_key is not None:
+            target = sorted(target, key=lambda k: k[tmp_key])
+
+    return source, target
+
+
+def compare_list_by_ignoring_keys(source_list, target_list, added, updated,
+                                  key, ignore_keys):
+    """
+    This function is used to compare the two list by ignoring the keys
+    specified in ignore_keys.
+    :param source_list:
+    :param target_list:
+    :param added:
+    :param updated:
+    :param key:
+    :param ignore_keys:
+    :return:
+    """
+    if type(target_list) is list and len(target_list) > 0:
+        tmp_target = None
+        for item in target_list:
+            if key in item and item[key] == source_list[key]:
+                tmp_target = copy.deepcopy(item)
+
+        if tmp_target is None:
+            added.append(source_list)
+        else:
+            source_with_ignored_keys = copy.deepcopy(source_list)
+            target_with_ignored_keys = copy.deepcopy(tmp_target)
+
+            # Remove ignore keys from source and target before comparison
+            for ig_key in ignore_keys:
+                if ig_key in source_with_ignored_keys:
+                    del source_with_ignored_keys[ig_key]
+                if ig_key in target_with_ignored_keys:
+                    del target_with_ignored_keys[ig_key]
+
+            if source_with_ignored_keys != target_with_ignored_keys:
+                updated.append(source_list)
+                target_list.remove(tmp_target)
+            elif source_with_ignored_keys == target_with_ignored_keys:
+                target_list.remove(tmp_target)
+    else:
+        added.append(source_list)
--- a/web/pgadmin/tools/schema_diff/tests/pg/10_plus/source.sql
+++ b/web/pgadmin/tools/schema_diff/tests/pg/10_plus/source.sql
@ -853,3 +853,20 @@ ALTER FOREIGN TABLE source.ft_diff_foreign_server_1
    OWNER TO postgres;
 ALTER FOREIGN TABLE source.ft_diff_foreign_server_1
    ADD CONSTRAINT cs1 CHECK ((fid > 200)) NO INHERIT;
+
+-- Test for RM #5350
+CREATE TABLE source.events_transactions
+(
+    event_code integer,
+    numerator integer,
+    account_token text COLLATE pg_catalog."default",
+    transaction_dt timestamp without time zone,
+    payment_method integer,
+    payment_pin integer,
+    approval text COLLATE pg_catalog."default",
+    amount integer,
+    file_dt timestamp without time zone DEFAULT CURRENT_TIMESTAMP,
+    file_name character varying(256) COLLATE pg_catalog."default",
+    transfer_dt timestamp without time zone,
+    transaction_type integer
+);
--- a/web/pgadmin/tools/schema_diff/tests/pg/10_plus/target.sql
+++ b/web/pgadmin/tools/schema_diff/tests/pg/10_plus/target.sql
@ -808,3 +808,20 @@ ALTER FOREIGN TABLE target.ft_diff_foreign_server_1
    OWNER TO postgres;
 ALTER FOREIGN TABLE target.ft_diff_foreign_server_1
    ADD CONSTRAINT cs2 CHECK ((fid > 200)) NO INHERIT;
+
+-- Test for RM #5350
+CREATE TABLE target.events_transactions
+(
+    event_code integer,
+    numerator integer,
+    account_token text COLLATE pg_catalog."default",
+    transaction_dt timestamp without time zone,
+    payment_method integer,
+    approval text COLLATE pg_catalog."default",
+    amount integer,
+    file_dt timestamp without time zone DEFAULT CURRENT_TIMESTAMP,
+    file_name character varying(256) COLLATE pg_catalog."default",
+    payment_pin integer,
+    transfer_dt timestamp without time zone,
+    transaction_type integer
+);
--- a/web/pgadmin/tools/schema_diff/tests/pg/11_plus/source.sql
+++ b/web/pgadmin/tools/schema_diff/tests/pg/11_plus/source.sql
@ -852,3 +852,20 @@ ALTER FOREIGN TABLE source.ft_diff_foreign_server_1
    OWNER TO postgres;
 ALTER FOREIGN TABLE source.ft_diff_foreign_server_1
    ADD CONSTRAINT cs1 CHECK ((fid > 200)) NO INHERIT;
+
+-- Test for RM #5350
+CREATE TABLE source.events_transactions
+(
+    event_code integer,
+    numerator integer,
+    account_token text COLLATE pg_catalog."default",
+    transaction_dt timestamp without time zone,
+    payment_method integer,
+    payment_pin integer,
+    approval text COLLATE pg_catalog."default",
+    amount integer,
+    file_dt timestamp without time zone DEFAULT CURRENT_TIMESTAMP,
+    file_name character varying(256) COLLATE pg_catalog."default",
+    transfer_dt timestamp without time zone,
+    transaction_type integer
+);
--- a/web/pgadmin/tools/schema_diff/tests/pg/11_plus/target.sql
+++ b/web/pgadmin/tools/schema_diff/tests/pg/11_plus/target.sql
@ -807,3 +807,20 @@ ALTER FOREIGN TABLE target.ft_diff_foreign_server_1
    OWNER TO postgres;
 ALTER FOREIGN TABLE target.ft_diff_foreign_server_1
    ADD CONSTRAINT cs2 CHECK ((fid > 200)) NO INHERIT;
+
+-- Test for RM #5350
+CREATE TABLE target.events_transactions
+(
+    event_code integer,
+    numerator integer,
+    account_token text COLLATE pg_catalog."default",
+    transaction_dt timestamp without time zone,
+    payment_method integer,
+    approval text COLLATE pg_catalog."default",
+    amount integer,
+    file_dt timestamp without time zone DEFAULT CURRENT_TIMESTAMP,
+    file_name character varying(256) COLLATE pg_catalog."default",
+    payment_pin integer,
+    transfer_dt timestamp without time zone,
+    transaction_type integer
+);
--- a/web/pgadmin/tools/schema_diff/tests/pg/12_plus/source.sql
+++ b/web/pgadmin/tools/schema_diff/tests/pg/12_plus/source.sql
@ -853,3 +853,20 @@ ALTER FOREIGN TABLE source.ft_diff_foreign_server_1
    OWNER TO postgres;
 ALTER FOREIGN TABLE source.ft_diff_foreign_server_1
    ADD CONSTRAINT cs1 CHECK ((fid > 200)) NO INHERIT;
+
+-- Test for RM #5350
+CREATE TABLE source.events_transactions
+(
+    event_code integer,
+    numerator integer,
+    account_token text COLLATE pg_catalog."default",
+    transaction_dt timestamp without time zone,
+    payment_method integer,
+    payment_pin integer,
+    approval text COLLATE pg_catalog."default",
+    amount integer,
+    file_dt timestamp without time zone DEFAULT CURRENT_TIMESTAMP,
+    file_name character varying(256) COLLATE pg_catalog."default",
+    transfer_dt timestamp without time zone,
+    transaction_type integer
+);
--- a/web/pgadmin/tools/schema_diff/tests/pg/12_plus/target.sql
+++ b/web/pgadmin/tools/schema_diff/tests/pg/12_plus/target.sql
@ -796,3 +796,20 @@ ALTER FOREIGN TABLE target.ft_diff_foreign_server_1
    OWNER TO postgres;
 ALTER FOREIGN TABLE target.ft_diff_foreign_server_1
    ADD CONSTRAINT cs2 CHECK ((fid > 200)) NO INHERIT;
+
+-- Test for RM #5350
+CREATE TABLE target.events_transactions
+(
+    event_code integer,
+    numerator integer,
+    account_token text COLLATE pg_catalog."default",
+    transaction_dt timestamp without time zone,
+    payment_method integer,
+    approval text COLLATE pg_catalog."default",
+    amount integer,
+    file_dt timestamp without time zone DEFAULT CURRENT_TIMESTAMP,
+    file_name character varying(256) COLLATE pg_catalog."default",
+    payment_pin integer,
+    transfer_dt timestamp without time zone,
+    transaction_type integer
+);
--- a/web/pgadmin/tools/schema_diff/tests/pg/9.2_plus/source.sql
+++ b/web/pgadmin/tools/schema_diff/tests/pg/9.2_plus/source.sql
@ -720,3 +720,20 @@ ALTER FOREIGN TABLE source.ft_diff_foreign_server_1
    OWNER TO postgres;
 ALTER FOREIGN TABLE source.ft_diff_foreign_server_1
    ADD CONSTRAINT cs1 CHECK ((fid > 200)) NO INHERIT;
+
+-- Test for RM #5350
+CREATE TABLE source.events_transactions
+(
+    event_code integer,
+    numerator integer,
+    account_token text COLLATE pg_catalog."default",
+    transaction_dt timestamp without time zone,
+    payment_method integer,
+    payment_pin integer,
+    approval text COLLATE pg_catalog."default",
+    amount integer,
+    file_dt timestamp without time zone DEFAULT CURRENT_TIMESTAMP,
+    file_name character varying(256) COLLATE pg_catalog."default",
+    transfer_dt timestamp without time zone,
+    transaction_type integer
+);
--- a/web/pgadmin/tools/schema_diff/tests/pg/9.2_plus/target.sql
+++ b/web/pgadmin/tools/schema_diff/tests/pg/9.2_plus/target.sql
@ -714,3 +714,20 @@ ALTER FOREIGN TABLE target.ft_diff_foreign_server_1
    OWNER TO postgres;
 ALTER FOREIGN TABLE target.ft_diff_foreign_server_1
    ADD CONSTRAINT cs2 CHECK ((fid > 200)) NO INHERIT;
+
+-- Test for RM #5350
+CREATE TABLE target.events_transactions
+(
+    event_code integer,
+    numerator integer,
+    account_token text COLLATE pg_catalog."default",
+    transaction_dt timestamp without time zone,
+    payment_method integer,
+    approval text COLLATE pg_catalog."default",
+    amount integer,
+    file_dt timestamp without time zone DEFAULT CURRENT_TIMESTAMP,
+    file_name character varying(256) COLLATE pg_catalog."default",
+    payment_pin integer,
+    transfer_dt timestamp without time zone,
+    transaction_type integer
+);
--- a/web/pgadmin/tools/schema_diff/tests/ppas/10_plus/source.sql
+++ b/web/pgadmin/tools/schema_diff/tests/ppas/10_plus/source.sql
@ -950,3 +950,20 @@ ALTER FOREIGN TABLE source.ft_diff_foreign_server_1
    OWNER TO enterprisedb;
 ALTER FOREIGN TABLE source.ft_diff_foreign_server_1
    ADD CONSTRAINT cs1 CHECK ((fid > 200)) NO INHERIT;
+
+-- Test for RM #5350
+CREATE TABLE source.events_transactions
+(
+    event_code integer,
+    numerator integer,
+    account_token text COLLATE pg_catalog."default",
+    transaction_dt timestamp without time zone,
+    payment_method integer,
+    payment_pin integer,
+    approval text COLLATE pg_catalog."default",
+    amount integer,
+    file_dt timestamp without time zone DEFAULT CURRENT_TIMESTAMP,
+    file_name character varying(256) COLLATE pg_catalog."default",
+    transfer_dt timestamp without time zone,
+    transaction_type integer
+);
--- a/web/pgadmin/tools/schema_diff/tests/ppas/10_plus/target.sql
+++ b/web/pgadmin/tools/schema_diff/tests/ppas/10_plus/target.sql
@ -895,3 +895,20 @@ ALTER FOREIGN TABLE target.ft_diff_foreign_server_1
    OWNER TO enterprisedb;
 ALTER FOREIGN TABLE target.ft_diff_foreign_server_1
    ADD CONSTRAINT cs2 CHECK ((fid > 200)) NO INHERIT;
+
+-- Test for RM #5350
+CREATE TABLE target.events_transactions
+(
+    event_code integer,
+    numerator integer,
+    account_token text COLLATE pg_catalog."default",
+    transaction_dt timestamp without time zone,
+    payment_method integer,
+    approval text COLLATE pg_catalog."default",
+    amount integer,
+    file_dt timestamp without time zone DEFAULT CURRENT_TIMESTAMP,
+    file_name character varying(256) COLLATE pg_catalog."default",
+    payment_pin integer,
+    transfer_dt timestamp without time zone,
+    transaction_type integer
+);
--- a/web/pgadmin/tools/schema_diff/tests/ppas/9.2_plus/source.sql
+++ b/web/pgadmin/tools/schema_diff/tests/ppas/9.2_plus/source.sql
@ -880,3 +880,20 @@ ALTER FOREIGN TABLE source.ft_diff_foreign_server_1
    OWNER TO enterprisedb;
 ALTER FOREIGN TABLE source.ft_diff_foreign_server_1
    ADD CONSTRAINT cs1 CHECK ((fid > 200)) NO INHERIT;
+
+-- Test for RM #5350
+CREATE TABLE source.events_transactions
+(
+    event_code integer,
+    numerator integer,
+    account_token text COLLATE pg_catalog."default",
+    transaction_dt timestamp without time zone,
+    payment_method integer,
+    payment_pin integer,
+    approval text COLLATE pg_catalog."default",
+    amount integer,
+    file_dt timestamp without time zone DEFAULT CURRENT_TIMESTAMP,
+    file_name character varying(256) COLLATE pg_catalog."default",
+    transfer_dt timestamp without time zone,
+    transaction_type integer
+);
--- a/web/pgadmin/tools/schema_diff/tests/ppas/9.2_plus/target.sql
+++ b/web/pgadmin/tools/schema_diff/tests/ppas/9.2_plus/target.sql
@ -864,3 +864,20 @@ ALTER FOREIGN TABLE target.ft_diff_foreign_server_1
    OWNER TO enterprisedb;
 ALTER FOREIGN TABLE target.ft_diff_foreign_server_1
    ADD CONSTRAINT cs2 CHECK ((fid > 200)) NO INHERIT;
+
+-- Test for RM #5350
+CREATE TABLE target.events_transactions
+(
+    event_code integer,
+    numerator integer,
+    account_token text COLLATE pg_catalog."default",
+    transaction_dt timestamp without time zone,
+    payment_method integer,
+    approval text COLLATE pg_catalog."default",
+    amount integer,
+    file_dt timestamp without time zone DEFAULT CURRENT_TIMESTAMP,
+    file_name character varying(256) COLLATE pg_catalog."default",
+    payment_pin integer,
+    transfer_dt timestamp without time zone,
+    transaction_type integer
+);