Re: Better formatting of functions in pg_dump - Mailing list pgsql-patches

From Heikki Linnakangas
Subject Re: Better formatting of functions in pg_dump
Date
Msg-id 486A194E.80803@enterprisedb.com
Whole thread Raw
In response to Re: Better formatting of functions in pg_dump  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-patches
Tom Lane wrote:
> "Greg Sabino Mullane" <greg@turnstep.com> writes:
>>> Why the random switching between newline-before and newline-after
>>> styles?  Please be consistent.
>
>> I thought they were all "after". On second glance, they still seem
>> all after?
>
> Oh, my mistake, I had failed to see that the patch was getting rid of
> newline-before style in this function.  I think you might have gone
> a bit overboard on adding whitespace, but the previous objection is
> nonsense, sorry.

Yeah, I like idea of moving the "metadata" stuff before the function
body, but the whitespace is a bit too much. You can fit
"   LANGUAGE plpgsql IMMUTABLE STRICT SECURITY DEFINER COST 100000" in
on one line without wrapping on a 80 col terminal. And we don't try to
guarantee any specific width anyway, you can get very long lines if the
function has a lot of arguments, for example.

I applied this simpler patch that just moves the "metadata" stuff before
the function body, leaving the whitespace as is (in newline-before style).

--
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com
*** src/bin/pg_dump/pg_dump.c
--- src/bin/pg_dump/pg_dump.c
***************
*** 6775,6788 **** dumpFunc(Archive *fout, FuncInfo *finfo)
      rettypename = getFormattedTypeName(finfo->prorettype, zeroAsOpaque);

      appendPQExpBuffer(q, "CREATE FUNCTION %s ", funcsig);
!     appendPQExpBuffer(q, "RETURNS %s%s\n    %s\n    LANGUAGE %s",
                        (proretset[0] == 't') ? "SETOF " : "",
!                       rettypename,
!                       asPart->data,
!                       fmtId(lanname));
!
      free(rettypename);

      if (provolatile[0] != PROVOLATILE_VOLATILE)
      {
          if (provolatile[0] == PROVOLATILE_IMMUTABLE)
--- 6775,6786 ----
      rettypename = getFormattedTypeName(finfo->prorettype, zeroAsOpaque);

      appendPQExpBuffer(q, "CREATE FUNCTION %s ", funcsig);
!     appendPQExpBuffer(q, "RETURNS %s%s",
                        (proretset[0] == 't') ? "SETOF " : "",
!                       rettypename);
      free(rettypename);

+     appendPQExpBuffer(q, "\n    LANGUAGE %s", fmtId(lanname));
      if (provolatile[0] != PROVOLATILE_VOLATILE)
      {
          if (provolatile[0] == PROVOLATILE_IMMUTABLE)
***************
*** 6850,6856 **** dumpFunc(Archive *fout, FuncInfo *finfo)
              appendStringLiteralAH(q, pos, fout);
      }

!     appendPQExpBuffer(q, ";\n");

      ArchiveEntry(fout, finfo->dobj.catId, finfo->dobj.dumpId,
                   funcsig_tag,
--- 6848,6854 ----
              appendStringLiteralAH(q, pos, fout);
      }

!     appendPQExpBuffer(q, "\n    %s;\n", asPart->data);

      ArchiveEntry(fout, finfo->dobj.catId, finfo->dobj.dumpId,
                   funcsig_tag,

pgsql-patches by date:

Previous
From: Simon Riggs
Date:
Subject: Re: [HACKERS] odd output in restore mode
Next
From: "Heikki Linnakangas"
Date:
Subject: Re: [UPDATED] A GUC variable to replace PGBE_ACTIVITY_SIZE