Обсуждение: [HACKERS] Preliminary results for proposed new pgindent implementation

Поиск
Список
Период
Сортировка

[HACKERS] Preliminary results for proposed new pgindent implementation

От
Tom Lane
Дата:
Over in this thread:
https://www.postgresql.org/message-id/flat/E1dAmxK-0006EE-1r%40gemulon.postgresql.org
we've been discussing switching to FreeBSD's version of "indent",
specifically the updated version that Piotr Stefaniak is working on,
as the basis for pgindent.  There seem to be a small number of bugs
that Piotr needs to fix, but overall it is looking remarkably good
to me.

To create a diff with not too much noise in it, I applied the hacks
(fixes?) already mentioned in the other thread, and I tweaked things
slightly to suppress changes in alignment of comments on #else and
#endif lines.  We might well want to reconsider that later, because it's
quite random right now (basically, #else comments are indented to column
33, #endif comments are indented to column 10, and for no reason at all
the latter do not have standard tab substitution).  But again, I'm trying
to drill down to the changes we want rather than the incidental ones.

At this point, the changes look pretty good, although still bulky.
I've attached a diff between current HEAD and re-indented HEAD
for anybody who wants to peruse all the details, but basically
what I'm seeing is:

* Improvements in formatting around sizeof and related constructs,
for example:

-       *phoned_word = palloc(sizeof(char) * strlen(word) +1);
+       *phoned_word = palloc(sizeof(char) * strlen(word) + 1);

* Likewise, operators after casts work better than before:

        res = shm_mq_send_bytes(mqh, sizeof(Size) - mqh->mqh_partial_bytes,
-                               ((char *) &nbytes) +mqh->mqh_partial_bytes,
+                               ((char *) &nbytes) + mqh->mqh_partial_bytes,
                                nowait, &bytes_written);

* Sane formatting of function typedefs, for example:

     * use buf as work area if NULL in-place copy
     */
    int         (*pull) (void *priv, PullFilter *src, int len,
-                                    uint8 **data_p, uint8 *buf, int buflen);
+                        uint8 **data_p, uint8 *buf, int buflen);
    void        (*free) (void *priv);
 };

* Non-typedef struct pointers are now formatted consistently, for example:

-mdcbuf_finish(struct MDCBufData * st)
+mdcbuf_finish(struct MDCBufData *st)

* Better handling of pointers with const/volatile qualifiers, for example:

-static bool shm_mq_wait_internal(volatile shm_mq *mq, PGPROC *volatile * ptr,
+static bool shm_mq_wait_internal(volatile shm_mq *mq, PGPROC *volatile *ptr,

* Better handling of PG_USED_FOR_ASSERTS_ONLY declarations, for example

    BlockNumber blkno;
    Buffer      buf;
-   Bucket new_bucket PG_USED_FOR_ASSERTS_ONLY = InvalidBucket;
+   Bucket      new_bucket PG_USED_FOR_ASSERTS_ONLY = InvalidBucket;
    bool        bucket_dirty = false;

* Corner cases where no space was left before a comment are fixed:

@@ -149,12 +149,12 @@ typedef struct RewriteStateData
    bool        rs_logical_rewrite;     /* do we need to do logical rewriting */
    TransactionId rs_oldest_xmin;       /* oldest xmin used by caller to
                                         * determine tuple visibility */
-   TransactionId rs_freeze_xid;/* Xid that will be used as freeze cutoff
-                                * point */
+   TransactionId rs_freeze_xid;    /* Xid that will be used as freeze cutoff
+                                    * point */
    TransactionId rs_logical_xmin;      /* Xid that will be used as cutoff
                                         * point for logical rewrites */
-   MultiXactId rs_cutoff_multi;/* MultiXactId that will be used as cutoff
-                                * point for multixacts */
+   MultiXactId rs_cutoff_multi;    /* MultiXactId that will be used as cutoff
+                                    * point for multixacts */
    MemoryContext rs_cxt;       /* for hash tables and entries and tuples in
                                 * them */
    XLogRecPtr  rs_begin_lsn;   /* XLogInsertLsn when starting the rewrite */


There are also changes that are not really wins, just different, but they
do arguably improve consistency.  One is that comments following "else"
and "case" are now always indented at least as far as column 33 (or
indent's -c parameter), whereas the old code would sometimes let them
slide to the left of that:

@@ -723,7 +723,7 @@ dblink_record_internal(FunctionCallInfo fcinfo, bool is_async)
                /* shouldn't happen */
                elog(ERROR, "wrong number of arguments");
        }
-       else    /* is_async */
+       else                    /* is_async */
        {
            /* get async result */
            conname = text_to_cstring(PG_GETARG_TEXT_PP(0));

This also happens for comments on a function's "}" close brace:

@@ -722,7 +722,7 @@ _metaphone(char *word,          /* IN */
    End_Phoned_Word;

    return (META_SUCCESS);
-}  /* END metaphone */
+}                              /* END metaphone */

It's hard to see these as anything except bug fixes, because the
old indent code certainly looks like it intends to always force
same-line columns to at least the -c column.  I haven't quite figured
out why it fails to, or where the new version fixed that.  Still,
it's a difference that isn't a particular benefit to us.

Another set of changes is slightly different handling of unrecognized
typedef names:

@@ -250,7 +250,7 @@ typedef enum
    PGSS_TRACK_NONE,            /* track no statements */
    PGSS_TRACK_TOP,             /* only top level statements */
    PGSS_TRACK_ALL              /* all statements, including nested ones */
-}  PGSSTrackLevel;
+}          PGSSTrackLevel;

The reason PGSSTrackLevel is "unrecognized" is that it's not in
typedefs.list, which is a deficiency in our typedef-collection
technology not in indent.  (I believe the problem is that there
are no variables declared with that typename, causing there to
not be any of the kind of symbol table entries we are looking for.)
The handling of such names was already slightly wonky, though;
note that the previous version was already differently indented
from what would happen if PGSSTrackLevel were known.

Another issue I'm noticing is that "extern C" declarations are
getting reformatted:

diff --git a/src/interfaces/ecpg/include/ecpgtype.h b/src/interfaces/ecpg/include/ecpgtype.h
index 7cc47e9..236d028 100644
--- a/src/interfaces/ecpg/include/ecpgtype.h
+++ b/src/interfaces/ecpg/include/ecpgtype.h
@@ -34,7 +34,7 @@
 #define _ECPGTYPE_H

 #ifdef __cplusplus
-extern     "C"
+extern "C"
 {
 #endif


The pgindent Perl script is fooling around with these already, and
I think what it's doing is probably not quite compatible with what
the new indent version does, but I haven't tracked it down yet.

All in all, this looks pretty darn good from here, and I'm thinking
we should push forward on it.

            regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Вложения

Re: [HACKERS] Preliminary results for proposed new pgindentimplementation

От
Stephen Frost
Дата:
Tom, all,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> All in all, this looks pretty darn good from here, and I'm thinking
> we should push forward on it.

+1.

Thanks!

Stephen

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

От
Robert Haas
Дата:
On Thu, May 18, 2017 at 11:00 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> * Improvements in formatting around sizeof and related constructs,
> for example:
>
> * Likewise, operators after casts work better than before:
>
> * Sane formatting of function typedefs, for example:
>
> * Non-typedef struct pointers are now formatted consistently, for example:
>
> * Better handling of pointers with const/volatile qualifiers, for example:
>
> * Better handling of PG_USED_FOR_ASSERTS_ONLY declarations, for example
>
> * Corner cases where no space was left before a comment are fixed:

Those all sound like good things.

> Another set of changes is slightly different handling of unrecognized
> typedef names:
>
> @@ -250,7 +250,7 @@ typedef enum
>     PGSS_TRACK_NONE,            /* track no statements */
>     PGSS_TRACK_TOP,             /* only top level statements */
>     PGSS_TRACK_ALL              /* all statements, including nested ones */
> -}  PGSSTrackLevel;
> +}          PGSSTrackLevel;
>
> The reason PGSSTrackLevel is "unrecognized" is that it's not in
> typedefs.list, which is a deficiency in our typedef-collection
> technology not in indent.  (I believe the problem is that there
> are no variables declared with that typename, causing there to
> not be any of the kind of symbol table entries we are looking for.)
> The handling of such names was already slightly wonky, though;
> note that the previous version was already differently indented
> from what would happen if PGSSTrackLevel were known.

This, however, doesn't sound so good.  Isn't there some way this can be fixed?

> All in all, this looks pretty darn good from here, and I'm thinking
> we should push forward on it.

What does that exactly mean concretely?

We've talked about pulling pgindent into our main repo, or posting a
link to a tarball someplace.  An intermediate plan might be to give it
its own repo, but on git.postgresql.org, which seems like it might
give us the best of both worlds.  But I really want something that's
going to be easy to set up and configure.  It took me years to be
brave enough to get the current pgindent set up.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: [HACKERS] Preliminary results for proposed new pgindent implementation

От
Tom Lane
Дата:
Robert Haas <robertmhaas@gmail.com> writes:
> On Thu, May 18, 2017 at 11:00 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> The reason PGSSTrackLevel is "unrecognized" is that it's not in
>> typedefs.list, which is a deficiency in our typedef-collection
>> technology not in indent.  (I believe the problem is that there
>> are no variables declared with that typename, causing there to
>> not be any of the kind of symbol table entries we are looking for.)

> This, however, doesn't sound so good.  Isn't there some way this can be fixed?

I'm intending to look into it, but I think it's mostly independent of
whether we replace pgindent itself.  The existing code has the same
problem, really.

One brute-force way we could deal with the problem is to have a "manual"
list of names to be treated as typedefs, in addition to whatever the
buildfarm produces.  I see no other way than that to get, for instance,
simplehash.h's SH_TYPE to be formatted as a typedef.  There are also
some typedefs that don't get formatted correctly because they are only
used for wonky options that no existing typedef-reporting buildfarm member
builds.  Manual addition might be the path of least resistance there too.

Now the other side of this coin is that, by definition, such typedefs
are not getting used in a huge number of places.  If we just had to
live with it, it might not be awful.

>> All in all, this looks pretty darn good from here, and I'm thinking
>> we should push forward on it.

> What does that exactly mean concretely?

That means I plan to continue putting effort into it with the goal of
making a switchover sometime pretty darn soon.  We do not have a very
wide window for fooling with pgindent rules, IMO --- once v11 development
starts I think we can't touch it again (until this time next year).

> We've talked about pulling pgindent into our main repo, or posting a
> link to a tarball someplace.  An intermediate plan might be to give it
> its own repo, but on git.postgresql.org, which seems like it might
> give us the best of both worlds.  But I really want something that's
> going to be easy to set up and configure.  It took me years to be
> brave enough to get the current pgindent set up.

Yes, moving the goalposts on ease-of-use is an important consideration
here.  What that says to me is that we ought to pull FreeBSD indent
into our tree, and provide Makefile support that makes it easy for
any developer to build it and put it into their PATH.  (I suppose
that means support in the MSVC scripts too, but somebody else will
have to do that part.)

We should also think hard about getting rid of the entab dependency,
to eliminate the other nonstandard prerequisite program.  We could
either accept that that will result in some tab-vs-space changes in
our code, or try to convert those steps in pgindent into pure Perl,
or try to convince Piotr to add an option to indent that will make
it do tabs the way we want (ie use a space not a tab if the tab
would only move one space anyway).

Lastly (and I've said this before, but you pushed back on it at
the time), if we're doing this then we're going all in.  That
means reformatting the back branches to match too.  That diff
is already big enough to be a disaster for back-patching, and
we haven't even considered whether we want to let pgindent adopt
less-inconsistent rules for comment indentation.  So I think that
as soon as the dust has settled in HEAD, we back-patch the addition
of FreeBSD indent, and the changes in pgindent proper, and then
run pgindent in each supported back branch.
        regards, tom lane



Re: [HACKERS] Preliminary results for proposed new pgindentimplementation

От
Stephen Frost
Дата:
Tom, all,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
> > On Thu, May 18, 2017 at 11:00 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> The reason PGSSTrackLevel is "unrecognized" is that it's not in
> >> typedefs.list, which is a deficiency in our typedef-collection
> >> technology not in indent.  (I believe the problem is that there
> >> are no variables declared with that typename, causing there to
> >> not be any of the kind of symbol table entries we are looking for.)
>
> > This, however, doesn't sound so good.  Isn't there some way this can be fixed?
>
> I'm intending to look into it, but I think it's mostly independent of
> whether we replace pgindent itself.  The existing code has the same
> problem, really.
>
> One brute-force way we could deal with the problem is to have a "manual"
> list of names to be treated as typedefs, in addition to whatever the
> buildfarm produces.  I see no other way than that to get, for instance,
> simplehash.h's SH_TYPE to be formatted as a typedef.  There are also
> some typedefs that don't get formatted correctly because they are only
> used for wonky options that no existing typedef-reporting buildfarm member
> builds.  Manual addition might be the path of least resistance there too.
>
> Now the other side of this coin is that, by definition, such typedefs
> are not getting used in a huge number of places.  If we just had to
> live with it, it might not be awful.

Dealing with the typedef lists in general is a bit of a pain to get
right, to make sure that new just-written code gets correctly indented.
Perhaps we could find a way to incorporate the buildfarm typedef lists
and a manual list and a locally generated/provided set in a simpler
fashion in general.

> >> All in all, this looks pretty darn good from here, and I'm thinking
> >> we should push forward on it.
>
> > What does that exactly mean concretely?
>
> That means I plan to continue putting effort into it with the goal of
> making a switchover sometime pretty darn soon.  We do not have a very
> wide window for fooling with pgindent rules, IMO --- once v11 development
> starts I think we can't touch it again (until this time next year).

Agreed.

> > We've talked about pulling pgindent into our main repo, or posting a
> > link to a tarball someplace.  An intermediate plan might be to give it
> > its own repo, but on git.postgresql.org, which seems like it might
> > give us the best of both worlds.  But I really want something that's
> > going to be easy to set up and configure.  It took me years to be
> > brave enough to get the current pgindent set up.
>
> Yes, moving the goalposts on ease-of-use is an important consideration
> here.  What that says to me is that we ought to pull FreeBSD indent
> into our tree, and provide Makefile support that makes it easy for
> any developer to build it and put it into their PATH.  (I suppose
> that means support in the MSVC scripts too, but somebody else will
> have to do that part.)

I'm not a huge fan of this, however.  Do we really need to carry around
the FreeBSD indent in our tree?  I had been expecting that these changes
would eventually result in a package that's available in the common
distributions (possibly from apt/yum.postgresql.org, at least until it's
in the main Debian-based and RHEL-based package systems).  Are you
thinking that we'll always have to have our own modified version?

> We should also think hard about getting rid of the entab dependency,
> to eliminate the other nonstandard prerequisite program.  We could
> either accept that that will result in some tab-vs-space changes in
> our code, or try to convert those steps in pgindent into pure Perl,
> or try to convince Piotr to add an option to indent that will make
> it do tabs the way we want (ie use a space not a tab if the tab
> would only move one space anyway).

What about perltidy itself..?  We don't include that in our tree either.

I do think it'd be good to if Piotr would add such an option, hopefully
that's agreeable.

> Lastly (and I've said this before, but you pushed back on it at
> the time), if we're doing this then we're going all in.  That
> means reformatting the back branches to match too.  That diff
> is already big enough to be a disaster for back-patching, and
> we haven't even considered whether we want to let pgindent adopt
> less-inconsistent rules for comment indentation.  So I think that
> as soon as the dust has settled in HEAD, we back-patch the addition
> of FreeBSD indent, and the changes in pgindent proper, and then
> run pgindent in each supported back branch.

Ugh.  This would be pretty painful, but I agree that back-patching
without doing re-indenting the back-branches would also suck, so I'm on
the fence about this.

Thanks!

Stephen

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

От
Tom Lane
Дата:
Stephen Frost <sfrost@snowman.net> writes:
> * Tom Lane (tgl@sss.pgh.pa.us) wrote:
>> Yes, moving the goalposts on ease-of-use is an important consideration
>> here.  What that says to me is that we ought to pull FreeBSD indent
>> into our tree, and provide Makefile support that makes it easy for
>> any developer to build it and put it into their PATH.  (I suppose
>> that means support in the MSVC scripts too, but somebody else will
>> have to do that part.)

> I'm not a huge fan of this, however.  Do we really need to carry around
> the FreeBSD indent in our tree?  I had been expecting that these changes
> would eventually result in a package that's available in the common
> distributions (possibly from apt/yum.postgresql.org, at least until it's
> in the main Debian-based and RHEL-based package systems).  Are you
> thinking that we'll always have to have our own modified version?

I certainly would rather that our version matched something that's under
active maintenance someplace.  But it seems like there are two good
arguments for having a copy in our tree:

* easy accessibility for PG developers

* at any given time we need to be using a specific "blessed" version,
so that all developers can get equivalent results.  There's pretty much
no chance of that happening if we depend on distro-provided packages,
even if those share a common upstream.

We've had reasonably decent luck with tracking the tzcode/tzdata packages
as local copies, so I feel like we're not taking on anything unreasonable
if our model is that we'll occasionally (not oftener than once per year)
update our copy to recent upstream and then re-indent using that.

> What about perltidy itself..?  We don't include that in our tree either.

Not being much of a Perl guy, I don't care one way or the other about
perltidy.  Somebody else can work on that if it needs work.
        regards, tom lane



Re: [HACKERS] Preliminary results for proposed new pgindentimplementation

От
Heikki Linnakangas
Дата:
On 05/19/2017 06:05 PM, Stephen Frost wrote:
> * Tom Lane (tgl@sss.pgh.pa.us) wrote:
>> Robert Haas <robertmhaas@gmail.com> writes:
>>> On Thu, May 18, 2017 at 11:00 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>>> The reason PGSSTrackLevel is "unrecognized" is that it's not in
>>>> typedefs.list, which is a deficiency in our typedef-collection
>>>> technology not in indent.  (I believe the problem is that there
>>>> are no variables declared with that typename, causing there to
>>>> not be any of the kind of symbol table entries we are looking for.)
>>
>>> This, however, doesn't sound so good.  Isn't there some way this can be fixed?
>>
>> I'm intending to look into it, but I think it's mostly independent of
>> whether we replace pgindent itself.  The existing code has the same
>> problem, really.
>>
>> One brute-force way we could deal with the problem is to have a "manual"
>> list of names to be treated as typedefs, in addition to whatever the
>> buildfarm produces.  I see no other way than that to get, for instance,
>> simplehash.h's SH_TYPE to be formatted as a typedef.  There are also
>> some typedefs that don't get formatted correctly because they are only
>> used for wonky options that no existing typedef-reporting buildfarm member
>> builds.  Manual addition might be the path of least resistance there too.
>>
>> Now the other side of this coin is that, by definition, such typedefs
>> are not getting used in a huge number of places.  If we just had to
>> live with it, it might not be awful.
>
> Dealing with the typedef lists in general is a bit of a pain to get
> right, to make sure that new just-written code gets correctly indented.
> Perhaps we could find a way to incorporate the buildfarm typedef lists
> and a manual list and a locally generated/provided set in a simpler
> fashion in general.

You can get a pretty good typedefs list just by looking for the pattern 
"} <type name>;". Something like this:

grep -o -h -I --perl-regexp -r "}\W(\w+);" src/ contrib/ | perl -pe 
's/}\W(.*);/\1/' | sort | uniq > typedefs.list

It won't cover system headers and non-struct typedefs, but it catches 
those simplehash typedefs and PGSSTrackLevel. Maybe we should run that 
and merge the result with the typedef lists we collect in the buildfarm.

- Heikki




Re: [HACKERS] Preliminary results for proposed new pgindent implementation

От
Tom Lane
Дата:
Heikki Linnakangas <hlinnaka@iki.fi> writes:
> You can get a pretty good typedefs list just by looking for the pattern 
> "} <type name>;".

That's going to catch a lot of things that are just variables, though.
It might be all right as long as there was manual filtering after it.
        regards, tom lane



Re: [HACKERS] Preliminary results for proposed new pgindentimplementation

От
Bruce Momjian
Дата:
On Fri, May 19, 2017 at 11:22:54AM -0400, Tom Lane wrote:
> We've had reasonably decent luck with tracking the tzcode/tzdata packages
> as local copies, so I feel like we're not taking on anything unreasonable
> if our model is that we'll occasionally (not oftener than once per year)
> update our copy to recent upstream and then re-indent using that.

I guess by having a copy in our tree we would overtly update our
version, rather than downloading whatever happens to be the most recent
version and finding changes by accident.

If we could download a specific version that had everything we need,
that would work too.

> > What about perltidy itself..?  We don't include that in our tree either.
> 
> Not being much of a Perl guy, I don't care one way or the other about
> perltidy.  Somebody else can work on that if it needs work.

We have agreed on a fixed version of perltidy and I added a download
link to pgindent/README.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +



Re: [HACKERS] Preliminary results for proposed new pgindentimplementation

От
Heikki Linnakangas
Дата:
On 05/19/2017 06:48 PM, Tom Lane wrote:
> Heikki Linnakangas <hlinnaka@iki.fi> writes:
>> You can get a pretty good typedefs list just by looking for the pattern
>> "} <type name>;".
>
> That's going to catch a lot of things that are just variables, though.
> It might be all right as long as there was manual filtering after it.

At a quick glance, there are only a couple of them. This two cases 
caught my eye. In twophase.c:

static struct xllist
{        StateFileChunk *head;           /* first data block in the chain */        StateFileChunk *tail;           /*
lastblock in chain */        uint32          num_chunks;        uint32          bytes_free;             /* free bytes
leftin 
 
tail block */        uint32          total_len;              /* total data bytes in 
chain */
}       records;

And this in informix.c:

static struct
{        long            val;        int                     maxdigits;        int                     digits;
int                    remaining;        char            sign;        char       *val_string;
 
}       value;

IMHO it would actually be an improvement if there was a space rather 
than a tab there. But I'm not sure what else it would mess up to 
consider those typedef names. And those are awfully generic names; 
wouldn't hurt to rename them, anyway.

- Heikki




Re: [HACKERS] Preliminary results for proposed new pgindent implementation

От
Robert Haas
Дата:
On Fri, May 19, 2017 at 11:22 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I certainly would rather that our version matched something that's under
> active maintenance someplace.  But it seems like there are two good
> arguments for having a copy in our tree:
>
> * easy accessibility for PG developers
>
> * at any given time we need to be using a specific "blessed" version,
> so that all developers can get equivalent results.  There's pretty much
> no chance of that happening if we depend on distro-provided packages,
> even if those share a common upstream.

Yeah, but those advantages could also be gained by putting the
pgindent tree on git.postgresql.org in a separate repository.  Having
it in the same repository as the actual PostgreSQL code is not
required nor, in my opinion, particularly desirable.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: [HACKERS] Preliminary results for proposed new pgindentimplementation

От
Andres Freund
Дата:
On 2017-05-19 12:21:52 -0400, Robert Haas wrote:
> On Fri, May 19, 2017 at 11:22 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > I certainly would rather that our version matched something that's under
> > active maintenance someplace.  But it seems like there are two good
> > arguments for having a copy in our tree:
> >
> > * easy accessibility for PG developers
> >
> > * at any given time we need to be using a specific "blessed" version,
> > so that all developers can get equivalent results.  There's pretty much
> > no chance of that happening if we depend on distro-provided packages,
> > even if those share a common upstream.
> 
> Yeah, but those advantages could also be gained by putting the
> pgindent tree on git.postgresql.org in a separate repository.  Having
> it in the same repository as the actual PostgreSQL code is not
> required nor, in my opinion, particularly desirable.

I'm of the contrary opinion.  A lot of the regular churn due to pgindent
right now is because it's inconvenient to run.  Having to clone a
separate repository, compile that project, put it into PATH (fun if
there's multiple versions), run pgindent, discover typedefs.list is out
of date, update, run, ...  is pretty much a guarantee that'll continue.
If we had a make indent that computed local typedefs list, *added* new
but not removed old ones, we could get much closer to just always being
properly indented.

The cost of putting it somewhere blow src/tools/pgindent seems fairly
minor.

- Andres



Re: [HACKERS] Preliminary results for proposed new pgindent implementation

От
Tom Lane
Дата:
Robert Haas <robertmhaas@gmail.com> writes:
> On Fri, May 19, 2017 at 11:22 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I certainly would rather that our version matched something that's under
>> active maintenance someplace.  But it seems like there are two good
>> arguments for having a copy in our tree:
>> 
>> * easy accessibility for PG developers
>> 
>> * at any given time we need to be using a specific "blessed" version,
>> so that all developers can get equivalent results.  There's pretty much
>> no chance of that happening if we depend on distro-provided packages,
>> even if those share a common upstream.

> Yeah, but those advantages could also be gained by putting the
> pgindent tree on git.postgresql.org in a separate repository.  Having
> it in the same repository as the actual PostgreSQL code is not
> required nor, in my opinion, particularly desirable.

It adds an extra step to what a developer has to do to get pgindent
up and running, so it doesn't seem to me like it's helping the goal
of reducing the setup overhead.
        regards, tom lane



Re: [HACKERS] Preliminary results for proposed new pgindent implementation

От
Tom Lane
Дата:
Heikki Linnakangas <hlinnaka@iki.fi> writes:
> On 05/19/2017 06:48 PM, Tom Lane wrote:
>> That's going to catch a lot of things that are just variables, though.
>> It might be all right as long as there was manual filtering after it.

> At a quick glance, there are only a couple of them. This two cases 
> caught my eye. In twophase.c:

> static struct xllist
> {
> ...
> }       records;

> IMHO it would actually be an improvement if there was a space rather 
> than a tab there.

Agreed, but if "records" were considered a typedef name, that would
likely screw up the formatting of code referencing it.  Maybe less
badly with this version of indent than our old one, not sure.

What I was just looking at is the possibility of absorbing struct
tags ("xllist" in the above) as if they were typedef names.  In
at least 95% of our usages, if a struct has a tag then the tag is
also the struct's typedef name.  The reason this is interesting
is that it looks like (on at least Linux and macOS) the debug info
captures struct tags even when it misses the corresponding typedef.
We could certainly create a coding rule that struct tags *must*
match struct typedef names for our own code, but I'm not sure what
violations of that convention might appear in system headers.
        regards, tom lane



Re: [HACKERS] Preliminary results for proposed new pgindent implementation

От
Tom Lane
Дата:
Andres Freund <andres@anarazel.de> writes:
> On 2017-05-19 12:21:52 -0400, Robert Haas wrote:
>> Yeah, but those advantages could also be gained by putting the
>> pgindent tree on git.postgresql.org in a separate repository.  Having
>> it in the same repository as the actual PostgreSQL code is not
>> required nor, in my opinion, particularly desirable.

> I'm of the contrary opinion.  A lot of the regular churn due to pgindent
> right now is because it's inconvenient to run.  Having to clone a
> separate repository, compile that project, put it into PATH (fun if
> there's multiple versions), run pgindent, discover typedefs.list is out
> of date, update, run, ...  is pretty much a guarantee that'll continue.
> If we had a make indent that computed local typedefs list, *added* new
> but not removed old ones, we could get much closer to just always being
> properly indented.

I hadn't really thought of automating it to that extent, but yeah,
that seems like an interesting prospect.

> The cost of putting it somewhere blow src/tools/pgindent seems fairly
> minor.

I think the main cost would be bloating distribution tarballs.  Although
we're talking about adding ~50K to tarballs that are already pushing 20MB,
so realistically who's going to notice?  If you want to cut the tarball
size, let's reopen the discussion about keeping release notes since the
dawn of time.

Also, having the support in distributed tarballs is not all bad, because
it would allow someone working from a tarball rather than a git pull
to have pgindent support.  Dunno if there are any such someones anymore,
but maybe they're out there.
        regards, tom lane



Re: [HACKERS] Preliminary results for proposed new pgindentimplementation

От
Alvaro Herrera
Дата:
Tom Lane wrote:
> Robert Haas <robertmhaas@gmail.com> writes:

> > Yeah, but those advantages could also be gained by putting the
> > pgindent tree on git.postgresql.org in a separate repository.  Having
> > it in the same repository as the actual PostgreSQL code is not
> > required nor, in my opinion, particularly desirable.
> 
> It adds an extra step to what a developer has to do to get pgindent
> up and running, so it doesn't seem to me like it's helping the goal
> of reducing the setup overhead.

I favor having indent in a separate repository in our Git server, for
these reasons

0. it's under our control (so we can change rules as we see fit)
1. we can have Piotr as a committer there
2. we can use the same pgindent version for all Pg branches

I'm thinking that whenever we change the indent rules, we would
re-indent supported back-branches, just as Tom's proposing we'd do now
(which I endorse).

We wouldn't change the rules often, but say if we leave some typedef
wonky behavior alone for now, and somebody happens to fix it in the
future, then the fix would apply not only to the current tree at the
time but also to all older trees, which makes sense.


Now, there is a process that can be followed to update a *patch* from an
"pre-indent upstream PG" to a "post-indent upstream PG", to avoid manual
work in rebasing the patch past pgindent.  This can easily be used on
branches that can be rebased, also (since they are essentially just a
collection of patches).  One problem that remains is that this doesn't
apply easily to branches that get merged without rebase.  I *think* it
should be possible to come up with a process that creates a merge commit
using pgindent, but I haven't tried.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



Re: [HACKERS] Preliminary results for proposed new pgindent implementation

От
Tom Lane
Дата:
I wrote:
> What I was just looking at is the possibility of absorbing struct
> tags ("xllist" in the above) as if they were typedef names.  In
> at least 95% of our usages, if a struct has a tag then the tag is
> also the struct's typedef name.  The reason this is interesting
> is that it looks like (on at least Linux and macOS) the debug info
> captures struct tags even when it misses the corresponding typedef.
> We could certainly create a coding rule that struct tags *must*
> match struct typedef names for our own code, but I'm not sure what
> violations of that convention might appear in system headers.

I did an experiment with seeing what would happen to the typedef list
if we included struct tags.  On my Linux box, that adds about 10%
more names (3343 instead of 3028).  A lot of them would be good to
have, but there are a lot of others that maybe not so much.  See
attached diff output.

I hesitate to suggest any rule as grotty as "take struct tags only
if they begin with an upper-case letter", but that would actually
work really well, looks like.

            regards, tom lane

--- typedefs.std    2017-05-19 14:41:15.357406399 -0400
+++ typedefs.log    2017-05-19 14:46:11.978739384 -0400
@@ -39,17 +39,24 @@
 AggSplit
 AggState
 AggStatePerAgg
+AggStatePerAggData
 AggStatePerGroup
+AggStatePerGroupData
 AggStatePerHash
+AggStatePerHashData
 AggStatePerPhase
+AggStatePerPhaseData
 AggStatePerTrans
+AggStatePerTransData
 AggStrategy
 Aggref
 AggrefExprState
 AlenState
 Alias
 AllocBlock
+AllocBlockData
 AllocChunk
+AllocChunkData
 AllocPointer
 AllocSet
 AllocSetContext
@@ -118,6 +125,7 @@
 ArrayExprIterState
 ArrayIOData
 ArrayIterator
+ArrayIteratorData
 ArrayMapState
 ArrayMetaState
 ArrayParseState
@@ -153,6 +161,7 @@
 BITVECP
 BMS_Comparison
 BMS_Membership
+BND
 BN_CTX
 BOX
 BTArrayKeyInfo
@@ -167,6 +176,7 @@
 BTPageStat
 BTPageState
 BTParallelScanDesc
+BTParallelScanDescData
 BTScanOpaque
 BTScanOpaqueData
 BTScanPos
@@ -251,6 +261,7 @@
 BufFile
 Buffer
 BufferAccessStrategy
+BufferAccessStrategyData
 BufferAccessStrategyType
 BufferCachePagesContext
 BufferCachePagesRec
@@ -263,6 +274,7 @@
 BuildAccumulator
 BuiltinScript
 BulkInsertState
+BulkInsertStateData
 CACHESIGN
 CAC_state
 CEOUC_WAIT_MODE
@@ -295,6 +307,7 @@
 CheckpointStatsData
 CheckpointerRequest
 CheckpointerShmemStruct
+CheckpointerSlotMapping
 Chromosome
 City
 CkptSortItem
@@ -351,12 +364,14 @@
 CompressorState
 ConditionVariable
 ConditionalStack
+ConditionalStackData
 ConfigData
 ConfigVariable
 ConnCacheEntry
 ConnCacheKey
 ConnStatusType
 ConnType
+ConnectionOption
 ConnectionStateEnum
 ConsiderSplitContext
 Const
@@ -424,6 +439,7 @@
 CustomExecMethods
 CustomOutPtrType
 CustomPath
+CustomPathMethods
 CustomScan
 CustomScanMethods
 CustomScanState
@@ -453,6 +469,7 @@
 DeclareCursorStmt
 DecodedBkpBlock
 DecodingOutputState
+DecomprData
 DefElem
 DefElemAction
 DefaultACLInfo
@@ -484,6 +501,7 @@
 DomainIOData
 DropBehavior
 DropOwnedStmt
+DropRelationCallbackState
 DropReplicationSlotCmd
 DropRoleStmt
 DropStmt
@@ -499,6 +517,10 @@
 DumpableObjectType
 DynamicFileList
 DynamicZoneAbbrev
+ECPGgeneric_varchar
+ECPGstruct_member
+ECPGtype
+ECPGtype_information_cache
 EC_KEY
 EDGE
 ENGINE
@@ -517,6 +539,7 @@
 EditableObjectType
 ElementsState
 EnableTimeoutParams
+EncStat
 EndBlobPtrType
 EndBlobsPtrType
 EndDataPtrType
@@ -607,6 +630,7 @@
 FieldStore
 File
 FileFdwExecutionState
+FileFdwOption
 FileFdwPlanState
 FileName
 FileNameMap
@@ -828,7 +852,9 @@
 GinPostingList
 GinQualCounts
 GinScanEntry
+GinScanEntryData
 GinScanKey
+GinScanKeyData
 GinScanOpaque
 GinScanOpaqueData
 GinState
@@ -844,6 +870,7 @@
 GistSplitUnion
 GistSplitVector
 GlobalTransaction
+GlobalTransactionData
 GrantObjectType
 GrantRoleStmt
 GrantStmt
@@ -900,8 +927,11 @@
 HashJoin
 HashJoinState
 HashJoinTable
+HashJoinTableData
 HashJoinTuple
+HashJoinTupleData
 HashMemoryChunk
+HashMemoryChunkData
 HashMetaPage
 HashMetaPageData
 HashPageOpaque
@@ -920,6 +950,7 @@
 HeadlineParsedText
 HeadlineWordEntry
 HeapScanDesc
+HeapScanDescData
 HeapTuple
 HeapTupleData
 HeapTupleFields
@@ -967,6 +998,7 @@
 IndexRuntimeKeyInfo
 IndexScan
 IndexScanDesc
+IndexScanDescData
 IndexScanState
 IndexStateFlagsAction
 IndexStmt
@@ -1008,6 +1040,7 @@
 IterateJsonStringValuesState
 JEntry
 JHashState
+JhashState
 Join
 JoinCostWorkspace
 JoinExpr
@@ -1132,6 +1165,7 @@
 LogicalTapeSet
 MAGIC
 MBuf
+MDCBufData
 MJEvalResult
 MVDependencies
 MVDependency
@@ -1152,6 +1186,7 @@
 MergeAppendState
 MergeJoin
 MergeJoinClause
+MergeJoinClauseData
 MergeJoinState
 MergePath
 MergeScanSelCache
@@ -1211,10 +1246,14 @@
 NullTestType
 Numeric
 NumericAggState
+NumericData
 NumericDigit
+NumericLong
+NumericShort
 NumericSortSupport
 NumericSumAccum
 NumericVar
+ONEXIT
 OP
 OSAPerGroupState
 OSAPerQueryState
@@ -1241,6 +1280,7 @@
 OidOptions
 OkeysState
 OldSerXidControl
+OldSerXidControlData
 OldSnapshotControlData
 OldToNewMapping
 OldToNewMappingData
@@ -1303,6 +1343,7 @@
 PGQueryClass
 PGRUsage
 PGSemaphore
+PGSemaphoreData
 PGSetenvStatusType
 PGShmemHeader
 PGTransactionStatusType
@@ -1449,7 +1490,9 @@
 ParallelContext
 ParallelExecutorInfo
 ParallelHeapScanDesc
+ParallelHeapScanDescData
 ParallelIndexScanDesc
+ParallelIndexScanDescData
 ParallelSlot
 ParallelState
 ParallelWorkerInfo
@@ -1459,6 +1502,7 @@
 ParamFetchHook
 ParamKind
 ParamListInfo
+ParamListInfoData
 ParamPathInfo
 ParamRef
 ParentMapEntry
@@ -1482,6 +1526,7 @@
 PartitionDispatchData
 PartitionElem
 PartitionKey
+PartitionKeyData
 PartitionListValue
 PartitionRangeBound
 PartitionRangeDatum
@@ -1561,6 +1606,8 @@
 Pg_magic_struct
 PipeProtoChunk
 PipeProtoHeader
+PktData
+PktStreamStat
 PlaceHolderInfo
 PlaceHolderVar
 Plan
@@ -1584,6 +1631,7 @@
 PopulateRecordsetState
 Port
 Portal
+PortalData
 PortalHashEnt
 PortalStatus
 PortalStrategy
@@ -1596,7 +1644,9 @@
 PredIterInfo
 PredIterInfoData
 PredXactList
+PredXactListData
 PredXactListElement
+PredXactListElementData
 PredicateLockData
 PredicateLockTargetType
 PrepareStmt
@@ -1672,6 +1722,7 @@
 RBOrderControl
 RBTree
 RBTreeIterator
+RELCACHECALLBACK
 RIX
 RI_CompareHashEntry
 RI_CompareKey
@@ -1680,7 +1731,9 @@
 RI_QueryKey
 RTEKind
 RWConflict
+RWConflictData
 RWConflictPoolHeader
+RWConflictPoolHeaderData
 Range
 RangeBound
 RangeBox
@@ -1790,6 +1843,7 @@
 ReservoirStateData
 ResourceArray
 ResourceOwner
+ResourceOwnerData
 ResourceReleaseCallback
 ResourceReleaseCallbackItem
 ResourceReleasePhase
@@ -1805,6 +1859,7 @@
 RewriteMappingFile
 RewriteRule
 RewriteState
+RewriteStateData
 RmgrData
 RmgrDescData
 RmgrId
@@ -1836,6 +1891,7 @@
 SISeg
 SMgrRelation
 SMgrRelationData
+SN_env
 SPELL
 SPIPlanPtr
 SPITupleTable
@@ -1847,6 +1903,7 @@
 SQLDropObject
 SQLFunctionCache
 SQLFunctionCachePtr
+SQLFunctionParseInfo
 SQLFunctionParseInfoPtr
 SQLValueFunction
 SQLValueFunctionOp
@@ -1855,6 +1912,7 @@
 SSL_CTX
 STRLEN
 SV
+SYSCACHECALLBACK
 SampleScan
 SampleScanGetSampleSize_function
 SampleScanState
@@ -1895,6 +1953,7 @@
 SetOpPath
 SetOpState
 SetOpStatePerGroup
+SetOpStatePerGroupData
 SetOpStrategy
 SetOperation
 SetOperationStmt
@@ -2030,6 +2089,7 @@
 Syn
 SyncRepConfigData
 SysScanDesc
+SysScanDescData
 SyscacheCallbackFunction
 SystemRowsSamplerData
 SystemSamplerData
@@ -2064,6 +2124,7 @@
 TSQuery
 TSQueryData
 TSQueryParserState
+TSQueryParserStateData
 TSQuerySign
 TSReadPointer
 TSTemplateInfo
@@ -2072,6 +2133,7 @@
 TSVectorBuildState
 TSVectorData
 TSVectorParseState
+TSVectorParseStateData
 TSVectorStat
 TState
 TStoreState
@@ -2176,6 +2238,7 @@
 TupleHashEntryData
 TupleHashIterator
 TupleHashTable
+TupleHashTableData
 TupleQueueReader
 TupleRemapClass
 TupleRemapInfo
@@ -2290,6 +2353,7 @@
 WindowStatePerAgg
 WindowStatePerAggData
 WindowStatePerFunc
+WindowStatePerFuncData
 WithCheckOption
 WithClause
 WordEntry
@@ -2347,6 +2411,7 @@
 XactCallbackItem
 XactEvent
 XactLockTableWaitInfo
+XidCache
 XidStatus
 XmlExpr
 XmlExprOp
@@ -2357,11 +2422,79 @@
 YYSTYPE
 YY_BUFFER_STATE
 YY_CHAR
+ZipStat
+_DestReceiver
+_FuncCandidateList
+_IndexList
+_MdfdVec
+_PQconninfoOption
+_PQprintOpt
+_PublicationInfo
+_PublicationRelInfo
 _SPI_connection
 _SPI_plan
+_SubscriptionInfo
+_TTOffList
+_accessMethodInfo
+_aggInfo
+_archiveHandle
+_attrDefInfo
+_avl_node
+_avl_tree
+_blobInfo
+_castInfo
+_cfgInfo
+_collInfo
+_constraintInfo
+_convInfo
+_defaultACLInfo
+_defines
+_dictInfo
+_dumpOptions
+_dumpableObject
+_evttriggerInfo
+_extensionInfo
+_extensionMemberId
+_fdwInfo
+_foreignServerInfo
+_funcInfo
+_helpStruct
+_if_value
+_include_path
+_indxInfo
+_inhInfo
+_internalPQconninfoOption
+_namespaceInfo
+_opclassInfo
+_opfamilyInfo
+_oprInfo
+_outputContext
+_param
+_pivot_field
+_policyInfo
+_procLangInfo
+_prsInfo
+_psqlSettings
+_restoreOptions
+_ruleInfo
+_shellTypeInfo
+_statsExtInfo
+_tableDataInfo
+_tableInfo
+_tmplInfo
+_tocEntry
+_transformInfo
+_triggerInfo
+_typeInfo
+_variable
+_yy_buffer
 acquireLocksOnSubLinks_context
+addrinfo
+adhoc_opts
 adjust_appendrel_attrs_context
+aff_struct
 allocfunc
+am_propname
 ambeginscan_function
 ambuild_function
 ambuildempty_function
@@ -2375,6 +2508,7 @@
 aminitparallelscan_function
 aminsert_function
 ammarkpos_function
+among
 amoptions_function
 amparallelrescan_function
 amproperty_function
@@ -2382,10 +2516,18 @@
 amrestrpos_function
 amvacuumcleanup_function
 amvalidate_function
+arc
+arcbatch
+arcp
+arguments
 array_iter
 array_unnest_fctx
 assign_collations_context
+assignment
+attrDefault
+auto_mem
 autovac_table
+av
 av_relation
 avl_dbase
 avl_node
@@ -2400,11 +2542,20 @@
 bitmapword
 bits32
 bits8
+bkend
+block
 bool
 brin_column_state
+buftag
 bytea
 cached_re_str
+cachedesc
+carc
 cashKEY
+catcache
+catcacheheader
+catclist
+catctup
 cfp
 check_agg_arguments_context
 check_function_callback
@@ -2414,18 +2565,34 @@
 check_ungrouped_columns_context
 chkpass
 chr
+cipher_info
 clock_t
 cmpEntriesArg
 cmpfunc
+cname
+cnfa
 codes_t
 coercion
 collation_cache_entry
 color
+colordesc
+colormap
 colormaprange
+config_bool
+config_enum
+config_enum_entry
+config_generic
+config_int
+config_real
+config_string
 config_var_value
+connection
+constrCheck
 contain_aggs_of_level_context
+context
 convert_testexpr_context
 copy_data_source_cb
+copy_options
 core_YYSTYPE
 core_yy_extra_type
 core_yyscan_t
@@ -2435,24 +2602,39 @@
 createdb_failure_params
 crosstab_HashEnt
 crosstab_cat_desc
+crosstab_hashent
+cursor
+cv
+cvec
+datapagemap
+datapagemap_iterator
 datapagemap_iterator_t
 datapagemap_t
 dateKEY
 datetkn
 dce_uuid_t
+ddlinfo
+debug_expect
 decimal
 deparse_columns
 deparse_context
 deparse_expr_cxt
 deparse_namespace
+descriptor
+descriptor_item
 destructor
 dev_t
+df_files
+dfa
+digest_info
 directory_fctx
+dirent
 disassembledLeaf
 dlist_head
 dlist_iter
 dlist_mutable_iter
 dlist_node
+dropmsgstrings
 ds_state
 dsa_area
 dsa_area_control
@@ -2476,6 +2658,9 @@
 ec_member_foreign_arg
 ec_member_matches_arg
 emit_log_hook_type
+encoding_match
+epoll_event
+error_desc
 eval_const_expressions_context
 event_trigger_command_tag_check_result
 event_trigger_support_data
@@ -2485,6 +2670,7 @@
 fd_set
 fe_scram_state
 fe_scram_state_enum
+fetch_desc
 file_action_t
 file_entry_t
 file_type_t
@@ -2499,12 +2685,17 @@
 flex_int32_t
 float4
 float4KEY
+float4key
 float8
 float8KEY
+float8key
 fmNodePtr
 fmgr_hook_type
+fmgr_security_definer_cache
+fns
 foreign_glob_cxt
 foreign_loc_cxt
+fp_info
 freefunc
 fsec_t
 gbt_vsrt_arg
@@ -2515,6 +2706,7 @@
 generate_series_timestamp_fctx
 generate_series_timestamptz_fctx
 generate_subscripts_fctx
+generator
 get_agg_clause_costs_context
 get_attavgwidth_hook_type
 get_index_stats_hook_type
@@ -2536,45 +2728,63 @@
 gistxlogPage
 gistxlogPageSplit
 gistxlogPageUpdate
+group
 grouping_sets_data
 gseg_picksplit_item
 gtrgm_consistent_cache
+guc_stack
+guts
+gv
 gzFile
 hashfunc
 hbaPort
+he
 heap_page_items_state
 help_handler
 hlCheck
+hostent
 hstoreCheckKeyLen_t
 hstoreCheckValLen_t
 hstorePairs_t
 hstoreUniquePairs_t
 hstoreUpgrade_t
+hv
 hyperLogLogState
 ifState
+ifaddrs
 import_error_callback_arg
+in6_addr
+in_addr
+index
 indexed_tlist
 inet
 inetKEY
 inet_struct
+inetkey
 inline_error_callback_arg
 ino_t
 inquiry
 instr_time
 int16
 int16KEY
+int16key
 int2vector
 int32
 int32KEY
 int32_t
+int32key
 int64
 int64KEY
+int64key
 int8
 internalPQconninfoOption
+interpreter
 intptr_t
 intvKEY
+io
 itemIdSort
 itemIdSortData
+itimerval
 join_search_hook_type
 json_aelem_action
 json_ofield_action
@@ -2582,18 +2792,23 @@
 json_struct_action
 keyEntryData
 key_t
+lc_time_T
 lclContext
 lclTocEntry
+lconv
 leafSegmentInfo
 line_t
+lineptr
 locale_t
 locate_agg_of_level_context
 locate_var_of_level_context
 locate_windowfunc_context
 logstreamer_param
+loop
 lquery
 lquery_level
 lquery_variant
+lsinfo
 ltree
 ltree_gist
 ltree_level
@@ -2604,11 +2819,13 @@
 macaddr
 macaddr8
 macaddr_sortsupport_state
+magic
 map_variable_attnos_context
 max_parallel_hazard_context
 mb2wchar_with_len_converter
 mbcharacter_incrementer
 mbdisplaylen_converter
+mbinterval
 mblen_converter
 mbverifier
 md5_ctxt
@@ -2619,18 +2836,27 @@
 movedb_failure_params
 mxact
 mxtruncinfo
+nameData
 needs_fmgr_hook_type
+nfa
 nodeitem
 normal_rand_fctx
 ntile_context
 numeric
 object_access_hook_type
+object_type_map
 off_t
 oidKEY
 oidvector
 on_dsm_detach_callback
 on_exit_nicely_callback
+op
+opclasscacheent
+option
+options
 ossl_EVP_cipher_func
+ossl_cipher
+ossl_cipher_lookup
 output_type
 pagetable_hash
 pagetable_iterator
@@ -2639,9 +2865,17 @@
 pairingheap_node
 parallel_worker_main_type
 parse_error_callback_arg
+passwd
+pct_info
 pendingPosition
+pgDataValue
+pgLobjfuncs
+pgMessageField
+pgNotify
 pgParameterStatus
 pg_atomic_uint32
+pg_cancel
+pg_conn
 pg_conn_host
 pg_conn_host_type
 pg_conv_map
@@ -2652,12 +2886,15 @@
 pg_enc2gettext
 pg_enc2name
 pg_encname
+pg_encoding
 pg_int64
 pg_local_to_utf_combined
+pg_locale_struct
 pg_locale_t
 pg_mb_radix_tree
 pg_on_exit_callback
 pg_re_flags
+pg_result
 pg_saslprep_rc
 pg_sha224_ctx
 pg_sha256_ctx
@@ -2665,6 +2902,7 @@
 pg_sha512_ctx
 pg_stack_base_t
 pg_time_t
+pg_tm
 pg_tz
 pg_tz_cache
 pg_tzenum
@@ -2676,6 +2914,9 @@
 pg_wchar_tbl
 pgp_armor_headers_state
 pgpid_t
+pgresAttDesc
+pgresAttValue
+pgresParamDesc
 pgsocket
 pgsql_thing_t
 pgssEntry
@@ -2701,16 +2942,21 @@
 plpgsql_CastHashEntry
 plpgsql_CastHashKey
 plpgsql_HashEnt
+plpgsql_hashent
 pltcl_call_state
 pltcl_interp_desc
 pltcl_proc_desc
 pltcl_proc_key
 pltcl_proc_ptr
 pltcl_query_desc
+pollfd
+portalhashent
 pos_trgm
 post_parse_analyze_hook_type
 pqbool
 pqsigfunc
+prep
+prepared_statement
 printQueryOpt
 printTableContent
 printTableFooter
@@ -2736,6 +2982,12 @@
 pull_vars_context
 pullup_replace_vars_context
 pushdown_safety_info
+px_alias
+px_cipher
+px_combo
+px_crypt_algo
+px_digest
+px_hmac
 qsort_arg_comparator
 query_pathkeys_callback
 radius_attribute
@@ -2755,6 +3007,7 @@
 regmatch_t
 regoff_t
 regproc
+relidcacheent
 relopt_bool
 relopt_gen
 relopt_int
@@ -2770,18 +3023,25 @@
 rendezvousHashEntry
 replace_rte_variables_callback
 replace_rte_variables_context
+rerr
 rewrite_event
+rix
+rlimit
 rm_detail_t
 role_auth_extra
 row_security_policy_hook_type
+rule
+rusage
 save_buffer
 scram_HMAC_ctx
 scram_state
 scram_state_enum
 sem_t
+separator
 sequence_magic
 set_join_pathlist_hook_type
 set_rel_pathlist_hook_type
+sha1_ctxt
 shm_mq
 shm_mq_handle
 shm_mq_iovec
@@ -2790,9 +3050,12 @@
 shm_toc_entry
 shm_toc_estimator
 shmem_startup_hook_type
+shmid_ds
 sig_atomic_t
+sigaction
 sigjmp_buf
 signedbitmapword
+sigpipe_info
 sigset_t
 size_t
 slist_head
@@ -2800,8 +3063,15 @@
 slist_mutable_iter
 slist_node
 slock_t
+smalldfa
 smgrid
+sockaddr
+sockaddr_in
+sockaddr_in6
+sockaddr_storage
+sockaddr_un
 socklen_t
+spell_struct
 spgBulkDeleteState
 spgChooseIn
 spgChooseOut
@@ -2827,33 +3097,54 @@
 spgxlogVacuumRoot
 split_pathtarget_context
 sql_error_callback_arg
+sqlca_t
+sqlda_compat
+sqlda_struct
+sqlname
 sqlparseInfo
 sqlparseState
+sqlvar_compat
+sqlvar_struct
 ss_lru_item_t
 ss_scan_location_t
 ss_scan_locations_t
+sset
 ssize_t
 standard_qp_extra
+stat
+state
+statement
 stemmer_module
 stmtCacheEntry
 storeInfo
 storeRes_func
 stream_stop_callback
+su_symbol
+subre
+subst
 substitute_actual_parameters_context
 substitute_actual_srf_parameters_context
 substitute_multiple_relids_context
+sv
 svtype
 symbol
 tablespaceinfo
+tablesync_start_time_mapping
 teReqs
 teSection
 temp_tablespaces_extra
+termios
 text
+this_type
 timeKEY
 time_t
 timeout_handler_proc
 timeout_params
+timespec
+timeval
 tlist_vinfo
+tm
+tms
 toast_compress_header
 transferMode
 trgm
@@ -2862,14 +3153,24 @@
 tsKEY
 ts_db_fctx
 ts_tokentype
+tsearch_config_match
 tsearch_readline_state
+ttinfo
+tupleConstr
+tupleDesc
 tuplehash_hash
 tuplehash_iterator
 txid
+typedefs
+typinfo
+typmap
 tzEntry
+tzhead
+tztry
 u_char
 u_int
 uchr
+ucred
 uid_t
 uint16
 uint16_t
@@ -2886,6 +3187,7 @@
 unicode_linestyle
 unit_conversion
 unlogged_relation_entry
+user_args
 utf_local_conversion_func
 uuidKEY
 uuid_sortsupport_state
@@ -2894,11 +3196,18 @@
 va_list
 vacuumingOptions
 validate_string_relopt
+var_list
 varatt_expanded
+varatt_external
+varatt_indirect
 varattrib_1b
 varattrib_1b_e
 varattrib_4b
+variable
+varlena
+vars
 vbits
+vfd
 walrcv_check_conninfo_fn
 walrcv_connect_fn
 walrcv_create_slot_fn
@@ -2913,6 +3222,8 @@
 walrcv_startstreaming_fn
 wchar2mb_with_len_converter
 wchar_t
+when
+winsize
 wint_t
 xl_brin_createidx
 xl_brin_desummarize
@@ -2994,6 +3305,7 @@
 xl_xact_subxacts
 xl_xact_twophase
 xl_xact_xinfo
+xllist
 xmlBuffer
 xmlBufferPtr
 xmlChar
@@ -3016,9 +3328,12 @@
 xsltSecurityPrefsPtr
 xsltStylesheetPtr
 xsltTransformContextPtr
+yy_buffer_state
 yy_parser
 yy_size_t
 yy_state_type
+yy_trans_info
+yyguts_t
 yyscan_t
 yytype_int16
 yytype_int8

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Preliminary results for proposed new pgindentimplementation

От
Peter Eisentraut
Дата:
On 5/19/17 13:31, Alvaro Herrera wrote:
> I favor having indent in a separate repository in our Git server, for
> these reasons

I am also in favor of that.

> 0. it's under our control (so we can change rules as we see fit)
> 1. we can have Piotr as a committer there
> 2. we can use the same pgindent version for all Pg branches

3. We can use pgindent for external code.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



Re: [HACKERS] Preliminary results for proposed new pgindentimplementation

От
Peter Eisentraut
Дата:
On 5/19/17 11:22, Tom Lane wrote:
> I certainly would rather that our version matched something that's under
> active maintenance someplace.  But it seems like there are two good
> arguments for having a copy in our tree:

Is pgindent going to be indented by pgindent?

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



Re: [HACKERS] Preliminary results for proposed new pgindent implementation

От
Tom Lane
Дата:
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> On 5/19/17 11:22, Tom Lane wrote:
>> I certainly would rather that our version matched something that's under
>> active maintenance someplace.  But it seems like there are two good
>> arguments for having a copy in our tree:

> Is pgindent going to be indented by pgindent?

If we were going to keep it in our tree, I'd plan to add an exclusion
rule to keep pgindent from touching it, as we already have for assorted
other files that are copied from external projects.  However, it seems
like "keep it in a separate repo" is winning, so it's moot.
        regards, tom lane



Re: [HACKERS] Preliminary results for proposed new pgindent implementation

От
Tom Lane
Дата:
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> On 5/19/17 13:31, Alvaro Herrera wrote:
>> I favor having indent in a separate repository in our Git server, for
>> these reasons

> I am also in favor of that.

>> 0. it's under our control (so we can change rules as we see fit)
>> 1. we can have Piotr as a committer there
>> 2. we can use the same pgindent version for all Pg branches

> 3. We can use pgindent for external code.

Now that we've about reached the point of actually making the change,
we need to come to a resolution on where we're keeping the new indent
code.  I thought that Alvaro's point 1 above (we can give Piotr a
commit bit) was the only really compelling argument for putting it
into a separate repo rather than into our main tree.  In other aspects
that's a loser --- in particular, it would be hard to have different
indent versions for different PG branches, if we chose to run things
that way.  However, I gather from Piotr's recent remarks[1] that
he's not actually excited about doing continuing maintenance on
indent, so that advantage now seems illusory.  In any case we'd
need to keep such a repo pretty well locked down: if it's changing,
and different developers pull from it at different times, then
we're going to have people working with different indent behaviors,
which will make nobody happy.

So I'm back to the position that we ought to stick the indent
code under src/tools/ in our main repo.  Is anyone really
seriously against that?
        regards, tom lane

[1]
https://www.postgresql.org/message-id/VI1PR03MB119959F4B65F000CA7CD9F6BF2CC0%40VI1PR03MB1199.eurprd03.prod.outlook.com



Re: [HACKERS] Preliminary results for proposed new pgindent implementation

От
Tom Lane
Дата:
There was some discussion upthread about how we'd like pgindent not to
do weird things with string literals that wrap around the end of the
line a little bit.  I looked into that and found that it's actually
a generic behavior for any line that's within parentheses: normally,
such a line will get lined up with the parens, like this:

        foobar(baz,
               baz2,
               baz3,
               ...

but if the line would wrap when indented that much, and backing off
lets it not wrap, then it backs off.

I experimented with disabling that logic and just always aligning
to the paren indentation.  That fixes the weird cases with continued
string literals, but it also makes for a heck of a lot of other changes.
The full diff is too big to post here, but I've attached a selection
of diff hunks to give you an idea.  I'm not really sure if I like this
better than pgindent's traditional behavior --- but it's arguably less
confusing.

An intermediate position that we could consider is to disable the back-off
logic only when the line starts with a string literal.  I haven't actually
coded this but it looks like it would be easy, if grotty.

Or we could leave it alone.

Thoughts?

            regards, tom lane

diff -ur pgsql/contrib/amcheck/verify_nbtree.c pgsql-dup/contrib/amcheck/verify_nbtree.c
--- pgsql/contrib/amcheck/verify_nbtree.c    2017-06-16 12:31:36.900504080 -0400
+++ pgsql-dup/contrib/amcheck/verify_nbtree.c    2017-06-16 12:35:21.042052427 -0400
@@ -240,8 +240,8 @@
         ereport(ERROR,
                 (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                  errmsg("cannot access temporary tables of other sessions"),
-             errdetail("Index \"%s\" is associated with temporary relation.",
-                       RelationGetRelationName(rel))));
+                 errdetail("Index \"%s\" is associated with temporary relation.",
+                           RelationGetRelationName(rel))));

     if (!IndexIsValid(rel->rd_index))
         ereport(ERROR,
@@ -411,12 +411,12 @@
                 ereport(ERROR,
                         (errcode(ERRCODE_INDEX_CORRUPTED),
                          errmsg("block %u fell off the end of index \"%s\"",
-                             current, RelationGetRelationName(state->rel))));
+                                current, RelationGetRelationName(state->rel))));
             else
                 ereport(DEBUG1,
                         (errcode(ERRCODE_NO_DATA),
                          errmsg("block %u of index \"%s\" ignored",
-                             current, RelationGetRelationName(state->rel))));
+                                current, RelationGetRelationName(state->rel))));
             goto nextpage;
         }
         else if (nextleveldown.leftmost == InvalidBlockNumber)
@@ -433,14 +433,14 @@
                 if (!P_LEFTMOST(opaque))
                     ereport(ERROR,
                             (errcode(ERRCODE_INDEX_CORRUPTED),
-                           errmsg("block %u is not leftmost in index \"%s\"",
-                             current, RelationGetRelationName(state->rel))));
+                             errmsg("block %u is not leftmost in index \"%s\"",
+                                    current, RelationGetRelationName(state->rel))));

                 if (level.istruerootlevel && !P_ISROOT(opaque))
                     ereport(ERROR,
                             (errcode(ERRCODE_INDEX_CORRUPTED),
-                          errmsg("block %u is not true root in index \"%s\"",
-                             current, RelationGetRelationName(state->rel))));
+                             errmsg("block %u is not true root in index \"%s\"",
+                                    current, RelationGetRelationName(state->rel))));
             }

             /*
@@ -488,7 +488,7 @@
                      errmsg("left link/right link pair in index \"%s\" not in agreement",
                             RelationGetRelationName(state->rel)),
                      errdetail_internal("Block=%u left block=%u left link from block=%u.",
-                                  current, leftcurrent, opaque->btpo_prev)));
+                                        current, leftcurrent, opaque->btpo_prev)));

         /* Check level, which must be valid for non-ignorable page */
         if (level.level != opaque->btpo.level)
@@ -497,7 +497,7 @@
                      errmsg("leftmost down link for level points to block in index \"%s\" whose level is not one level
down",
                             RelationGetRelationName(state->rel)),
                      errdetail_internal("Block pointed to=%u expected level=%u level in pointed to block=%u.",
-                                 current, level.level, opaque->btpo.level)));
+                                        current, level.level, opaque->btpo.level)));

         /* Verify invariants for page -- all important checks occur here */
         bt_target_page_check(state);
@@ -508,8 +508,8 @@
         if (current == leftcurrent || current == opaque->btpo_prev)
             ereport(ERROR,
                     (errcode(ERRCODE_INDEX_CORRUPTED),
-              errmsg("circular link chain found in block %u of index \"%s\"",
-                     current, RelationGetRelationName(state->rel))));
+                     errmsg("circular link chain found in block %u of index \"%s\"",
+                            current, RelationGetRelationName(state->rel))));

         leftcurrent = current;
         current = opaque->btpo_next;
@@ -665,17 +665,17 @@
                     (errcode(ERRCODE_INDEX_CORRUPTED),
                      errmsg("item order invariant violated for index \"%s\"",
                             RelationGetRelationName(state->rel)),
-               errdetail_internal("Lower index tid=%s (points to %s tid=%s) "
-                                  "higher index tid=%s (points to %s tid=%s) "
-                                  "page lsn=%X/%X.",
-                                  itid,
-                                  P_ISLEAF(topaque) ? "heap" : "index",
-                                  htid,
-                                  nitid,
-                                  P_ISLEAF(topaque) ? "heap" : "index",
-                                  nhtid,
-                                  (uint32) (state->targetlsn >> 32),
-                                  (uint32) state->targetlsn)));
+                     errdetail_internal("Lower index tid=%s (points to %s tid=%s) "
+                                        "higher index tid=%s (points to %s tid=%s) "
+                                        "page lsn=%X/%X.",
+                                        itid,
+                                        P_ISLEAF(topaque) ? "heap" : "index",
+                                        htid,
+                                        nitid,
+                                        P_ISLEAF(topaque) ? "heap" : "index",
+                                        nhtid,
+                                        (uint32) (state->targetlsn >> 32),
+                                        (uint32) state->targetlsn)));
         }

         /*
@@ -824,7 +824,7 @@
         ereport(DEBUG1,
                 (errcode(ERRCODE_NO_DATA),
                  errmsg("level %u leftmost page of index \"%s\" was found deleted or half dead",
-                    opaque->btpo.level, RelationGetRelationName(state->rel)),
+                        opaque->btpo.level, RelationGetRelationName(state->rel)),
                  errdetail_internal("Deleted page found when building scankey from right sibling.")));

         /* Be slightly more pro-active in freeing this memory, just in case */
@@ -1053,7 +1053,7 @@
                      errmsg("down-link lower bound invariant violated for index \"%s\"",
                             RelationGetRelationName(state->rel)),
                      errdetail_internal("Parent block=%u child index tid=(%u,%u) parent page lsn=%X/%X.",
-                                      state->targetblock, childblock, offset,
+                                        state->targetblock, childblock, offset,
                                         (uint32) (state->targetlsn >> 32),
                                         (uint32) state->targetlsn)));
     }
@@ -1228,21 +1228,21 @@
     if (P_ISLEAF(opaque) && !P_ISDELETED(opaque) && opaque->btpo.level != 0)
         ereport(ERROR,
                 (errcode(ERRCODE_INDEX_CORRUPTED),
-            errmsg("invalid leaf page level %u for block %u in index \"%s\"",
-        opaque->btpo.level, blocknum, RelationGetRelationName(state->rel))));
+                 errmsg("invalid leaf page level %u for block %u in index \"%s\"",
+                        opaque->btpo.level, blocknum, RelationGetRelationName(state->rel))));

     if (blocknum != BTREE_METAPAGE && !P_ISLEAF(opaque) &&
         !P_ISDELETED(opaque) && opaque->btpo.level == 0)
         ereport(ERROR,
                 (errcode(ERRCODE_INDEX_CORRUPTED),
-         errmsg("invalid internal page level 0 for block %u in index \"%s\"",
-                opaque->btpo.level, RelationGetRelationName(state->rel))));
+                 errmsg("invalid internal page level 0 for block %u in index \"%s\"",
+                        opaque->btpo.level, RelationGetRelationName(state->rel))));

     if (!P_ISLEAF(opaque) && P_HAS_GARBAGE(opaque))
         ereport(ERROR,
                 (errcode(ERRCODE_INDEX_CORRUPTED),
-           errmsg("internal page block %u in index \"%s\" has garbage items",
-                  blocknum, RelationGetRelationName(state->rel))));
+                 errmsg("internal page block %u in index \"%s\" has garbage items",
+                        blocknum, RelationGetRelationName(state->rel))));

     return page;
 }
diff -ur pgsql/contrib/auth_delay/auth_delay.c pgsql-dup/contrib/auth_delay/auth_delay.c
--- pgsql/contrib/auth_delay/auth_delay.c    2017-06-16 12:31:37.732524682 -0400
+++ pgsql-dup/contrib/auth_delay/auth_delay.c    2017-06-16 12:35:21.855072546 -0400
@@ -57,7 +57,7 @@
 {
     /* Define custom GUC variables */
     DefineCustomIntVariable("auth_delay.milliseconds",
-             "Milliseconds to delay before reporting authentication failure",
+                            "Milliseconds to delay before reporting authentication failure",
                             NULL,
                             &auth_delay_milliseconds,
                             0,
diff -ur pgsql/contrib/auto_explain/auto_explain.c pgsql-dup/contrib/auto_explain/auto_explain.c
--- pgsql/contrib/auto_explain/auto_explain.c    2017-06-16 12:31:39.711573688 -0400
+++ pgsql-dup/contrib/auto_explain/auto_explain.c    2017-06-16 12:35:23.808120875 -0400
@@ -74,8 +74,8 @@
 {
     /* Define custom GUC variables. */
     DefineCustomIntVariable("auto_explain.log_min_duration",
-         "Sets the minimum execution time above which plans will be logged.",
-                         "Zero prints all plans. -1 turns this feature off.",
+                            "Sets the minimum execution time above which plans will be logged.",
+                            "Zero prints all plans. -1 turns this feature off.",
                             &auto_explain_log_min_duration,
                             -1,
                             -1, INT_MAX / 1000,
@@ -120,7 +120,7 @@

     DefineCustomBoolVariable("auto_explain.log_triggers",
                              "Include trigger statistics in plans.",
-                        "This has no effect unless log_analyze is also set.",
+                             "This has no effect unless log_analyze is also set.",
                              &auto_explain_log_triggers,
                              false,
                              PGC_SUSET,
diff -ur pgsql/contrib/bloom/bloom.h pgsql-dup/contrib/bloom/bloom.h
--- pgsql/contrib/bloom/bloom.h    2017-06-16 12:31:40.051582106 -0400
+++ pgsql-dup/contrib/bloom/bloom.h    2017-06-16 12:35:24.139129065 -0400
@@ -111,8 +111,8 @@
  */
 typedef BlockNumber FreeBlockNumberArray[
                                          MAXALIGN_DOWN(
-        BLCKSZ - SizeOfPageHeaderData - MAXALIGN(sizeof(BloomPageOpaqueData))
-       - MAXALIGN(sizeof(uint16) * 2 + sizeof(uint32) + sizeof(BloomOptions))
+                                                       BLCKSZ - SizeOfPageHeaderData -
MAXALIGN(sizeof(BloomPageOpaqueData))
+                                                       - MAXALIGN(sizeof(uint16) * 2 + sizeof(uint32) +
sizeof(BloomOptions))
                                                        ) / sizeof(BlockNumber)
 ];

diff -ur pgsql/contrib/bloom/blvacuum.c pgsql-dup/contrib/bloom/blvacuum.c
--- pgsql/contrib/bloom/blvacuum.c    2017-06-16 12:31:39.988580546 -0400
+++ pgsql-dup/contrib/bloom/blvacuum.c    2017-06-16 12:35:24.077127532 -0400
@@ -84,7 +84,7 @@
          */
         itup = itupPtr = BloomPageGetTuple(&state, page, FirstOffsetNumber);
         itupEnd = BloomPageGetTuple(&state, page,
-                              OffsetNumberNext(BloomPageGetMaxOffset(page)));
+                                    OffsetNumberNext(BloomPageGetMaxOffset(page)));
         while (itup < itupEnd)
         {
             /* Do we have to delete this tuple? */
@@ -108,7 +108,7 @@

         /* Assert that we counted correctly */
         Assert(itupPtr == BloomPageGetTuple(&state, page,
-                             OffsetNumberNext(BloomPageGetMaxOffset(page))));
+                                            OffsetNumberNext(BloomPageGetMaxOffset(page))));

         /*
          * Add page to new notFullPage list if we will not mark page as
diff -ur pgsql/contrib/btree_gin/btree_gin.c pgsql-dup/contrib/btree_gin/btree_gin.c
--- pgsql/contrib/btree_gin/btree_gin.c    2017-06-16 12:31:37.825526986 -0400
+++ pgsql-dup/contrib/btree_gin/btree_gin.c    2017-06-16 12:35:21.948074848 -0400
@@ -115,8 +115,8 @@
                                                  data->typecmp,
                                                  fcinfo->flinfo,
                                                  PG_GET_COLLATION(),
-                                   (data->strategy == BTLessStrategyNumber ||
-                                 data->strategy == BTLessEqualStrategyNumber)
+                                                 (data->strategy == BTLessStrategyNumber ||
+                                                  data->strategy == BTLessEqualStrategyNumber)
                                                  ? data->datum : a,
                                                  b));

diff -ur pgsql/contrib/btree_gist/btree_cash.c pgsql-dup/contrib/btree_gist/btree_cash.c
--- pgsql/contrib/btree_gist/btree_cash.c    2017-06-16 12:31:37.630522156 -0400
+++ pgsql-dup/contrib/btree_gist/btree_cash.c    2017-06-16 12:35:21.753070023 -0400
@@ -203,8 +203,8 @@
 gbt_cash_picksplit(PG_FUNCTION_ARGS)
 {
     PG_RETURN_POINTER(gbt_num_picksplit(
-                                    (GistEntryVector *) PG_GETARG_POINTER(0),
-                                      (GIST_SPLITVEC *) PG_GETARG_POINTER(1),
+                                        (GistEntryVector *) PG_GETARG_POINTER(0),
+                                        (GIST_SPLITVEC *) PG_GETARG_POINTER(1),
                                         &tinfo, fcinfo->flinfo
                                         ));
 }
diff -ur pgsql/contrib/btree_gist/btree_date.c pgsql-dup/contrib/btree_gist/btree_date.c
--- pgsql/contrib/btree_gist/btree_date.c    2017-06-16 12:31:37.376515867 -0400
+++ pgsql-dup/contrib/btree_gist/btree_date.c    2017-06-16 12:35:21.505063884 -0400
@@ -87,8 +87,8 @@
 {
     /* we assume the difference can't overflow */
     Datum        diff = DirectFunctionCall2(date_mi,
-                                     DateADTGetDatum(*((const DateADT *) a)),
-                                    DateADTGetDatum(*((const DateADT *) b)));
+                                           DateADTGetDatum(*((const DateADT *) a)),
+                                           DateADTGetDatum(*((const DateADT *) b)));

     return (float8) Abs(DatumGetInt32(diff));
 }
@@ -210,14 +210,14 @@
     diff = DatumGetInt32(DirectFunctionCall2(
                                              date_mi,
                                              DateADTGetDatum(newentry->upper),
-                                         DateADTGetDatum(origentry->upper)));
+                                             DateADTGetDatum(origentry->upper)));

     res = Max(diff, 0);

     diff = DatumGetInt32(DirectFunctionCall2(
                                              date_mi,
-                                           DateADTGetDatum(origentry->lower),
-                                          DateADTGetDatum(newentry->lower)));
+                                             DateADTGetDatum(origentry->lower),
+                                             DateADTGetDatum(newentry->lower)));

     res += Max(diff, 0);

@@ -227,8 +227,8 @@
     {
         diff = DatumGetInt32(DirectFunctionCall2(
                                                  date_mi,
-                                           DateADTGetDatum(origentry->upper),
-                                         DateADTGetDatum(origentry->lower)));
+                                                 DateADTGetDatum(origentry->upper),
+                                                 DateADTGetDatum(origentry->lower)));
         *result += FLT_MIN;
         *result += (float) (res / ((double) (res + diff)));
         *result *= (FLT_MAX / (((GISTENTRY *) PG_GETARG_POINTER(0))->rel->rd_att->natts + 1));
@@ -242,8 +242,8 @@
 gbt_date_picksplit(PG_FUNCTION_ARGS)
 {
     PG_RETURN_POINTER(gbt_num_picksplit(
-                                    (GistEntryVector *) PG_GETARG_POINTER(0),
-                                      (GIST_SPLITVEC *) PG_GETARG_POINTER(1),
+                                        (GistEntryVector *) PG_GETARG_POINTER(0),
+                                        (GIST_SPLITVEC *) PG_GETARG_POINTER(1),
                                         &tinfo, fcinfo->flinfo
                                         ));
 }
diff -ur pgsql/contrib/btree_gist/btree_enum.c pgsql-dup/contrib/btree_gist/btree_enum.c
--- pgsql/contrib/btree_gist/btree_enum.c    2017-06-16 12:31:37.659522874 -0400
+++ pgsql-dup/contrib/btree_gist/btree_enum.c    2017-06-16 12:35:21.782070739 -0400
@@ -169,8 +169,8 @@
 gbt_enum_picksplit(PG_FUNCTION_ARGS)
 {
     PG_RETURN_POINTER(gbt_num_picksplit(
-                                    (GistEntryVector *) PG_GETARG_POINTER(0),
-                                      (GIST_SPLITVEC *) PG_GETARG_POINTER(1),
+                                        (GistEntryVector *) PG_GETARG_POINTER(0),
+                                        (GIST_SPLITVEC *) PG_GETARG_POINTER(1),
                                         &tinfo, fcinfo->flinfo
                                         ));
 }
diff -ur pgsql/contrib/btree_gist/btree_float4.c pgsql-dup/contrib/btree_gist/btree_float4.c
--- pgsql/contrib/btree_gist/btree_float4.c    2017-06-16 12:31:37.536519828 -0400
+++ pgsql-dup/contrib/btree_gist/btree_float4.c    2017-06-16 12:35:21.660067721 -0400
@@ -196,8 +196,8 @@
 gbt_float4_picksplit(PG_FUNCTION_ARGS)
 {
     PG_RETURN_POINTER(gbt_num_picksplit(
-                                    (GistEntryVector *) PG_GETARG_POINTER(0),
-                                      (GIST_SPLITVEC *) PG_GETARG_POINTER(1),
+                                        (GistEntryVector *) PG_GETARG_POINTER(0),
+                                        (GIST_SPLITVEC *) PG_GETARG_POINTER(1),
                                         &tinfo, fcinfo->flinfo
                                         ));
 }
diff -ur pgsql/contrib/btree_gist/btree_float8.c pgsql-dup/contrib/btree_gist/btree_float8.c
--- pgsql/contrib/btree_gist/btree_float8.c    2017-06-16 12:31:37.392516263 -0400
+++ pgsql-dup/contrib/btree_gist/btree_float8.c    2017-06-16 12:35:21.520064256 -0400
@@ -203,8 +203,8 @@
 gbt_float8_picksplit(PG_FUNCTION_ARGS)
 {
     PG_RETURN_POINTER(gbt_num_picksplit(
-                                    (GistEntryVector *) PG_GETARG_POINTER(0),
-                                      (GIST_SPLITVEC *) PG_GETARG_POINTER(1),
+                                        (GistEntryVector *) PG_GETARG_POINTER(0),
+                                        (GIST_SPLITVEC *) PG_GETARG_POINTER(1),
                                         &tinfo, fcinfo->flinfo
                                         ));
 }
diff -ur pgsql/contrib/btree_gist/btree_inet.c pgsql-dup/contrib/btree_gist/btree_inet.c
--- pgsql/contrib/btree_gist/btree_inet.c    2017-06-16 12:31:37.551520200 -0400
+++ pgsql-dup/contrib/btree_gist/btree_inet.c    2017-06-16 12:35:21.675068092 -0400
@@ -133,7 +133,7 @@
     key.upper = (GBT_NUMKEY *) &kkk->upper;

     PG_RETURN_BOOL(gbt_num_consistent(&key, (void *) &query,
-                       &strategy, GIST_LEAF(entry), &tinfo, fcinfo->flinfo));
+                                      &strategy, GIST_LEAF(entry), &tinfo, fcinfo->flinfo));
 }


@@ -165,8 +165,8 @@
 gbt_inet_picksplit(PG_FUNCTION_ARGS)
 {
     PG_RETURN_POINTER(gbt_num_picksplit(
-                                    (GistEntryVector *) PG_GETARG_POINTER(0),
-                                      (GIST_SPLITVEC *) PG_GETARG_POINTER(1),
+                                        (GistEntryVector *) PG_GETARG_POINTER(0),
+                                        (GIST_SPLITVEC *) PG_GETARG_POINTER(1),
                                         &tinfo, fcinfo->flinfo
                                         ));
 }
diff -ur pgsql/contrib/btree_gist/btree_int2.c pgsql-dup/contrib/btree_gist/btree_int2.c
--- pgsql/contrib/btree_gist/btree_int2.c    2017-06-16 12:31:37.645522528 -0400
+++ pgsql-dup/contrib/btree_gist/btree_int2.c    2017-06-16 12:35:21.768070393 -0400
@@ -202,8 +202,8 @@
 gbt_int2_picksplit(PG_FUNCTION_ARGS)
 {
     PG_RETURN_POINTER(gbt_num_picksplit(
-                                    (GistEntryVector *) PG_GETARG_POINTER(0),
-                                      (GIST_SPLITVEC *) PG_GETARG_POINTER(1),
+                                        (GistEntryVector *) PG_GETARG_POINTER(0),
+                                        (GIST_SPLITVEC *) PG_GETARG_POINTER(1),
                                         &tinfo, fcinfo->flinfo
                                         ));
 }
diff -ur pgsql/contrib/btree_gist/btree_time.c pgsql-dup/contrib/btree_gist/btree_time.c
--- pgsql/contrib/btree_gist/btree_time.c    2017-06-16 12:31:37.568520620 -0400
+++ pgsql-dup/contrib/btree_gist/btree_time.c    2017-06-16 12:35:21.692068512 -0400
@@ -289,15 +289,15 @@

     intr = DatumGetIntervalP(DirectFunctionCall2(
                                                  time_mi_time,
-                                        TimeADTGetDatumFast(newentry->upper),
-                                     TimeADTGetDatumFast(origentry->upper)));
+                                                 TimeADTGetDatumFast(newentry->upper),
+                                                 TimeADTGetDatumFast(origentry->upper)));
     res = INTERVAL_TO_SEC(intr);
     res = Max(res, 0);

     intr = DatumGetIntervalP(DirectFunctionCall2(
                                                  time_mi_time,
-                                       TimeADTGetDatumFast(origentry->lower),
-                                      TimeADTGetDatumFast(newentry->lower)));
+                                                 TimeADTGetDatumFast(origentry->lower),
+                                                 TimeADTGetDatumFast(newentry->lower)));
     res2 = INTERVAL_TO_SEC(intr);
     res2 = Max(res2, 0);

@@ -309,8 +309,8 @@
     {
         intr = DatumGetIntervalP(DirectFunctionCall2(
                                                      time_mi_time,
-                                       TimeADTGetDatumFast(origentry->upper),
-                                     TimeADTGetDatumFast(origentry->lower)));
+                                                     TimeADTGetDatumFast(origentry->upper),
+                                                     TimeADTGetDatumFast(origentry->lower)));
         *result += FLT_MIN;
         *result += (float) (res / (res + INTERVAL_TO_SEC(intr)));
         *result *= (FLT_MAX / (((GISTENTRY *) PG_GETARG_POINTER(0))->rel->rd_att->natts + 1));
@@ -324,8 +324,8 @@
 gbt_time_picksplit(PG_FUNCTION_ARGS)
 {
     PG_RETURN_POINTER(gbt_num_picksplit(
-                                    (GistEntryVector *) PG_GETARG_POINTER(0),
-                                      (GIST_SPLITVEC *) PG_GETARG_POINTER(1),
+                                        (GistEntryVector *) PG_GETARG_POINTER(0),
+                                        (GIST_SPLITVEC *) PG_GETARG_POINTER(1),
                                         &tinfo, fcinfo->flinfo
                                         ));
 }
diff -ur pgsql/contrib/btree_gist/btree_ts.c pgsql-dup/contrib/btree_gist/btree_ts.c
--- pgsql/contrib/btree_gist/btree_ts.c    2017-06-16 12:31:37.442517501 -0400
+++ pgsql-dup/contrib/btree_gist/btree_ts.c    2017-06-16 12:35:21.568065444 -0400
@@ -387,8 +387,8 @@
 gbt_ts_picksplit(PG_FUNCTION_ARGS)
 {
     PG_RETURN_POINTER(gbt_num_picksplit(
-                                    (GistEntryVector *) PG_GETARG_POINTER(0),
-                                      (GIST_SPLITVEC *) PG_GETARG_POINTER(1),
+                                        (GistEntryVector *) PG_GETARG_POINTER(0),
+                                        (GIST_SPLITVEC *) PG_GETARG_POINTER(1),
                                         &tinfo, fcinfo->flinfo
                                         ));
 }
diff -ur pgsql/contrib/btree_gist/btree_utils_var.c pgsql-dup/contrib/btree_gist/btree_utils_var.c
--- pgsql/contrib/btree_gist/btree_utils_var.c    2017-06-16 12:31:37.521519457 -0400
+++ pgsql-dup/contrib/btree_gist/btree_utils_var.c    2017-06-16 12:35:21.645067350 -0400
@@ -402,8 +402,8 @@
         *res = 0.0;
     else if (!(((*tinfo->f_cmp) (nk.lower, ok.lower, collation, flinfo) >= 0 ||
                 gbt_bytea_pf_match(ok.lower, nk.lower, tinfo)) &&
-             ((*tinfo->f_cmp) (nk.upper, ok.upper, collation, flinfo) <= 0 ||
-              gbt_bytea_pf_match(ok.upper, nk.upper, tinfo))))
+               ((*tinfo->f_cmp) (nk.upper, ok.upper, collation, flinfo) <= 0 ||
+                gbt_bytea_pf_match(ok.upper, nk.upper, tinfo))))
     {
         Datum        d = PointerGetDatum(0);
         double        dres;
diff -ur pgsql/contrib/btree_gist/btree_uuid.c pgsql-dup/contrib/btree_gist/btree_uuid.c
--- pgsql/contrib/btree_gist/btree_uuid.c    2017-06-16 12:31:37.584521017 -0400
+++ pgsql-dup/contrib/btree_gist/btree_uuid.c    2017-06-16 12:35:21.707068883 -0400
@@ -150,7 +150,7 @@

     PG_RETURN_BOOL(
                    gbt_num_consistent(&key, (void *) query, &strategy,
-                                    GIST_LEAF(entry), &tinfo, fcinfo->flinfo)
+                                      GIST_LEAF(entry), &tinfo, fcinfo->flinfo)
         );
 }

@@ -220,8 +220,8 @@
 gbt_uuid_picksplit(PG_FUNCTION_ARGS)
 {
     PG_RETURN_POINTER(gbt_num_picksplit(
-                                    (GistEntryVector *) PG_GETARG_POINTER(0),
-                                      (GIST_SPLITVEC *) PG_GETARG_POINTER(1),
+                                        (GistEntryVector *) PG_GETARG_POINTER(0),
+                                        (GIST_SPLITVEC *) PG_GETARG_POINTER(1),
                                         &tinfo, fcinfo->flinfo
                                         ));
 }
diff -ur pgsql/contrib/cube/cube.c pgsql-dup/contrib/cube/cube.c
--- pgsql/contrib/cube/cube.c    2017-06-16 12:31:37.174510865 -0400
+++ pgsql-dup/contrib/cube/cube.c    2017-06-16 12:35:21.303058886 -0400
@@ -483,7 +483,7 @@
             union_d = cube_union_v0(datum_alpha, datum_beta);
             rt_cube_size(union_d, &size_union);
             inter_d = DatumGetNDBOX(DirectFunctionCall2(cube_inter,
-                          entryvec->vector[i].key, entryvec->vector[j].key));
+                                                        entryvec->vector[i].key, entryvec->vector[j].key));
             rt_cube_size(inter_d, &size_inter);
             size_waste = size_union - size_inter;

@@ -1354,15 +1354,15 @@
         {
             case CubeKNNDistanceTaxicab:
                 retval = DatumGetFloat8(DirectFunctionCall2(distance_taxicab,
-                             PointerGetDatum(cube), PointerGetDatum(query)));
+                                                            PointerGetDatum(cube), PointerGetDatum(query)));
                 break;
             case CubeKNNDistanceEuclid:
                 retval = DatumGetFloat8(DirectFunctionCall2(cube_distance,
-                             PointerGetDatum(cube), PointerGetDatum(query)));
+                                                            PointerGetDatum(cube), PointerGetDatum(query)));
                 break;
             case CubeKNNDistanceChebyshev:
                 retval = DatumGetFloat8(DirectFunctionCall2(distance_chebyshev,
-                             PointerGetDatum(cube), PointerGetDatum(query)));
+                                                            PointerGetDatum(cube), PointerGetDatum(query)));
                 break;
             default:
                 elog(ERROR, "unrecognized cube strategy number: %d", strategy);
diff -ur pgsql/contrib/dblink/dblink.c pgsql-dup/contrib/dblink/dblink.c
--- pgsql/contrib/dblink/dblink.c    2017-06-16 12:31:37.778525821 -0400
+++ pgsql-dup/contrib/dblink/dblink.c    2017-06-16 12:35:21.902073710 -0400
@@ -179,7 +179,7 @@

 static void
 dblink_get_conn(char *conname_or_str,
-       PGconn *volatile *conn_p, char **conname_p, volatile bool *freeconn_p)
+                PGconn *volatile *conn_p, char **conname_p, volatile bool *freeconn_p)
 {
     remoteConn *rconn = getConnectionByName(conname_or_str);
     PGconn       *conn;
@@ -207,9 +207,9 @@

             PQfinish(conn);
             ereport(ERROR,
-               (errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION),
-                errmsg("could not establish connection"),
-                errdetail_internal("%s", msg)));
+                    (errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION),
+                     errmsg("could not establish connection"),
+                     errdetail_internal("%s", msg)));
         }
         dblink_security_check(conn, rconn);
         if (PQclientEncoding(conn) != GetDatabaseEncoding())
@@ -869,8 +869,8 @@
                     /* failed to determine actual type of RECORD */
                     ereport(ERROR,
                             (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                        errmsg("function returning record called in context "
-                               "that cannot accept type record")));
+                             errmsg("function returning record called in context "
+                                    "that cannot accept type record")));
                     break;
                 default:
                     /* result type isn't composite */
@@ -909,7 +909,7 @@
                 nestlevel = applyRemoteGucs(conn);

             oldcontext = MemoryContextSwitchTo(
-                                    rsinfo->econtext->ecxt_per_query_memory);
+                                               rsinfo->econtext->ecxt_per_query_memory);
             tupstore = tuplestore_begin_heap(true, false, work_mem);
             rsinfo->setResult = tupstore;
             rsinfo->setDesc = tupdesc;
@@ -1036,7 +1036,7 @@
             attinmeta = TupleDescGetAttInMetadata(tupdesc);

             oldcontext = MemoryContextSwitchTo(
-                                    rsinfo->econtext->ecxt_per_query_memory);
+                                               rsinfo->econtext->ecxt_per_query_memory);
             tupstore = tuplestore_begin_heap(true, false, work_mem);
             rsinfo->setResult = tupstore;
             rsinfo->setDesc = tupdesc;
@@ -1460,8 +1460,8 @@
         {
             PQclear(res);
             ereport(ERROR,
-                  (errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
-                   errmsg("statement returning results not allowed")));
+                    (errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
+                     errmsg("statement returning results not allowed")));
         }
     }
     PG_CATCH();
@@ -1980,7 +1980,7 @@
             ereport(ERROR,
                     (errcode(ERRCODE_FDW_OUT_OF_MEMORY),
                      errmsg("out of memory"),
-             errdetail("could not get libpq's default connection options")));
+                     errdetail("could not get libpq's default connection options")));
     }

     /* Validate each supplied option. */
@@ -2179,7 +2179,7 @@
             appendStringInfoChar(&buf, ',');

         appendStringInfoString(&buf,
-                      quote_ident_cstr(NameStr(tupdesc->attrs[i]->attname)));
+                               quote_ident_cstr(NameStr(tupdesc->attrs[i]->attname)));
         needComma = true;
     }

@@ -2242,7 +2242,7 @@
             appendStringInfoString(&buf, " AND ");

         appendStringInfoString(&buf,
-               quote_ident_cstr(NameStr(tupdesc->attrs[pkattnum]->attname)));
+                               quote_ident_cstr(NameStr(tupdesc->attrs[pkattnum]->attname)));

         if (tgt_pkattvals[i] != NULL)
             appendStringInfo(&buf, " = %s",
@@ -2296,7 +2296,7 @@
             appendStringInfoString(&buf, ", ");

         appendStringInfo(&buf, "%s = ",
-                      quote_ident_cstr(NameStr(tupdesc->attrs[i]->attname)));
+                         quote_ident_cstr(NameStr(tupdesc->attrs[i]->attname)));

         key = get_attnum_pk_pos(pkattnums, pknumatts, i);

@@ -2325,7 +2325,7 @@
             appendStringInfoString(&buf, " AND ");

         appendStringInfoString(&buf,
-               quote_ident_cstr(NameStr(tupdesc->attrs[pkattnum]->attname)));
+                               quote_ident_cstr(NameStr(tupdesc->attrs[pkattnum]->attname)));

         val = tgt_pkattvals[i];

@@ -2351,7 +2351,7 @@

     rawstr_text = cstring_to_text(rawstr);
     result_text = DatumGetTextPP(DirectFunctionCall1(quote_ident,
-                                              PointerGetDatum(rawstr_text)));
+                                                     PointerGetDatum(rawstr_text)));
     result = text_to_cstring(result_text);

     return result;
@@ -2416,7 +2416,7 @@
             appendStringInfoString(&buf, "NULL");
         else
             appendStringInfoString(&buf,
-                      quote_ident_cstr(NameStr(tupdesc->attrs[i]->attname)));
+                                   quote_ident_cstr(NameStr(tupdesc->attrs[i]->attname)));
     }

     appendStringInfo(&buf, " FROM %s WHERE ", relname);
@@ -2429,7 +2429,7 @@
             appendStringInfoString(&buf, " AND ");

         appendStringInfoString(&buf,
-               quote_ident_cstr(NameStr(tupdesc->attrs[pkattnum]->attname)));
+                               quote_ident_cstr(NameStr(tupdesc->attrs[pkattnum]->attname)));

         if (src_pkattvals[i] != NULL)
             appendStringInfo(&buf, " = %s",
@@ -2619,10 +2619,10 @@
                 pfree(rconn);

             ereport(ERROR,
-                  (errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
-                   errmsg("password is required"),
-                   errdetail("Non-superuser cannot connect if the server does not request a password."),
-                   errhint("Target server's authentication method must be changed.")));
+                    (errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
+                     errmsg("password is required"),
+                     errdetail("Non-superuser cannot connect if the server does not request a password."),
+                     errhint("Target server's authentication method must be changed.")));
         }
     }
 }
@@ -2661,9 +2661,9 @@

         if (!connstr_gives_password)
             ereport(ERROR,
-                  (errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
-                   errmsg("password is required"),
-                   errdetail("Non-superusers must provide a password in the connection string.")));
+                    (errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
+                     errmsg("password is required"),
+                     errdetail("Non-superusers must provide a password in the connection string.")));
     }
 }

@@ -2724,8 +2724,8 @@
              message_detail ? errdetail_internal("%s", message_detail) : 0,
              message_hint ? errhint("%s", message_hint) : 0,
              message_context ? errcontext("%s", message_context) : 0,
-          errcontext("Error occurred on dblink connection named \"%s\": %s.",
-                     dblink_context_conname, dblink_context_msg)));
+             errcontext("Error occurred on dblink connection named \"%s\": %s.",
+                        dblink_context_conname, dblink_context_msg)));
 }

 /*
@@ -2760,7 +2760,7 @@
             ereport(ERROR,
                     (errcode(ERRCODE_FDW_OUT_OF_MEMORY),
                      errmsg("out of memory"),
-             errdetail("could not get libpq's default connection options")));
+                     errdetail("could not get libpq's default connection options")));
     }

     /* first gather the server connstr options */
diff -ur pgsql/contrib/hstore/hstore_op.c pgsql-dup/contrib/hstore/hstore_op.c
--- pgsql/contrib/hstore/hstore_op.c    2017-06-16 12:31:39.468567670 -0400
+++ pgsql-dup/contrib/hstore/hstore_op.c    2017-06-16 12:35:23.576115134 -0400
@@ -101,8 +101,8 @@
     if (key_count > MaxAllocSize / sizeof(Pairs))
         ereport(ERROR,
                 (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
-              errmsg("number of pairs (%d) exceeds the maximum allowed (%d)",
-                     key_count, (int) (MaxAllocSize / sizeof(Pairs)))));
+                 errmsg("number of pairs (%d) exceeds the maximum allowed (%d)",
+                        key_count, (int) (MaxAllocSize / sizeof(Pairs)))));

     key_pairs = palloc(sizeof(Pairs) * key_count);

@@ -181,7 +181,7 @@
     for (i = 0; i < nkeys; i++)
     {
         int            idx = hstoreFindKey(hs, &lowbound,
-                                      key_pairs[i].key, key_pairs[i].keylen);
+                                        key_pairs[i].key, key_pairs[i].keylen);

         if (idx >= 0)
         {
@@ -215,7 +215,7 @@
     for (i = 0; i < nkeys; i++)
     {
         int            idx = hstoreFindKey(hs, &lowbound,
-                                      key_pairs[i].key, key_pairs[i].keylen);
+                                        key_pairs[i].key, key_pairs[i].keylen);

         if (idx < 0)
         {
@@ -546,8 +546,8 @@
         if (difference >= 0)
         {
             HS_COPYITEM(ed, bufd, pd,
-                      HSTORE_KEY(es2, ps2, s2idx), HSTORE_KEYLEN(es2, s2idx),
-                    HSTORE_VALLEN(es2, s2idx), HSTORE_VALISNULL(es2, s2idx));
+                        HSTORE_KEY(es2, ps2, s2idx), HSTORE_KEYLEN(es2, s2idx),
+                        HSTORE_VALLEN(es2, s2idx), HSTORE_VALISNULL(es2, s2idx));
             ++s2idx;
             if (difference == 0)
                 ++s1idx;
@@ -555,8 +555,8 @@
         else
         {
             HS_COPYITEM(ed, bufd, pd,
-                      HSTORE_KEY(es1, ps1, s1idx), HSTORE_KEYLEN(es1, s1idx),
-                    HSTORE_VALLEN(es1, s1idx), HSTORE_VALISNULL(es1, s1idx));
+                        HSTORE_KEY(es1, ps1, s1idx), HSTORE_KEYLEN(es1, s1idx),
+                        HSTORE_VALLEN(es1, s1idx), HSTORE_VALISNULL(es1, s1idx));
             ++s1idx;
         }
     }
@@ -614,8 +614,8 @@
         else
         {
             out_datums[i] = PointerGetDatum(
-                      cstring_to_text_with_len(HSTORE_VAL(entries, ptr, idx),
-                                               HSTORE_VALLEN(entries, idx)));
+                                            cstring_to_text_with_len(HSTORE_VAL(entries, ptr, idx),
+                                                                     HSTORE_VALLEN(entries, idx)));
             out_nulls[i] = false;
         }
     }
@@ -667,7 +667,7 @@
     for (i = 0; i < nkeys; ++i)
     {
         int            idx = hstoreFindKey(hs, &lastidx,
-                                      key_pairs[i].key, key_pairs[i].keylen);
+                                        key_pairs[i].key, key_pairs[i].keylen);

         if (idx >= 0)
         {
@@ -760,7 +760,7 @@
         else
         {
             text       *item = cstring_to_text_with_len(HSTORE_VAL(entries, base, i),
-                                                  HSTORE_VALLEN(entries, i));
+                                                        HSTORE_VALLEN(entries, i));

             d[i] = PointerGetDatum(item);
             nulls[i] = false;
@@ -811,7 +811,7 @@
         else
         {
             text       *item = cstring_to_text_with_len(HSTORE_VAL(entries, base, i),
-                                                  HSTORE_VALLEN(entries, i));
+                                                        HSTORE_VALLEN(entries, i));

             out_datums[i * 2 + 1] = PointerGetDatum(item);
             out_nulls[i * 2 + 1] = false;
diff -ur pgsql/contrib/intarray/_int_bool.c pgsql-dup/contrib/intarray/_int_bool.c
--- pgsql/contrib/intarray/_int_bool.c    2017-06-16 12:31:37.287513663 -0400
+++ pgsql-dup/contrib/intarray/_int_bool.c    2017-06-16 12:35:21.418061732 -0400
@@ -506,8 +506,8 @@
     if (state.num > QUERYTYPEMAXITEMS)
         ereport(ERROR,
                 (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
-        errmsg("number of query items (%d) exceeds the maximum allowed (%d)",
-               state.num, (int) QUERYTYPEMAXITEMS)));
+                 errmsg("number of query items (%d) exceeds the maximum allowed (%d)",
+                        state.num, (int) QUERYTYPEMAXITEMS)));
     commonlen = COMPUTESIZE(state.num);

     query = (QUERYTYPE *) palloc(commonlen);
diff -ur pgsql/contrib/intarray/_int_gist.c pgsql-dup/contrib/intarray/_int_gist.c
--- pgsql/contrib/intarray/_int_gist.c    2017-06-16 12:31:37.236512400 -0400
+++ pgsql-dup/contrib/intarray/_int_gist.c    2017-06-16 12:35:21.368060495 -0400
@@ -83,7 +83,7 @@
         case RTOldContainedByStrategyNumber:
             if (GIST_LEAF(entry))
                 retval = inner_int_contains(query,
-                                  (ArrayType *) DatumGetPointer(entry->key));
+                                            (ArrayType *) DatumGetPointer(entry->key));
             else
                 retval = inner_int_overlap((ArrayType *) DatumGetPointer(entry->key),
                                            query);
diff -ur pgsql/contrib/intarray/_int_op.c pgsql-dup/contrib/intarray/_int_op.c
--- pgsql/contrib/intarray/_int_op.c    2017-06-16 12:31:37.203511583 -0400
+++ pgsql-dup/contrib/intarray/_int_op.c    2017-06-16 12:35:21.334059653 -0400
@@ -49,8 +49,8 @@
     PG_RETURN_BOOL(!DatumGetBool(
                                  DirectFunctionCall2(
                                                      _int_same,
-                                       PointerGetDatum(PG_GETARG_POINTER(0)),
-                                        PointerGetDatum(PG_GETARG_POINTER(1))
+                                                     PointerGetDatum(PG_GETARG_POINTER(0)),
+                                                     PointerGetDatum(PG_GETARG_POINTER(1))
                                                      )
                                  ));
 }
diff -ur pgsql/contrib/intarray/_int_selfuncs.c pgsql-dup/contrib/intarray/_int_selfuncs.c
--- pgsql/contrib/intarray/_int_selfuncs.c    2017-06-16 12:31:37.303514059 -0400
+++ pgsql-dup/contrib/intarray/_int_selfuncs.c    2017-06-16 12:35:21.433062103 -0400
@@ -57,7 +57,7 @@
 {
     PG_RETURN_DATUM(DirectFunctionCall4(arraycontsel,
                                         PG_GETARG_DATUM(0),
-                                      ObjectIdGetDatum(OID_ARRAY_OVERLAP_OP),
+                                        ObjectIdGetDatum(OID_ARRAY_OVERLAP_OP),
                                         PG_GETARG_DATUM(2),
                                         PG_GETARG_DATUM(3)));
 }
@@ -67,7 +67,7 @@
 {
     PG_RETURN_DATUM(DirectFunctionCall4(arraycontsel,
                                         PG_GETARG_DATUM(0),
-                                     ObjectIdGetDatum(OID_ARRAY_CONTAINS_OP),
+                                        ObjectIdGetDatum(OID_ARRAY_CONTAINS_OP),
                                         PG_GETARG_DATUM(2),
                                         PG_GETARG_DATUM(3)));
 }
@@ -77,7 +77,7 @@
 {
     PG_RETURN_DATUM(DirectFunctionCall4(arraycontsel,
                                         PG_GETARG_DATUM(0),
-                                    ObjectIdGetDatum(OID_ARRAY_CONTAINED_OP),
+                                        ObjectIdGetDatum(OID_ARRAY_CONTAINED_OP),
                                         PG_GETARG_DATUM(2),
                                         PG_GETARG_DATUM(3)));
 }
@@ -87,7 +87,7 @@
 {
     PG_RETURN_DATUM(DirectFunctionCall5(arraycontjoinsel,
                                         PG_GETARG_DATUM(0),
-                                      ObjectIdGetDatum(OID_ARRAY_OVERLAP_OP),
+                                        ObjectIdGetDatum(OID_ARRAY_OVERLAP_OP),
                                         PG_GETARG_DATUM(2),
                                         PG_GETARG_DATUM(3),
                                         PG_GETARG_DATUM(4)));
@@ -98,7 +98,7 @@
 {
     PG_RETURN_DATUM(DirectFunctionCall5(arraycontjoinsel,
                                         PG_GETARG_DATUM(0),
-                                     ObjectIdGetDatum(OID_ARRAY_CONTAINS_OP),
+                                        ObjectIdGetDatum(OID_ARRAY_CONTAINS_OP),
                                         PG_GETARG_DATUM(2),
                                         PG_GETARG_DATUM(3),
                                         PG_GETARG_DATUM(4)));
@@ -109,7 +109,7 @@
 {
     PG_RETURN_DATUM(DirectFunctionCall5(arraycontjoinsel,
                                         PG_GETARG_DATUM(0),
-                                    ObjectIdGetDatum(OID_ARRAY_CONTAINED_OP),
+                                        ObjectIdGetDatum(OID_ARRAY_CONTAINED_OP),
                                         PG_GETARG_DATUM(2),
                                         PG_GETARG_DATUM(3),
                                         PG_GETARG_DATUM(4)));
diff -ur pgsql/contrib/isn/isn.c pgsql-dup/contrib/isn/isn.c
--- pgsql/contrib/isn/isn.c    2017-06-16 12:31:37.905528967 -0400
+++ pgsql-dup/contrib/isn/isn.c    2017-06-16 12:35:22.030076877 -0400
@@ -887,8 +887,8 @@
         {
             ereport(ERROR,
                     (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
-            errmsg("invalid check digit for %s number: \"%s\", should be %c",
-                   isn_names[accept], str, (rcheck == 10) ? ('X') : (rcheck + '0'))));
+                     errmsg("invalid check digit for %s number: \"%s\", should be %c",
+                            isn_names[accept], str, (rcheck == 10) ? ('X') : (rcheck + '0'))));
         }
     }
     return false;
diff -ur pgsql/contrib/ltree/_ltree_op.c pgsql-dup/contrib/ltree/_ltree_op.c
--- pgsql/contrib/ltree/_ltree_op.c    2017-06-16 12:31:36.983506134 -0400
+++ pgsql-dup/contrib/ltree/_ltree_op.c    2017-06-16 12:35:21.125054481 -0400
@@ -53,7 +53,7 @@
     while (num > 0)
     {
         if (DatumGetBool(DirectFunctionCall2(callback,
-                             PointerGetDatum(item), PointerGetDatum(param))))
+                                             PointerGetDatum(item), PointerGetDatum(param))))
         {

             if (found)
diff -ur pgsql/contrib/ltree/lquery_op.c pgsql-dup/contrib/ltree/lquery_op.c
--- pgsql/contrib/ltree/lquery_op.c    2017-06-16 12:31:36.967505739 -0400
+++ pgsql-dup/contrib/ltree/lquery_op.c    2017-06-16 12:35:21.110054110 -0400
@@ -356,7 +356,7 @@
     while (num > 0)
     {
         if (DatumGetBool(DirectFunctionCall2(ltq_regex,
-                             PointerGetDatum(tree), PointerGetDatum(query))))
+                                             PointerGetDatum(tree), PointerGetDatum(query))))
         {

             res = true;
diff -ur pgsql/contrib/ltree/ltree.h pgsql-dup/contrib/ltree/ltree.h
--- pgsql/contrib/ltree/ltree.h    2017-06-16 12:31:37.035507422 -0400
+++ pgsql-dup/contrib/ltree/ltree.h    2017-06-16 12:35:21.173055670 -0400
@@ -161,7 +161,7 @@
 int            ltree_compare(const ltree *a, const ltree *b);
 bool        inner_isparent(const ltree *c, const ltree *p);
 bool compare_subnode(ltree_level *t, char *q, int len,
-            int (*cmpptr) (const char *, const char *, size_t), bool anyend);
+                int (*cmpptr) (const char *, const char *, size_t), bool anyend);
 ltree       *lca_inner(ltree **a, int len);
 int            ltree_strncasecmp(const char *a, const char *b, size_t s);

diff -ur pgsql/contrib/ltree/ltree_gist.c pgsql-dup/contrib/ltree/ltree_gist.c
--- pgsql/contrib/ltree/ltree_gist.c    2017-06-16 12:31:37.055507918 -0400
+++ pgsql-dup/contrib/ltree/ltree_gist.c    2017-06-16 12:35:21.193056164 -0400
@@ -672,8 +672,8 @@
             query = PG_GETARG_LQUERY(1);
             if (GIST_LEAF(entry))
                 res = DatumGetBool(DirectFunctionCall2(ltq_regex,
-                                              PointerGetDatum(LTG_NODE(key)),
-                                            PointerGetDatum((lquery *) query)
+                                                       PointerGetDatum(LTG_NODE(key)),
+                                                       PointerGetDatum((lquery *) query)
                                                        ));
             else
                 res = (gist_qe(key, (lquery *) query) && gist_between(key, (lquery *) query));
@@ -683,8 +683,8 @@
             query = PG_GETARG_LQUERY(1);
             if (GIST_LEAF(entry))
                 res = DatumGetBool(DirectFunctionCall2(ltxtq_exec,
-                                              PointerGetDatum(LTG_NODE(key)),
-                                            PointerGetDatum((lquery *) query)
+                                                       PointerGetDatum(LTG_NODE(key)),
+                                                       PointerGetDatum((lquery *) query)
                                                        ));
             else
                 res = gist_qtxt(key, (ltxtquery *) query);
@@ -694,8 +694,8 @@
             query = DatumGetPointer(PG_DETOAST_DATUM(PG_GETARG_DATUM(1)));
             if (GIST_LEAF(entry))
                 res = DatumGetBool(DirectFunctionCall2(lt_q_regex,
-                                              PointerGetDatum(LTG_NODE(key)),
-                                         PointerGetDatum((ArrayType *) query)
+                                                       PointerGetDatum(LTG_NODE(key)),
+                                                       PointerGetDatum((ArrayType *) query)
                                                        ));
             else
                 res = arrq_cons(key, (ArrayType *) query);
diff -ur pgsql/contrib/ltree/ltree_io.c pgsql-dup/contrib/ltree/ltree_io.c
--- pgsql/contrib/ltree/ltree_io.c    2017-06-16 12:31:37.105509156 -0400
+++ pgsql-dup/contrib/ltree/ltree_io.c    2017-06-16 12:35:21.239057302 -0400
@@ -61,8 +61,8 @@
     if (num + 1 > MaxAllocSize / sizeof(nodeitem))
         ereport(ERROR,
                 (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
-             errmsg("number of levels (%d) exceeds the maximum allowed (%d)",
-                    num + 1, (int) (MaxAllocSize / sizeof(nodeitem)))));
+                 errmsg("number of levels (%d) exceeds the maximum allowed (%d)",
+                        num + 1, (int) (MaxAllocSize / sizeof(nodeitem)))));
     list = lptr = (nodeitem *) palloc(sizeof(nodeitem) * (num + 1));
     ptr = buf;
     while (*ptr)
@@ -230,8 +230,8 @@
     if (num > MaxAllocSize / ITEMSIZE)
         ereport(ERROR,
                 (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
-             errmsg("number of levels (%d) exceeds the maximum allowed (%d)",
-                    num, (int) (MaxAllocSize / ITEMSIZE))));
+                 errmsg("number of levels (%d) exceeds the maximum allowed (%d)",
+                        num, (int) (MaxAllocSize / ITEMSIZE))));
     curqlevel = tmpql = (lquery_level *) palloc0(ITEMSIZE * num);
     ptr = buf;
     while (*ptr)
diff -ur pgsql/contrib/oid2name/oid2name.c pgsql-dup/contrib/oid2name/oid2name.c
--- pgsql/contrib/oid2name/oid2name.c    2017-06-16 12:31:36.917504501 -0400
+++ pgsql-dup/contrib/oid2name/oid2name.c    2017-06-16 12:35:21.060052872 -0400
@@ -211,7 +211,7 @@
     {
         eary      ->alloc *= 2;
         eary      ->array = (char **) pg_realloc(eary->array,
-                                               eary->alloc * sizeof(char *));
+                                                 eary->alloc * sizeof(char *));
     }

     eary      ->array[eary->num] = pg_strdup(eltname);
@@ -436,7 +436,7 @@
     snprintf(todo, sizeof(todo),
              "SELECT pg_catalog.pg_relation_filenode(c.oid) as \"Filenode\", relname as \"Table Name\" %s "
              "FROM pg_catalog.pg_class c "
-           "    LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace "
+             "    LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace "
              "    LEFT JOIN pg_catalog.pg_database d ON d.datname = pg_catalog.current_database(),"
              "    pg_catalog.pg_tablespace t "
              "WHERE relkind IN (" CppAsString2(RELKIND_RELATION) ","
@@ -507,7 +507,7 @@
     todo = psprintf(
                     "SELECT pg_catalog.pg_relation_filenode(c.oid) as \"Filenode\", relname as \"Table Name\" %s\n"
                     "FROM pg_catalog.pg_class c\n"
-         "    LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace\n"
+                    "    LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace\n"
                     "    LEFT JOIN pg_catalog.pg_database d ON d.datname = pg_catalog.current_database(),\n"
                     "    pg_catalog.pg_tablespace t\n"
                     "WHERE relkind IN (" CppAsString2(RELKIND_RELATION) ","
diff -ur pgsql/contrib/pageinspect/brinfuncs.c pgsql-dup/contrib/pageinspect/brinfuncs.c
--- pgsql/contrib/pageinspect/brinfuncs.c    2017-06-16 12:31:37.992531121 -0400
+++ pgsql-dup/contrib/pageinspect/brinfuncs.c    2017-06-16 12:35:22.114078954 -0400
@@ -226,7 +226,7 @@
             if (ItemIdIsUsed(itemId))
             {
                 dtup = brin_deform_tuple(bdesc,
-                                     (BrinTuple *) PageGetItem(page, itemId),
+                                         (BrinTuple *) PageGetItem(page, itemId),
                                          NULL);
                 attno = 1;
                 unusedItem = false;
diff -ur pgsql/contrib/pageinspect/btreefuncs.c pgsql-dup/contrib/pageinspect/btreefuncs.c
--- pgsql/contrib/pageinspect/btreefuncs.c    2017-06-16 12:31:38.010531567 -0400
+++ pgsql-dup/contrib/pageinspect/btreefuncs.c    2017-06-16 12:35:22.132079400 -0400
@@ -346,7 +346,7 @@
         if (RELATION_IS_OTHER_TEMP(rel))
             ereport(ERROR,
                     (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                errmsg("cannot access temporary tables of other sessions")));
+                     errmsg("cannot access temporary tables of other sessions")));

         if (blkno == 0)
             elog(ERROR, "block 0 is a meta page");
@@ -442,7 +442,7 @@
         if (raw_page_size < SizeOfPageHeaderData)
             ereport(ERROR,
                     (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-                  errmsg("input page too small (%d bytes)", raw_page_size)));
+                     errmsg("input page too small (%d bytes)", raw_page_size)));

         fctx = SRF_FIRSTCALL_INIT();
         mctx = MemoryContextSwitchTo(fctx->multi_call_memory_ctx);
diff -ur pgsql/contrib/pageinspect/ginfuncs.c pgsql-dup/contrib/pageinspect/ginfuncs.c
--- pgsql/contrib/pageinspect/ginfuncs.c    2017-06-16 12:31:38.061532829 -0400
+++ pgsql-dup/contrib/pageinspect/ginfuncs.c    2017-06-16 12:35:22.182080639 -0400
@@ -196,7 +196,7 @@
         if (opaq->flags != (GIN_DATA | GIN_LEAF | GIN_COMPRESSED))
             ereport(ERROR,
                     (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-                 errmsg("input page is not a compressed GIN data leaf page"),
+                     errmsg("input page is not a compressed GIN data leaf page"),
                      errdetail("Flags %04X, expected %04X",
                                opaq->flags,
                                (GIN_DATA | GIN_LEAF | GIN_COMPRESSED))));
diff -ur pgsql/contrib/pageinspect/hashfuncs.c pgsql-dup/contrib/pageinspect/hashfuncs.c
--- pgsql/contrib/pageinspect/hashfuncs.c    2017-06-16 12:31:38.046532458 -0400
+++ pgsql-dup/contrib/pageinspect/hashfuncs.c    2017-06-16 12:35:22.166080243 -0400
@@ -99,7 +99,7 @@
             case LH_BUCKET_PAGE | LH_OVERFLOW_PAGE:
                 ereport(ERROR,
                         (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-                      errmsg("page is not a hash bucket or overflow page")));
+                         errmsg("page is not a hash bucket or overflow page")));
             case LH_OVERFLOW_PAGE:
                 ereport(ERROR,
                         (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
diff -ur pgsql/contrib/pageinspect/heapfuncs.c pgsql-dup/contrib/pageinspect/heapfuncs.c
--- pgsql/contrib/pageinspect/heapfuncs.c    2017-06-16 12:31:38.027531988 -0400
+++ pgsql-dup/contrib/pageinspect/heapfuncs.c    2017-06-16 12:35:22.148079796 -0400
@@ -84,7 +84,7 @@
         else
             ereport(ERROR,
                     (errcode(ERRCODE_DATA_CORRUPTED),
-               errmsg("illegal character '%c' in t_bits string", str[off])));
+                     errmsg("illegal character '%c' in t_bits string", str[off])));

         if (off % 8 == 7)
             bits[off / 8] = byte;
@@ -132,7 +132,7 @@
         if (raw_page_size < SizeOfPageHeaderData)
             ereport(ERROR,
                     (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-                  errmsg("input page too small (%d bytes)", raw_page_size)));
+                     errmsg("input page too small (%d bytes)", raw_page_size)));

         fctx = SRF_FIRSTCALL_INIT();
         mctx = MemoryContextSwitchTo(fctx->multi_call_memory_ctx);
@@ -236,7 +236,7 @@
                     bits_len =
                         ((tuphdr->t_infomask2 & HEAP_NATTS_MASK) / 8 + 1) * 8;
                     values[11] = CStringGetTextDatum(
-                                     bits_to_text(tuphdr->t_bits, bits_len));
+                                                     bits_to_text(tuphdr->t_bits, bits_len));
                 }
                 else
                     nulls[11] = true;
@@ -384,7 +384,7 @@
     if (tupdata_len != off)
         ereport(ERROR,
                 (errcode(ERRCODE_DATA_CORRUPTED),
-            errmsg("end of tuple reached without looking at all its data")));
+                 errmsg("end of tuple reached without looking at all its data")));

     return makeArrayResult(raw_attrs, CurrentMemoryContext);
 }
diff -ur pgsql/contrib/pageinspect/rawpage.c pgsql-dup/contrib/pageinspect/rawpage.c
--- pgsql/contrib/pageinspect/rawpage.c    2017-06-16 12:31:37.962530377 -0400
+++ pgsql-dup/contrib/pageinspect/rawpage.c    2017-06-16 12:35:22.084078213 -0400
@@ -311,7 +311,7 @@
     if (raw_page_size != BLCKSZ)
         ereport(ERROR,
                 (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-          errmsg("incorrect size of input page (%d bytes)", raw_page_size)));
+                 errmsg("incorrect size of input page (%d bytes)", raw_page_size)));

     page = (PageHeader) VARDATA(raw_page);

diff -ur pgsql/contrib/passwordcheck/passwordcheck.c pgsql-dup/contrib/passwordcheck/passwordcheck.c
--- pgsql/contrib/passwordcheck/passwordcheck.c    2017-06-16 12:31:39.279562991 -0400
+++ pgsql-dup/contrib/passwordcheck/passwordcheck.c    2017-06-16 12:35:23.390110531 -0400
@@ -112,7 +112,7 @@
         if (!pwd_has_letter || !pwd_has_nonletter)
             ereport(ERROR,
                     (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-               errmsg("password must contain both letters and nonletters")));
+                     errmsg("password must contain both letters and nonletters")));

 #ifdef USE_CRACKLIB
         /* call cracklib to check password */
diff -ur pgsql/contrib/pg_prewarm/pg_prewarm.c pgsql-dup/contrib/pg_prewarm/pg_prewarm.c
--- pgsql/contrib/pg_prewarm/pg_prewarm.c    2017-06-16 12:31:39.211561306 -0400
+++ pgsql-dup/contrib/pg_prewarm/pg_prewarm.c    2017-06-16 12:35:23.321108824 -0400
@@ -138,8 +138,8 @@
         if (last_block < 0 || last_block >= nblocks)
             ereport(ERROR,
                     (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-            errmsg("ending block number must be between 0 and " INT64_FORMAT,
-                   nblocks - 1)));
+                     errmsg("ending block number must be between 0 and " INT64_FORMAT,
+                            nblocks - 1)));
     }

     /* Now we're ready to do the real work. */
diff -ur pgsql/contrib/pg_standby/pg_standby.c pgsql-dup/contrib/pg_standby/pg_standby.c
--- pgsql/contrib/pg_standby/pg_standby.c    2017-06-16 12:31:36.875503461 -0400
+++ pgsql-dup/contrib/pg_standby/pg_standby.c    2017-06-16 12:35:21.017051809 -0400
@@ -256,7 +256,7 @@
                  * in case this worries you.
                  */
                 if (IsXLogFileName(xlde->d_name) &&
-                  strcmp(xlde->d_name + 8, exclusiveCleanupFileName + 8) < 0)
+                    strcmp(xlde->d_name + 8, exclusiveCleanupFileName + 8) < 0)
                 {
 #ifdef WIN32
                     snprintf(WALFilePath, sizeof(WALFilePath), "%s\\%s", archiveLocation, xlde->d_name);
@@ -523,7 +523,7 @@
            "Main intended use as restore_command in recovery.conf:\n"
            "  restore_command = 'pg_standby [OPTION]... ARCHIVELOCATION %%f %%p %%r'\n"
            "e.g.\n"
-    "  restore_command = 'pg_standby /mnt/server/archiverdir %%f %%p %%r'\n");
+           "  restore_command = 'pg_standby /mnt/server/archiverdir %%f %%p %%r'\n");
     printf("\nReport bugs to <pgsql-bugs@postgresql.org>.\n");
 }

diff -ur pgsql/contrib/pg_stat_statements/pg_stat_statements.c
pgsql-dup/contrib/pg_stat_statements/pg_stat_statements.c
--- pgsql/contrib/pg_stat_statements/pg_stat_statements.c    2017-06-16 12:31:38.373540556 -0400
+++ pgsql-dup/contrib/pg_stat_statements/pg_stat_statements.c    2017-06-16 12:35:22.493088334 -0400
@@ -358,7 +358,7 @@
      * Define (or redefine) custom GUC variables.
      */
     DefineCustomIntVariable("pg_stat_statements.max",
-      "Sets the maximum number of statements tracked by pg_stat_statements.",
+                            "Sets the maximum number of statements tracked by pg_stat_statements.",
                             NULL,
                             &pgss_max,
                             5000,
@@ -371,7 +371,7 @@
                             NULL);

     DefineCustomEnumVariable("pg_stat_statements.track",
-               "Selects which statements are tracked by pg_stat_statements.",
+                             "Selects which statements are tracked by pg_stat_statements.",
                              NULL,
                              &pgss_track,
                              PGSS_TRACK_TOP,
@@ -383,7 +383,7 @@
                              NULL);

     DefineCustomBoolVariable("pg_stat_statements.track_utility",
-       "Selects whether utility commands are tracked by pg_stat_statements.",
+                             "Selects whether utility commands are tracked by pg_stat_statements.",
                              NULL,
                              &pgss_track_utility,
                              true,
@@ -394,7 +394,7 @@
                              NULL);

     DefineCustomBoolVariable("pg_stat_statements.save",
-               "Save pg_stat_statements statistics across server shutdowns.",
+                             "Save pg_stat_statements statistics across server shutdowns.",
                              NULL,
                              &pgss_save,
                              true,
diff -ur pgsql/contrib/pgcrypto/crypt-blowfish.c pgsql-dup/contrib/pgcrypto/crypt-blowfish.c
--- pgsql/contrib/pgcrypto/crypt-blowfish.c    2017-06-16 12:31:38.957555016 -0400
+++ pgsql-dup/contrib/pgcrypto/crypt-blowfish.c    2017-06-16 12:35:23.067102539 -0400
@@ -737,7 +737,7 @@

     memcpy(output, setting, 7 + 22 - 1);
     output[7 + 22 - 1] = BF_itoa64[(int)
-                         BF_atoi64[(int) setting[7 + 22 - 1] - 0x20] & 0x30];
+                                   BF_atoi64[(int) setting[7 + 22 - 1] - 0x20] & 0x30];

 /* This has to be bug-compatible with the original implementation, so
  * only encode 23 of the 24 bytes. :-) */
diff -ur pgsql/contrib/pgcrypto/crypt-gensalt.c pgsql-dup/contrib/pgcrypto/crypt-gensalt.c
--- pgsql/contrib/pgcrypto/crypt-gensalt.c    2017-06-16 12:31:38.609546399 -0400
+++ pgsql-dup/contrib/pgcrypto/crypt-gensalt.c    2017-06-16 12:35:22.729094174 -0400
@@ -23,7 +23,7 @@

 char *
 _crypt_gensalt_traditional_rn(unsigned long count,
-                  const char *input, int size, char *output, int output_size)
+                              const char *input, int size, char *output, int output_size)
 {
     if (size < 2 || output_size < 2 + 1 || (count && count != 25))
     {
@@ -41,7 +41,7 @@

 char *
 _crypt_gensalt_extended_rn(unsigned long count,
-                  const char *input, int size, char *output, int output_size)
+                           const char *input, int size, char *output, int output_size)
 {
     unsigned long value;

@@ -77,7 +77,7 @@

 char *
 _crypt_gensalt_md5_rn(unsigned long count,
-                  const char *input, int size, char *output, int output_size)
+                      const char *input, int size, char *output, int output_size)
 {
     unsigned long value;

@@ -159,7 +159,7 @@

 char *
 _crypt_gensalt_blowfish_rn(unsigned long count,
-                  const char *input, int size, char *output, int output_size)
+                           const char *input, int size, char *output, int output_size)
 {
     if (size < 16 || output_size < 7 + 22 + 1 ||
         (count && (count < 4 || count > 31)))
diff -ur pgsql/contrib/pgcrypto/px-crypt.h pgsql-dup/contrib/pgcrypto/px-crypt.h
--- pgsql/contrib/pgcrypto/px-crypt.h    2017-06-16 12:31:38.388540927 -0400
+++ pgsql-dup/contrib/pgcrypto/px-crypt.h    2017-06-16 12:35:22.507088681 -0400
@@ -57,13 +57,13 @@

 /* crypt-gensalt.c */
 char *_crypt_gensalt_traditional_rn(unsigned long count,
-                 const char *input, int size, char *output, int output_size);
+                              const char *input, int size, char *output, int output_size);
 char *_crypt_gensalt_extended_rn(unsigned long count,
-                 const char *input, int size, char *output, int output_size);
+                           const char *input, int size, char *output, int output_size);
 char *_crypt_gensalt_md5_rn(unsigned long count,
-                 const char *input, int size, char *output, int output_size);
+                      const char *input, int size, char *output, int output_size);
 char *_crypt_gensalt_blowfish_rn(unsigned long count,
-                 const char *input, int size, char *output, int output_size);
+                           const char *input, int size, char *output, int output_size);

 /* disable 'extended DES crypt' */
 /* #define DISABLE_XDES */
diff -ur pgsql/src/bin/psql/describe.c pgsql-dup/src/bin/psql/describe.c
--- pgsql/src/bin/psql/describe.c    2017-06-16 12:31:06.247745021 -0400
+++ pgsql-dup/src/bin/psql/describe.c    2017-06-16 12:34:50.771303324 -0400
@@ -69,7 +69,7 @@
     printfPQExpBuffer(&buf,
                       "SELECT n.nspname as \"%s\",\n"
                       "  p.proname AS \"%s\",\n"
-                 "  pg_catalog.format_type(p.prorettype, NULL) AS \"%s\",\n",
+                      "  pg_catalog.format_type(p.prorettype, NULL) AS \"%s\",\n",
                       gettext_noop("Schema"),
                       gettext_noop("Name"),
                       gettext_noop("Result data type"));
@@ -78,7 +78,7 @@
         appendPQExpBuffer(&buf,
                           "  CASE WHEN p.pronargs = 0\n"
                           "    THEN CAST('*' AS pg_catalog.text)\n"
-                     "    ELSE pg_catalog.pg_get_function_arguments(p.oid)\n"
+                          "    ELSE pg_catalog.pg_get_function_arguments(p.oid)\n"
                           "  END AS \"%s\",\n",
                           gettext_noop("Argument data types"));
     else if (pset.sversion >= 80200)
@@ -88,7 +88,7 @@
                           "    ELSE\n"
                           "    pg_catalog.array_to_string(ARRAY(\n"
                           "      SELECT\n"
-                 "        pg_catalog.format_type(p.proargtypes[s.i], NULL)\n"
+                          "        pg_catalog.format_type(p.proargtypes[s.i], NULL)\n"
                           "      FROM\n"
                           "        pg_catalog.generate_series(0, pg_catalog.array_upper(p.proargtypes, 1)) AS s(i)\n"
                           "    ), ', ')\n"
@@ -96,13 +96,13 @@
                           gettext_noop("Argument data types"));
     else
         appendPQExpBuffer(&buf,
-             "  pg_catalog.format_type(p.proargtypes[0], NULL) AS \"%s\",\n",
+                          "  pg_catalog.format_type(p.proargtypes[0], NULL) AS \"%s\",\n",
                           gettext_noop("Argument data types"));

     appendPQExpBuffer(&buf,
-                 "  pg_catalog.obj_description(p.oid, 'pg_proc') as \"%s\"\n"
+                      "  pg_catalog.obj_description(p.oid, 'pg_proc') as \"%s\"\n"
                       "FROM pg_catalog.pg_proc p\n"
-       "     LEFT JOIN pg_catalog.pg_namespace n ON n.oid = p.pronamespace\n"
+                      "     LEFT JOIN pg_catalog.pg_namespace n ON n.oid = p.pronamespace\n"
                       "WHERE p.proisagg\n",
                       gettext_noop("Description"));

@@ -167,7 +167,7 @@
     {
         appendPQExpBuffer(&buf,
                           ",\n  amhandler AS \"%s\",\n"
-                      "  pg_catalog.obj_description(oid, 'pg_am') AS \"%s\"",
+                          "  pg_catalog.obj_description(oid, 'pg_am') AS \"%s\"",
                           gettext_noop("Handler"),
                           gettext_noop("Description"));
     }
@@ -223,15 +223,15 @@
     if (pset.sversion >= 90200)
         printfPQExpBuffer(&buf,
                           "SELECT spcname AS \"%s\",\n"
-                        "  pg_catalog.pg_get_userbyid(spcowner) AS \"%s\",\n"
-                        "  pg_catalog.pg_tablespace_location(oid) AS \"%s\"",
+                          "  pg_catalog.pg_get_userbyid(spcowner) AS \"%s\",\n"
+                          "  pg_catalog.pg_tablespace_location(oid) AS \"%s\"",
                           gettext_noop("Name"),
                           gettext_noop("Owner"),
                           gettext_noop("Location"));
     else
         printfPQExpBuffer(&buf,
                           "SELECT spcname AS \"%s\",\n"
-                        "  pg_catalog.pg_get_userbyid(spcowner) AS \"%s\",\n"
+                          "  pg_catalog.pg_get_userbyid(spcowner) AS \"%s\",\n"
                           "  spclocation AS \"%s\"",
                           gettext_noop("Name"),
                           gettext_noop("Owner"),
@@ -255,7 +255,7 @@

     if (verbose && pset.sversion >= 80200)
         appendPQExpBuffer(&buf,
-         ",\n  pg_catalog.shobj_description(oid, 'pg_tablespace') AS \"%s\"",
+                          ",\n  pg_catalog.shobj_description(oid, 'pg_tablespace') AS \"%s\"",
                           gettext_noop("Description"));

     appendPQExpBufferStr(&buf,
@@ -344,8 +344,8 @@

     if (pset.sversion >= 80400)
         appendPQExpBuffer(&buf,
-                    "  pg_catalog.pg_get_function_result(p.oid) as \"%s\",\n"
-                 "  pg_catalog.pg_get_function_arguments(p.oid) as \"%s\",\n"
+                          "  pg_catalog.pg_get_function_result(p.oid) as \"%s\",\n"
+                          "  pg_catalog.pg_get_function_arguments(p.oid) as \"%s\",\n"
                           " CASE\n"
                           "  WHEN p.proisagg THEN '%s'\n"
                           "  WHEN p.proiswindow THEN '%s'\n"
@@ -362,22 +362,22 @@
                           gettext_noop("Type"));
     else if (pset.sversion >= 80100)
         appendPQExpBuffer(&buf,
-                     "  CASE WHEN p.proretset THEN 'SETOF ' ELSE '' END ||\n"
-                  "  pg_catalog.format_type(p.prorettype, NULL) as \"%s\",\n"
+                          "  CASE WHEN p.proretset THEN 'SETOF ' ELSE '' END ||\n"
+                          "  pg_catalog.format_type(p.prorettype, NULL) as \"%s\",\n"
                           "  CASE WHEN proallargtypes IS NOT NULL THEN\n"
                           "    pg_catalog.array_to_string(ARRAY(\n"
                           "      SELECT\n"
                           "        CASE\n"
                           "          WHEN p.proargmodes[s.i] = 'i' THEN ''\n"
-                      "          WHEN p.proargmodes[s.i] = 'o' THEN 'OUT '\n"
-                    "          WHEN p.proargmodes[s.i] = 'b' THEN 'INOUT '\n"
-                 "          WHEN p.proargmodes[s.i] = 'v' THEN 'VARIADIC '\n"
+                          "          WHEN p.proargmodes[s.i] = 'o' THEN 'OUT '\n"
+                          "          WHEN p.proargmodes[s.i] = 'b' THEN 'INOUT '\n"
+                          "          WHEN p.proargmodes[s.i] = 'v' THEN 'VARIADIC '\n"
                           "        END ||\n"
                           "        CASE\n"
-             "          WHEN COALESCE(p.proargnames[s.i], '') = '' THEN ''\n"
+                          "          WHEN COALESCE(p.proargnames[s.i], '') = '' THEN ''\n"
                           "          ELSE p.proargnames[s.i] || ' '\n"
                           "        END ||\n"
-              "        pg_catalog.format_type(p.proallargtypes[s.i], NULL)\n"
+                          "        pg_catalog.format_type(p.proallargtypes[s.i], NULL)\n"
                           "      FROM\n"
                           "        pg_catalog.generate_series(1, pg_catalog.array_upper(p.proallargtypes, 1)) AS
s(i)\n"
                           "    ), ', ')\n"
@@ -385,10 +385,10 @@
                           "    pg_catalog.array_to_string(ARRAY(\n"
                           "      SELECT\n"
                           "        CASE\n"
-           "          WHEN COALESCE(p.proargnames[s.i+1], '') = '' THEN ''\n"
+                          "          WHEN COALESCE(p.proargnames[s.i+1], '') = '' THEN ''\n"
                           "          ELSE p.proargnames[s.i+1] || ' '\n"
                           "          END ||\n"
-                 "        pg_catalog.format_type(p.proargtypes[s.i], NULL)\n"
+                          "        pg_catalog.format_type(p.proargtypes[s.i], NULL)\n"
                           "      FROM\n"
                           "        pg_catalog.generate_series(0, pg_catalog.array_upper(p.proargtypes, 1)) AS s(i)\n"
                           "    ), ', ')\n"
@@ -407,9 +407,9 @@
                           gettext_noop("Type"));
     else
         appendPQExpBuffer(&buf,
-                     "  CASE WHEN p.proretset THEN 'SETOF ' ELSE '' END ||\n"
-                  "  pg_catalog.format_type(p.prorettype, NULL) as \"%s\",\n"
-                    "  pg_catalog.oidvectortypes(p.proargtypes) as \"%s\",\n"
+                          "  CASE WHEN p.proretset THEN 'SETOF ' ELSE '' END ||\n"
+                          "  pg_catalog.format_type(p.prorettype, NULL) as \"%s\",\n"
+                          "  pg_catalog.oidvectortypes(p.proargtypes) as \"%s\",\n"
                           "  CASE\n"
                           "    WHEN p.proisagg THEN '%s'\n"
                           "    WHEN p.prorettype = 'pg_catalog.trigger'::pg_catalog.regtype THEN '%s'\n"
@@ -447,8 +447,8 @@
                               gettext_noop("unsafe"),
                               gettext_noop("Parallel"));
         appendPQExpBuffer(&buf,
-                       ",\n pg_catalog.pg_get_userbyid(p.proowner) as \"%s\""
-                 ",\n CASE WHEN prosecdef THEN '%s' ELSE '%s' END AS \"%s\"",
+                          ",\n pg_catalog.pg_get_userbyid(p.proowner) as \"%s\""
+                          ",\n CASE WHEN prosecdef THEN '%s' ELSE '%s' END AS \"%s\"",
                           gettext_noop("Owner"),
                           gettext_noop("definer"),
                           gettext_noop("invoker"),
@@ -458,7 +458,7 @@
         appendPQExpBuffer(&buf,
                           ",\n l.lanname as \"%s\""
                           ",\n p.prosrc as \"%s\""
-                ",\n pg_catalog.obj_description(p.oid, 'pg_proc') as \"%s\"",
+                          ",\n pg_catalog.obj_description(p.oid, 'pg_proc') as \"%s\"",
                           gettext_noop("Language"),
                           gettext_noop("Source code"),
                           gettext_noop("Description"));
@@ -466,11 +466,11 @@

     appendPQExpBufferStr(&buf,
                          "\nFROM pg_catalog.pg_proc p"
-    "\n     LEFT JOIN pg_catalog.pg_namespace n ON n.oid = p.pronamespace\n");
+                         "\n     LEFT JOIN pg_catalog.pg_namespace n ON n.oid = p.pronamespace\n");

     if (verbose)
         appendPQExpBufferStr(&buf,
-           "     LEFT JOIN pg_catalog.pg_language l ON l.oid = p.prolang\n");
+                             "     LEFT JOIN pg_catalog.pg_language l ON l.oid = p.prolang\n");

     have_where = false;

@@ -530,7 +530,7 @@
             if (needs_or)
                 appendPQExpBufferStr(&buf, "       OR ");
             appendPQExpBufferStr(&buf,
-                "p.prorettype = 'pg_catalog.trigger'::pg_catalog.regtype\n");
+                                 "p.prorettype = 'pg_catalog.trigger'::pg_catalog.regtype\n");
             needs_or = true;
         }
         if (showWindow)
@@ -634,7 +634,7 @@
     if (verbose)
     {
         appendPQExpBuffer(&buf,
-                     "  pg_catalog.pg_get_userbyid(t.typowner) AS \"%s\",\n",
+                          "  pg_catalog.pg_get_userbyid(t.typowner) AS \"%s\",\n",
                           gettext_noop("Owner"));
     }
     if (verbose && pset.sversion >= 90200)

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Preliminary results for proposed new pgindentimplementation

От
Andres Freund
Дата:
Hi,

On 2017-06-16 13:10:50 -0400, Tom Lane wrote:
> I experimented with disabling that logic and just always aligning
> to the paren indentation.  That fixes the weird cases with continued
> string literals, but it also makes for a heck of a lot of other changes.
> The full diff is too big to post here, but I've attached a selection
> of diff hunks to give you an idea.  I'm not really sure if I like this
> better than pgindent's traditional behavior --- but it's arguably less
> confusing.
> 
> An intermediate position that we could consider is to disable the back-off
> logic only when the line starts with a string literal.  I haven't actually
> coded this but it looks like it would be easy, if grotty.

I think the current logic is pretty horrible, primarily because it's so
hard to get to manually.  I could live with both of these proposed
changes, the selection of the changes you posted looks like it could be
improved by code changes, but that's obviously a large amount of work.
The heuristic also seems to make sense.

At this point however I wonder whether just moving to the new tool on
its own wouldn't be a big enough change - we could just delay that
decision until we've got the rest done at least.

- Andres



Re: [HACKERS] Preliminary results for proposed new pgindent implementation

От
Tom Lane
Дата:
Andres Freund <andres@anarazel.de> writes:
> I think the current logic is pretty horrible, primarily because it's so
> hard to get to manually.

Yes, I think that's really the big argument against it: no editor on
the face of the planet will indent code that way to start with.

> I could live with both of these proposed
> changes, the selection of the changes you posted looks like it could be
> improved by code changes, but that's obviously a large amount of work.

In the end, the only thing that fixes this sort of stuff is to be more
rigid about making the code fit into 80 columns to begin with.  I get
the impression though that a lot of people work in editor windows that
are wider than that, so the code looks fine to them when it slops over
a bit.

> At this point however I wonder whether just moving to the new tool on
> its own wouldn't be a big enough change - we could just delay that
> decision until we've got the rest done at least.

I'm torn between that approach and "let's just have one big flag day
and get it over with".  I think having the rules incrementally changing
from one release to the next will be a huge headache.

I do intend to apply the diffs to HEAD in multiple steps, just to
make them more reviewable.  But I think we should probably absorb
all the changes we want into v10, not leave some for later cycles.
        regards, tom lane



Re: [HACKERS] Preliminary results for proposed new pgindentimplementation

От
Bruce Momjian
Дата:
On Fri, Jun 16, 2017 at 01:34:01PM -0400, Tom Lane wrote:
> > I could live with both of these proposed
> > changes, the selection of the changes you posted looks like it could be
> > improved by code changes, but that's obviously a large amount of work.
> 
> In the end, the only thing that fixes this sort of stuff is to be more
> rigid about making the code fit into 80 columns to begin with.  I get
> the impression though that a lot of people work in editor windows that
> are wider than that, so the code looks fine to them when it slops over
> a bit.

Yes, it is all about <80 column output.  The current pgindent does
everything possible to accomplish that --- the question is whether we
want uglier code to do it.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +



Re: [HACKERS] Preliminary results for proposed new pgindentimplementation

От
Andres Freund
Дата:
On 2017-06-16 13:44:30 -0400, Bruce Momjian wrote:
> On Fri, Jun 16, 2017 at 01:34:01PM -0400, Tom Lane wrote:
> > > I could live with both of these proposed
> > > changes, the selection of the changes you posted looks like it could be
> > > improved by code changes, but that's obviously a large amount of work.
> > 
> > In the end, the only thing that fixes this sort of stuff is to be more
> > rigid about making the code fit into 80 columns to begin with.  I get
> > the impression though that a lot of people work in editor windows that
> > are wider than that, so the code looks fine to them when it slops over
> > a bit.
> 
> Yes, it is all about <80 column output.  The current pgindent does
> everything possible to accomplish that --- the question is whether we
> want uglier code to do it.

For me personally the misindentation is way uglier than a too long line.
I think a number of those long-lines are there because pgindent
sometimes re-indents lines that are continuations of previous ones
pretty far, making it hard to reduce indentation.

- Andres



Re: [HACKERS] Preliminary results for proposed new pgindentimplementation

От
Andres Freund
Дата:
On 2017-06-16 13:34:01 -0400, Tom Lane wrote:
> > I could live with both of these proposed
> > changes, the selection of the changes you posted looks like it could be
> > improved by code changes, but that's obviously a large amount of work.
> 
> In the end, the only thing that fixes this sort of stuff is to be more
> rigid about making the code fit into 80 columns to begin with.  I get
> the impression though that a lot of people work in editor windows that
> are wider than that, so the code looks fine to them when it slops over
> a bit.

That, but maybe also that it's often slightly too long line vs. weird
multiline mess.  A good number of things pgindent indents weirdly can be
prevented by just not adding a linebreak, which isn't a great fix...


> > At this point however I wonder whether just moving to the new tool on
> > its own wouldn't be a big enough change - we could just delay that
> > decision until we've got the rest done at least.
> 
> I'm torn between that approach and "let's just have one big flag day
> and get it over with".

I don't have a strong opinion on this.


> I think having the rules incrementally changing from one release to
> the next will be a huge headache.

Yea, I was more thinking of getting the new indent in, and then making
the followup decisions a few days after.


> I do intend to apply the diffs to HEAD in multiple steps, just to
> make them more reviewable.  But I think we should probably absorb
> all the changes we want into v10, not leave some for later cycles.

Btw, how much are you planning to backpatch these?

Greetings,

Andres Freund



Re: [HACKERS] Preliminary results for proposed new pgindent implementation

От
Tom Lane
Дата:
Andres Freund <andres@anarazel.de> writes:
> On 2017-06-16 13:44:30 -0400, Bruce Momjian wrote:
>> Yes, it is all about <80 column output.  The current pgindent does
>> everything possible to accomplish that --- the question is whether we
>> want uglier code to do it.

> For me personally the misindentation is way uglier than a too long line.

I'm coming around to that opinion too.  We have many source lines that
are a bit too long, or a lot too long if someone decided they didn't
want to split an error message across lines.  pgindent "fixes" that
in some places but not others (if it would have to go left of the
prevailing statement indent, it gives up and indents to the paren level
anyway).  On balance it's just weird.  Better to indent normally and
let the programmer decide if she wants to break the lines differently
to keep them from wrapping.

I assume though that Piotr wants an option to preserve that behavior.
I'm happy to write up a patch for bsdindent that adds a switch
controlling this, but is there any rhyme or reason to the way its
switches are named?
        regards, tom lane



Re: [HACKERS] Preliminary results for proposed new pgindent implementation

От
Tom Lane
Дата:
Andres Freund <andres@anarazel.de> writes:
> On 2017-06-16 13:34:01 -0400, Tom Lane wrote:
>> I do intend to apply the diffs to HEAD in multiple steps, just to
>> make them more reviewable.  But I think we should probably absorb
>> all the changes we want into v10, not leave some for later cycles.

> Btw, how much are you planning to backpatch these?

Well, that's something we need to discuss.  I originally argued for
back-patching the new rules, whatever they are (ie, run the new
pgindent on the back branches whenever we've agreed that the dust
has settled).  But I'm starting to realize that that's likely to
be horrid for anyone who's carrying out-of-tree patches, as I know
a lot of packagers do for instance.  We have to trade off our own
inconvenience in making back-patches against inconvenience to
people who are maintaining private patchsets.

One idea that occurs to me after a few minutes' thought is to
announce that we will reindent the back branches, but not till
around the time of v10 final release.  Once v10 is out, anybody
who's carrying a private patchset will be needing to think about
rebasing it on top of reindented code anyway, so dealing with that
in the back branches at the same time might be a bit less work.

Or we could leave the back branches alone and anticipate five
years worth of pain in back-patching.  I don't find that very
appetizing personally, but it might be the easiest sell to the
majority of the community, since very few of us do back-patching
work on a regular basis.
        regards, tom lane



Re: [HACKERS] Preliminary results for proposed new pgindentimplementation

От
Bruce Momjian
Дата:
On Fri, Jun 16, 2017 at 02:23:00PM -0400, Tom Lane wrote:
> Well, that's something we need to discuss.  I originally argued for
> back-patching the new rules, whatever they are (ie, run the new
> pgindent on the back branches whenever we've agreed that the dust
> has settled).  But I'm starting to realize that that's likely to
> be horrid for anyone who's carrying out-of-tree patches, as I know
> a lot of packagers do for instance.  We have to trade off our own
> inconvenience in making back-patches against inconvenience to
> people who are maintaining private patchsets.

Can't they sync up to just before our pgindent commit and run pgindent
on their own code base?

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +



Re: [HACKERS] Preliminary results for proposed new pgindentimplementation

От
Andres Freund
Дата:
On 2017-06-16 14:42:38 -0400, Bruce Momjian wrote:
> On Fri, Jun 16, 2017 at 02:23:00PM -0400, Tom Lane wrote:
> > Well, that's something we need to discuss.  I originally argued for
> > back-patching the new rules, whatever they are (ie, run the new
> > pgindent on the back branches whenever we've agreed that the dust
> > has settled).  But I'm starting to realize that that's likely to
> > be horrid for anyone who's carrying out-of-tree patches, as I know
> > a lot of packagers do for instance.  We have to trade off our own
> > inconvenience in making back-patches against inconvenience to
> > people who are maintaining private patchsets.
> 
> Can't they sync up to just before our pgindent commit and run pgindent
> on their own code base?

That doesn't really help that much if you have a series of patches that
you want to keep independent, e.g. because you might want to submit to
postgres.  And you'll also get a bunch of annoying to resolve merge
conflicts, even if they're easier to resolve with that methodology.

- Andres



Re: [HACKERS] Preliminary results for proposed new pgindentimplementation

От
Bruce Momjian
Дата:
On Fri, Jun 16, 2017 at 11:54:06AM -0700, Andres Freund wrote:
> On 2017-06-16 14:42:38 -0400, Bruce Momjian wrote:
> > On Fri, Jun 16, 2017 at 02:23:00PM -0400, Tom Lane wrote:
> > > Well, that's something we need to discuss.  I originally argued for
> > > back-patching the new rules, whatever they are (ie, run the new
> > > pgindent on the back branches whenever we've agreed that the dust
> > > has settled).  But I'm starting to realize that that's likely to
> > > be horrid for anyone who's carrying out-of-tree patches, as I know
> > > a lot of packagers do for instance.  We have to trade off our own
> > > inconvenience in making back-patches against inconvenience to
> > > people who are maintaining private patchsets.
> > 
> > Can't they sync up to just before our pgindent commit and run pgindent
> > on their own code base?
> 
> That doesn't really help that much if you have a series of patches that
> you want to keep independent, e.g. because you might want to submit to
> postgres.  And you'll also get a bunch of annoying to resolve merge
> conflicts, even if they're easier to resolve with that methodology.

I think we have to ask how much we want to make things easier for people
with modified but continually-updated Postgres trees vs. our
community-tree developers.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +



Re: [HACKERS] Preliminary results for proposed new pgindent implementation

От
Tom Lane
Дата:
One other thing I'd like to do while we're changing this stuff is
to get rid of the need for entab/detab.  Right now, after doing
all the other work, my copy of pgindent is running the code through
detab and then entab so as to match the old decisions about how to
represent whitespace (ie, as spaces or tabs).  This is grotty as
can be.  I managed to tweak bsdindent so that its output matches
what entab would do, by dint of the attached patch, which implements
the rule "use a space instead of a tab if the tab would only move
one column and we don't need another tab after it".  (I think entab
is being weird with the second half of that rule, but if I remove it,
I get circa a thousand lines of invisible whitespace changes; probably
better not to deal with those.  With no patch at all, just letting
bsdindent do what it does now, there's circa ten thousand changed lines.)

Unless Piotr objects, I propose to add another switch to bsdindent
that selects this behavior, and then we can drop entab, removing
another impediment to getting pgindent working.

            regards, tom lane
diff -ur /home/postgres/freebsd_indent/indent.c ./indent.c
--- /home/postgres/freebsd_indent/indent.c    2017-06-13 11:53:59.474524563 -0400
+++ ./indent.c    2017-06-16 15:41:53.515416614 -0400
@@ -1275,7 +1275,7 @@

     CHECK_SIZE_CODE(cur_dec_ind / tabsize);
     while ((tpos = tabsize * (1 + pos / tabsize)) <= cur_dec_ind) {
-        *e_code++ = '\t';
+        *e_code++ = (tpos > pos + 1 || cur_dec_ind >= tpos + tabsize) ? '\t' : ' ';
         pos = tpos;
     }
     }
diff -ur /home/postgres/freebsd_indent/io.c ./io.c
--- /home/postgres/freebsd_indent/io.c    2017-06-13 11:53:59.475524587 -0400
+++ ./io.c    2017-06-16 15:42:47.686762023 -0400
@@ -399,7 +399,7 @@
     int tcur;

     while ((tcur = tabsize * (1 + (curr - 1) / tabsize) + 1) <= target) {
-        putc('\t', output);
+        putc((tcur > curr + 1 || target >= tcur + tabsize) ? '\t' : ' ', output);
         curr = tcur;
     }
     }

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Preliminary results for proposed new pgindentimplementation

От
Bruce Momjian
Дата:
On Fri, Jun 16, 2017 at 03:56:47PM -0400, Tom Lane wrote:
> can be.  I managed to tweak bsdindent so that its output matches
> what entab would do, by dint of the attached patch, which implements
> the rule "use a space instead of a tab if the tab would only move
> one column and we don't need another tab after it".  (I think entab
> is being weird with the second half of that rule, but if I remove it,
> I get circa a thousand lines of invisible whitespace changes; probably
> better not to deal with those.  With no patch at all, just letting
> bsdindent do what it does now, there's circa ten thousand changed lines.)

Yeah, entab was designed to do that, via this C comment:
/* * Is the next character going to be a tab?  We do tab * replacement in the current spot if the next char is * going
tobe a tab and ignore min_spaces. */
 


--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +



Re: [HACKERS] Preliminary results for proposed new pgindentimplementation

От
Piotr Stefaniak
Дата:
On 2017-06-16 21:56, Tom Lane wrote:
> One other thing I'd like to do while we're changing this stuff is
> to get rid of the need for entab/detab.  Right now, after doing
> all the other work, my copy of pgindent is running the code through
> detab and then entab so as to match the old decisions about how to
> represent whitespace (ie, as spaces or tabs).  This is grotty as
> can be.  I managed to tweak bsdindent so that its output matches
> what entab would do, by dint of the attached patch, which implements
> the rule "use a space instead of a tab if the tab would only move
> one column and we don't need another tab after it".  (I think entab
> is being weird with the second half of that rule, but if I remove it,
> I get circa a thousand lines of invisible whitespace changes; probably
> better not to deal with those.  With no patch at all, just letting
> bsdindent do what it does now, there's circa ten thousand changed lines.)
>
> Unless Piotr objects, I propose to add another switch to bsdindent
> that selects this behavior, and then we can drop entab, removing
> another impediment to getting pgindent working.

I understand the reasoning, but this is a very specific need and I think
not at all universal for anyone else in the future. One of the bugs
listed in indent's manpage is that it "has more switches than ls(1)". So
currently I'm against pushing an option for the above upstream, to the
FreeBSD repository.

Why not add this to the already non-empty list of custom patches?



Re: [HACKERS] Preliminary results for proposed new pgindentimplementation

От
Piotr Stefaniak
Дата:
On 2017-06-16 20:11, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
>> On 2017-06-16 13:44:30 -0400, Bruce Momjian wrote:
>>> Yes, it is all about <80 column output.  The current pgindent does
>>> everything possible to accomplish that --- the question is whether we
>>> want uglier code to do it.
> 
>> For me personally the misindentation is way uglier than a too long line.

> 
> I assume though that Piotr wants an option to preserve that behavior.
> I'm happy to write up a patch for bsdindent that adds a switch
> controlling this, but is there any rhyme or reason to the way its
> switches are named?

I don't want to preserve the current behavior at all, but I might need
to add an option for choosing one or the other if users of FreeBSD
indent protest.

I don't have a good name for it. The best I can do is -lpl ("-lp long
lines too").  Can I see the patch?

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

От
Tom Lane
Дата:
Piotr Stefaniak <postgres@piotr-stefaniak.me> writes:
> On 2017-06-16 20:11, Tom Lane wrote:
>> I assume though that Piotr wants an option to preserve that behavior.
>> I'm happy to write up a patch for bsdindent that adds a switch
>> controlling this, but is there any rhyme or reason to the way its
>> switches are named?

> I don't want to preserve the current behavior at all, but I might need
> to add an option for choosing one or the other if users of FreeBSD
> indent protest.

> I don't have a good name for it. The best I can do is -lpl ("-lp long
> lines too").  Can I see the patch?

Here's a patch.  An alternative switch name might be -lpa ("-lp always")
but I'm not set on that.

            regards, tom lane

diff -pudr indent-curr/args.c indent-lpl/args.c
--- indent-curr/args.c    2017-06-16 11:06:53.329712682 -0400
+++ indent-lpl/args.c    2017-06-16 17:43:56.473230024 -0400
@@ -125,6 +125,7 @@ struct pro {
     {"i", PRO_INT, 8, 0, &ps.ind_size},
     {"lc", PRO_INT, 0, 0, &block_comment_max_col},
     {"ldi", PRO_INT, -1, 0, &ps.local_decl_indent},
+    {"lpl", PRO_BOOL, false, ON, &lineup_to_parens_always},
     {"lp", PRO_BOOL, true, ON, &lineup_to_parens},
     {"l", PRO_INT, 78, 0, &max_col},
     {"nbacc", PRO_BOOL, false, OFF, &blanklines_around_conditional_compilation},
@@ -143,6 +144,7 @@ struct pro {
     {"nfc1", PRO_BOOL, true, OFF, &format_col1_comments},
     {"nfcb", PRO_BOOL, true, OFF, &format_block_comments},
     {"nip", PRO_BOOL, true, OFF, &ps.indent_parameters},
+    {"nlpl", PRO_BOOL, false, OFF, &lineup_to_parens_always},
     {"nlp", PRO_BOOL, true, OFF, &lineup_to_parens},
     {"npcs", PRO_BOOL, false, OFF, &proc_calls_space},
     {"npro", PRO_SPECIAL, 0, IGN, 0},
diff -pudr indent-curr/indent.1 indent-lpl/indent.1
--- indent-curr/indent.1    2017-06-16 17:18:05.697722416 -0400
+++ indent-lpl/indent.1    2017-06-16 17:26:53.203823690 -0400
@@ -74,6 +74,7 @@
 .Op Fl \&lc Ns Ar n
 .Op Fl \&ldi Ns Ar n
 .Op Fl \&lp | Fl nlp
+.Op Fl \&lpl | Fl nlpl
 .Op Fl npro
 .Op Fl P Ns Ar file
 .Op Fl pcs | Fl npcs
@@ -388,6 +389,19 @@ p1\ =\ first_procedure(second_procedure(
 \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ third_procedure(p4,
 \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ p5));
 .Ed
+.It Fl \&lpl , nlpl
+With
+.Fl \&lpl ,
+code surrounded by parentheses in continuation lines is lined up even if it
+would extend past the right margin.
+With
+.Fl \&nlpl
+(the default), such a line that would extend past the right margin is moved
+left to keep it within the margin, if that does not require placing it to
+the left of the prevailing indentation level.
+These switches have no effect if
+.Fl nlp
+is selected.
 .It Fl npro
 Causes the profile files,
 .Sq Pa ./.indent.pro
diff -pudr indent-curr/indent.c indent-lpl/indent.c
--- indent-curr/indent.c    2017-06-13 11:53:59.474524563 -0400
+++ indent-lpl/indent.c    2017-06-16 17:29:11.924267996 -0400
@@ -160,6 +160,7 @@ main(int argc, char **argv)
 #ifdef undef
     max_col = 78;        /* -l78 */
     lineup_to_parens = 1;    /* -lp */
+    lineup_to_parens_always = 0;    /* -nlpl */
     ps.ljust_decl = 0;        /* -ndj */
     ps.com_ind = 33;        /* -c33 */
     star_comment_cont = 1;    /* -sc */
diff -pudr indent-curr/indent_globs.h indent-lpl/indent_globs.h
--- indent-curr/indent_globs.h    2017-06-16 11:06:53.329712682 -0400
+++ indent-lpl/indent_globs.h    2017-06-16 17:30:14.664826384 -0400
@@ -185,6 +185,8 @@ int         continuation_indent;/* set t
                  * code and continuation lines */
 int         lineup_to_parens;    /* if true, continued code within parens will
                  * be lined up to the open paren */
+int         lineup_to_parens_always;    /* if true, do not attempt to keep
+                     * lined-up code within the margin */
 int         Bill_Shannon;    /* true iff a blank should always be inserted
                  * after sizeof */
 int         blanklines_after_declarations_at_proctop;    /* This is vaguely
diff -pudr indent-curr/io.c indent-lpl/io.c
--- indent-curr/io.c    2017-06-13 11:53:59.475524587 -0400
+++ indent-lpl/io.c    2017-06-16 17:31:11.233230896 -0400
@@ -221,6 +221,8 @@ compute_code_target(void)
     if (!lineup_to_parens)
         target_col += continuation_indent
         * (2 * continuation_indent == ps.ind_size ? 1 : ps.paren_level);
+    else if (lineup_to_parens_always)
+        target_col = paren_target;
     else {
         int w;
         int t = paren_target;

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

От
Tom Lane
Дата:
Piotr Stefaniak <postgres@piotr-stefaniak.me> writes:
> On 2017-06-16 21:56, Tom Lane wrote:
>> Unless Piotr objects, I propose to add another switch to bsdindent
>> that selects this behavior, and then we can drop entab, removing
>> another impediment to getting pgindent working.

> I understand the reasoning, but this is a very specific need and I think
> not at all universal for anyone else in the future. One of the bugs
> listed in indent's manpage is that it "has more switches than ls(1)". So
> currently I'm against pushing an option for the above upstream, to the
> FreeBSD repository.

> Why not add this to the already non-empty list of custom patches?

Umm ... I thought the idea was to get to the point where the list of
custom patches *is* empty.  Except for carrying our own Makefile of
course.  I'd be sad if we needed a fork just for this.

What I'm testing with right now has just four differences from your repo:

1. This workaround for what I believe you agree is a bug:

-        ps.in_decl = ps.decl_on_line = ps.last_token != type_def;
+        ps.in_decl = ps.decl_on_line = true;

2. The long-lines adjustment I just sent you a patch for.

3. The tab-vs-space difference under discussion here.

4. A temporary hack affecting the indentation of comments on the same line
(forcing them to a multiple of 8 spaces even though tabsize is 4).  I have
every intention of dropping that one later; I just don't want to deal with
comment reindentation at the same time as these other things.
        regards, tom lane



Re: [HACKERS] Preliminary results for proposed new pgindentimplementation

От
Piotr Stefaniak
Дата:
On 2017-06-17 00:02, Tom Lane wrote:
> Piotr Stefaniak <postgres@piotr-stefaniak.me> writes:
>> On 2017-06-16 21:56, Tom Lane wrote:
>>> Unless Piotr objects, I propose to add another switch to bsdindent
>>> that selects this behavior, and then we can drop entab, removing
>>> another impediment to getting pgindent working.
> 
>> I understand the reasoning, but this is a very specific need and I think
>> not at all universal for anyone else in the future. One of the bugs
>> listed in indent's manpage is that it "has more switches than ls(1)". So
>> currently I'm against pushing an option for the above upstream, to the
>> FreeBSD repository.
> 
>> Why not add this to the already non-empty list of custom patches?
> 
> Umm ... I thought the idea was to get to the point where the list of
> custom patches *is* empty.  Except for carrying our own Makefile of
> course.  I'd be sad if we needed a fork just for this.
> 
> What I'm testing with right now has just four differences from your repo:

There are also the "portability fixes" and they're the main problem.

I've simply removed things like capsicum or __FBSDID() because I thought
it wouldn't be a problem since Postgres will have its own copy of indent
anyway (so that its behavior is not a moving target). I can ifdef-out
them instead of removing entirely, I just didn't think it was important
anymore.

I expect to be in trouble for replacing err() and errx(), though.


> 1. This workaround for what I believe you agree is a bug:
> 
> -        ps.in_decl = ps.decl_on_line = ps.last_token != type_def;
> +        ps.in_decl = ps.decl_on_line = true;

That will need a proper fix...

> 2. The long-lines adjustment I just sent you a patch for.

That looks very good.

> 3. The tab-vs-space difference under discussion here.

I can be convinced to make it another option upstream. But I dislike it
nevertheless.

> 4. A temporary hack affecting the indentation of comments on the same line
> (forcing them to a multiple of 8 spaces even though tabsize is 4).  I have
> every intention of dropping that one later; I just don't want to deal with
> comment reindentation at the same time as these other things.

Great!

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

От
Tom Lane
Дата:
Piotr Stefaniak <postgres@piotr-stefaniak.me> writes:
> On 2017-06-17 00:02, Tom Lane wrote:
>> What I'm testing with right now has just four differences from your repo:

> There are also the "portability fixes" and they're the main problem.

Fair enough.

> I've simply removed things like capsicum or __FBSDID() because I thought
> it wouldn't be a problem since Postgres will have its own copy of indent
> anyway (so that its behavior is not a moving target). I can ifdef-out
> them instead of removing entirely, I just didn't think it was important
> anymore.

We should be able to deal with those via some #define hackery, no?

> I expect to be in trouble for replacing err() and errx(), though.

Understood.  I think we could deal with this by providing err() and
errx() in a support file that would be part of our distribution but
not yours.
        regards, tom lane



Re: [HACKERS] Preliminary results for proposed new pgindent implementation

От
Robert Haas
Дата:
On Fri, Jun 16, 2017 at 10:51 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> So I'm back to the position that we ought to stick the indent
> code under src/tools/ in our main repo.  Is anyone really
> seriously against that?

Is it under the same license as everything else?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: [HACKERS] Preliminary results for proposed new pgindent implementation

От
Tom Lane
Дата:
Robert Haas <robertmhaas@gmail.com> writes:
> On Fri, Jun 16, 2017 at 10:51 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> So I'm back to the position that we ought to stick the indent
>> code under src/tools/ in our main repo.  Is anyone really
>> seriously against that?

> Is it under the same license as everything else?

Hm, now that you mention it, interesting point.  I was about to
answer that it's under the standard BSD license, but looking closer,
I see that these files still quote the old 4-clause BSD license text
(with the "advertising" clause).  We need to get them adjusted
to be 3-clause with no advertising.  Looking into a FreeBSD source
tree locally, I see that FreeBSD has removed the advertising clause
from all their files --- sometimes without even remembering to
renumber the old clause 4 to clause 3 ;-).  So Piotr needs to do
likewise to conform with FreeBSD policy.  Once he has, it'll be
identical text to the other BSD-origin files we have in our tree,
eg src/port/getopt.c.

Piotr: if you're unclear on the rationale for this, see the bottom of
https://www.freebsd.org/copyright/license.html
        regards, tom lane



Re: [HACKERS] Preliminary results for proposed new pgindent implementation

От
Tom Lane
Дата:
I wrote:
> Piotr Stefaniak <postgres@piotr-stefaniak.me> writes:
>> There are also the "portability fixes" and they're the main problem.

> Fair enough.

I spent some time looking into this.  I reverted your commits
198457848ae5c86bec3336a9437dd5aa30f480c2 (Replace err.h functions with
standard C equivalents) and fb10acb040b90bdcbad09defd303363db29257d1
(Remove inclusion of sys/cdefs.h) locally and tried to build without
those.  I've successfully worked around the err.h change by adding
cut-down versions of FreeBSD 11's err.h and err.c to the fileset
(see attached).  However, it's proving impossible to work around having
"#include <sys/cdefs.h>" as the first live code in the files.  I thought
maybe we could provide a dummy cdefs.h file, but that breaks things on
platforms where cdefs.h is a real thing and is relied on by other system
headers --- which includes both Linux and BSD.  It seems we would have
to have something like #ifdef HAVE_SYS_CDEFS_H, but that is already a
departure from FreeBSD practice.

So what I'm currently thinking is that we have to diverge from the
FreeBSD sources to the extent of removing #include <sys/cdefs.h>
and the __FBSDID() calls, and instead inserting #include "c.h" to
pick up PG's own portability definitions.  The thing that forced me
into the latter is that there seems no way to avoid compiler warnings
if we don't decorate the declarations of err() and errx() with noreturn
and printf-format attributes --- and we need c.h to provide portable
ways of writing those.  But there are probably other portability things
that we'll need c.h for, anyway, especially if we want to make it work
on Windows.  So I'm thinking this is a small and easily maintainable
difference from the upstream FreeBSD files.

When I inserted #include "c.h", I got duplicate-macro-definition warnings
about "true" and "false", so I would ask you to add this:

--- freebsd_indent/indent_globs.h    2017-06-16 11:06:53.329712682 -0400
+++ new/indent_globs.h    2017-06-17 14:45:41.388015754 -0400
@@ -43,8 +43,12 @@
                  * of code */
 
 
+#ifndef false
 #define false 0
+#endif
+#ifndef true
 #define true  1
+#endif
 
 
 FILE       *input;        /* the fid for the input file */

Other than that, I think this is a workable compromise on the
portability questions.

            regards, tom lane

/*-
 * Copyright (c) 1993
 *    The Regents of the University of California.  All rights reserved.
 *
 * Redistribution and use in source and binary forms, with or without
 * modification, are permitted provided that the following conditions
 * are met:
 * 1. Redistributions of source code must retain the above copyright
 *    notice, this list of conditions and the following disclaimer.
 * 2. Redistributions in binary form must reproduce the above copyright
 *    notice, this list of conditions and the following disclaimer in the
 *    documentation and/or other materials provided with the distribution.
 * 3. Neither the name of the University nor the names of its contributors
 *    may be used to endorse or promote products derived from this software
 *    without specific prior written permission.
 *
 * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
 * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
 * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
 * ARE DISCLAIMED.  IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
 * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
 * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
 * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
 * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
 * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
 * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 * SUCH DAMAGE.
 *
 *    @(#)err.h    8.1 (Berkeley) 6/2/93
 * $FreeBSD: stable/11/include/err.h 203964 2010-02-16 19:39:50Z imp $
 */

#ifndef _ERR_H_
#define    _ERR_H_

/*
 * This is cut down to just the minimum that we need to build indent.
 */

void    err(int, const char *, ...)
  pg_attribute_noreturn() pg_attribute_printf(2, 3);
void    errx(int, const char *, ...)
  pg_attribute_noreturn() pg_attribute_printf(2, 3);

#endif /* !_ERR_H_ */
/*-
 * Copyright (c) 1993
 *    The Regents of the University of California.  All rights reserved.
 *
 * Redistribution and use in source and binary forms, with or without
 * modification, are permitted provided that the following conditions
 * are met:
 * 1. Redistributions of source code must retain the above copyright
 *    notice, this list of conditions and the following disclaimer.
 * 2. Redistributions in binary form must reproduce the above copyright
 *    notice, this list of conditions and the following disclaimer in the
 *    documentation and/or other materials provided with the distribution.
 * 3. Neither the name of the University nor the names of its contributors
 *    may be used to endorse or promote products derived from this software
 *    without specific prior written permission.
 *
 * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
 * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
 * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
 * ARE DISCLAIMED.  IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
 * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
 * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
 * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
 * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
 * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
 * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 * SUCH DAMAGE.
 */

/*
 * This is cut down to just the minimum that we need to build indent.
 */
#include "c.h"

#include <err.h>
#include <errno.h>
#include <stdarg.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

void
err(int eval, const char *fmt, ...)
{
    int code = errno;
    va_list ap;
    va_start(ap, fmt);
    if (fmt != NULL) {
        vfprintf(stderr, fmt, ap);
        fprintf(stderr, ": ");
    }
    fprintf(stderr, "%s\n", strerror(code));
    va_end(ap);
    exit(eval);
}

void
errx(int eval, const char *fmt, ...)
{
    va_list ap;
    va_start(ap, fmt);
    if (fmt != NULL)
        vfprintf(stderr, fmt, ap);
    fprintf(stderr, "\n");
    va_end(ap);
    exit(eval);
}

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Preliminary results for proposed new pgindentimplementation

От
Peter Eisentraut
Дата:
On 6/16/17 10:51, Tom Lane wrote:
> So I'm back to the position that we ought to stick the indent
> code under src/tools/ in our main repo.  Is anyone really
> seriously against that?

I think it would be better to have it separate.

Other than for reasons of principle and general modularity of the world,
I would like this to be available separately for separate download,
packaging, etc. to it can be applied to extension projects without
having to download and build (a specific version of) PostgreSQL.  The
code formatting in extension projects is, um, questionable.  In fact, if
it's a better indent period, I would like to package it for the general
public.

If the vote is to put it into the tree, I would request not to do it in
PG10.  At this point, we should be winding things down and not open up
new areas of activity.  There is a chance that if this goes in (or
anywhere else), there will be a stream of requests along the lines of:
doesn't build on Windows, doesn't build on AIX, doesn't build on
PowerPC, doesn't build on this other Windows variant, the tests don't
run, the tests don't run on Windows, it doesn't build in vpath, it
doesn't work on the buildfarm, and so on.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



Re: [HACKERS] Preliminary results for proposed new pgindentimplementation

От
Piotr Stefaniak
Дата:
On 2017-06-17 21:55, Tom Lane wrote:
> I spent some time looking into this.  I reverted your commits
> 198457848ae5c86bec3336a9437dd5aa30f480c2 (Replace err.h functions with
> standard C equivalents) and fb10acb040b90bdcbad09defd303363db29257d1
> (Remove inclusion of sys/cdefs.h) locally and tried to build without
> those.

I wanted to mirror that move, but forgot to not rebase the repository,
so I removed those two commits instead of committing their negatives.
Sorry about that.

> I've successfully worked around the err.h change by adding
> cut-down versions of FreeBSD 11's err.h and err.c to the fileset
> (see attached).

I thought about something like:
#ifdef __FreeBSD__
#include <err.h>
#define ERR(...) err(__VA_ARGS__)
#define ERRX(...) errx(__VA_ARGS__)
#else
#include "err.h"
#endif

and then call ERR() and ERRX() instead of err() and errx(). But that
requires C99. And I would have a very hard time convincing anyone that
it makes any sense from FreeBSD's perspective, since indent is part of
the base system, where <err.h> is guaranteed to exist.

Perhaps it would be best for everyone if indent was moved out of FreeBSD
base, so that portability arguments would make more sense. But that
would take time and some debate.

> However, it's proving impossible to work around having
> "#include <sys/cdefs.h>" as the first live code in the files.  I thought
> maybe we could provide a dummy cdefs.h file, but that breaks things on
> platforms where cdefs.h is a real thing and is relied on by other system
> headers --- which includes both Linux and BSD.  It seems we would have
> to have something like #ifdef HAVE_SYS_CDEFS_H, but that is already a
> departure from FreeBSD practice.

I was thinking if I could get away with putting those into #ifdef
__FreeBSD__ ... #endif. I think that it might be feasible unlike the
idea above. I could be wrong.

> So what I'm currently thinking is that we have to diverge from the
> FreeBSD sources to the extent of removing #include <sys/cdefs.h>
> and the __FBSDID() calls, and instead inserting #include "c.h" to
> pick up PG's own portability definitions.  The thing that forced me
> into the latter is that there seems no way to avoid compiler warnings
> if we don't decorate the declarations of err() and errx() with noreturn
> and printf-format attributes --- and we need c.h to provide portable
> ways of writing those.  But there are probably other portability things
> that we'll need c.h for, anyway, especially if we want to make it work
> on Windows.  So I'm thinking this is a small and easily maintainable
> difference from the upstream FreeBSD files.

That works for me.

> When I inserted #include "c.h", I got duplicate-macro-definition warnings
> about "true" and "false", so I would ask you to add this:

Done.




Re: [HACKERS] Preliminary results for proposed new pgindent implementation

От
Tom Lane
Дата:
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> On 6/16/17 10:51, Tom Lane wrote:
>> So I'm back to the position that we ought to stick the indent
>> code under src/tools/ in our main repo.  Is anyone really
>> seriously against that?

> I think it would be better to have it separate.

> Other than for reasons of principle and general modularity of the world,
> I would like this to be available separately for separate download,
> packaging, etc. to it can be applied to extension projects without
> having to download and build (a specific version of) PostgreSQL.  The
> code formatting in extension projects is, um, questionable.  In fact, if
> it's a better indent period, I would like to package it for the general
> public.

Well, the direction I'm headed in for addressing the portability issues
is to make it depend on the usual PG build environment, notably c.h and
libpgport.  If we don't want it in-tree, it can be built using PGXS,
but it'll still require a PG installation somewhere in order to get built.
Making it independent of both FreeBSD and PG is a significantly larger
project, and one I don't personally intend to tackle.  (And, if someone
does tackle that, I don't exactly see why having our own copy in-tree
would stop them.)

However ... off-list discussion with Piotr indicates that he's unwilling
to touch the license text without permission from FreeBSD core and/or
legal teams.  While the 4-clause license is certainly no impediment to
using indent, we don't want any such text in our tree, so that seems
like a showstopper, at least until the license question is resolved.

Accordingly, I'll proceed with setting up a repo for it on
git.postgresql.org.

> If the vote is to put it into the tree, I would request not to do it in
> PG10.  At this point, we should be winding things down and not open up
> new areas of activity.

I'm confused by this.  Are you objecting to switching to the new indent
version for v10?
        regards, tom lane



Re: [HACKERS] Preliminary results for proposed new pgindent implementation

От
Robert Haas
Дата:
On Sat, Jun 17, 2017 at 5:41 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
> On 6/16/17 10:51, Tom Lane wrote:
>> So I'm back to the position that we ought to stick the indent
>> code under src/tools/ in our main repo.  Is anyone really
>> seriously against that?
>
> I think it would be better to have it separate.

+1.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: [HACKERS] Preliminary results for proposed new pgindentimplementation

От
Stephen Frost
Дата:
* Robert Haas (robertmhaas@gmail.com) wrote:
> On Sat, Jun 17, 2017 at 5:41 PM, Peter Eisentraut
> <peter.eisentraut@2ndquadrant.com> wrote:
> > On 6/16/17 10:51, Tom Lane wrote:
> >> So I'm back to the position that we ought to stick the indent
> >> code under src/tools/ in our main repo.  Is anyone really
> >> seriously against that?
> >
> > I think it would be better to have it separate.
>
> +1.

+1.

Thanks!

Stephen

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

От
Tom Lane
Дата:
Stephen Frost <sfrost@snowman.net> writes:
> * Robert Haas (robertmhaas@gmail.com) wrote:
>> On Sat, Jun 17, 2017 at 5:41 PM, Peter Eisentraut
>> <peter.eisentraut@2ndquadrant.com> wrote:
>>> On 6/16/17 10:51, Tom Lane wrote:
>>>> So I'm back to the position that we ought to stick the indent
>>>> code under src/tools/ in our main repo.  Is anyone really
>>>> seriously against that?

>>> I think it would be better to have it separate.

>> +1.

> +1.

Given the license issues raised downthread, we have no choice in
the short term.  So I have a request in to create a separate repo
on git.postgresql.org (whose chain do I need to pull to get that
approved, btw?)
        regards, tom lane



Re: [HACKERS] Preliminary results for proposed new pgindentimplementation

От
Stephen Frost
Дата:
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Stephen Frost <sfrost@snowman.net> writes:
> > * Robert Haas (robertmhaas@gmail.com) wrote:
> >> On Sat, Jun 17, 2017 at 5:41 PM, Peter Eisentraut
> >> <peter.eisentraut@2ndquadrant.com> wrote:
> >>> On 6/16/17 10:51, Tom Lane wrote:
> >>>> So I'm back to the position that we ought to stick the indent
> >>>> code under src/tools/ in our main repo.  Is anyone really
> >>>> seriously against that?
>
> >>> I think it would be better to have it separate.
>
> >> +1.
>
> > +1.
>
> Given the license issues raised downthread, we have no choice in
> the short term.  So I have a request in to create a separate repo
> on git.postgresql.org (whose chain do I need to pull to get that
> approved, btw?)

uhhhh, that would probably be pginfra in some capacity, but I don't
recall seeing any notification of such a request.

I will follow up with those responsible,  #blamemagnus

Thanks!

Stephen