Hi Chao-San.

A couple of comments for your v1-0001 cleanup patch.

======
src/bin/pg_dump/pg_dumpall.c

I guess you were just making the minimal changes, but I thought
parseDumpFormat could have been simplified more by removing that local
variable entirely.

BEFORE
if (pg_strcasecmp(format, "c") == 0)
  archFormat = archCustom;
else if (pg_strcasecmp(format, "custom") == 0)
  archFormat = archCustom;
else if (pg_strcasecmp(format, "d") == 0)
  archFormat = archDirectory;
else if (pg_strcasecmp(format, "directory") == 0)
  archFormat = archDirectory;
else if (pg_strcasecmp(format, "p") == 0)
  archFormat = archNull;
else if (pg_strcasecmp(format, "plain") == 0)
  archFormat = archNull;
else if (pg_strcasecmp(format, "t") == 0)
  archFormat = archTar;
else if (pg_strcasecmp(format, "tar") == 0)
  archFormat = archTar;

SUGGESTION
if (pg_strcasecmp(format, "c") == 0 ||
  pg_strcasecmp(format, "custom") == 0)
  return archCustom;

if (pg_strcasecmp(format, "d") == 0 ||
  pg_strcasecmp(format, "directory") == 0)
  return archDirectory;

if (pg_strcasecmp(format, "p") == 0 ||
  pg_strcasecmp(format, "plain") == 0)
  return archNull;

if (pg_strcasecmp(format, "t") == 0 ||
  pg_strcasecmp(format, "tar") == 0)
  return archTar;

======
src/bin/psql/describe.c

I know you were addressing only "new" issues, but it seemed a bit
strange to fix only this one when there was the same issue earlier
(~line 1780) in the same function.

if (tableinfo.relkind == RELKIND_SEQUENCE)
{
 PGresult   *result = NULL;
 printQueryOpt myopt = pset.popt;

======
Kind Regards,
Peter Smith.
Fujitsu Australia


Reply via email to