Fix restore helper hides actual err (#100)

The commit 4371a94 moved the LoopEnd label so that the err variable began to be
overwritten and it will no longer be used below. To fix it, this patch renames
the err variable returned by flushAndCloseRestoreWriter to errPipe.

The integration test has been adapted to error-free behavior.

end_to_end/end_to_end_suite_test.go

@@ -2519,6 +2519,19 @@ LANGUAGE plpgsql NO SQL;`)
 			Expect(string(output)).To(MatchRegexp(`Error loading data into table public.d: COPY d, line 1: "\d,d": ERROR: null value in column "c" violates not-null constraint`))
 			Expect(string(output)).To(MatchRegexp(`Error loading data into table public.e: COPY e, line 1: "\d,e": ERROR: null value in column "c" violates not-null constraint`))
 			Expect(string(output)).To(ContainSubstring(`Encountered 3 error(s) during table data restore`))
+			homeDir := os.Getenv("HOME")
+			helperFiles, _ := path.Glob(path.Join(homeDir, "gpAdminLogs/gpbackup_helper_*"))
+			helperCommand := exec.Command("grep", "Error copying data", helperFiles[len(helperFiles)-1])
+			helperOutput, _ := helperCommand.CombinedOutput()
+			Expect(string(helperOutput)).To(ContainSubstring(`Segment 2: Oid 16397, Batch 0: Error encountered: Error copying data: write`))
+			Expect(string(helperOutput)).To(ContainSubstring(`Segment 0: Oid 16397, Batch 0: Error encountered: Error copying data: write`))
+			Expect(string(helperOutput)).To(ContainSubstring(`Segment 1: Oid 16397, Batch 0: Error encountered: Error copying data: write`))
+			Expect(string(helperOutput)).To(ContainSubstring(`Segment 2: Oid 16403, Batch 0: Error encountered: Error copying data: write`))
+			Expect(string(helperOutput)).To(ContainSubstring(`Segment 0: Oid 16403, Batch 0: Error encountered: Error copying data: write`))
+			Expect(string(helperOutput)).To(ContainSubstring(`Segment 1: Oid 16403, Batch 0: Error encountered: Error copying data: write`))
+			Expect(string(helperOutput)).To(ContainSubstring(`Segment 2: Oid 16409, Batch 0: Error encountered: Error copying data: write`))
+			Expect(string(helperOutput)).To(ContainSubstring(`Segment 0: Oid 16409, Batch 0: Error encountered: Error copying data: write`))
+			Expect(string(helperOutput)).To(ContainSubstring(`Segment 1: Oid 16409, Batch 0: Error encountered: Error copying data: write`))
 			assertArtifactsCleaned("20240710143553")
 			testhelper.AssertQueryRuns(restoreConn, "DROP TABLE a; DROP TABLE b; DROP TABLE c; DROP TABLE d; DROP TABLE e; DROP TABLE f; DROP TABLE g; DROP TABLE h;")
 		})

helper/restore_helper.go

@@ -348,9 +348,9 @@ func doRestoreAgent() error {
 
 	LoopEnd:
 		logInfo(fmt.Sprintf("Oid %d, Batch %d: Closing pipe %s", tableOid, batchNum, currentPipe))
-		err = flushAndCloseRestoreWriter(currentPipe, tableOid)
-		if err != nil {
-			logVerbose(fmt.Sprintf("Oid %d, Batch %d: Failed to flush and close pipe: %s", tableOid, batchNum, err))
+		errPipe := flushAndCloseRestoreWriter(currentPipe, tableOid)
+		if errPipe != nil {
+			logVerbose(fmt.Sprintf("Oid %d, Batch %d: Failed to flush and close pipe: %s", tableOid, batchNum, errPipe))
 		}
 
 		logVerbose(fmt.Sprintf("Oid %d, Batch %d: End batch restore", tableOid, batchNum))
@@ -367,7 +367,7 @@ func doRestoreAgent() error {
 		}
 
 		logVerbose(fmt.Sprintf("Oid %d, Batch %d: Attempt to delete pipe %s", tableOid, batchNum, currentPipe))
-		errPipe := deletePipe(currentPipe)
+		errPipe = deletePipe(currentPipe)
 		if errPipe != nil {
 			logError("Oid %d, Batch %d: Failed to remove pipe %s: %v", tableOid, batchNum, currentPipe, errPipe)
 		}

integration/helper_test.go

@@ -372,11 +372,9 @@ options:
 			err = helperCmd.Start()
 			Expect(err).ToNot(HaveOccurred())
 
-			for _, i := range []int{1, 3} {
-				contents, _ := ioutil.ReadFile(fmt.Sprintf("%s_%d_0", pipeFile, i))
-				// Empty output
-				Expect(contents).To(Equal([]byte{}))
-			}
+			contents, _ := ioutil.ReadFile(fmt.Sprintf("%s_%d_0", pipeFile, 1))
+			// Empty output
+			Expect(contents).To(Equal([]byte{}))
 
 			err = helperCmd.Wait()
 			Expect(err).To(HaveOccurred())