postgresql modules: use postgres.exec_sql instead of __exec_sql methods (#57674)

* postgresql modules: use postgres.exec_sql instead of __exec_sql methods

* postgresql modules: use exec_sql, added changelog fragment

* Update changelogs/fragments/57674-postgres_modules_use_exec_sql_instead_of_methods.yml

Co-Authored-By: Felix Fontein <felix@fontein.de>

changelogs/fragments/57674-postgres_modules_use_exec_sql_instead_of_methods.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - postgresql modules - use ``module_utils.postgres.exec_sql`` function instead of ``__exec_sql`` method (https://github.com/ansible/ansible/pull/57674)

lib/ansible/modules/database/postgresql/postgresql_membership.py

@@ -144,7 +144,11 @@ except ImportError:
 
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.database import SQLParseError, pg_quote_identifier
-from ansible.module_utils.postgres import connect_to_db, postgres_common_argument_spec
+from ansible.module_utils.postgres import (
+    connect_to_db,
+    exec_sql,
+    postgres_common_argument_spec,
+)
 from ansible.module_utils._text import to_native
 
 
@@ -173,7 +177,7 @@ class PgMembership(object):
 
                 query = "GRANT %s TO %s" % ((pg_quote_identifier(group, 'role'),
                                             (pg_quote_identifier(role, 'role'))))
-                self.changed = self.__exec_sql(query, ddl=True)
+                self.changed = exec_sql(self, query, ddl=True)
 
                 if self.changed:
                     self.granted[group].append(role)
@@ -191,7 +195,7 @@ class PgMembership(object):
 
                 query = "REVOKE %s FROM %s" % ((pg_quote_identifier(group, 'role'),
                                                (pg_quote_identifier(role, 'role'))))
-                self.changed = self.__exec_sql(query, ddl=True)
+                self.changed = exec_sql(self, query, ddl=True)
 
                 if self.changed:
                     self.revoked[group].append(role)
@@ -206,7 +210,7 @@ class PgMembership(object):
                  "FROM pg_catalog.pg_roles r "
                  "WHERE r.rolname = '%s'" % dst_role)
 
-        res = self.__exec_sql(query, add_to_executed=False)
+        res = exec_sql(self, query, add_to_executed=False)
         membership = []
         if res:
             membership = res[0][0]
@@ -252,22 +256,7 @@ class PgMembership(object):
         self.target_roles = [r for r in self.target_roles if r not in self.non_existent_roles]
 
     def __role_exists(self, role):
-        return self.__exec_sql("SELECT 1 FROM pg_roles WHERE rolname = '%s'" % role, add_to_executed=False)
+        return exec_sql(self, "SELECT 1 FROM pg_roles WHERE rolname = '%s'" % role, add_to_executed=False)
-
-    def __exec_sql(self, query, ddl=False, add_to_executed=True):
-        try:
-            self.cursor.execute(query)
-
-            if add_to_executed:
-                self.executed_queries.append(query)
-
-            if not ddl:
-                res = self.cursor.fetchall()
-                return res
-            return True
-        except Exception as e:
-            self.module.fail_json(msg="Cannot execute SQL '%s': %s" % (query, to_native(e)))
-        return False
 
 
 # ===========================================

lib/ansible/modules/database/postgresql/postgresql_owner.py

@@ -158,7 +158,11 @@ except ImportError:
 
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.database import SQLParseError, pg_quote_identifier
-from ansible.module_utils.postgres import connect_to_db, postgres_common_argument_spec
+from ansible.module_utils.postgres import (
+    connect_to_db,
+    exec_sql,
+    postgres_common_argument_spec,
+)
 from ansible.module_utils._text import to_native
 
 
@@ -238,7 +242,7 @@ class PgOwnership(object):
         query.append('TO %s' % pg_quote_identifier(self.role, 'role'))
         query = ' '.join(query)
 
-        self.changed = self.__exec_sql(query, ddl=True)
+        self.changed = exec_sql(self, query, ddl=True)
 
     def set_owner(self, obj_type, obj_name):
         """Change owner of a database object.
@@ -326,85 +330,59 @@ class PgOwnership(object):
                      "WHERE matviewname = '%s' "
                      "AND matviewowner = '%s'" % (self.obj_name, self.role))
 
-        return self.__exec_sql(query, add_to_executed=False)
+        return exec_sql(self, query, add_to_executed=False)
 
     def __set_db_owner(self):
         """Set the database owner."""
         query = "ALTER DATABASE %s OWNER TO %s" % (pg_quote_identifier(self.obj_name, 'database'),
                                                    pg_quote_identifier(self.role, 'role'))
-        self.changed = self.__exec_sql(query, ddl=True)
+        self.changed = exec_sql(self, query, ddl=True)
 
     def __set_func_owner(self):
         """Set the function owner."""
         query = "ALTER FUNCTION %s OWNER TO %s" % (self.obj_name,
                                                    pg_quote_identifier(self.role, 'role'))
-        self.changed = self.__exec_sql(query, ddl=True)
+        self.changed = exec_sql(self, query, ddl=True)
 
     def __set_seq_owner(self):
         """Set the sequence owner."""
         query = "ALTER SEQUENCE %s OWNER TO %s" % (pg_quote_identifier(self.obj_name, 'table'),
                                                    pg_quote_identifier(self.role, 'role'))
-        self.changed = self.__exec_sql(query, ddl=True)
+        self.changed = exec_sql(self, query, ddl=True)
 
     def __set_schema_owner(self):
         """Set the schema owner."""
         query = "ALTER SCHEMA %s OWNER TO %s" % (pg_quote_identifier(self.obj_name, 'schema'),
                                                  pg_quote_identifier(self.role, 'role'))
-        self.changed = self.__exec_sql(query, ddl=True)
+        self.changed = exec_sql(self, query, ddl=True)
 
     def __set_table_owner(self):
         """Set the table owner."""
         query = "ALTER TABLE %s OWNER TO %s" % (pg_quote_identifier(self.obj_name, 'table'),
                                                 pg_quote_identifier(self.role, 'role'))
-        self.changed = self.__exec_sql(query, ddl=True)
+        self.changed = exec_sql(self, query, ddl=True)
 
     def __set_tablespace_owner(self):
         """Set the tablespace owner."""
         query = "ALTER TABLESPACE %s OWNER TO %s" % (pg_quote_identifier(self.obj_name, 'database'),
                                                      pg_quote_identifier(self.role, 'role'))
-        self.changed = self.__exec_sql(query, ddl=True)
+        self.changed = exec_sql(self, query, ddl=True)
 
     def __set_view_owner(self):
         """Set the view owner."""
         query = "ALTER VIEW %s OWNER TO %s" % (pg_quote_identifier(self.obj_name, 'table'),
                                                pg_quote_identifier(self.role, 'role'))
-        self.changed = self.__exec_sql(query, ddl=True)
+        self.changed = exec_sql(self, query, ddl=True)
 
     def __set_mat_view_owner(self):
         """Set the materialized view owner."""
         query = "ALTER MATERIALIZED VIEW %s OWNER TO %s" % (pg_quote_identifier(self.obj_name, 'table'),
                                                             pg_quote_identifier(self.role, 'role'))
-        self.changed = self.__exec_sql(query, ddl=True)
+        self.changed = exec_sql(self, query, ddl=True)
 
     def __role_exists(self, role):
         """Return True if role exists, otherwise return Fasle."""
-        return self.__exec_sql("SELECT 1 FROM pg_roles WHERE rolname = '%s'" % role, add_to_executed=False)
+        return exec_sql(self, "SELECT 1 FROM pg_roles WHERE rolname = '%s'" % role, add_to_executed=False)
-
-    def __exec_sql(self, query, ddl=False, add_to_executed=True):
-        """Execute SQL.
-
-        Return a query result if possible or True/False if ddl=True arg was passed.
-        It's necessary for statements that don't return any result (like DDL queries).
-
-        Arguments:
-            query (str): SQL query to execute.
-            ddl (bool): Must return True or False instead of rows
-                (typical for DDL queries) (default False).
-            add_to_executed (bool): Append the query to self.executed_queries attr.
-        """
-        try:
-            self.cursor.execute(query)
-
-            if add_to_executed:
-                self.executed_queries.append(query)
-
-            if not ddl:
-                res = self.cursor.fetchall()
-                return res
-            return True
-        except Exception as e:
-            self.module.fail_json(msg="Cannot execute SQL '%s': %s" % (query, to_native(e)))
-        return False
 
 
 # ===========================================

lib/ansible/modules/database/postgresql/postgresql_ping.py

@@ -80,7 +80,11 @@ except ImportError:
 
 from ansible.module_utils.basic import AnsibleModule, missing_required_lib
 from ansible.module_utils.database import SQLParseError
-from ansible.module_utils.postgres import connect_to_db, postgres_common_argument_spec
+from ansible.module_utils.postgres import (
+    connect_to_db,
+    exec_sql,
+    postgres_common_argument_spec,
+)
 from ansible.module_utils._text import to_native
 from ansible.module_utils.six import iteritems
 
@@ -103,7 +107,7 @@ class PgPing(object):
 
     def get_pg_version(self):
         query = "SELECT version()"
-        raw = self.__exec_sql(query)[0][0]
+        raw = exec_sql(self, query, add_to_executed=False)[0][0]
         if raw:
             self.is_available = True
             raw = raw.split()[1].split('.')
@@ -112,16 +116,6 @@ class PgPing(object):
                 minor=int(raw[1]),
             )
 
-    def __exec_sql(self, query):
-        try:
-            self.cursor.execute(query)
-            res = self.cursor.fetchall()
-            if res:
-                return res
-        except Exception as e:
-            self.module.fail_json("Unable to execute '%s': %s" % (query, to_native(e)))
-
-        return False
 
 # ===========================================
 # Module execution.

lib/ansible/modules/database/postgresql/postgresql_slot.py

@@ -150,7 +150,11 @@ except ImportError:
 
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.database import SQLParseError
-from ansible.module_utils.postgres import connect_to_db, postgres_common_argument_spec
+from ansible.module_utils.postgres import (
+    connect_to_db,
+    exec_sql,
+    postgres_common_argument_spec,
+)
 from ansible.module_utils._text import to_native
 
 
@@ -192,37 +196,22 @@ class PgSlot(object):
         elif kind == 'logical':
             query = "SELECT pg_create_logical_replication_slot('%s', '%s')" % (self.name, output_plugin)
 
-        self.changed = self.__exec_sql(query, ddl=True)
+        self.changed = exec_sql(self, query, ddl=True)
 
     def drop(self):
         if not self.exists:
             return False
 
         query = "SELECT pg_drop_replication_slot('%s')" % self.name
-        self.changed = self.__exec_sql(query, ddl=True)
+        self.changed = exec_sql(self, query, ddl=True)
 
     def __slot_exists(self):
         query = "SELECT slot_type FROM pg_replication_slots WHERE slot_name = '%s'" % self.name
-        res = self.__exec_sql(query, add_to_executed=False)
+        res = exec_sql(self, query, add_to_executed=False)
         if res:
             self.exists = True
             self.kind = res[0][0]
 
-    def __exec_sql(self, query, ddl=False, add_to_executed=True):
-        try:
-            self.cursor.execute(query)
-
-            if add_to_executed:
-                self.executed_queries.append(query)
-
-            if not ddl:
-                res = self.cursor.fetchall()
-                return res
-            return True
-        except Exception as e:
-            self.module.fail_json(msg="Cannot execute SQL '%s': %s" % (query, to_native(e)))
-        return False
-
 
 # ===========================================
 # Module execution.

lib/ansible/modules/database/postgresql/postgresql_table.py

@@ -237,7 +237,11 @@ except ImportError:
 
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.database import SQLParseError, pg_quote_identifier
-from ansible.module_utils.postgres import connect_to_db, postgres_common_argument_spec
+from ansible.module_utils.postgres import (
+    connect_to_db,
+    exec_sql,
+    postgres_common_argument_spec,
+)
 from ansible.module_utils._text import to_native
 
 
@@ -278,7 +282,7 @@ class Table(object):
                  "INNER JOIN pg_namespace AS n ON c.relnamespace = n.oid "
                  "WHERE t.tablename = '%s' "
                  "AND n.nspname = '%s'" % (tblname, schema))
-        res = self.__exec_sql(query)
+        res = exec_sql(self, query, add_to_executed=False)
         if res:
             self.exists = True
             self.info = dict(
@@ -354,8 +358,7 @@ class Table(object):
         if tblspace:
             query += " TABLESPACE %s" % pg_quote_identifier(tblspace, 'database')
 
-        if self.__exec_sql(query, ddl=True):
+        if exec_sql(self, query, ddl=True):
-            self.executed_queries.append(query)
             changed = True
 
         if owner:
@@ -402,8 +405,7 @@ class Table(object):
         if tblspace:
             query += " TABLESPACE %s" % pg_quote_identifier(tblspace, 'database')
 
-        if self.__exec_sql(query, ddl=True):
+        if exec_sql(self, query, ddl=True):
-            self.executed_queries.append(query)
             changed = True
 
         if owner:
@@ -413,20 +415,17 @@ class Table(object):
 
     def truncate(self):
         query = "TRUNCATE TABLE %s" % pg_quote_identifier(self.name, 'table')
-        self.executed_queries.append(query)
+        return exec_sql(self, query, ddl=True)
-        return self.__exec_sql(query, ddl=True)
 
     def rename(self, newname):
         query = "ALTER TABLE %s RENAME TO %s" % (pg_quote_identifier(self.name, 'table'),
                                                  pg_quote_identifier(newname, 'table'))
-        self.executed_queries.append(query)
+        return exec_sql(self, query, ddl=True)
-        return self.__exec_sql(query, ddl=True)
 
     def set_owner(self, username):
         query = "ALTER TABLE %s OWNER TO %s" % (pg_quote_identifier(self.name, 'table'),
                                                 pg_quote_identifier(username, 'role'))
-        self.executed_queries.append(query)
+        return exec_sql(self, query, ddl=True)
-        return self.__exec_sql(query, ddl=True)
 
     def drop(self, cascade=False):
         if not self.exists:
@@ -435,30 +434,16 @@ class Table(object):
         query = "DROP TABLE %s" % pg_quote_identifier(self.name, 'table')
         if cascade:
             query += " CASCADE"
-        self.executed_queries.append(query)
+        return exec_sql(self, query, ddl=True)
-        return self.__exec_sql(query, ddl=True)
 
     def set_tblspace(self, tblspace):
         query = "ALTER TABLE %s SET TABLESPACE %s" % (pg_quote_identifier(self.name, 'table'),
                                                       pg_quote_identifier(tblspace, 'database'))
-        self.executed_queries.append(query)
+        return exec_sql(self, query, ddl=True)
-        return self.__exec_sql(query, ddl=True)
 
     def set_stor_params(self, params):
         query = "ALTER TABLE %s SET (%s)" % (pg_quote_identifier(self.name, 'table'), params)
-        self.executed_queries.append(query)
+        return exec_sql(self, query, ddl=True)
-        return self.__exec_sql(query, ddl=True)
-
-    def __exec_sql(self, query, ddl=False):
-        try:
-            self.cursor.execute(query)
-            if not ddl:
-                res = self.cursor.fetchall()
-                return res
-            return True
-        except Exception as e:
-            self.module.fail_json(msg="Cannot execute SQL '%s': %s" % (query, to_native(e)))
-        return False
 
 
 # ===========================================

lib/ansible/modules/database/postgresql/postgresql_tablespace.py

@@ -173,7 +173,11 @@ except ImportError:
 
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.database import SQLParseError, pg_quote_identifier
-from ansible.module_utils.postgres import connect_to_db, postgres_common_argument_spec
+from ansible.module_utils.postgres import (
+    connect_to_db,
+    exec_sql,
+    postgres_common_argument_spec,
+)
 from ansible.module_utils._text import to_native
 
 
@@ -216,14 +220,14 @@ class PgTablespace(object):
         """Get tablespace information."""
 
         # Check that spcoptions exists:
-        opt = self.__exec_sql("SELECT 1 FROM information_schema.columns "
+        opt = exec_sql(self, "SELECT 1 FROM information_schema.columns "
-                              "WHERE table_name = 'pg_tablespace' "
+                             "WHERE table_name = 'pg_tablespace' "
-                              "AND column_name = 'spcoptions'", add_to_executed=False)
+                             "AND column_name = 'spcoptions'", add_to_executed=False)
 
         # For 9.1 version and earlier:
-        location = self.__exec_sql("SELECT 1 FROM information_schema.columns "
+        location = exec_sql(self, "SELECT 1 FROM information_schema.columns "
-                                   "WHERE table_name = 'pg_tablespace' "
+                                  "WHERE table_name = 'pg_tablespace' "
-                                   "AND column_name = 'spclocation'", add_to_executed=False)
+                                  "AND column_name = 'spclocation'", add_to_executed=False)
         if location:
             location = 'spclocation'
         else:
@@ -243,7 +247,7 @@ class PgTablespace(object):
                      "ON t.spcowner = r.oid "
                      "WHERE t.spcname = '%s'" % (location, self.name))
 
-        res = self.__exec_sql(query, add_to_executed=False)
+        res = exec_sql(self, query, add_to_executed=False)
 
         if not res:
             self.exists = False
@@ -273,7 +277,7 @@ class PgTablespace(object):
         """
 
         query = ("CREATE TABLESPACE %s LOCATION '%s'" % (pg_quote_identifier(self.name, 'database'), location))
-        return self.__exec_sql(query, ddl=True)
+        return exec_sql(self, query, ddl=True)
 
     def drop(self):
         """Drop tablespace.
@@ -281,7 +285,7 @@ class PgTablespace(object):
         Return True if success, otherwise, return False.
         """
 
-        return self.__exec_sql("DROP TABLESPACE %s" % pg_quote_identifier(self.name, 'database'), ddl=True)
+        return exec_sql(self, "DROP TABLESPACE %s" % pg_quote_identifier(self.name, 'database'), ddl=True)
 
     def set_owner(self, new_owner):
         """Set tablespace owner.
@@ -296,7 +300,7 @@ class PgTablespace(object):
             return False
 
         query = "ALTER TABLESPACE %s OWNER TO %s" % (pg_quote_identifier(self.name, 'database'), new_owner)
-        return self.__exec_sql(query, ddl=True)
+        return exec_sql(self, query, ddl=True)
 
     def rename(self, newname):
         """Rename tablespace.
@@ -309,7 +313,7 @@ class PgTablespace(object):
 
         query = "ALTER TABLESPACE %s RENAME TO %s" % (pg_quote_identifier(self.name, 'database'), newname)
         self.new_name = newname
-        return self.__exec_sql(query, ddl=True)
+        return exec_sql(self, query, ddl=True)
 
     def set_settings(self, new_settings):
         """Set tablespace settings (options).
@@ -349,7 +353,7 @@ class PgTablespace(object):
         """
 
         query = "ALTER TABLESPACE %s RESET (%s)" % (pg_quote_identifier(self.name, 'database'), setting)
-        return self.__exec_sql(query, ddl=True)
+        return exec_sql(self, query, ddl=True)
 
     def __set_setting(self, setting):
         """Set tablespace setting.
@@ -361,33 +365,7 @@ class PgTablespace(object):
         """
 
         query = "ALTER TABLESPACE %s SET (%s)" % (pg_quote_identifier(self.name, 'database'), setting)
-        return self.__exec_sql(query, ddl=True)
+        return exec_sql(self, query, ddl=True)
-
-    def __exec_sql(self, query, ddl=False, add_to_executed=True):
-        """Execute SQL query.
-
-        Return a query result if possible or True/False if ddl=True arg was passed.
-        It's necessary for statements that don't return any result (like DDL queries).
-
-        args:
-            query (str) -- SQL query to execute
-            ddl (bool) -- must return True or False instead of rows (typical for DDL queries)
-            add_to_executed (bool) -- append the query to self.executed_queries list
-        """
-
-        try:
-            self.cursor.execute(query)
-
-            if add_to_executed:
-                self.executed_queries.append(query)
-
-            if not ddl:
-                res = self.cursor.fetchall()
-                return res
-            return True
-        except Exception as e:
-            self.module.fail_json(msg="Cannot execute SQL '%s': %s" % (query, to_native(e)))
-        return False
 
 
 # ===========================================