Fix formatting a view's SQL being treated as a column change that drops and re-creates the view. #9838

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

@@ -912,6 +912,107 @@ def _get_create_view_sql(self, data):
 
         return False, '', sql
 
+    # A character that may appear in an unquoted identifier, and so cannot
+    # immediately precede the start of a dollar-quoted string or an E''
+    # string's prefix.
+    _IDENT_CHAR = re.compile(r'[A-Za-z0-9_$\u0080-\uffff]')
+    _DOLLAR_TAG = re.compile(
+        r'\$(?:[A-Za-z_\u0080-\uffff][A-Za-z0-9_\u0080-\uffff]*)?\$')
+
+    @staticmethod
+    def _tokenise_definition(definition):
+        """
+        Split a view definition into plain SQL and quoted parts, in the same
+        shape as re.split() with one capturing group: even indexes hold plain
+        SQL and odd indexes hold a quoted identifier, string literal or
+        dollar-quoted string exactly as written. This follows PostgreSQL's
+        lexer: '' and "" are escaped quotes, E'' strings also take backslash
+        escapes, block comments nest, and a $ that continues an identifier
+        does not start a dollar-quoted string. Comments are replaced with a
+        space in the plain SQL. A construct left unterminated runs to the
+        end of the definition.
+        """
+        def follows_ident(pos):
+            return pos > 0 and \
+                ViewNode._IDENT_CHAR.match(definition[pos - 1]) is not None
+
+        parts = ['']
+        i, n = 0, len(definition)
+        while i < n:
+            ch = definition[i]
+            pair = definition[i:i + 2]
+            if pair == '--':
+                end = definition.find('\n', i)
+                i = n if end < 0 else end
+                parts[-1] += ' '
+            elif pair == '/*':
+                depth, i = 1, i + 2
+                while i < n and depth:
+                    pair = definition[i:i + 2]
+                    if pair == '/*':
+                        depth, i = depth + 1, i + 2
+                    elif pair == '*/':
+                        depth, i = depth - 1, i + 2
+                    else:
+                        i += 1
+                parts[-1] += ' '
+            elif ch in '"\'':
+                escapes = ch == "'" and i > 0 and \
+                    definition[i - 1] in 'eE' and not follows_ident(i - 1)
+                j = i + 1
+                while j < n:
+                    if escapes and definition[j] == '\\':
+                        j += 2
+                    elif definition[j] != ch:
+                        j += 1
+                    elif definition[j + 1:j + 2] == ch:
+                        j += 2
+                    else:
+                        break
+                j = min(j + 1, n)
+                parts.extend([definition[i:j], ''])
+                i = j
+            else:
+                tag = None if ch != '$' or follows_ident(i) else \
+                    ViewNode._DOLLAR_TAG.match(definition, i)
+                if tag is None:
+                    parts[-1] += ch
+                    i += 1
+                    continue
+                end = definition.find(tag.group(0), tag.end())
+                j = n if end < 0 else end + len(tag.group(0))
+                parts.extend([definition[i:j], ''])
+                i = j
+        return parts
+
+    @staticmethod
+    def _split_definition(definition):
+        """
+        Split a view definition at each FROM keyword and normalise the parts
+        so that only changes that can affect the view's columns are noticed.
+        The contents of quoted identifiers and string literals are kept
+        exactly as written, although the double quotes around an identifier
+        are dropped so that "name" still matches name. Everything else is
+        lower-cased (keywords, function names, type names and unquoted
+        identifiers are case-insensitive) and has its non-word characters
+        removed, so a pure reformat, such as one from the SQL formatter, is
+        not mistaken for a change to the column list. FROM is only matched as
+        a whole word outside quotes and comments, so identifiers such as
+        valid_from, and comments such as /* from */, do not end the select
+        list early.
+        """
+        segments = ['']
+        parts = ViewNode._tokenise_definition(definition or '')
+        for i, part in enumerate(parts):
+            if i % 2:
+                segments[-1] += part[1:-1] if part[0] == '"' else part
+                continue
+            pieces = [re.sub(r"\W", "", piece)
+                      for piece in re.split(r'\bfrom\b', part.lower())]
+            segments[-1] += pieces[0]
+            segments.extend(pieces[1:])
+        return segments
+
     def _get_definition_data(self, vid, data, old_data, res, acls):
         """
         Check and process definition data.
@@ -925,9 +1026,9 @@ def _get_definition_data(self, vid, data, old_data, res, acls):
         if not retun False with error msg empty('')
         """
         if 'definition' in data and self.manager.server_type == 'pg':
-            new_def = re.sub(r"\W", "", data['definition']).split('FROM')
-            old_def = re.sub(r"\W", "", res['rows'][0]['definition']
-                             ).split('FROM')
+            new_def = ViewNode._split_definition(data['definition'])
+            old_def = ViewNode._split_definition(
+                res['rows'][0]['definition'])
             if 'definition' in data and (
                 len(old_def) > 1 or len(new_def) > 1
             ) and (

web/pgadmin/browser/server_groups/servers/databases/schemas/views/static/js/view.ui.js

@@ -14,6 +14,119 @@ import SecLabelSchema from '../../../../../static/js/sec_label.ui';
 import { isEmptyString } from 'sources/validators';
 import { getPrivilegesForTableAndLikeObjects } from '../../../tables/static/js/table.ui';
 
+// A character that may appear in an unquoted identifier, and so cannot
+// immediately precede the start of a dollar-quoted string or an E'' string's
+// prefix.
+const IDENT_CHAR = /[A-Za-z0-9_$\u0080-\uffff]/;
+const DOLLAR_TAG = /\$(?:[A-Za-z_\u0080-\uffff][A-Za-z0-9_\u0080-\uffff]*)?\$/y;
+
+/*
+ * Split a view definition into plain SQL and quoted parts, in the same shape
+ * as String.split() with one capturing group: even indexes hold plain SQL and
+ * odd indexes hold a quoted identifier, string literal or dollar-quoted string
+ * exactly as written. This follows PostgreSQL's lexer: '' and "" are escaped
+ * quotes, E'' strings also take backslash escapes, block comments nest, and a
+ * $ that continues an identifier does not start a dollar-quoted string.
+ * Comments are replaced with a space in the plain SQL. A construct left
+ * unterminated runs to the end of the definition. This must stay in step with
+ * ViewNode._tokenise_definition() on the server.
+ */
+function tokeniseViewDefinition(definition) {
+  const followsIdent = (pos) => pos > 0 && IDENT_CHAR.test(definition[pos - 1]);
+  const n = definition.length;
+  let parts = [''];
+  let i = 0;
+  while (i < n) {
+    const ch = definition[i];
+    let pair = definition.slice(i, i + 2);
+    if (pair == '--') {
+      const end = definition.indexOf('\n', i);
+      i = end < 0 ? n : end;
+      parts[parts.length - 1] += ' ';
+    } else if (pair == '/*') {
+      let depth = 1;
+      i += 2;
+      while (i < n && depth) {
+        pair = definition.slice(i, i + 2);
+        if (pair == '/*') {
+          depth += 1;
+          i += 2;
+        } else if (pair == '*/') {
+          depth -= 1;
+          i += 2;
+        } else {
+          i += 1;
+        }
+      }
+      parts[parts.length - 1] += ' ';
+    } else if (ch == '"' || ch == '\'') {
+      const escapes = ch == '\'' && i > 0 && 'eE'.includes(definition[i - 1]) &&
+        !followsIdent(i - 1);
+      let j = i + 1;
+      while (j < n) {
+        if (escapes && definition[j] == '\\') {
+          j += 2;
+        } else if (definition[j] != ch) {
+          j += 1;
+        } else if (definition[j + 1] == ch) {
+          j += 2;
+        } else {
+          break;
+        }
+      }
+      j = Math.min(j + 1, n);
+      parts.push(definition.slice(i, j), '');
+      i = j;
+    } else {
+      let tag = null;
+      if (ch == '$' && !followsIdent(i)) {
+        DOLLAR_TAG.lastIndex = i;
+        tag = DOLLAR_TAG.exec(definition);
+      }
+      if (tag === null) {
+        parts[parts.length - 1] += ch;
+        i += 1;
+        continue;
+      }
+      const end = definition.indexOf(tag[0], i + tag[0].length);
+      const j = end < 0 ? n : end + tag[0].length;
+      parts.push(definition.slice(i, j), '');
+      i = j;
+    }
+  }
+  return parts;
+}
+
+/*
+ * Split a view definition at each FROM keyword and normalise the parts so
+ * that only changes that can affect the view's columns are noticed. The
+ * contents of quoted identifiers and string literals are kept exactly as
+ * written, although the double quotes around an identifier are dropped so
+ * that "name" still matches name. Everything else is lower-cased (SQL
+ * keywords, function names, type names and unquoted identifiers are
+ * case-insensitive) and has its whitespace removed, so a pure reformat (for
+ * example with the SQL formatter, which can change keyword, function and data
+ * type case) does not look like a change to the column list. FROM is only
+ * matched as a whole word outside quotes and comments, so identifiers such as
+ * valid_from, and comments such as /* from *\/, do not end the select list
+ * early.
+ */
+export function splitViewDefinition(definition) {
+  let segments = [''];
+  tokeniseViewDefinition(definition ?? '').forEach((part, i) => {
+    if (i % 2 == 1) {
+      segments[segments.length - 1] += part.startsWith('"') ?
+        part.slice(1, -1) : part;
+      return;
+    }
+    let pieces = part.toLowerCase().split(/\bfrom\b/).map(
+      (piece) => piece.replace(/\s/g, '')
+    );
+    segments[segments.length - 1] += pieces[0];
+    segments.push(...pieces.slice(1));
+  });
+  return segments;
+}
 
 export default class ViewSchema extends BaseUISchema {
   constructor(getPrivilegeRoleSchema, nodeInfo, fieldOptions={}, initValues={}) {
@@ -147,15 +260,11 @@ export default class ViewSchema extends BaseUISchema {
           return false;
         }
 
-        let old_def = obj.origData.definition?.replace(
-            /\s/gi, ''
-          ).split('FROM'),
+        let old_def = splitViewDefinition(obj.origData.definition),
           new_def = [];
 
         if (!_.isUndefined(state.definition)) {
-          new_def = state.definition.replace(
-            /\s/gi, ''
-          ).split('FROM');
+          new_def = splitViewDefinition(state.definition);
         }
 
         if ((old_def.length != new_def.length) || (

web/pgadmin/browser/server_groups/servers/databases/schemas/views/tests/test_views_get_msql.py

@@ -68,13 +68,22 @@ def runTest(self):
         schema node."""
 
         if self.is_positive_test:
-            url_encode_data = {"oid": self.view_id,
-                               "comment": self.data['comment']}
+            url_encode_data = {"oid": self.view_id}
+            for key in ('comment', 'definition'):
+                if key in self.data:
+                    url_encode_data[key] = self.data[key]
 
             response = views_utils.api_get_msql(self, url_encode_data)
 
             # Assert response
             utils.assert_status_code(self, response)
+
+            result = self.expected_data.get('test_result_data', {})
+            sql = json.loads(response.data.decode('utf-8'))['data']
+            if 'sql_contains' in result:
+                self.assertIn(result['sql_contains'], sql)
+            if 'sql_not_contains' in result:
+                self.assertNotIn(result['sql_not_contains'], sql)
         else:
             if 'view_id' in self.data:
                 # Non-existing view id

web/pgadmin/browser/server_groups/servers/databases/schemas/views/tests/test_views_split_definition.py

@@ -0,0 +1,85 @@
+##########################################################################
+#
+# pgAdmin 4 - PostgreSQL Tools
+#
+# Copyright (C) 2013 - 2026, The pgAdmin Development Team
+# This software is released under the PostgreSQL Licence
+#
+##########################################################################
+
+from pgadmin.utils.route import BaseTestGenerator
+from pgadmin.browser.server_groups.servers.databases.schemas.views import \
+    ViewNode
+
+
+class ViewsSplitDefinitionTestCase(BaseTestGenerator):
+    """The select list compared to decide whether a view must be dropped
+    and re-created ignores reformatting, but notices changes to quoted
+    identifiers and is not cut short by identifiers containing 'from'."""
+
+    scenarios = [
+        ('Reformat with different case and whitespace is ignored', dict(
+            old=' SELECT id,\n    upper(a) AS ua,\n    (d)::text AS ds\n'
+                '   FROM t1 t;',
+            new='SELECT\n\tid,\n\tUPPER(a) AS ua,\n\t(d)::TEXT AS ds\n'
+                'FROM\n\tt1 t;',
+            same=True)),
+        ('Case change in a quoted identifier is noticed', dict(
+            old='SELECT 1 AS "Name" FROM t1',
+            new='SELECT 1 AS "NAME" FROM t1',
+            same=False)),
+        ('Punctuation change in a quoted identifier is noticed', dict(
+            old='SELECT 1 AS "a-b" FROM t1',
+            new='SELECT 1 AS "ab" FROM t1',
+            same=False)),
+        ('Quoting an identifier that needs no quotes is ignored', dict(
+            old='SELECT id, name FROM t1',
+            new='SELECT "id", "name" FROM t1',
+            same=True)),
+        ('Column removed after one named like FROM is noticed', dict(
+            old='SELECT a AS fromage, b FROM t1',
+            new='SELECT a AS fromage FROM t1',
+            same=False)),
+        ('Column removed after valid_from is noticed', dict(
+            old='SELECT id, valid_from, name FROM t1',
+            new='SELECT id, valid_from FROM t1',
+            same=False)),
+        ('FROM inside a string literal does not split', dict(
+            old="SELECT 'x FROM y' AS a, b FROM t1",
+            new="SELECT 'x FROM y' AS a FROM t1",
+            same=False)),
+        ('FROM inside a block comment does not split', dict(
+            old='SELECT a /* from */, b FROM t1',
+            new='SELECT a /* from */ FROM t1',
+            same=False)),
+        ('FROM inside a nested block comment does not split', dict(
+            old='SELECT a /* x /* y */ from */, b FROM t1',
+            new='SELECT a /* x /* y */ from */ FROM t1',
+            same=False)),
+        ('FROM inside a line comment does not split', dict(
+            old='SELECT a -- from\n, b FROM t1',
+            new='SELECT a -- from\nFROM t1',
+            same=False)),
+        ('Adding or removing a comment is ignored', dict(
+            old='SELECT a, /* note */ b -- the end\nFROM t1',
+            new='SELECT a, b FROM t1',
+            same=True)),
+        ('FROM inside a dollar-quoted string does not split', dict(
+            old='SELECT $q$ from $$ \' $q$ AS a, b FROM t1',
+            new='SELECT $q$ from $$ \' $q$ AS a FROM t1',
+            same=False)),
+        ('FROM inside an E string with escapes does not split', dict(
+            old="SELECT E'it\\'s from' AS a, b FROM t1",
+            new="SELECT E'it\\'s from' AS a FROM t1",
+            same=False)),
+        ('A $ inside an identifier does not start a dollar quote', dict(
+            old='SELECT a$q$, b FROM t1',
+            new='SELECT a$q$ FROM t1',
+            same=False)),
+    ]
+
+    def runTest(self):
+        old_def = ViewNode._split_definition(self.old)
+        new_def = ViewNode._split_definition(self.new)
+        self.assertEqual(len(old_def), 2)
+        self.assertEqual(old_def[0] == new_def[0], self.same)