Fixes syntax issues in backup/restore and gparray.

These are minor fixes identified through static code analysis.

Add unit tests to gprestore filter and restore.

Authors: Chumki Roy and Chris Hajas

gpMgmt/bin/gpcrondump

@@ -859,9 +859,9 @@ class GpCronDump(Operation):
             if self.context.history:
                 # This certainly does not belong under DumpDatabase, due to CLI assumptions. Note the use of options_list.
                 UpdateHistoryTable(self.context,
-                                   self.options_list,
                                    time_start = dump_outcome['time_start'],
                                    time_end = dump_outcome['time_end'],
+                                   options_list = self.options_list,
                                    dump_exit_status = dump_outcome['exit_status'],
                                    timestamp = post_dump_outcome['timestamp'],
                                    pseudo_exit_status = current_exit_status).run()

gpMgmt/bin/gppylib/gparray.py

@@ -766,7 +766,7 @@ class Segment:
         if self.primaryDB.isSegmentPrimary(current_role=True):
             return self.primaryDB
         else:
-            for mirror in mirrorDBs:
+            for mirror in self.mirrorDBs:
                 if mirror.isSegmentPrimary(current_role=True):
                     return mirror
 
@@ -2265,6 +2265,7 @@ class GpArray:
         datadirs = {}
         used_ports = {}
         used_replication_ports = {}
+        hostname = ""
         for db in self.getDbList(True):
             datadir = db.getSegmentDataDirectory()
             hostname = db.getSegmentHostName()

gpMgmt/bin/gppylib/operations/backup_utils.py

@@ -426,7 +426,7 @@ def get_lines_from_zipped_file(fname):
     try:
         for line in fd:
             content.append(line.strip('\n'))
-    except err:
+    except Exception as err:
         raise Exception("Error reading from file %s: %s" % (fname, err))
     finally:
         fd.close()

gpMgmt/bin/gppylib/operations/restore.py

@@ -446,7 +446,7 @@ class RestoreDatabase(Operation):
                     try:
                         execSQL(conn, analyze_table)
                     except Exception as e:
-                        raise Exception('Issue with \'ANALYZE\' of restored table \'%s\' in \'%s\' database' % (restore_table, self.context.restore_db))
+                        raise Exception('Issue with \'ANALYZE\' of restored table \'%s\' in \'%s\' database' % (tbl, self.context.restore_db))
                     else:
                         num_sqls += 1
                         if num_sqls == 1000: # The choice of batch size was choosen arbitrarily

gpMgmt/bin/gppylib/operations/test/unit/test_unit_restore.py

@@ -1017,6 +1017,14 @@ CREATE DATABASE monkey WITH TEMPLATE = template0 ENCODING = 'UTF8' OWNER = thisg
         self.context.change_schema = 'newschema'
         self.restore._analyze_restore_tables()
 
+    @patch('gppylib.operations.restore.execSQL', side_effect=Exception())
+    @patch('gppylib.operations.backup_utils.dbconn.DbURL')
+    @patch('gppylib.operations.backup_utils.dbconn.connect')
+    def test_analyze_restore_tables_execSQL_failed(self, mock1, mock2, mock3):
+        self.context.restore_db = 'db1'
+        self.context.restore_tables = ['public.t1', 'public.t2']
+        self.assertRaisesRegexp(Exception, 'Issue with \'ANALYZE\' of restored table \'"public"."t1"\' in \'db1\' database', self.restore._analyze_restore_tables)
+
     @patch('os.path.exists', side_effect=[True, False])
     def test_validate_metadata_file_with_compression_exists(self, mock):
         compressed_file = 'compressed_file.gz'

gpMgmt/bin/gppylib/test/unit/test_unit_gprestore_filter.py

@@ -7,7 +7,8 @@ from gppylib import gplog
 from mock import patch
 from gppylib.mainUtils import ExceptionNoStackTraceNeeded
 from gprestore_filter import get_table_schema_set, extract_schema, extract_table, \
-                            process_data, get_table_info, process_schema, check_valid_schema, check_valid_table, check_dropped_table
+                            process_data, get_table_info, process_schema, check_valid_schema, check_valid_table, \
+                            check_dropped_table, get_table_from_alter_table
 
 
 logger = gplog.get_unittest_logger()
@@ -68,6 +69,42 @@ class GpRestoreFilterTestCase(unittest.TestCase):
         with self.assertRaisesRegexp(Exception, "Failed to extract table name"):
             table = extract_table(line)
 
+    def test_get_table_from_alter_table_with_schemaname(self):
+        line = 'ALTER TABLE schema1.table1 OWNER TO gpadmin;'
+        alter_expr = "ALTER TABLE"
+        res = get_table_from_alter_table(line, alter_expr)
+        self.assertEqual(res, 'table1')
+
+    def test_get_table_from_alter_table_without_schemaname(self):
+        line = 'ALTER TABLE table1 OWNER TO gpadmin;'
+        alter_expr = "ALTER TABLE"
+        res = get_table_from_alter_table(line, alter_expr)
+        self.assertEqual(res, 'table1')
+
+    def test_get_table_from_alter_table_with_specialchar(self):
+        line = 'ALTER TABLE Tab#$_1 OWNER TO gpadmin;'
+        alter_expr = "ALTER TABLE"
+        res = get_table_from_alter_table(line, alter_expr)
+        self.assertEqual(res, 'Tab#$_1')
+
+    def test_get_table_from_alter_table_with_specialchar_and_schema(self):
+        line = 'ALTER TABLE "Foo#$1"."Tab#$_1" OWNER TO gpadmin;'
+        alter_expr = "ALTER TABLE"
+        res = get_table_from_alter_table(line, alter_expr)
+        self.assertEqual(res, '"Tab#$_1"')
+
+    def test_get_table_from_alter_table_with_specialchar(self):
+        line = 'ALTER TABLE "T a""b#$_1" OWNER TO gpadmin;'
+        alter_expr = "ALTER TABLE"
+        res = get_table_from_alter_table(line, alter_expr)
+        self.assertEqual(res, '"T a""b#$_1"')
+
+    def test_get_table_from_alter_table_with_specialchar_and_double_quoted_schema(self):
+        line = 'ALTER TABLE "schema1".table1 OWNER TO gpadmin;'
+        alter_expr = "ALTER TABLE"
+        res = get_table_from_alter_table(line, alter_expr)
+        self.assertEqual(res, 'table1')
+
     def test_process_data00(self):
 
         test_case_buf = """

gpMgmt/bin/gprestore_filter.py

@@ -71,17 +71,17 @@ def get_table_from_alter_table(line, alter_expr):
     has_special_chars = True if last_double_quote_idx != -1 else False
 
     if not has_schema_table_fmt and not has_special_chars:
-        line[len(alter_expr):].split()[0]
+        return line[len(alter_expr):].split()[0]
     elif has_schema_table_fmt and not has_special_chars:
         full_table_name = line[len(alter_expr):].split()[0]
         _, table = split_fqn(full_table_name)
         return table
     elif not has_schema_table_fmt and has_special_chars:
-        return line[len(alter_expr) : last_double_quote_idx + 1]
+        return line[len(alter_expr) + 1 : last_double_quote_idx + 1]
     else:
         if dot_separator_idx < last_double_quote_idx:
             # table name is double quoted
-            full_table_name = line[len(alter_expr) : last_double_idx + 1]
+            full_table_name = line[len(alter_expr) : last_double_quote_idx + 1]
         else:
             # only schema name double quoted
             ending_space_idx = line.find(' ', dot_separator_idx)