Fix an issue where command and statements were parsed incorrectly for Rules. Fixes #5115

Fixed issue where removing command or statements altogether was not generating modified SQL.

docs/en_US/release_notes_4_19.rst

@@ -22,4 +22,5 @@ Housekeeping
 Bug fixes
 *********
 
-| `Issue #5107 <https://redmine.postgresql.org/issues/5107>`_ -  Set proper focus on tab navigation for file manager dialog.
+| `Issue #5107 <https://redmine.postgresql.org/issues/5107>`_ -  Set proper focus on tab navigation for file manager dialog.
+| `Issue #5115 <https://redmine.postgresql.org/issues/5115>`_ -  Fix an issue where command and statements were parsed incorrectly for Rules.

web/pgadmin/browser/server_groups/servers/databases/schemas/tables/rules/static/js/rule.js

@@ -169,10 +169,10 @@ define('pgadmin.node.rule', [
             allowClear: false,
           },
           options:[
-            {label: 'Select', value: 'Select'},
+            {label: 'SELECT', value: 'SELECT'},
-            {label: 'Insert', value: 'Insert'},
+            {label: 'INSERT', value: 'INSERT'},
-            {label: 'Update', value: 'Update'},
+            {label: 'UPDATE', value: 'UPDATE'},
-            {label: 'Delete', value: 'Delete'},
+            {label: 'DELETE', value: 'DELETE'},
           ],
         },
         {

web/pgadmin/browser/server_groups/servers/databases/schemas/tables/rules/tests/default/alter_delete_event_rule.sql

@@ -5,7 +5,7 @@
 CREATE OR REPLACE RULE "test_delete_rule1_$%{}[]()&*^!@""'`\/#" AS
     ON DELETE TO public.test_emp_rule
     DO INSTEAD
-DELETE FROM test_emp_rule
+(DELETE FROM test_emp_rule
-  WHERE test_emp_rule.name = old.name;
+  WHERE (test_emp_rule.name = old.name));
 
 COMMENT ON RULE "test_delete_rule1_$%{}[]()&*^!@""'`\/#" ON public.test_emp_rule IS 'This is a delete rule';

web/pgadmin/browser/server_groups/servers/databases/schemas/tables/rules/tests/default/alter_delete_event_rule_msql.sql

@@ -3,7 +3,7 @@ ALTER RULE "test_delete_rule_$%{}[]()&*^!@""'`\/#" ON public.test_emp_rule RENAM
 CREATE OR REPLACE RULE "test_delete_rule1_$%{}[]()&*^!@""'`\/#" AS
     ON DELETE TO public.test_emp_rule
     DO INSTEAD
-DELETE FROM test_emp_rule
+(DELETE FROM test_emp_rule
-  WHERE test_emp_rule.name = old.name;
+  WHERE test_emp_rule.name = old.name);
 
 COMMENT ON RULE "test_delete_rule1_$%{}[]()&*^!@""'`\/#" ON public.test_emp_rule IS 'This is a delete rule';

web/pgadmin/browser/server_groups/servers/databases/schemas/tables/rules/tests/default/alter_insert_event_rule.sql

@@ -4,9 +4,9 @@
 
 CREATE OR REPLACE RULE "test_insert_rule1_$%{}[]()&*^!@""'`\/#" AS
     ON INSERT TO public.test_emp_rule
-    WHERE new.salary > 8000
+    WHERE (new.salary > 8000)
     DO INSTEAD
-UPDATE test_emp_rule SET salary = 8000
+(UPDATE test_emp_rule SET salary = 8000
-  WHERE test_emp_rule.emp_id = new.emp_id;
+  WHERE (test_emp_rule.emp_id = new.emp_id));
 
 COMMENT ON RULE "test_insert_rule1_$%{}[]()&*^!@""'`\/#" ON public.test_emp_rule IS 'This is a insert rule';

web/pgadmin/browser/server_groups/servers/databases/schemas/tables/rules/tests/default/alter_insert_event_rule_complex.sql

@@ -0,0 +1,17 @@
+-- Rule: "test_insert_rule1_$%{}[]()&*^!@""'`\/#" ON public.test_emp_rule
+
+-- DROP Rule "test_insert_rule1_$%{}[]()&*^!@""'`\/#" ON public.test_emp_rule;
+
+CREATE OR REPLACE RULE "test_insert_rule1_$%{}[]()&*^!@""'`\/#" AS
+    ON INSERT TO public.test_emp_rule
+    WHERE (new.salary > 8000)
+    DO INSTEAD
+( UPDATE test_emp_rule SET salary = 8000
+  WHERE (test_emp_rule.emp_id = new.emp_id);
+ INSERT INTO test_emp_rule (emp_id, name, salary)  SELECT 1,
+            'xyz',
+            2000
+          WHERE (1 = 2);
+);
+
+COMMENT ON RULE "test_insert_rule1_$%{}[]()&*^!@""'`\/#" ON public.test_emp_rule IS 'This is a insert rule';

web/pgadmin/browser/server_groups/servers/databases/schemas/tables/rules/tests/default/alter_insert_event_rule_complex_msql.sql

@@ -0,0 +1,8 @@
+CREATE OR REPLACE RULE "test_insert_rule1_$%{}[]()&*^!@""'`\/#" AS
+    ON INSERT TO public.test_emp_rule
+    WHERE (new.salary > 8000)
+    DO INSTEAD
+(UPDATE test_emp_rule SET salary = 8000
+	WHERE test_emp_rule.emp_id = new.emp_id;
+INSERT INTO test_emp_rule(emp_id, name, salary)
+SELECT 1, 'xyz', 2000 WHERE 1=2);

web/pgadmin/browser/server_groups/servers/databases/schemas/tables/rules/tests/default/alter_insert_event_rule_msql.sql

@@ -2,9 +2,9 @@ ALTER RULE "test_insert_rule_$%{}[]()&*^!@""'`\/#" ON public.test_emp_rule RENAM
 
 CREATE OR REPLACE RULE "test_insert_rule1_$%{}[]()&*^!@""'`\/#" AS
     ON INSERT TO public.test_emp_rule
-    WHERE new.salary > 8000
+    WHERE (new.salary > 8000)
     DO INSTEAD
-UPDATE test_emp_rule SET salary = 8000
+(UPDATE test_emp_rule SET salary = 8000
-	WHERE test_emp_rule.emp_id = new.emp_id;
+	WHERE test_emp_rule.emp_id = new.emp_id);
 
 COMMENT ON RULE "test_insert_rule1_$%{}[]()&*^!@""'`\/#" ON public.test_emp_rule IS 'This is a insert rule';

web/pgadmin/browser/server_groups/servers/databases/schemas/tables/rules/tests/default/alter_update_event_rule.sql

@@ -4,9 +4,9 @@
 
 CREATE OR REPLACE RULE "test_update_rule1_$%{}[]()&*^!@""'`\/#" AS
     ON UPDATE TO public.test_emp_rule
-    WHERE old.name = 'Sam'::text
+    WHERE (old.name = 'Sam'::text)
     DO INSTEAD
-UPDATE test_emp_rule SET salary = new.salary
+(UPDATE test_emp_rule SET salary = new.salary
-  WHERE test_emp_rule.name = 'Joe'::text;
+  WHERE (test_emp_rule.name = 'Joe'::text));
 
 COMMENT ON RULE "test_update_rule1_$%{}[]()&*^!@""'`\/#" ON public.test_emp_rule IS 'This is a update rule';

web/pgadmin/browser/server_groups/servers/databases/schemas/tables/rules/tests/default/alter_update_event_rule_msql.sql

@@ -2,9 +2,9 @@ ALTER RULE "test_update_rule_$%{}[]()&*^!@""'`\/#" ON public.test_emp_rule RENAM
 
 CREATE OR REPLACE RULE "test_update_rule1_$%{}[]()&*^!@""'`\/#" AS
     ON UPDATE TO public.test_emp_rule
-    WHERE old.name = 'Sam'
+    WHERE (old.name = 'Sam')
     DO INSTEAD
-UPDATE test_emp_rule SET salary = new.salary
+(UPDATE test_emp_rule SET salary = new.salary
-	WHERE test_emp_rule.name = 'Joe';
+	WHERE test_emp_rule.name = 'Joe');
 
 COMMENT ON RULE "test_update_rule1_$%{}[]()&*^!@""'`\/#" ON public.test_emp_rule IS 'This is a update rule';

web/pgadmin/browser/server_groups/servers/databases/schemas/tables/rules/tests/default/create_delete_event_rule.sql

@@ -4,5 +4,4 @@
 
 CREATE OR REPLACE RULE "test_delete_rule_$%{}[]()&*^!@""'`\/#" AS
     ON DELETE TO public.test_emp_rule
-    DO
+    DO NOTHING;
-NOTHING;

web/pgadmin/browser/server_groups/servers/databases/schemas/tables/rules/tests/default/create_delete_event_rule_msql.sql

@@ -1,4 +1,3 @@
 CREATE OR REPLACE RULE "test_delete_rule_$%{}[]()&*^!@""'`\/#" AS
     ON DELETE TO public.test_emp_rule
-    DO
+    DO NOTHING;
-  NOTHING;

web/pgadmin/browser/server_groups/servers/databases/schemas/tables/rules/tests/default/create_insert_event_rule.sql

@@ -4,7 +4,7 @@
 
 CREATE OR REPLACE RULE "test_insert_rule_$%{}[]()&*^!@""'`\/#" AS
     ON INSERT TO public.test_emp_rule
-    WHERE new.salary > 5000
+    WHERE (new.salary > 5000)
     DO
-UPDATE test_emp_rule SET salary = 5000
+(UPDATE test_emp_rule SET salary = 5000
-  WHERE test_emp_rule.emp_id = new.emp_id;
+  WHERE (test_emp_rule.emp_id = new.emp_id));

web/pgadmin/browser/server_groups/servers/databases/schemas/tables/rules/tests/default/create_insert_event_rule_msql.sql

@@ -1,6 +1,6 @@
 CREATE OR REPLACE RULE "test_insert_rule_$%{}[]()&*^!@""'`\/#" AS
     ON INSERT TO public.test_emp_rule
-    WHERE new.salary > 5000
+    WHERE (new.salary > 5000)
     DO
-UPDATE test_emp_rule SET salary = 5000
+(UPDATE test_emp_rule SET salary = 5000
-  WHERE test_emp_rule.emp_id = new.emp_id;
+  WHERE test_emp_rule.emp_id = new.emp_id);

web/pgadmin/browser/server_groups/servers/databases/schemas/tables/rules/tests/default/create_update_event_rule.sql

@@ -4,7 +4,7 @@
 
 CREATE OR REPLACE RULE "test_update_rule_$%{}[]()&*^!@""'`\/#" AS
     ON UPDATE TO public.test_emp_rule
-    WHERE old.name = 'Joe'::text
+    WHERE (old.name = 'Joe'::text)
     DO
-UPDATE test_emp_rule SET salary = new.salary
+(UPDATE test_emp_rule SET salary = new.salary
-  WHERE test_emp_rule.name = 'Sam'::text;
+  WHERE (test_emp_rule.name = 'Sam'::text));

web/pgadmin/browser/server_groups/servers/databases/schemas/tables/rules/tests/default/create_update_event_rule_msql.sql

@@ -1,6 +1,6 @@
 CREATE OR REPLACE RULE "test_update_rule_$%{}[]()&*^!@""'`\/#" AS
     ON UPDATE TO public.test_emp_rule
-    WHERE old.name = 'Joe'
+    WHERE (old.name = 'Joe')
     DO
-UPDATE test_emp_rule SET salary = new.salary
+(UPDATE test_emp_rule SET salary = new.salary
-  WHERE test_emp_rule.name = 'Sam';
+  WHERE test_emp_rule.name = 'Sam');

web/pgadmin/browser/server_groups/servers/databases/schemas/tables/rules/tests/default/test.json

@@ -57,6 +57,17 @@
       },
       "expected_sql_file": "alter_insert_event_rule.sql",
       "expected_msql_file": "alter_insert_event_rule_msql.sql"
+    }, {
+      "type": "alter",
+      "name": "Alter Rule for insert event with complex commands",
+      "endpoint": "NODE-rule.obj_id",
+      "sql_endpoint": "NODE-rule.sql_id",
+      "msql_endpoint": "NODE-rule.msql_id",
+      "data": {
+        "statements": "UPDATE test_emp_rule SET salary = 8000\n\tWHERE test_emp_rule.emp_id = new.emp_id;\nINSERT INTO test_emp_rule(emp_id, name, salary)\nSELECT 1, 'xyz', 2000 WHERE 1=2;"
+      },
+      "expected_sql_file": "alter_insert_event_rule_complex.sql",
+      "expected_msql_file": "alter_insert_event_rule_complex_msql.sql"
     }, {
       "type": "delete",
       "name": "Drop Rule",

web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/rules/sql/create.sql

@@ -9,17 +9,17 @@
 CREATE OR REPLACE RULE {{ conn|qtIdent(data.name) }} AS
     ON {{ data.event|upper if data.event else 'SELECT' }} TO {{ conn|qtIdent(data.schema, data.view) }}
 {% if data.condition %}
-    WHERE {{ data.condition }}
+    WHERE ({{ data.condition }})
 {% endif %}
     DO{% if data.do_instead in ['true', True] %}
-{{ ' INSTEAD' }}
+{{ ' INSTEAD' }}{% else %}{{ '' }}{% endif %}
+{% if data.statements is defined and data.statements.strip() in ['', 'NOTHING'] %}
+ NOTHING;
+{% elif data.statements is defined %}
+
+({{ data.statements.rstrip(';') }});
 {% else %}
-{{ '' }}
+ NOTHING;
-{% endif %}
-{% if data.statements %}
-{{ data.statements.rstrip(';') }};
-{% else %}
-  NOTHING;
 {% endif %}
 {% if data.comment %}
 

web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/rules/sql/properties.sql

@@ -3,6 +3,8 @@
 SELECT
     rw.oid AS oid,
     rw.rulename AS name,
+    rw.ev_type,
+    rw.is_instead,
     relname AS view,
     CASE WHEN relkind = 'r' THEN TRUE ELSE FALSE END AS parentistable,
     nspname AS schema,
@@ -10,7 +12,7 @@ SELECT
     {# ===== Check whether it is system rule or not ===== #}
     CASE WHEN rw.rulename = '_RETURN' THEN True ELSE False END AS system_rule,
     CASE WHEN rw.ev_enabled != 'D' THEN True ELSE False END AS enabled,
-    pg_get_ruledef(rw.oid, true) AS definition
+    pg_get_ruledef(rw.oid) AS definition
 FROM
     pg_rewrite rw
 JOIN pg_class cl ON cl.oid=rw.ev_class

web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/rules/sql/update.sql

@@ -8,22 +8,24 @@
 ALTER RULE {{ conn|qtIdent(o_data.name) }} ON {{ conn|qtIdent(o_data.schema, o_data.view) }} RENAME TO {{ conn|qtIdent(data.name) }};
 
 {% endif %}
-{% if data.event or data.do_instead is defined or data.condition or data.statements %}
+{% if data.event or data.do_instead is defined or data.condition is defined or data.statements is defined %}
 CREATE OR REPLACE RULE {{ conn|qtIdent(rule_name) }} AS
     ON {% if data.event and data.event != o_data.event %}{{ data.event|upper }}{% else %}{{ o_data.event|upper }}{% endif %}
  TO {{ conn|qtIdent(o_data.schema, o_data.view) }}
 {% if data.condition and o_data.condition != data.condition %}
-    WHERE {{ data.condition }}
+    WHERE ({{ data.condition }})
 {% elif data.condition is not defined and o_data.condition %}
-    WHERE {{ o_data.condition }}
+    WHERE ({{ o_data.condition }})
 {% endif %}
-    DO {% if (('do_instead' not in data and o_data.do_instead in ['true', True]) or (data.do_instead in ['true', True])) %}{{ 'INSTEAD' }}{% endif %}
+    DO{% if (('do_instead' not in data and o_data.do_instead in ['true', True]) or (data.do_instead in ['true', True])) %}{{ ' INSTEAD' }}{% endif %}
-{% if data.statements and data.statements != o_data.statements %}
+{% if data.statements and data.statements != o_data.statements and data.statements.strip() in ['', 'NOTHING']%}
+ NOTHING;
+{% elif data.statements and data.statements != o_data.statements %}
 
-{{ data.statements.rstrip(';') }};
+({{ data.statements.rstrip(';') }});
 {% elif data.statements is not defined and o_data.statements %}
 
-{{ o_data.statements.rstrip(';') }};
+({{ o_data.statements.rstrip(';') }});
 {% else %}
  NOTHING;
 {% endif %}

web/pgadmin/browser/server_groups/servers/databases/schemas/utils.py

@@ -428,32 +428,43 @@ def parse_rule_definition(res):
         res_data = res['rows'][0]
         data_def = res_data['definition']
         import re
-        # Parse data for event
-        e_match = re.search(r"ON\s+(.*)\s+TO", data_def)
-        event_data = e_match.group(1) if e_match is not None else None
-        event = event_data if event_data is not None else ''
-
-        # Parse data for do instead
-        inst_match = re.search(r"\s+(INSTEAD)\s+", data_def)
-        instead_data = inst_match.group(1) if inst_match is not None else None
-        instead = True if instead_data is not None else False
 
         # Parse data for condition
-        condition_match = re.search(r"(?:WHERE)\s+([\s\S]*)\s+(?:DO)",
+        condition = ''
-                                    data_def)
+        condition_part_match = re.search(
-        condition_data = condition_match.group(1) \
+            r"((?:ON)\s+(?:[\s\S]+?)"
-            if condition_match is not None else None
+            r"(?:TO)\s+(?:[\s\S]+?)(?:DO))", data_def)
-        condition = condition_data if condition_data is not None else ''
+        if condition_part_match is not None:
+            condition_part = condition_part_match.group(1)
 
-        # Parse data for statements
+            condition_match = re.search(
+                r"(?:WHERE)\s+(\([\s\S]*\))\s+(?:DO)", condition_part)
+
+            if condition_match is not None:
+                condition = condition_match.group(1)
+                # also remove enclosing brackets
+                if condition.startswith('(') and condition.endswith(')'):
+                    condition = condition[1:-1]
+
+            # Parse data for statements
         statement_match = re.search(
-            r"(?:DO\s+)(?:INSTEAD\s+)?((.|\n)*)", data_def)
+            r"(?:DO\s+)(?:INSTEAD\s+)?([\s\S]*)(?:;)", data_def)
-        statement_data = statement_match.group(1) if statement_match else None
+
-        statement = statement_data if statement_data is not None else ''
+        statement = ''
+        if statement_match is not None:
+            statement = statement_match.group(1)
+            # also remove enclosing brackets
+            if statement.startswith('(') and statement.endswith(')'):
+                statement = statement[1:-1]
 
         # set columns parse data
-        res_data['event'] = event.lower().capitalize()
+        res_data['event'] = {
-        res_data['do_instead'] = instead
+            '1': 'SELECT',
+            '2': 'UPDATE',
+            '3': 'INSERT',
+            '4': 'DELETE'
+        }[res_data['ev_type']]
+        res_data['do_instead'] = res_data['is_instead']
         res_data['statements'] = statement
         res_data['condition'] = condition
     except Exception as e: