Quantcast

Preliminary results for proposed new pgindent implementation

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
20 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Preliminary results for proposed new pgindent implementation

Tom Lane-2
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 ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

new-pgindent-changes-1.patch.gz (154K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Preliminary results for proposed new pgindent implementation

Stephen Frost
Tom, all,

* Tom Lane ([hidden email]) wrote:
> All in all, this looks pretty darn good from here, and I'm thinking
> we should push forward on it.

+1.

Thanks!

Stephen

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Preliminary results for proposed new pgindent implementation

Robert Haas
In reply to this post by Tom Lane-2
On Thu, May 18, 2017 at 11:00 PM, Tom Lane <[hidden email]> 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


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Preliminary results for proposed new pgindent implementation

Tom Lane-2
Robert Haas <[hidden email]> writes:
> On Thu, May 18, 2017 at 11:00 PM, Tom Lane <[hidden email]> 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


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Preliminary results for proposed new pgindent implementation

Stephen Frost
Tom, all,

* Tom Lane ([hidden email]) wrote:

> Robert Haas <[hidden email]> writes:
> > On Thu, May 18, 2017 at 11:00 PM, Tom Lane <[hidden email]> 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

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Preliminary results for proposed new pgindent implementation

Tom Lane-2
Stephen Frost <[hidden email]> writes:
> * Tom Lane ([hidden email]) 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


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Preliminary results for proposed new pgindent implementation

Heikki Linnakangas
In reply to this post by Stephen Frost
On 05/19/2017 06:05 PM, Stephen Frost wrote:

> * Tom Lane ([hidden email]) wrote:
>> Robert Haas <[hidden email]> writes:
>>> On Thu, May 18, 2017 at 11:00 PM, Tom Lane <[hidden email]> 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



--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Preliminary results for proposed new pgindent implementation

Tom Lane-2
Heikki Linnakangas <[hidden email]> 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


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Preliminary results for proposed new pgindent implementation

Bruce Momjian
In reply to this post by Tom Lane-2
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  <[hidden email]>        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 +


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Preliminary results for proposed new pgindent implementation

Heikki Linnakangas
In reply to this post by Tom Lane-2
On 05/19/2017 06:48 PM, Tom Lane wrote:
> Heikki Linnakangas <[hidden email]> 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;           /* last block in chain */
         uint32          num_chunks;
         uint32          bytes_free;             /* free bytes left in
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



--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Preliminary results for proposed new pgindent implementation

Robert Haas
In reply to this post by Tom Lane-2
On Fri, May 19, 2017 at 11:22 AM, Tom Lane <[hidden email]> 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


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Preliminary results for proposed new pgindent implementation

Andres Freund
On 2017-05-19 12:21:52 -0400, Robert Haas wrote:

> On Fri, May 19, 2017 at 11:22 AM, Tom Lane <[hidden email]> 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


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Preliminary results for proposed new pgindent implementation

Tom Lane-2
In reply to this post by Robert Haas
Robert Haas <[hidden email]> writes:

> On Fri, May 19, 2017 at 11:22 AM, Tom Lane <[hidden email]> 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


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Preliminary results for proposed new pgindent implementation

Tom Lane-2
In reply to this post by Heikki Linnakangas
Heikki Linnakangas <[hidden email]> 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


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Preliminary results for proposed new pgindent implementation

Tom Lane-2
In reply to this post by Andres Freund
Andres Freund <[hidden email]> 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


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Preliminary results for proposed new pgindent implementation

Alvaro Herrera-9
In reply to this post by Tom Lane-2
Tom Lane wrote:
> Robert Haas <[hidden email]> 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


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Preliminary results for proposed new pgindent implementation

Tom Lane-2
In reply to this post by Tom Lane-2
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 ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Preliminary results for proposed new pgindent implementation

Peter Eisentraut-6
In reply to this post by Alvaro Herrera-9
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


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Preliminary results for proposed new pgindent implementation

Peter Eisentraut-6
In reply to this post by Tom Lane-2
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


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Preliminary results for proposed new pgindent implementation

Tom Lane-2
Peter Eisentraut <[hidden email]> 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


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Previous Thread Next Thread
Loading...