Fix logfile_getname()

There are two issues with current logfile_getname()
- First, it doesn't honor the `gp_log_format` GUC. Originally,
when the format is CSV, then `.csv` is used, and when format is TEXT,
the `.log` if used. However, currently the `.csv` is always used regardless
of the `gp_log_format` settings, hence make the content of the file
and suffix inconsistent.

- Second, it mistakenly generate logs with wrong suffix `.csv.csv`
during logfile_rotate(), due to the wrong assumption of the filename
always containing `.log` when suffix is NULL. Also, due to the calling
sequence of the logfile_rotate, an extra empty file is generated, e.g.
in this case, the file with `.csv.csv` is always empty.

Fix in this patch bring back original GPDB behavior. After the fix,
we generated correct extension, however, still an empty extra log file
generated during log rotation.

However, a separate refactoring is required to clean up all the API
changes in all the callers of logfile_getname(), since the parameter
`suffix` is no longer needed. Also, the calling of logfile_rotate()
to fix the extra empty log file issue.
Signed-off-by: NXin Zhang <xzhang@pivotal.io>

src/backend/postmaster/syslogger.c

@@ -2085,9 +2085,11 @@ logfile_rotate(bool time_based_rotation, bool size_based_rotation,
 /*
  * construct logfile name using timestamp information
  *
- * If suffix isn't NULL, append it to the name, replacing any ".log"
+ * In Postgres, if suffix isn't NULL, append it to the name, replacing any ".log"
  * that may be in the pattern.
  *
+ * In GPDB, parameter suffix is not used. A separate refactor is needed for the API change.
+ *
  * Result is palloc'd.
  */
 static char *
@@ -2096,6 +2098,7 @@ logfile_getname(pg_time_t timestamp, const char *suffix, const char *log_directo
     char	   *filename;
     int			len;
 #define CSV_SUFFIX ".csv"
+#define LOG_SUFFIX ".log"
 
     filename = palloc(MAXPGPATH);
 
@@ -2116,10 +2119,6 @@ logfile_getname(pg_time_t timestamp, const char *suffix, const char *log_directo
                  log_file_pattern, (unsigned long) timestamp);
     }
 
-	/* GPDB_83_MERGE_FIXME: The upstream code does the ".log" -> ".csv" transformation,
-	 * but not the other one. I doubt we really need that, so I think we should revert
-	 * this to the way it is in upstream, but will need to double-check. */
-#if 0
 	/*
 	 * If the logging format is 'TEXT' and the filename ends with ".csv",
 	 * replace ".csv" with ".log".
@@ -2127,10 +2126,12 @@ logfile_getname(pg_time_t timestamp, const char *suffix, const char *log_directo
 	 * If the logging format is 'CSV' and the filename does not end with ".csv",
 	 * replace the last four characters in the filename with ".cvs".
 	 */
-	{
 	if (strlen(filename) - sizeof(CSV_SUFFIX) + 1 > 0)
+	{
 		suffix = filename + (strlen(filename) - sizeof(CSV_SUFFIX) + 1);
+	}
 	else
+	{
 		/*
 		 * Point the suffix to the end of string if the length of
 		 * the filename is less than ".csv".
@@ -2138,24 +2139,15 @@ logfile_getname(pg_time_t timestamp, const char *suffix, const char *log_directo
 		suffix = filename + strlen(filename);
 	}
 	
-	if (gp_log_format == 0 && pg_strcasecmp(suffix, CSV_SUFFIX) == 0)
+	if (gp_log_format == 0 && pg_strcasecmp(suffix, LOG_SUFFIX) != 0)
 	{
-		snprintf(suffix, sizeof(CSV_SUFFIX), ".log");
+		snprintf(suffix, sizeof(LOG_SUFFIX), LOG_SUFFIX);
 	}
 	
 	if (gp_log_format == 1 && pg_strcasecmp(suffix, CSV_SUFFIX) != 0)
 	{
 		snprintf(suffix, sizeof(CSV_SUFFIX), CSV_SUFFIX);
 	}
-#endif
-
-	if (suffix != NULL)
-	{
-		len = strlen(filename);
-		if (len > 4 && (strcmp(filename + (len - 4), ".log") == 0))
-			len -= 4;
-		strlcpy(filename + len, suffix, MAXPGPATH - len);
-	}
 
     return filename;
 }