Refactor postgresql modules (#291)

* Refactor postgresql_idx to simplify code

Have refactored the postgresql_idx module to:
* Use the class schema in the drop function rather than a passed in one
* Remove the if/else and just return the bool in the drop and create
  functions

* Refactor postgresql_ext module

Have refactored the postgresql_ext module to:
* Remove an unused exception
* Simplify logic in the create and update functions
* Use list comprehension to simplify getting the available version

* Refactor postgresql_user_obj_stat_info module

Have refactored the postgresql_user_obj_stat_info module to:
* Simplify the logic in some of the functions
* Reduce duplicate code

* Add changelog fragment

changelogs/fragments/291-postgresql_refactor_modules.yml

@@ -0,0 +1,5 @@
+---
+minor_changes:
+  - postgresql_ext - refactor to simplify and remove dead code (https://github.com/ansible-collections/community.general/pull/291)
+  - postgresql_idx - refactor to simplify code (https://github.com/ansible-collections/community.general/pull/291)
+  - postgresql_user_obj_stat_info - refactor to simplify code (https://github.com/ansible-collections/community.general/pull/291)

plugins/modules/database/postgresql/postgresql_ext.py

@@ -195,10 +195,6 @@ from ansible.module_utils._text import to_native
 executed_queries = []
 
 
-class NotSupportedError(Exception):
-    pass
-
-
 # ===========================================
 # PostgreSQL module specific support methods.
 #
@@ -231,32 +227,33 @@ def ext_update_version(cursor, ext, version):
       ext (str) -- extension name
       version (str) -- extension version
     """
+    query = "ALTER EXTENSION \"%s\" UPDATE" % ext
+    params = {}
+
     if version != 'latest':
-        query = ("ALTER EXTENSION \"%s\"" % ext)
-        cursor.execute(query + " UPDATE TO %(ver)s", {'ver': version})
-        executed_queries.append(cursor.mogrify(query + " UPDATE TO %(ver)s", {'ver': version}))
-    else:
-        query = ("ALTER EXTENSION \"%s\" UPDATE" % ext)
-        cursor.execute(query)
-        executed_queries.append(query)
+        query += " TO %(ver)s"
+        params['ver'] = version
+
+    cursor.execute(query, params)
+    executed_queries.append(cursor.mogrify(query, params))
+
     return True
 
 
 def ext_create(cursor, ext, schema, cascade, version):
     query = "CREATE EXTENSION \"%s\"" % ext
+    params = {}
+
     if schema:
         query += " WITH SCHEMA \"%s\"" % schema
     if version:
         query += " VERSION %(ver)s"
+        params['ver'] = version
     if cascade:
         query += " CASCADE"
 
-    if version:
-        cursor.execute(query, {'ver': version})
-        executed_queries.append(cursor.mogrify(query, {'ver': version}))
-    else:
-        cursor.execute(query)
-        executed_queries.append(query)
+    cursor.execute(query, params)
+    executed_queries.append(cursor.mogrify(query, params))
     return True
 
 
@@ -292,12 +289,11 @@ def ext_get_versions(cursor, ext):
     cursor.execute(query, {'ext': ext})
     res = cursor.fetchall()
 
-    available_versions = []
-    if res:
-        # Make the list of available versions:
-        for line in res:
-            if LooseVersion(line[0]) > LooseVersion(current_version):
-                available_versions.append(line['version'])
+    available_versions = [
+        line['version']
+        for line in res
+        if LooseVersion(line['version']) > LooseVersion(current_version)
+    ]
 
     if current_version == '0':
         current_version = False
@@ -372,7 +368,7 @@ def main():
                                              "the passed version is not available" % (version, curr_version))
 
                 # If the specific version is passed and it is higher that the current version:
-                if curr_version and version:
+                if curr_version:
                     if LooseVersion(curr_version) < LooseVersion(version):
                         if module.check_mode:
                             changed = True

plugins/modules/database/postgresql/postgresql_idx.py

@@ -145,6 +145,7 @@ notes:
 
 author:
 - Andrew Klychkov (@Andersson007)
+- Thomas O'Donnell (@andytom)
 
 extends_documentation_fragment:
 - community.general.postgres
@@ -418,12 +419,9 @@ class Index(object):
 
         self.executed_query = query
 
-        if exec_sql(self, query, return_bool=True, add_to_executed=False):
-            return True
+        return exec_sql(self, query, return_bool=True, add_to_executed=False)
 
-        return False
-
-    def drop(self, schema, cascade=False, concurrent=True):
+    def drop(self, cascade=False, concurrent=True):
         """Drop PostgreSQL index.
 
         Return True if success, otherwise, return False.
@@ -444,20 +442,14 @@ class Index(object):
         if concurrent:
             query += ' CONCURRENTLY'
 
-        if not schema:
-            query += ' "public"."%s"' % self.name
-        else:
-            query += ' "%s"."%s"' % (schema, self.name)
+        query += ' "%s"."%s"' % (self.schema, self.name)
 
         if cascade:
             query += ' CASCADE'
 
         self.executed_query = query
 
-        if exec_sql(self, query, return_bool=True, add_to_executed=False):
-            return True
-
-        return False
+        return exec_sql(self, query, return_bool=True, add_to_executed=False)
 
 
 # ===========================================
@@ -579,7 +571,7 @@ def main():
             kw['query'] = index.executed_query
 
     else:
-        changed = index.drop(schema, cascade, concurrent)
+        changed = index.drop(cascade, concurrent)
 
         if changed:
             kw['state'] = 'absent'

plugins/modules/database/postgresql/postgresql_user_obj_stat_info.py

@@ -191,13 +191,12 @@ class PgUserObjStatInfo():
 
     def get_func_stat(self):
         """Get function statistics and fill out self.info dictionary."""
-        if not self.schema:
-            query = "SELECT * FROM pg_stat_user_functions"
-            result = exec_sql(self, query, add_to_executed=False)
-        else:
+        query = "SELECT * FROM pg_stat_user_functions"
+        if self.schema:
             query = "SELECT * FROM pg_stat_user_functions WHERE schemaname = %s"
-            result = exec_sql(self, query, query_params=(self.schema,),
-                              add_to_executed=False)
+
+        result = exec_sql(self, query, query_params=(self.schema,),
+                          add_to_executed=False)
 
         if not result:
             return
@@ -209,13 +208,12 @@ class PgUserObjStatInfo():
 
     def get_idx_stat(self):
         """Get index statistics and fill out self.info dictionary."""
-        if not self.schema:
-            query = "SELECT * FROM pg_stat_user_indexes"
-            result = exec_sql(self, query, add_to_executed=False)
-        else:
+        query = "SELECT * FROM pg_stat_user_indexes"
+        if self.schema:
             query = "SELECT * FROM pg_stat_user_indexes WHERE schemaname = %s"
-            result = exec_sql(self, query, query_params=(self.schema,),
-                              add_to_executed=False)
+
+        result = exec_sql(self, query, query_params=(self.schema,),
+                          add_to_executed=False)
 
         if not result:
             return
@@ -227,13 +225,12 @@ class PgUserObjStatInfo():
 
     def get_tbl_stat(self):
         """Get table statistics and fill out self.info dictionary."""
-        if not self.schema:
-            query = "SELECT * FROM pg_stat_user_tables"
-            result = exec_sql(self, query, add_to_executed=False)
-        else:
+        query = "SELECT * FROM pg_stat_user_tables"
+        if self.schema:
             query = "SELECT * FROM pg_stat_user_tables WHERE schemaname = %s"
-            result = exec_sql(self, query, query_params=(self.schema,),
-                              add_to_executed=False)
+
+        result = exec_sql(self, query, query_params=(self.schema,),
+                          add_to_executed=False)
 
         if not result:
             return
@@ -262,30 +259,22 @@ class PgUserObjStatInfo():
                     self.info[info_key][elem[schema_key]][elem[name_key]][key] = val
 
             if info_key in ('tables', 'indexes'):
-                relname = elem[name_key]
                 schemaname = elem[schema_key]
-                if not self.schema:
-                    result = exec_sql(self, "SELECT pg_relation_size ('%s.%s')" % (schemaname, relname),
-                                      add_to_executed=False)
-                else:
-                    relname = '%s.%s' % (self.schema, relname)
-                    result = exec_sql(self, "SELECT pg_relation_size (%s)",
-                                      query_params=(relname,),
-                                      add_to_executed=False)
+                if self.schema:
+                    schemaname = self.schema
+
+                relname = '%s.%s' % (schemaname, elem[name_key])
+
+                result = exec_sql(self, "SELECT pg_relation_size (%s)",
+                                  query_params=(relname,),
+                                  add_to_executed=False)
 
                 self.info[info_key][elem[schema_key]][elem[name_key]]['size'] = result[0][0]
 
                 if info_key == 'tables':
-                    relname = elem[name_key]
-                    schemaname = elem[schema_key]
-                    if not self.schema:
-                        result = exec_sql(self, "SELECT pg_total_relation_size ('%s.%s')" % (schemaname, relname),
-                                          add_to_executed=False)
-                    else:
-                        relname = '%s.%s' % (self.schema, relname)
-                        result = exec_sql(self, "SELECT pg_total_relation_size (%s)",
-                                          query_params=(relname,),
-                                          add_to_executed=False)
+                    result = exec_sql(self, "SELECT pg_total_relation_size (%s)",
+                                      query_params=(relname,),
+                                      add_to_executed=False)
 
                     self.info[info_key][elem[schema_key]][elem[name_key]]['total_size'] = result[0][0]