With RC1 nearing, when should I run pgindent?  This is usually the time
I do it.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Tue, 20 Mar 2001 18:36:53 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
pgindent run?
> With RC1 nearing, when should I run pgindent?  This is usually the time
> I do it.

Does the silence mean I should pick a date to run this?

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Wed, 21 Mar 2001 14:43:22 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: pgindent run?
> Bruce Momjian writes:
> 
> > With RC1 nearing, when should I run pgindent?  This is usually the time
> > I do it.
> 
> Are there any severely mis-indented files?

Not sure.  I think there are some.  It doesn't do anything unless there
is mis-indenting, so it is pretty safe and has always been done in the
past.  It obviously only affects new changes since the last run.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Wed, 21 Mar 2001 15:14:33 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: pgindent run?
On Wed, 21 Mar 2001, Bruce Momjian wrote:

> > With RC1 nearing, when should I run pgindent?  This is usually the time
> > I do it.
>
> Does the silence mean I should pick a date to run this?

Since I'm going to end up re-rolling RC1, do a run tonight on her, so that
any problems that arise from pgindent this time can be caught with those
testing RC1 ...



Wed, 21 Mar 2001 16:20:34 -0400 (AST)
The Hermit Hacker <scrappy@hub.org>
Re: pgindent run?
> On Wed, 21 Mar 2001, Bruce Momjian wrote:
> 
> > > With RC1 nearing, when should I run pgindent?  This is usually the time
> > > I do it.
> >
> > Does the silence mean I should pick a date to run this?
> 
> Since I'm going to end up re-rolling RC1, do a run tonight on her, so that
> any problems that arise from pgindent this time can be caught with those
> testing RC1 ...

Good idea.  It is well tested, but you never know.  

Peter, this is the optimial time to do it because no one has any
outstanding patches at this point.  Seems this is the only good time.

Unless someone says otherwise, I will do the run tonight.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Wed, 21 Mar 2001 15:21:52 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: pgindent run?
Bruce Momjian writes:

> With RC1 nearing, when should I run pgindent?  This is usually the time
> I do it.

Are there any severely mis-indented files?

-- 
Peter Eisentraut      peter_e@gmx.net       http://yi.org/peter-e/


Wed, 21 Mar 2001 21:22:32 +0100 (CET)
Peter Eisentraut <peter_e@gmx.net>
Re: pgindent run?
> Bruce Momjian writes:
> 
> > Peter, this is the optimial time to do it because no one has any
> > outstanding patches at this point.  Seems this is the only good time.
> 
> Actually, I have quite a few outstanding patches.  I got screwed by this
> last time around as well.  But I understand that this might be the best
> time.

That you are holding?  Yes, I have a few to at my new Unapplied
Patches web page:

	http://candle.pha.pa.us/cgi-bin/pgpatches

The good news is that these will apply fine to 7.2 unless they touch an
area that needed indenting.  The problem of not doing it is that the
code starts to look different after a while and takes on a chaotic feel.

This is probably the time when there are the fewest oustanding patches,
I guess.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Wed, 21 Mar 2001 15:48:35 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: pgindent run?
Bruce Momjian writes:

> Peter, this is the optimial time to do it because no one has any
> outstanding patches at this point.  Seems this is the only good time.

Actually, I have quite a few outstanding patches.  I got screwed by this
last time around as well.  But I understand that this might be the best
time.

-- 
Peter Eisentraut      peter_e@gmx.net       http://yi.org/peter-e/


Wed, 21 Mar 2001 21:54:01 +0100 (CET)
Peter Eisentraut <peter_e@gmx.net>
Re: pgindent run?

OK, I am going to have dinner and then get started on the pgindent run.

I have also noticed we have some comments like:

	/* ----
         * one word
         * ----
         */

that look funny in a few places.  I propose:

	/* one word */

to be consistent.


> With RC1 nearing, when should I run pgindent?  This is usually the time
> I do it.
> 
> -- 
>   Bruce Momjian                        |  http://candle.pha.pa.us
>   pgman@candle.pha.pa.us               |  (610) 853-3000
>   +  If your life is a hard drive,     |  830 Blythe Avenue
>   +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
> 
> http://www.postgresql.org/search.mpl
> 


-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Wed, 21 Mar 2001 18:12:36 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: pgindent run?
On Wed, 21 Mar 2001, Bruce Momjian wrote:

>
> OK, I am going to have dinner and then get started on the pgindent run.
>
> I have also noticed we have some comments like:
>
> 	/* ----
>          * one word
>          * ----
>          */
>
> that look funny in a few places.  I propose:
>
> 	/* one word */
>
> to be consistent.

to be consistent with what ... ?  isn't:

/* ----------
 * comment
 * ----------
 */

the standard?

>
>
> > With RC1 nearing, when should I run pgindent?  This is usually the time
> > I do it.
> >
> > --
> >   Bruce Momjian                        |  http://candle.pha.pa.us
> >   pgman@candle.pha.pa.us               |  (610) 853-3000
> >   +  If your life is a hard drive,     |  830 Blythe Avenue
> >   +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
> >
> > ---------------------------(end of broadcast)---------------------------
> > TIP 6: Have you searched our list archives?
> >
> > http://www.postgresql.org/search.mpl
> >
>
>
> --
>   Bruce Momjian                        |  http://candle.pha.pa.us
>   pgman@candle.pha.pa.us               |  (610) 853-3000
>   +  If your life is a hard drive,     |  830 Blythe Avenue
>   +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
>

Marc G. Fournier                   ICQ#7615664               IRC Nick: Scrappy
Systems Administrator @ hub.org
primary: scrappy@hub.org           secondary: scrappy@{freebsd|postgresql}.org


Wed, 21 Mar 2001 19:25:49 -0400 (AST)
The Hermit Hacker <scrappy@hub.org>
Re: pgindent run?
> On Wed, 21 Mar 2001, Bruce Momjian wrote:
> 
> >
> > OK, I am going to have dinner and then get started on the pgindent run.
> >
> > I have also noticed we have some comments like:
> >
> > 	/* ----
> >          * one word
> >          * ----
> >          */
> >
> > that look funny in a few places.  I propose:
> >
> > 	/* one word */
> >
> > to be consistent.
> 
> to be consistent with what ... ?  isn't:
> 
> /* ----------
>  * comment
>  * ----------
>  */
> 
> the standard?

Sorry.  It has been a while since I studied this.  The issue is the
dashes, not the block comments.  /* --- is needed for multi-line comment
where you want to preserve the layout, but in other cases, it prevents
comment layout and looks kind of heavy.  I eyeball each change to make
sure it is clean so:

	/* ---
	 * test
	 * ---
	 */

becomes the cleaner:

	/*
	 * test
	 */

This makes the comment easier to read.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Wed, 21 Mar 2001 20:24:19 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: pgindent run?
Bruce Momjian <pgman@candle.pha.pa.us> writes:
>> With RC1 nearing, when should I run pgindent?  This is usually the time
>> I do it.

> Does the silence mean I should pick a date to run this?

If you're going to do it before the release, I think you should do it
*before* we wrap RC1.  I've said before and will say again that I think
it's utter folly to run pgindent at the conclusion of the test cycle.
I've been around this project for three major release cycles and we have
seen errors introduced by pgindent in two of them.  I don't trust
pgindent to be bug-free and I don't believe you should either.

			regards, tom lane

Wed, 21 Mar 2001 22:58:49 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: pgindent run? 
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> >> With RC1 nearing, when should I run pgindent?  This is usually the time
> >> I do it.
> 
> > Does the silence mean I should pick a date to run this?
> 
> If you're going to do it before the release, I think you should do it
> *before* we wrap RC1.  I've said before and will say again that I think
> it's utter folly to run pgindent at the conclusion of the test cycle.
> I've been around this project for three major release cycles and we have
> seen errors introduced by pgindent in two of them.  I don't trust
> pgindent to be bug-free and I don't believe you should either.

OK, running now.  Should I run it at another time or never?

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Wed, 21 Mar 2001 23:00:00 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: pgindent run?
>> Are there any severely mis-indented files?

There are some new contrib modules that are nowhere close to our
indent conventions; also a good deal of foreign-key-related stuff
in the parser that needs to be cleaned up.  So we should run it.

I've always felt that it'd be smarter to run pgindent at the start
of a development cycle, not the end, but I've been unable to convince
Bruce of that ...

			regards, tom lane

Wed, 21 Mar 2001 23:08:14 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: pgindent run? 
> >> Are there any severely mis-indented files?
> 
> There are some new contrib modules that are nowhere close to our
> indent conventions; also a good deal of foreign-key-related stuff
> in the parser that needs to be cleaned up.  So we should run it.
> 
> I've always felt that it'd be smarter to run pgindent at the start
> of a development cycle, not the end, but I've been unable to convince
> Bruce of that ...

Hey, I am open to whatever people want to do.  Just remember that we
accumulate lots of patches/development during the slow time before
development, and those patches become harder to apply.  Peter E has some
already.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Wed, 21 Mar 2001 23:10:43 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: pgindent run?
* Bruce Momjian <pgman@candle.pha.pa.us> [010321 22:11]:
> > >> Are there any severely mis-indented files?
> > 
> > There are some new contrib modules that are nowhere close to our
> > indent conventions; also a good deal of foreign-key-related stuff
> > in the parser that needs to be cleaned up.  So we should run it.
> > 
> > I've always felt that it'd be smarter to run pgindent at the start
> > of a development cycle, not the end, but I've been unable to convince
> > Bruce of that ...
> 
> Hey, I am open to whatever people want to do.  Just remember that we
> accumulate lots of patches/development during the slow time before
> development, and those patches become harder to apply.  Peter E has some
> already.
How about:
1) just AFTER release
2) just BEFORE Beta 

LER

> 
> -- 
>   Bruce Momjian                        |  http://candle.pha.pa.us
>   pgman@candle.pha.pa.us               |  (610) 853-3000
>   +  If your life is a hard drive,     |  830 Blythe Avenue
>   +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo@postgresql.org so that your
> message can get through to the mailing list cleanly
-- 
Larry Rosenman                     http://www.lerctr.org/~ler
Phone: +1 972-414-9812                 E-Mail: ler@lerctr.org
US Mail: 1905 Steamboat Springs Drive, Garland, TX 75044-6749

Wed, 21 Mar 2001 22:12:26 -0600
Larry Rosenman <ler@lerctr.org>
Re: pgindent run?
On Wed, 21 Mar 2001, Bruce Momjian wrote:

> > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > >> With RC1 nearing, when should I run pgindent?  This is usually the time
> > >> I do it.
> >
> > > Does the silence mean I should pick a date to run this?
> >
> > If you're going to do it before the release, I think you should do it
> > *before* we wrap RC1.  I've said before and will say again that I think
> > it's utter folly to run pgindent at the conclusion of the test cycle.
> > I've been around this project for three major release cycles and we have
> > seen errors introduced by pgindent in two of them.  I don't trust
> > pgindent to be bug-free and I don't believe you should either.
>
> OK, running now.  Should I run it at another time or never?

I'll put my vote on Tom's side of things ... run if after the release,
right at the start of the next development cycle, so that any bugs that
crop up aren't just as we are trying to release ...

Hell, maybe once then and once *just* as we are going into first beta of a
release ... Tom?


Thu, 22 Mar 2001 00:16:13 -0400 (AST)
The Hermit Hacker <scrappy@hub.org>
Re: pgindent run?
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Hey, I am open to whatever people want to do.  Just remember that we
> accumulate lots of patches/development during the slow time before
> development, and those patches become harder to apply.  Peter E has some
> already.

Why not start a devel cycle by (a) branching the tree, (b) applying
all held-over patches, and then (c) running pgindent?

I'd probably wait a week or so between (a) and (c) to let people push
in whatever they have pending.  But in general it seems a lot safer
to pgindent at the front end of the cycle not the back end.

			regards, tom lane

Wed, 21 Mar 2001 23:35:07 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: pgindent run? 
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Hey, I am open to whatever people want to do.  Just remember that we
> > accumulate lots of patches/development during the slow time before
> > development, and those patches become harder to apply.  Peter E has some
> > already.
> 
> Why not start a devel cycle by (a) branching the tree, (b) applying
> all held-over patches, and then (c) running pgindent?

If people can get their patches in all at one time, that would work. 
The only problem there is that people who supply patches against 7.1
will not match the 7.2 tree, and we get those patches from people for
months.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Wed, 21 Mar 2001 23:45:17 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: pgindent run?
On Wed, 21 Mar 2001, Bruce Momjian wrote:

> > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > > Hey, I am open to whatever people want to do.  Just remember that we
> > > accumulate lots of patches/development during the slow time before
> > > development, and those patches become harder to apply.  Peter E has some
> > > already.
> >
> > Why not start a devel cycle by (a) branching the tree, (b) applying
> > all held-over patches, and then (c) running pgindent?
>
> If people can get their patches in all at one time, that would work.
> The only problem there is that people who supply patches against 7.1
> will not match the 7.2 tree, and we get those patches from people for
> months.

and those patches should only be applied to the v7.1 branch ... we are
suggesting (or, at least, I am) is that you pgindent *HEAD* after we've
branched off v7.1 ...

... that way, we go into the new dev cycle "clean", but we doon't mess up
the *STABLE* tree ...


Thu, 22 Mar 2001 00:48:09 -0400 (AST)
The Hermit Hacker <scrappy@hub.org>
Re: pgindent run?
> > If people can get their patches in all at one time, that would work.
> > The only problem there is that people who supply patches against 7.1
> > will not match the 7.2 tree, and we get those patches from people for
> > months.
> 
> and those patches should only be applied to the v7.1 branch ... we are
> suggesting (or, at least, I am) is that you pgindent *HEAD* after we've
> branched off v7.1 ...
> 
> ... that way, we go into the new dev cycle "clean", but we doon't mess up
> the *STABLE* tree ...

But we get patches from 7.0.X now that get applied to 7.1.  All our
developers are not working on CVS trees.  Many use their main trees and
send in the occasional patch.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Wed, 21 Mar 2001 23:50:46 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: pgindent run?
On Wed, 21 Mar 2001, Bruce Momjian wrote:

> > > If people can get their patches in all at one time, that would work.
> > > The only problem there is that people who supply patches against 7.1
> > > will not match the 7.2 tree, and we get those patches from people for
> > > months.
> >
> > and those patches should only be applied to the v7.1 branch ... we are
> > suggesting (or, at least, I am) is that you pgindent *HEAD* after we've
> > branched off v7.1 ...
> >
> > ... that way, we go into the new dev cycle "clean", but we doon't mess up
> > the *STABLE* tree ...
>
> But we get patches from 7.0.X now that get applied to 7.1.  All our
> developers are not working on CVS trees.  Many use their main trees and
> send in the occasional patch.

and most times, those have to be merged into the source tree due to
extensive changes anyway ... maybe we should just get rid of the use of
pgindent altogether?  its not something that I've ever seen required on
other projects I've worked on ... in general, most projects seem to
require that a submit'd patch from an older release be at least tested on
the newest CVS, and with nightly snapshots being created as it is, I
really don't see why such a requirement is a bad thing ...



Thu, 22 Mar 2001 00:57:19 -0400 (AST)
The Hermit Hacker <scrappy@hub.org>
Re: pgindent run?
> and most times, those have to be merged into the source tree due to
> extensive changes anyway ... maybe we should just get rid of the use of
> pgindent altogether?  its not something that I've ever seen required on
> other projects I've worked on ... in general, most projects seem to
> require that a submit'd patch from an older release be at least tested on
> the newest CVS, and with nightly snapshots being created as it is, I
> really don't see why such a requirement is a bad thing ...

In an ideal world, people would test on CVS but in reality, the patches
are usually pretty small and if they fix the problem, we apply them.

Seems like a lot of work just to avoid pgindent.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Wed, 21 Mar 2001 23:59:24 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: pgindent run?
On Wed, 21 Mar 2001, Bruce Momjian wrote:

> > and most times, those have to be merged into the source tree due to
> > extensive changes anyway ... maybe we should just get rid of the use of
> > pgindent altogether?  its not something that I've ever seen required on
> > other projects I've worked on ... in general, most projects seem to
> > require that a submit'd patch from an older release be at least tested on
> > the newest CVS, and with nightly snapshots being created as it is, I
> > really don't see why such a requirement is a bad thing ...
>
> In an ideal world, people would test on CVS but in reality, the patches
> are usually pretty small and if they fix the problem, we apply them.
>
> Seems like a lot of work just to avoid pgindent.

If they are small, then why is pgindent required?  And if they are large,
is it too much to ask that the person submitting tests the patch to make
sure its even applicable in the newest snapshot?



Thu, 22 Mar 2001 01:08:48 -0400 (AST)
The Hermit Hacker <scrappy@hub.org>
Re: pgindent run?
> > > and most times, those have to be merged into the source tree due to
> > > extensive changes anyway ... maybe we should just get rid of the use of
> > > pgindent altogether?  its not something that I've ever seen required on
> > > other projects I've worked on ... in general, most projects seem to
> > > require that a submit'd patch from an older release be at least tested on
> > > the newest CVS, and with nightly snapshots being created as it is, I
> > > really don't see why such a requirement is a bad thing ...
> >
> > In an ideal world, people would test on CVS but in reality, the patches
> > are usually pretty small and if they fix the problem, we apply them.
> >
> > Seems like a lot of work just to avoid pgindent.
> 
> If they are small, then why is pgindent required?  And if they are large,
> is it too much to ask that the person submitting tests the patch to make
> sure its even applicable in the newest snapshot?

The problem is that the small ones don't apply cleanly if they don't
match the indenting in the source.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Thu, 22 Mar 2001 00:10:06 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: pgindent run?
The Hermit Hacker <scrappy@hub.org> writes:
> and most times, those have to be merged into the source tree due to
> extensive changes anyway ... maybe we should just get rid of the use of
> pgindent altogether?

I think pgindent is a good thing; the style of different parts of the
code would vary too much without it.  I'm only unhappy about the risk
issues of running it at this late stage of the release cycle.

			regards, tom lane

Thu, 22 Mar 2001 00:11:36 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: pgindent run? 
> The Hermit Hacker <scrappy@hub.org> writes:
> > and most times, those have to be merged into the source tree due to
> > extensive changes anyway ... maybe we should just get rid of the use of
> > pgindent altogether?
> 
> I think pgindent is a good thing; the style of different parts of the
> code would vary too much without it.  I'm only unhappy about the risk
> issues of running it at this late stage of the release cycle.

This is the usual�discussion.  Some like it, some don't like the risk,
some don't like the timing.  I don't think we ever came up with a better
time than before RC, though I think we could do it a little earlier in
beta if people were not holding patches during that period.  It is the
beta patching folks that we have the most control over.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Thu, 22 Mar 2001 00:13:28 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: pgindent run?
On Thu, 22 Mar 2001, Bruce Momjian wrote:

> > > > and most times, those have to be merged into the source tree due to
> > > > extensive changes anyway ... maybe we should just get rid of the use of
> > > > pgindent altogether?  its not something that I've ever seen required on
> > > > other projects I've worked on ... in general, most projects seem to
> > > > require that a submit'd patch from an older release be at least tested on
> > > > the newest CVS, and with nightly snapshots being created as it is, I
> > > > really don't see why such a requirement is a bad thing ...
> > >
> > > In an ideal world, people would test on CVS but in reality, the patches
> > > are usually pretty small and if they fix the problem, we apply them.
> > >
> > > Seems like a lot of work just to avoid pgindent.
> >
> > If they are small, then why is pgindent required?  And if they are large,
> > is it too much to ask that the person submitting tests the patch to make
> > sure its even applicable in the newest snapshot?
>
> The problem is that the small ones don't apply cleanly if they don't
> match the indenting in the source.

but ... if they are small, manually merging isn't that big of a deal ...
and if anyone else has been working in that code since release, there is a
chance it won't mergef cleanly ...

Quite frankly, I'm for pgindent after branch and before beta ...


Thu, 22 Mar 2001 01:14:16 -0400 (AST)
The Hermit Hacker <scrappy@hub.org>
Re: pgindent run?
> > The problem is that the small ones don't apply cleanly if they don't
> > match the indenting in the source.
> 
> but ... if they are small, manually merging isn't that big of a deal ...
> and if anyone else has been working in that code since release, there is a
> chance it won't mergef cleanly ...

Yes, they can be manually merged, but that is much more error-prone than
pgindent itself, at least with me patching them.  :-)

Yes, I agree there is a risk.  I was quite scared the first time I ran
it on the tree and did the commit.  At this point, there are very few
changes to it, so I feel a little better, and the stuff gets caught
somehow if there is a problem.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Thu, 22 Mar 2001 00:17:22 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: pgindent run?
* Bruce Momjian <pgman@candle.pha.pa.us> [010321 21:14] wrote:
> > The Hermit Hacker <scrappy@hub.org> writes:
> > > and most times, those have to be merged into the source tree due to
> > > extensive changes anyway ... maybe we should just get rid of the use of
> > > pgindent altogether?
> > 
> > I think pgindent is a good thing; the style of different parts of the
> > code would vary too much without it.  I'm only unhappy about the risk
> > issues of running it at this late stage of the release cycle.
> 
> This is the usualï¿½discussion.  Some like it, some don't like the risk,
> some don't like the timing.  I don't think we ever came up with a better
> time than before RC, though I think we could do it a little earlier in
> beta if people were not holding patches during that period.  It is the
> beta patching folks that we have the most control over.

It seems that you guys are dead set on using this pgindent tool,
this is cool, we'd probably use some indentation tool on the FreeBSD
sources if there was one that met our code style(9) guidelines.

With that said, I really scares the crud out of me to see those massive
pg_indent runs right before you guys do a release.

It would make a lot more sense to force a pgindent run after applying
each patch.  This way you don't loose the history.

You want to be upset with yourself Bruce?  Go into a directory and type:

cvs annotate <any file that's been pgindented>

cvs annotate is a really, really handy tool, unfortunetly these
indent runs remove this very useful tool as well as do a major job
of obfuscating the code changes.

It's not like you guys have a massive devel team with new people each
week that have a steep committer learning curve ahead of them, making
pgindent as patches are applied should work.

There's also the argument that a developer's pgindent may force a
contributor to resolve conflicts, while this is true, it's also
true that you guys expect diffs to be in context format, comments
to be in english, function prototypes to be new style, etc, etc..

I think contributors can deal with this.

just my usual 20 cents. :)

-- 
-Alfred Perlstein - [bright@wintelcom.net|alfred@freebsd.org]
Daemon News Magazine in your snail-mail! http://magazine.daemonnews.org/

Wed, 21 Mar 2001 23:21:03 -0800
Alfred Perlstein <bright@wintelcom.net>
Re: pgindent run?
> * Bruce Momjian <pgman@candle.pha.pa.us> [010321 21:14] wrote:
> > > The Hermit Hacker <scrappy@hub.org> writes:
> > > > and most times, those have to be merged into the source tree due to
> > > > extensive changes anyway ... maybe we should just get rid of the use of
> > > > pgindent altogether?
> > > 
> > > I think pgindent is a good thing; the style of different parts of the
> > > code would vary too much without it.  I'm only unhappy about the risk
> > > issues of running it at this late stage of the release cycle.
> > 
> > This is the usual?discussion.  Some like it, some don't like the risk,
> > some don't like the timing.  I don't think we ever came up with a better
> > time than before RC, though I think we could do it a little earlier in
> > beta if people were not holding patches during that period.  It is the
> > beta patching folks that we have the most control over.
> 
> It seems that you guys are dead set on using this pgindent tool,
> this is cool, we'd probably use some indentation tool on the FreeBSD
> sources if there was one that met our code style(9) guidelines.

You don't notice the value of pgindent until you have some code that
hasn't been run through it.  For example, ODBC was not run through until
this release, and I had a terrible time trying to understand the code
because it didn't _look_ like the rest of the code.  Now that pgindent
is run, it looks more normal, and I am sure that will encourage more
people to get in and make changes.

It gives more of a "I know where I am" feeling to the code.  It
certainly doesn't make anything possible that wasn't possible before,
but it does encourage people, and that is pretty powerful.

I can't tell you how many times I have had to fix someone's contributed
code that hadn't been through pgindent yet, and the problems I had
trying to understand it.  I have even copied the file, pgindented it,
read through the copy, then went back and fixed the original code.  (I
can't run pgindent during development cycle, only when I have 100%
control over the code and outstanding patches people may have.)

As far as FreeBSD, I guarantee you will see major benefits to community
participation by running the script.  You will have to hand-review all
the changes after the first run to make sure it didn't wack out some
wierd piece of code, but after that you will be pretty OK.  The only
issue is that the person who takes this on is a taking a major risk of
exposure to ridicule if it fails.

I remember doing it the first time, and being really scared I would
lethally brake the code.  Indenting is one of those efforts that has
this _big_ risk componient when it is performed, and you get the small,
steady benefit of doing it for months after.

> With that said, I really scares the crud out of me to see those massive
> pg_indent runs right before you guys do a release.
> 
> It would make a lot more sense to force a pgindent run after applying
> each patch.  This way you don't loose the history.


Yes, we have considered that.  The problem there is that sometimes
people supply a patch, do some more work on their production source,
then supply other patches to fix new problems.  If we pgindent for every
CVS commit, we then are changing the supplied patch, which means any new
patches that person sends do not match their previous patch, and we get
into hand edits again.

I know we ask for context diff's, but anytime a patch applies with some
offset, if the offset is large, I have to make sure there wasn't some
other identical context of code that may have been found by the patch
program and applied incorrectly.

A silent patch apply is safe;  if it reports a large offset, I have to
investigate.

> You want to be upset with yourself Bruce?  Go into a directory and type:
> 
> cvs annotate <any file that's been pgindented>
> 
> cvs annotate is a really, really handy tool, unfortunetly these
> indent runs remove this very useful tool as well as do a major job
> of obfuscating the code changes.

I have never seen that feature.  I don't even see it in my cvs manual
page.  It is great, and yes, I clearly wack that out for pgindent runs.
Maybe pgindent for every commit is the way to go.

> It's not like you guys have a massive devel team with new people each
> week that have a steep committer learning curve ahead of them, making
> pgindent as patches are applied should work.

I imagine we can get CVS to do that automatically.  The number of patch
on top of another patch is pretty rare and it would solve the other
stated problems.


> There's also the argument that a developer's pgindent may force a
> contributor to resolve conflicts, while this is true, it's also
> true that you guys expect diffs to be in context format, comments
> to be in english, function prototypes to be new style, etc, etc..
> 
> I think contributors can deal with this.

If someone submits a massive patch, and we apply it, all patches after
that that they give us will not apply cleanly because they still have
the old format.  The other argument for not doing pgindent on cvs commit
is that if someone is working in an area of the code, they should be
able to format that code as they like to see it.  They may be working in
there for months.  Only during release are is their _style_ removed.

On a side note, the idea of having people submit patches only against
current CVS seems bad to me.  If people are running production machines
and they develop a patch and test it there, I want the patch that works
on their machine and can make sure it applies here.  Having them
download CVS and do the merge themselves seems really risky, especially
because they probably can't test the CVS in production.  The CVS may not
even run properly.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Thu, 22 Mar 2001 09:49:19 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: pgindent run?
Alfred Perlstein <bright@wintelcom.net> writes:
> cvs annotate is a really, really handy tool, unfortunetly these
> indent runs remove this very useful tool as well as do a major job
> of obfuscating the code changes.

I think this is a good reason for *not* applying pgindent on an
incremental basis, but only once per release cycle.  That at least
lets cvs annotate be useful within a cycle.

			regards, tom lane

Thu, 22 Mar 2001 09:56:06 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: pgindent run? 
> Alfred Perlstein <bright@wintelcom.net> writes:
> > cvs annotate is a really, really handy tool, unfortunetly these
> > indent runs remove this very useful tool as well as do a major job
> > of obfuscating the code changes.
> 
> I think this is a good reason for *not* applying pgindent on an
> incremental basis, but only once per release cycle.  That at least
> lets cvs annotate be useful within a cycle.

What about having it happen on every CVS commit? 

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Thu, 22 Mar 2001 10:07:23 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: pgindent run?
> It seems that you guys are dead set on using this pgindent tool,
> this is cool, we'd probably use some indentation tool on the FreeBSD
> sources if there was one that met our code style(9) guidelines.

I would liken running pgindent to having a nice looking store or
website.  No one is going to go to a website or a store only because it
looks nice, but having it look nice does keep people coming back.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Thu, 22 Mar 2001 10:09:38 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: pgindent run?
Bruce Momjian <pgman@candle.pha.pa.us> writes:
>> Alfred Perlstein <bright@wintelcom.net> writes:
> cvs annotate is a really, really handy tool, unfortunetly these
> indent runs remove this very useful tool as well as do a major job
> of obfuscating the code changes.
>> 
>> I think this is a good reason for *not* applying pgindent on an
>> incremental basis, but only once per release cycle.  That at least
>> lets cvs annotate be useful within a cycle.

> What about having it happen on every CVS commit? 

Try that and you'll get all-out war.  It's tough enough keeping in sync
with the repository already.

			regards, tom lane

Thu, 22 Mar 2001 10:36:24 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: pgindent run? 
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> >> Alfred Perlstein <bright@wintelcom.net> writes:
> > cvs annotate is a really, really handy tool, unfortunetly these
> > indent runs remove this very useful tool as well as do a major job
> > of obfuscating the code changes.
> >> 
> >> I think this is a good reason for *not* applying pgindent on an
> >> incremental basis, but only once per release cycle.  That at least
> >> lets cvs annotate be useful within a cycle.
> 
> > What about having it happen on every CVS commit? 
> 
> Try that and you'll get all-out war.  It's tough enough keeping in sync
> with the repository already.

Oh, well.  I tried.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Thu, 22 Mar 2001 10:37:14 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: pgindent run?
> > Hey, I am open to whatever people want to do.  Just remember that we
> > accumulate lots of patches/development during the slow time before
> > development, and those patches become harder to apply.  Peter E has some
> > already.
> 
> This argument seems irrelevant when given the choice of before release or
> after start of new cycle.  From an analytical point of view (and slightly
> idealized), these are limits approaching the same points in time, from
> opposite sides.  Thus, the second choice seems the infinitely better
> option.

Yes, the bigger problem is people running our most recent stable release
not matching the current CVS sources.  I think early beta is the time to
do this next time.  That has the fewest patches crossing over time.  In
7.1, we had the WAL patches still being worked on until near the end,
but once Tom put that mega-patch in last week, we could have done it
then.


-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Thu, 22 Mar 2001 11:12:04 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: pgindent run?
Bruce Momjian writes:

> > >> Are there any severely mis-indented files?
> >
> > There are some new contrib modules that are nowhere close to our
> > indent conventions; also a good deal of foreign-key-related stuff
> > in the parser that needs to be cleaned up.  So we should run it.
> >
> > I've always felt that it'd be smarter to run pgindent at the start
> > of a development cycle, not the end, but I've been unable to convince
> > Bruce of that ...
>
> Hey, I am open to whatever people want to do.  Just remember that we
> accumulate lots of patches/development during the slow time before
> development, and those patches become harder to apply.  Peter E has some
> already.

This argument seems irrelevant when given the choice of before release or
after start of new cycle.  From an analytical point of view (and slightly
idealized), these are limits approaching the same points in time, from
opposite sides.  Thus, the second choice seems the infinitely better
option.

-- 
Peter Eisentraut      peter_e@gmx.net       http://yi.org/peter-e/


Thu, 22 Mar 2001 17:12:59 +0100 (CET)
Peter Eisentraut <peter_e@gmx.net>
Re: pgindent run?
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> I think early beta is the time to
> do this next time.  That has the fewest patches crossing over time.

That would work too, particularly if you give people a few days' notice.
("Get your patches in now, or expect to have to reformat...")

			regards, tom lane

Thu, 22 Mar 2001 11:14:23 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: pgindent run? 
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > I think early beta is the time to
> > do this next time.  That has the fewest patches crossing over time.
> 
> That would work too, particularly if you give people a few days' notice.
> ("Get your patches in now, or expect to have to reformat...")

Yes, I did try that in a previous release and got the, "Oh, I am still
working on X."  I will be more insistent in the future that we get this
done earlier.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Thu, 22 Mar 2001 11:16:14 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: pgindent run?
* Bruce Momjian <pgman@candle.pha.pa.us> [010322 06:49] wrote:
> > * Bruce Momjian <pgman@candle.pha.pa.us> [010321 21:14] wrote:
> > > > The Hermit Hacker <scrappy@hub.org> writes:
> > > > > and most times, those have to be merged into the source tree due to
> > > > > extensive changes anyway ... maybe we should just get rid of the use of
> > > > > pgindent altogether?
> > > > 
> > > > I think pgindent is a good thing; the style of different parts of the
> > > > code would vary too much without it.  I'm only unhappy about the risk
> > > > issues of running it at this late stage of the release cycle.
> > > 
> > > This is the usual?discussion.  Some like it, some don't like the risk,
> > > some don't like the timing.  I don't think we ever came up with a better
> > > time than before RC, though I think we could do it a little earlier in
> > > beta if people were not holding patches during that period.  It is the
> > > beta patching folks that we have the most control over.
> > 
> > It seems that you guys are dead set on using this pgindent tool,
> > this is cool, we'd probably use some indentation tool on the FreeBSD
> > sources if there was one that met our code style(9) guidelines.
> 
> You don't notice the value of pgindent until you have some code that
> hasn't been run through it.  For example, ODBC was not run through until
> this release, and I had a terrible time trying to understand the code
> because it didn't _look_ like the rest of the code.  Now that pgindent
> is run, it looks more normal, and I am sure that will encourage more
> people to get in and make changes.

In FreeBSD we will simply refuse to apply patches that don't at least
somewhat adhere to our coding standards.  Word of mouth keeps people
submitting patches that are correct.

> As far as FreeBSD, I guarantee you will see major benefits to community
> participation by running the script.  You will have to hand-review all
> the changes after the first run to make sure it didn't wack out some
> wierd piece of code, but after that you will be pretty OK.  The only
> issue is that the person who takes this on is a taking a major risk of
> exposure to ridicule if it fails.

This scares me to death, has indent done this before to you?

If it has, here's a nifty trick someone started doing on our project,
whenever he came across some file that was terribly formatted (*) he'd
compile it as is, then copy the object files somewhere else, reformat
it and then recompile.  He'd then use the md5(1) command to verify
that in reality nothing had changed.

> > With that said, I really scares the crud out of me to see those massive
> > pg_indent runs right before you guys do a release.
> > 
> > It would make a lot more sense to force a pgindent run after applying
> > each patch.  This way you don't loose the history.
> 
> Yes, we have considered that.  The problem there is that sometimes
> people supply a patch, do some more work on their production source,
> then supply other patches to fix new problems.  If we pgindent for every
> CVS commit, we then are changing the supplied patch, which means any new
> patches that person sends do not match their previous patch, and we get
> into hand edits again.
> 
> I know we ask for context diff's, but anytime a patch applies with some
> offset, if the offset is large, I have to make sure there wasn't some
> other identical context of code that may have been found by the patch
> program and applied incorrectly.
> 
> A silent patch apply is safe;  if it reports a large offset, I have to
> investigate.

This really goes without saying.  I think it would be cool to setup
a site one day that explains the proper way to contribute to each
project, I still occasionally get smacked upside the head for doing
something like putting an RCS/CVS tag in the wrong spot in a file.
:)

> > You want to be upset with yourself Bruce?  Go into a directory and type:
> > 
> > cvs annotate <any file that's been pgindented>
> > 
> > cvs annotate is a really, really handy tool, unfortunetly these
> > indent runs remove this very useful tool as well as do a major job
> > of obfuscating the code changes.
> 
> I have never seen that feature.  I don't even see it in my cvs manual
> page.  It is great, and yes, I clearly wack that out for pgindent runs.
> Maybe pgindent for every commit is the way to go.

yeah. :(

> > It's not like you guys have a massive devel team with new people each
> > week that have a steep committer learning curve ahead of them, making
> > pgindent as patches are applied should work.
> 
> I imagine we can get CVS to do that automatically.  The number of patch
> on top of another patch is pretty rare and it would solve the other
> stated problems.
> 
> 
> > There's also the argument that a developer's pgindent may force a
> > contributor to resolve conflicts, while this is true, it's also
> > true that you guys expect diffs to be in context format, comments
> > to be in english, function prototypes to be new style, etc, etc..
> > 
> > I think contributors can deal with this.
> 
> If someone submits a massive patch, and we apply it, all patches after
> that that they give us will not apply cleanly because they still have
> the old format.  The other argument for not doing pgindent on cvs commit
> is that if someone is working in an area of the code, they should be
> able to format that code as they like to see it.  They may be working in
> there for months.  Only during release are is their _style_ removed.

And how exactly does that make sense?  You guys have a long beta period,
this give people that sit in thier own little corner of the code (OBDC
for instance) stuck with a major conflict resolution after release when
they go to add patches they held off on during beta.  You also suddenly
make the code look completely forien to the contributor... what if he
has a major issue with the style of pgindent?  It would make a lot more
sense to explain this up front...
  "Say, Alfred, I noticed you use two space indents, that's gross,
   have you run your code through pgindent as explained in the
   contributor's guide at http://www.postgresql.org/faq/cont....?"

> On a side note, the idea of having people submit patches only against
> current CVS seems bad to me.  If people are running production machines
> and they develop a patch and test it there, I want the patch that works
> on their machine and can make sure it applies here.  Having them
> download CVS and do the merge themselves seems really risky, especially
> because they probably can't test the CVS in production.  The CVS may not
> even run properly.

Well that's true, but how confident are you that the patch applied to
the CVS version has the same effect as the -release version?

-- 
-Alfred Perlstein - [bright@wintelcom.net|alfred@freebsd.org]
Daemon News Magazine in your snail-mail! http://magazine.daemonnews.org/

Thu, 22 Mar 2001 09:23:41 -0800
Alfred Perlstein <bright@wintelcom.net>
Re: pgindent run?
> > You don't notice the value of pgindent until you have some code that
> > hasn't been run through it.  For example, ODBC was not run through until
> > this release, and I had a terrible time trying to understand the code
> > because it didn't _look_ like the rest of the code.  Now that pgindent
> > is run, it looks more normal, and I am sure that will encourage more
> > people to get in and make changes.
> 
> In FreeBSD we will simply refuse to apply patches that don't at least
> somewhat adhere to our coding standards.  Word of mouth keeps people
> submitting patches that are correct.

Yes, but most individuals can't be bothered to do that all the time, and
automated tools make it much better.

Also, we aren't too big on rejecting patches because they don't meet our
indentation standards.  After they get involved, they start to follow
it, but usually it takes a while for that to happen, and if you enforce
it right away, you risk losing that person.


> > As far as FreeBSD, I guarantee you will see major benefits to community
> > participation by running the script.  You will have to hand-review all
> > the changes after the first run to make sure it didn't wack out some
> > wierd piece of code, but after that you will be pretty OK.  The only
> > issue is that the person who takes this on is a taking a major risk of
> > exposure to ridicule if it fails.
> 
> This scares me to death, has indent done this before to you?

No, not really.  It is more the size of the diff that scares you.

> If it has, here's a nifty trick someone started doing on our project,
> whenever he came across some file that was terribly formatted (*) he'd
> compile it as is, then copy the object files somewhere else, reformat
> it and then recompile.  He'd then use the md5(1) command to verify
> that in reality nothing had changed.

Yes, I have considered taking the 'postgres' binary and doing a cmp
against the two versions.  I think I did that the first time I ran
pgindent.  You need to skip over the object timestamp headers, but other
than that, it should work.

> > I know we ask for context diff's, but anytime a patch applies with some
> > offset, if the offset is large, I have to make sure there wasn't some
> > other identical context of code that may have been found by the patch
> > program and applied incorrectly.
> > 
> > A silent patch apply is safe;  if it reports a large offset, I have to
> > investigate.
> 
> This really goes without saying.  I think it would be cool to setup
> a site one day that explains the proper way to contribute to each
> project, I still occasionally get smacked upside the head for doing
> something like putting an RCS/CVS tag in the wrong spot in a file.
> :)

We do have the developers FAQ, which goes into some detail about it,
question #1.

> > I have never seen that feature.  I don't even see it in my cvs manual
> > page.  It is great, and yes, I clearly wack that out for pgindent runs.
> > Maybe pgindent for every commit is the way to go.
> 
> yeah. :(


> > If someone submits a massive patch, and we apply it, all patches after
> > that that they give us will not apply cleanly because they still have
> > the old format.  The other argument for not doing pgindent on cvs commit
> > is that if someone is working in an area of the code, they should be
> > able to format that code as they like to see it.  They may be working in
> > there for months.  Only during release are is their _style_ removed.
> 
> And how exactly does that make sense?  You guys have a long beta period,
> this give people that sit in thier own little corner of the code (OBDC
> for instance) stuck with a major conflict resolution after release when
> they go to add patches they held off on during beta.  You also suddenly
> make the code look completely forien to the contributor... what if he
> has a major issue with the style of pgindent?  It would make a lot more
> sense to explain this up front...
>   "Say, Alfred, I noticed you use two space indents, that's gross,
>    have you run your code through pgindent as explained in the
>    contributor's guide at http://www.postgresql.org/faq/cont....?"

But we have the FAQ item to explain the format.  I don't think we want
people personally formatting code to meet their style between releases
because it discourages others from looking at the code.  We sort of give
them their own format for the release cycle, then reign it back in just
before release.  Seems like a good balance to me.

> 
> > On a side note, the idea of having people submit patches only against
> > current CVS seems bad to me.  If people are running production machines
> > and they develop a patch and test it there, I want the patch that works
> > on their machine and can make sure it applies here.  Having them
> > download CVS and do the merge themselves seems really risky, especially
> > because they probably can't test the CVS in production.  The CVS may not
> > even run properly.
> 
> Well that's true, but how confident are you that the patch applied to
> the CVS version has the same effect as the -release version?

If there is no offset change, that means the lines are still in the
exact same spot, and no lines were added/removed above, and the code
around the patch hasn't been changed.  There is a chance that stuff
could slip in around that, but we have the beta test cycle and
reviewer's eyes to keep those in check.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Thu, 22 Mar 2001 12:33:13 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: pgindent run?
* Bruce Momjian <pgman@candle.pha.pa.us> [010322 07:12] wrote:
> > It seems that you guys are dead set on using this pgindent tool,
> > this is cool, we'd probably use some indentation tool on the FreeBSD
> > sources if there was one that met our code style(9) guidelines.
> 
> I would liken running pgindent to having a nice looking store or
> website.  No one is going to go to a website or a store only because it
> looks nice, but having it look nice does keep people coming back.

I'm not saying I don't like pgindent, I'm saying I don't like
pgindent's effect on the CVS history.

-- 
-Alfred Perlstein - [bright@wintelcom.net|alfred@freebsd.org]
Instead of asking why a piece of software is using "1970s technology,"
start asking why software is ignoring 30 years of accumulated wisdom.

Thu, 22 Mar 2001 09:36:17 -0800
Alfred Perlstein <bright@wintelcom.net>
Re: pgindent run?
> * Bruce Momjian <pgman@candle.pha.pa.us> [010322 07:12] wrote:
> > > It seems that you guys are dead set on using this pgindent tool,
> > > this is cool, we'd probably use some indentation tool on the FreeBSD
> > > sources if there was one that met our code style(9) guidelines.
> > 
> > I would liken running pgindent to having a nice looking store or
> > website.  No one is going to go to a website or a store only because it
> > looks nice, but having it look nice does keep people coming back.
> 
> I'm not saying I don't like pgindent, I'm saying I don't like
> pgindent's effect on the CVS history.

How do we get around this problem?

Also, I now remember that the problem with CVS auto-pgindenting, as Tom
mentioned, is that once you cvs commit, you would have to cvs update
again because your source tree wouldn't match cvs anymore.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Thu, 22 Mar 2001 12:54:11 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: pgindent run?
Bruce Momjian wrote:
> 
> You don't notice the value of pgindent until you have some code that
> hasn't been run through it.  For example, ODBC was not run through until
> this release, and I had a terrible time trying to understand the code
> because it didn't _look_ like the rest of the code.  Now that pgindent
> is run, it looks more normal, and I am sure that will encourage more
> people to get in and make changes.
> 

I see now the following comment in interfaces/odbc/statement.c.
Though it's mine(probably), it's hard for me to read.
Please tell me how to prevent pgindent from changing
comments.

 /*
  * Basically we don't have to begin a transaction in autocommit mode
  * because Postgres backend runs in autocomit mode. We issue "BEGIN"
  * in the following cases. 1) we use declare/fetch and the statement
  * is SELECT (because declare/fetch must be called in a transaction).
  * 2) we are in autocommit off state and the statement isn't of type
  * OTHER.
  */

regards,
Hiroshi Inoue

Fri, 23 Mar 2001 09:37:39 +0900
Hiroshi Inoue <Inoue@tpf.co.jp>
Re: pgindent run?
Hiroshi Inoue <Inoue@tpf.co.jp> writes:
> Please tell me how to prevent pgindent from changing
> comments.

Put dashes at the start and end of the comment block, eg

	/*----------
	 * comment here
	 *----------
	 */

I'm not sure exactly how many dashes are needed --- I usually use ten as
shown above.

			regards, tom lane

Thu, 22 Mar 2001 19:38:47 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: pgindent run? 
I have applied the following patch to fix wrapping of comparisons in
comment text, for Tom Lane.

If others find comments that were mis-wrapped, I would be glad to fix
them.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Index: contrib/spi/refint.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/contrib/spi/refint.c,v
retrieving revision 1.13
diff -c -r1.13 refint.c
*** contrib/spi/refint.c	2000/12/03 20:45:31	1.13
--- contrib/spi/refint.c	2001/03/23 04:37:25
***************
*** 399,417 ****
  		{
  			relname = args2[0];
  
! 			/*
! 			 * For 'R'estrict action we construct SELECT query - SELECT 1
! 			 * FROM _referencing_relation_ WHERE Fkey1 = $1 [AND Fkey2 =
! 			 * $2 [...]] - to check is tuple referenced or not.
  			 */
  			if (action == 'r')
  
  				sprintf(sql, "select 1 from %s where ", relname);
  
! 			/*
! 			 * For 'C'ascade action we construct DELETE query - DELETE
! 			 * FROM _referencing_relation_ WHERE Fkey1 = $1 [AND Fkey2 =
! 			 * $2 [...]] - to delete all referencing tuples.
  			 */
  
  			/*
--- 399,427 ----
  		{
  			relname = args2[0];
  
! 			/*---------
! 			 * For 'R'estrict action we construct SELECT query:
! 			 *
! 			 *  SELECT 1
! 			 *	FROM _referencing_relation_
! 			 *	WHERE Fkey1 = $1 [AND Fkey2 = $2 [...]]
! 			 *
! 			 *  to check is tuple referenced or not.
! 			 *---------
  			 */
  			if (action == 'r')
  
  				sprintf(sql, "select 1 from %s where ", relname);
  
! 			/*---------
! 			 * For 'C'ascade action we construct DELETE query
! 			 *
! 			 *	DELETE
! 			 *	FROM _referencing_relation_
! 			 *	WHERE Fkey1 = $1 [AND Fkey2 = $2 [...]]
! 			 *
! 			 * to delete all referencing tuples.
! 			 *---------
  			 */
  
  			/*
Index: src/backend/access/gist/gistscan.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/access/gist/gistscan.c,v
retrieving revision 1.32
diff -c -r1.32 gistscan.c
*** src/backend/access/gist/gistscan.c	2001/03/22 03:59:12	1.32
--- src/backend/access/gist/gistscan.c	2001/03/23 04:37:26
***************
*** 143,151 ****
  			for (i = 0; i < s->numberOfKeys; i++)
  			{
  
! 				/*
  				 * s->keyData[i].sk_procedure =
! 				 * index_getprocid(s->relation, 1, GIST_CONSISTENT_PROC);
  				 */
  				s->keyData[i].sk_procedure
  					= RelationGetGISTStrategy(s->relation, s->keyData[i].sk_attno,
--- 143,152 ----
  			for (i = 0; i < s->numberOfKeys; i++)
  			{
  
! 				/*----------
  				 * s->keyData[i].sk_procedure =
! 				 * 		index_getprocid(s->relation, 1, GIST_CONSISTENT_PROC);
! 				 *----------
  				 */
  				s->keyData[i].sk_procedure
  					= RelationGetGISTStrategy(s->relation, s->keyData[i].sk_attno,
Index: src/backend/access/hash/hashsearch.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/access/hash/hashsearch.c,v
retrieving revision 1.25
diff -c -r1.25 hashsearch.c
*** src/backend/access/hash/hashsearch.c	2001/01/24 19:42:47	1.25
--- src/backend/access/hash/hashsearch.c	2001/03/23 04:37:26
***************
*** 334,342 ****
  				while (offnum > maxoff)
  				{
  
! 					/*
! 					 * either this page is empty (maxoff ==
! 					 * InvalidOffsetNumber) or we ran off the end.
  					 */
  					_hash_readnext(rel, &buf, &page, &opaque);
  					if (BufferIsInvalid(buf))
--- 334,344 ----
  				while (offnum > maxoff)
  				{
  
! 					/*--------
! 					 * either this page is empty
! 					 * (maxoff == InvalidOffsetNumber)
! 					 * or we ran off the end.
! 					 *--------
  					 */
  					_hash_readnext(rel, &buf, &page, &opaque);
  					if (BufferIsInvalid(buf))
***************
*** 382,390 ****
  				while (offnum < FirstOffsetNumber)
  				{
  
! 					/*
! 					 * either this page is empty (offnum ==
! 					 * InvalidOffsetNumber) or we ran off the end.
  					 */
  					_hash_readprev(rel, &buf, &page, &opaque);
  					if (BufferIsInvalid(buf))
--- 384,394 ----
  				while (offnum < FirstOffsetNumber)
  				{
  
! 					/*---------
! 					 * either this page is empty
! 					 * (offnum == InvalidOffsetNumber)
! 					 * or we ran off the end.
! 					 *---------
  					 */
  					_hash_readprev(rel, &buf, &page, &opaque);
  					if (BufferIsInvalid(buf))
Index: src/backend/access/heap/tuptoaster.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/access/heap/tuptoaster.c,v
retrieving revision 1.19
diff -c -r1.19 tuptoaster.c
*** src/backend/access/heap/tuptoaster.c	2001/03/22 06:16:07	1.19
--- src/backend/access/heap/tuptoaster.c	2001/03/23 04:37:26
***************
*** 458,466 ****
  		int32		biggest_size = MAXALIGN(sizeof(varattrib));
  		Datum		old_value;
  
! 		/*
! 		 * Search for the biggest yet inlined attribute with attstorage =
! 		 * 'x' or 'e'
  		 */
  		for (i = 0; i < numAttrs; i++)
  		{
--- 458,467 ----
  		int32		biggest_size = MAXALIGN(sizeof(varattrib));
  		Datum		old_value;
  
! 		/*------
! 		 * Search for the biggest yet inlined attribute with
! 		 * attstorage equals 'x' or 'e'
! 		 *------
  		 */
  		for (i = 0; i < numAttrs; i++)
  		{
***************
*** 572,580 ****
  		int32		biggest_size = MAXALIGN(sizeof(varattrib));
  		Datum		old_value;
  
! 		/*
! 		 * Search for the biggest yet inlined attribute with attstorage =
! 		 * 'm'
  		 */
  		for (i = 0; i < numAttrs; i++)
  		{
--- 573,582 ----
  		int32		biggest_size = MAXALIGN(sizeof(varattrib));
  		Datum		old_value;
  
! 		/*--------
! 		 * Search for the biggest yet inlined attribute with
! 		 * attstorage = 'm'
! 		 *--------
  		 */
  		for (i = 0; i < numAttrs; i++)
  		{
Index: src/backend/access/nbtree/nbtsearch.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/access/nbtree/nbtsearch.c,v
retrieving revision 1.65
diff -c -r1.65 nbtsearch.c
*** src/backend/access/nbtree/nbtsearch.c	2001/03/22 06:16:07	1.65
--- src/backend/access/nbtree/nbtsearch.c	2001/03/23 04:37:27
***************
*** 584,591 ****
  
  	/*
  	 * At this point we are positioned at the first item >= scan key, or
! 	 * possibly at the end of a page on which all the existing items are <
! 	 * scan key and we know that everything on later pages is >= scan key.
  	 * We could step forward in the latter case, but that'd be a waste of
  	 * time if we want to scan backwards.  So, it's now time to examine
  	 * the scan strategy to find the exact place to start the scan.
--- 584,593 ----
  
  	/*
  	 * At this point we are positioned at the first item >= scan key, or
! 	 * possibly at the end of a page on which all the existing items are 
! 	 * greater than the scan key and we know that everything on later pages
! 	 * is less than or equal to scan key.
!      *
  	 * We could step forward in the latter case, but that'd be a waste of
  	 * time if we want to scan backwards.  So, it's now time to examine
  	 * the scan strategy to find the exact place to start the scan.
Index: src/backend/access/nbtree/nbtutils.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/access/nbtree/nbtutils.c,v
retrieving revision 1.43
diff -c -r1.43 nbtutils.c
*** src/backend/access/nbtree/nbtutils.c	2001/03/22 03:59:15	1.43
--- src/backend/access/nbtree/nbtutils.c	2001/03/23 04:37:27
***************
*** 412,419 ****
  			if (DatumGetBool(test))
  				xform[j].sk_argument = cur->sk_argument;
  			else if (j == (BTEqualStrategyNumber - 1))
! 				so->qual_ok = false;	/* key == a && key == b, but a !=
! 										 * b */
  		}
  		else
  		{
--- 412,419 ----
  			if (DatumGetBool(test))
  				xform[j].sk_argument = cur->sk_argument;
  			else if (j == (BTEqualStrategyNumber - 1))
! 				so->qual_ok = false;
! 			/* key == a && key == b, but a != b */
  		}
  		else
  		{
Index: src/backend/commands/command.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/commands/command.c,v
retrieving revision 1.124
diff -c -r1.124 command.c
*** src/backend/commands/command.c	2001/03/22 06:16:11	1.124
--- src/backend/commands/command.c	2001/03/23 04:37:28
***************
*** 1034,1044 ****
  	ScanKeyEntryInitialize(&scankeys[0], 0x0, Anum_pg_attrdef_adrelid,
  						   F_OIDEQ, ObjectIdGetDatum(myrelid));
  
! 	/*
  	 * Oops pg_attrdef doesn't have (adrelid,adnum) index
! 	 * ScanKeyEntryInitialize(&scankeys[1], 0x0, Anum_pg_attrdef_adnum,
! 	 * F_INT2EQ, Int16GetDatum(attnum)); sysscan =
! 	 * systable_beginscan(adrel, AttrDefaultIndex, 2, scankeys);
  	 */
  	sysscan = systable_beginscan(adrel, AttrDefaultIndex, 1, scankeys);
  	while (HeapTupleIsValid(tup = systable_getnext(sysscan)))
--- 1034,1046 ----
  	ScanKeyEntryInitialize(&scankeys[0], 0x0, Anum_pg_attrdef_adrelid,
  						   F_OIDEQ, ObjectIdGetDatum(myrelid));
  
! 	/*--------
  	 * Oops pg_attrdef doesn't have (adrelid,adnum) index
! 	 *
! 	 *	ScanKeyEntryInitialize(&scankeys[1], 0x0, Anum_pg_attrdef_adnum,
! 	 * 								F_INT2EQ, Int16GetDatum(attnum));
! 	 *	sysscan = systable_beginscan(adrel, AttrDefaultIndex, 2, scankeys);
! 	 *--------
  	 */
  	sysscan = systable_beginscan(adrel, AttrDefaultIndex, 1, scankeys);
  	while (HeapTupleIsValid(tup = systable_getnext(sysscan)))
Index: src/backend/commands/_deadcode/version.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/commands/_deadcode/version.c,v
retrieving revision 1.25
diff -c -r1.25 version.c
*** src/backend/commands/_deadcode/version.c	2001/01/24 19:42:53	1.25
--- src/backend/commands/_deadcode/version.c	2001/03/23 04:37:29
***************
*** 77,85 ****
  eval_as_new_xact(char *query)
  {
  
! 	/*
  	 * WARNING! do not uncomment the following lines WARNING!
! 	 * CommitTransactionCommand(); StartTransactionCommand();
  	 */
  	CommandCounterIncrement();
  	pg_exec_query(query);
--- 77,88 ----
  eval_as_new_xact(char *query)
  {
  
! 	/*------
  	 * WARNING! do not uncomment the following lines WARNING!
! 	 *
! 	 *	CommitTransactionCommand();
! 	 *	StartTransactionCommand();
! 	 *------
  	 */
  	CommandCounterIncrement();
  	pg_exec_query(query);
Index: src/backend/executor/execQual.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/executor/execQual.c,v
retrieving revision 1.84
diff -c -r1.84 execQual.c
*** src/backend/executor/execQual.c	2001/03/22 03:59:26	1.84
--- src/backend/executor/execQual.c	2001/03/23 04:37:30
***************
*** 1499,1506 ****
  	 * and another array that holds the isDone status for each targetlist
  	 * item. The isDone status is needed so that we can iterate,
  	 * generating multiple tuples, when one or more tlist items return
! 	 * sets.  (We expect the caller to call us again if we return *isDone
! 	 * = ExprMultipleResult.)
  	 */
  	if (nodomains > NPREALLOCDOMAINS)
  	{
--- 1499,1507 ----
  	 * and another array that holds the isDone status for each targetlist
  	 * item. The isDone status is needed so that we can iterate,
  	 * generating multiple tuples, when one or more tlist items return
! 	 * sets.  (We expect the caller to call us again if we return:
! 	 *
! 	 *	isDone = ExprMultipleResult.)
  	 */
  	if (nodomains > NPREALLOCDOMAINS)
  	{
Index: src/backend/executor/nodeLimit.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/executor/nodeLimit.c,v
retrieving revision 1.5
diff -c -r1.5 nodeLimit.c
*** src/backend/executor/nodeLimit.c	2001/03/22 06:16:13	1.5
--- src/backend/executor/nodeLimit.c	2001/03/23 04:37:30
***************
*** 79,86 ****
  		 * tuple in the offset region before we can return NULL.
  		 * Otherwise we won't be correctly aligned to start going forward
  		 * again.  So, although you might think we can quit when position
! 		 * = offset + 1, we have to fetch a subplan tuple first, and then
! 		 * exit when position = offset.
  		 */
  		if (ScanDirectionIsForward(direction))
  		{
--- 79,86 ----
  		 * tuple in the offset region before we can return NULL.
  		 * Otherwise we won't be correctly aligned to start going forward
  		 * again.  So, although you might think we can quit when position
! 		 * equals offset + 1, we have to fetch a subplan tuple first, and
! 		 * then exit when position = offset.
  		 */
  		if (ScanDirectionIsForward(direction))
  		{
Index: src/backend/executor/nodeMergejoin.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/executor/nodeMergejoin.c,v
retrieving revision 1.44
diff -c -r1.44 nodeMergejoin.c
*** src/backend/executor/nodeMergejoin.c	2001/03/22 06:16:13	1.44
--- src/backend/executor/nodeMergejoin.c	2001/03/23 04:37:31
***************
*** 240,249 ****
  			break;
  		}
  
! 		/*
  		 * ok, the compare clause failed so we test if the keys are
! 		 * equal... if key1 != key2, we return false. otherwise key1 =
! 		 * key2 so we move on to the next pair of keys.
  		 */
  		const_value = ExecEvalExpr((Node *) lfirst(eqclause),
  								   econtext,
--- 240,250 ----
  			break;
  		}
  
! 		/*-----------
  		 * ok, the compare clause failed so we test if the keys are
! 		 * equal... if key1 != key2, we return false. otherwise
! 		 * key1 = key2 so we move on to the next pair of keys.
! 		 *-----------
  		 */
  		const_value = ExecEvalExpr((Node *) lfirst(eqclause),
  								   econtext,
Index: src/backend/optimizer/path/clausesel.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/optimizer/path/clausesel.c,v
retrieving revision 1.42
diff -c -r1.42 clausesel.c
*** src/backend/optimizer/path/clausesel.c	2001/03/22 03:59:34	1.42
--- src/backend/optimizer/path/clausesel.c	2001/03/23 04:37:31
***************
*** 297,305 ****
  			else
  			{
  
! 				/*
! 				 * We have found two similar clauses, such as x < y AND x
! 				 * < z.  Keep only the more restrictive one.
  				 */
  				if (rqelem->lobound > s2)
  					rqelem->lobound = s2;
--- 297,307 ----
  			else
  			{
  
! 				/*------
! 				 * We have found two similar clauses, such as
! 				 * x < y AND x < z.
! 				 * Keep only the more restrictive one.
! 				 *------
  				 */
  				if (rqelem->lobound > s2)
  					rqelem->lobound = s2;
***************
*** 315,323 ****
  			else
  			{
  
! 				/*
! 				 * We have found two similar clauses, such as x > y AND x
! 				 * > z.  Keep only the more restrictive one.
  				 */
  				if (rqelem->hibound > s2)
  					rqelem->hibound = s2;
--- 317,327 ----
  			else
  			{
  
! 				/*------
! 				 * We have found two similar clauses, such as
! 				 * x > y AND x > z.
! 				 * Keep only the more restrictive one.
! 				 *------
  				 */
  				if (rqelem->hibound > s2)
  					rqelem->hibound = s2;
Index: src/backend/optimizer/path/indxpath.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/optimizer/path/indxpath.c,v
retrieving revision 1.103
diff -c -r1.103 indxpath.c
*** src/backend/optimizer/path/indxpath.c	2001/03/22 03:59:35	1.103
--- src/backend/optimizer/path/indxpath.c	2001/03/23 04:37:33
***************
*** 1986,1994 ****
  	expr = make_opclause(op, leftop, (Var *) con);
  	result = makeList1(expr);
  
! 	/*
! 	 * If we can create a string larger than the prefix, we can say "x <
! 	 * greaterstr".
  	 */
  	greaterstr = make_greater_string(prefix, datatype);
  	if (greaterstr)
--- 1986,1995 ----
  	expr = make_opclause(op, leftop, (Var *) con);
  	result = makeList1(expr);
  
! 	/*-------
! 	 * If we can create a string larger than the prefix, we can say
! 	 * "x < greaterstr".
! 	 *-------
  	 */
  	greaterstr = make_greater_string(prefix, datatype);
  	if (greaterstr)
Index: src/backend/rewrite/rewriteDefine.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/rewrite/rewriteDefine.c,v
retrieving revision 1.60
diff -c -r1.60 rewriteDefine.c
*** src/backend/rewrite/rewriteDefine.c	2001/03/22 06:16:16	1.60
--- src/backend/rewrite/rewriteDefine.c	2001/03/23 04:37:33
***************
*** 130,139 ****
  
  #ifdef NOT_USED
  
! 	/*
  	 * on retrieve to class.attribute do instead nothing is converted to
! 	 * 'on retrieve to class.attribute do instead retrieve (attribute =
! 	 * NULL)' --- this is also a terrible hack that works well -- glass
  	 */
  	if (is_instead && !*action && eslot_string && event_type == CMD_SELECT)
  	{
--- 130,143 ----
  
  #ifdef NOT_USED
  
! 	/*---------
  	 * on retrieve to class.attribute do instead nothing is converted to
! 	 * 'on retrieve to class.attribute do instead:
! 	 *
! 	 *	 retrieve (attribute = NULL)'
! 	 *
! 	 * this is also a terrible hack that works well -- glass
! 	 *---------
  	 */
  	if (is_instead && !*action && eslot_string && event_type == CMD_SELECT)
  	{
Index: src/backend/storage/ipc/ipc.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/storage/ipc/ipc.c,v
retrieving revision 1.65
diff -c -r1.65 ipc.c
*** src/backend/storage/ipc/ipc.c	2001/03/22 06:16:16	1.65
--- src/backend/storage/ipc/ipc.c	2001/03/23 04:37:34
***************
*** 404,410 ****
  	 * and entering the semop() call.  If a cancel/die interrupt occurs in
  	 * that window, we would fail to notice it until after we acquire the
  	 * lock (or get another interrupt to escape the semop()).  We can
! 	 * avoid this problem by temporarily setting ImmediateInterruptOK =
  	 * true before we do CHECK_FOR_INTERRUPTS; then, a die() interrupt in
  	 * this interval will execute directly.  However, there is a huge
  	 * pitfall: there is another window of a few instructions after the
--- 404,410 ----
  	 * and entering the semop() call.  If a cancel/die interrupt occurs in
  	 * that window, we would fail to notice it until after we acquire the
  	 * lock (or get another interrupt to escape the semop()).  We can
! 	 * avoid this problem by temporarily setting ImmediateInterruptOK to
  	 * true before we do CHECK_FOR_INTERRUPTS; then, a die() interrupt in
  	 * this interval will execute directly.  However, there is a huge
  	 * pitfall: there is another window of a few instructions after the
Index: src/backend/storage/ipc/sinval.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/storage/ipc/sinval.c,v
retrieving revision 1.28
diff -c -r1.28 sinval.c
*** src/backend/storage/ipc/sinval.c	2001/03/22 03:59:45	1.28
--- src/backend/storage/ipc/sinval.c	2001/03/23 04:37:34
***************
*** 319,329 ****
  				xid < FirstTransactionId || xid >= snapshot->xmax)
  			{
  
! 				/*
! 				 * Seems that there is no sense to store xid >=
! 				 * snapshot->xmax (what we got from ReadNewTransactionId
! 				 * above) in snapshot->xip - we just assume that all xacts
  				 * with such xid-s are running and may be ignored.
  				 */
  				continue;
  			}
--- 319,331 ----
  				xid < FirstTransactionId || xid >= snapshot->xmax)
  			{
  
! 				/*--------
! 				 * Seems that there is no sense to store
! 				 * 		xid >= snapshot->xmax
! 				 * (what we got from ReadNewTransactionId above)
! 				 * in snapshot->xip.  We just assume that all xacts
  				 * with such xid-s are running and may be ignored.
+ 				 *--------
  				 */
  				continue;
  			}
Index: src/backend/utils/adt/formatting.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/utils/adt/formatting.c,v
retrieving revision 1.35
diff -c -r1.35 formatting.c
*** src/backend/utils/adt/formatting.c	2001/03/22 06:16:17	1.35
--- src/backend/utils/adt/formatting.c	2001/03/23 04:37:36
***************
*** 2846,2854 ****
  	else if (tmfc->yy)
  	{
  
! 		/*
! 		 * 2-digit year: '00' ... '69'	= 2000 ... 2069 '70' ... '99'  =
! 		 * 1970 ... 1999
  		 */
  		tm->tm_year = tmfc->yy;
  
--- 2846,2856 ----
  	else if (tmfc->yy)
  	{
  
! 		/*---------
! 		 * 2-digit year:
! 		 * '00' ... '69'  = 2000 ... 2069
! 		 * '70' ... '99'  = 1970 ... 1999
! 		 *---------
  		 */
  		tm->tm_year = tmfc->yy;
  
***************
*** 2860,2868 ****
  	else if (tmfc->yyy)
  	{
  
! 		/*
! 		 * 3-digit year: '100' ... '999' = 1100 ... 1999 '000' ... '099' =
! 		 * 2000 ... 2099
  		 */
  		tm->tm_year = tmfc->yyy;
  
--- 2862,2872 ----
  	else if (tmfc->yyy)
  	{
  
! 		/*---------
! 		 * 3-digit year:
! 		 *	'100' ... '999' = 1100 ... 1999
! 		 *	'000' ... '099' = 2000 ... 2099
! 		 *---------
  		 */
  		tm->tm_year = tmfc->yyy;
  
Index: src/backend/utils/adt/selfuncs.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/utils/adt/selfuncs.c,v
retrieving revision 1.86
diff -c -r1.86 selfuncs.c
*** src/backend/utils/adt/selfuncs.c	2001/03/22 03:59:54	1.86
--- src/backend/utils/adt/selfuncs.c	2001/03/23 04:37:38
***************
*** 1642,1650 ****
  							   Int32GetDatum(SEL_CONSTANT | SEL_RIGHT)));
  	pfree(DatumGetPointer(prefixcon));
  
! 	/*
! 	 * If we can create a string larger than the prefix, say "x <
! 	 * greaterstr".
  	 */
  	greaterstr = make_greater_string(prefix, datatype);
  	if (greaterstr)
--- 1642,1651 ----
  							   Int32GetDatum(SEL_CONSTANT | SEL_RIGHT)));
  	pfree(DatumGetPointer(prefixcon));
  
! 	/*-------
! 	 * If we can create a string larger than the prefix, say
! 	 *	"x < greaterstr".
! 	 *-------
  	 */
  	greaterstr = make_greater_string(prefix, datatype);
  	if (greaterstr)
Index: src/backend/utils/cache/lsyscache.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/utils/cache/lsyscache.c,v
retrieving revision 1.51
diff -c -r1.51 lsyscache.c
*** src/backend/utils/cache/lsyscache.c	2001/03/22 03:59:57	1.51
--- src/backend/utils/cache/lsyscache.c	2001/03/23 04:37:39
***************
*** 272,278 ****
  
  	/*
  	 * VACUUM ANALYZE has not been run for this table. Produce an estimate
! 	 * = 1/numtuples.  This may produce unreasonably small estimates for
  	 * large tables, so limit the estimate to no less than min_estimate.
  	 */
  	dispersion = 1.0 / (double) ntuples;
--- 272,278 ----
  
  	/*
  	 * VACUUM ANALYZE has not been run for this table. Produce an estimate
! 	 * of 1/numtuples.  This may produce unreasonably small estimates for
  	 * large tables, so limit the estimate to no less than min_estimate.
  	 */
  	dispersion = 1.0 / (double) ntuples;
Index: src/backend/utils/cache/relcache.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/utils/cache/relcache.c,v
retrieving revision 1.129
diff -c -r1.129 relcache.c
*** src/backend/utils/cache/relcache.c	2001/03/22 03:59:57	1.129
--- src/backend/utils/cache/relcache.c	2001/03/23 04:37:40
***************
*** 2833,2843 ****
  	 * the descriptors, nail them into cache so we never lose them.
  	 */
  
! 	/*
! 	 * Removed the following ProcessingMode change -- inoue At this point
! 	 * 1) Catalog Cache isn't initialized 2) Relation Cache for the
! 	 * following critical indexes aren't built oldmode =
! 	 * GetProcessingMode(); SetProcessingMode(BootstrapProcessing);
  	 */
  
  	bi.infotype = INFO_RELNAME;
--- 2833,2846 ----
  	 * the descriptors, nail them into cache so we never lose them.
  	 */
  
! 	/*---------
! 	 * Removed the following ProcessingMode change -- inoue
! 	 * At this point
! 	 * 1) Catalog Cache isn't initialized
! 	 * 2) Relation Cache for the following critical indexes aren't built
! 	 * oldmode = GetProcessingMode();
! 	 * SetProcessingMode(BootstrapProcessing);
! 	 *---------
  	 */
  
  	bi.infotype = INFO_RELNAME;
Index: src/backend/utils/sort/tuplesort.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/utils/sort/tuplesort.c,v
retrieving revision 1.14
diff -c -r1.14 tuplesort.c
*** src/backend/utils/sort/tuplesort.c	2001/03/22 04:00:09	1.14
--- src/backend/utils/sort/tuplesort.c	2001/03/23 04:37:42
***************
*** 129,136 ****
  	 * kind of tuple we are sorting from the routines that don't need to
  	 * know it. They are set up by the tuplesort_begin_xxx routines.
  	 *
! 	 * Function to compare two tuples; result is per qsort() convention, ie,
! 	 * <0, 0, >0 according as a<b, a=b, a>b.
  	 */
  	int			(*comparetup) (Tuplesortstate *state, const void *a, const void *b);
  
--- 129,138 ----
  	 * kind of tuple we are sorting from the routines that don't need to
  	 * know it. They are set up by the tuplesort_begin_xxx routines.
  	 *
! 	 * Function to compare two tuples; result is per qsort() convention,
! 	 * ie:
! 	 *
! 	 * 	<0, 0, >0 according as a<b, a=b, a>b.
  	 */
  	int			(*comparetup) (Tuplesortstate *state, const void *a, const void *b);
  
Index: src/bin/pg_dump/pg_backup_db.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/bin/pg_dump/pg_backup_db.c,v
retrieving revision 1.16
diff -c -r1.16 pg_backup_db.c
*** src/bin/pg_dump/pg_backup_db.c	2001/03/22 04:00:12	1.16
--- src/bin/pg_dump/pg_backup_db.c	2001/03/23 04:37:42
***************
*** 473,481 ****
  				qry += loc + 1;
  				isEnd = (strcmp(AH->pgCopyBuf->data, "\\.\n") == 0);
  
! 				/*
! 				 * fprintf(stderr, "Sending '%s' via COPY (at end =
! 				 * %d)\n\n", AH->pgCopyBuf->data, isEnd);
  				 */
  
  				if (PQputline(AH->connection, AH->pgCopyBuf->data) != 0)
--- 473,482 ----
  				qry += loc + 1;
  				isEnd = (strcmp(AH->pgCopyBuf->data, "\\.\n") == 0);
  
! 				/*---------
! 				 * fprintf(stderr, "Sending '%s' via
! 				 *		COPY (at end = %d)\n\n", AH->pgCopyBuf->data, isEnd);
! 				 *---------
  				 */
  
  				if (PQputline(AH->connection, AH->pgCopyBuf->data) != 0)
Index: src/bin/pg_dump/pg_dump.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/bin/pg_dump/pg_dump.c,v
retrieving revision 1.196
diff -c -r1.196 pg_dump.c
*** src/bin/pg_dump/pg_dump.c	2001/03/22 04:00:14	1.196
--- src/bin/pg_dump/pg_dump.c	2001/03/23 04:37:45
***************
*** 4405,4412 ****
  	/*
  	 * The logic we use for restoring sequences is as follows: -   Add a
  	 * basic CREATE SEQUENCE statement (use last_val for start if called
! 	 * == 'f', else use min_val for start_val). -	Add a 'SETVAL(seq,
! 	 * last_val, iscalled)' at restore-time iff we load data
  	 */
  
  	if (!dataOnly)
--- 4405,4414 ----
  	/*
  	 * The logic we use for restoring sequences is as follows: -   Add a
  	 * basic CREATE SEQUENCE statement (use last_val for start if called
! 	 * with 'f', else use min_val for start_val).
! 	 *
! 	 *	Add a 'SETVAL(seq, last_val, iscalled)' at restore-time iff
! 	 *  we load data
  	 */
  
  	if (!dataOnly)
Index: src/bin/pg_dump/pg_dump.h
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/bin/pg_dump/pg_dump.h,v
retrieving revision 1.59
diff -c -r1.59 pg_dump.h
*** src/bin/pg_dump/pg_dump.h	2001/03/22 04:00:15	1.59
--- src/bin/pg_dump/pg_dump.h	2001/03/23 04:37:45
***************
*** 158,165 ****
  {
  	char	   *oid;
  	char	   *oprname;
! 	char	   *oprkind;		/* "b" = binary, "l" = left unary, "r" =
! 								 * right unary */
  	char	   *oprcode;		/* operator function name */
  	char	   *oprleft;		/* left operand type */
  	char	   *oprright;		/* right operand type */
--- 158,169 ----
  {
  	char	   *oid;
  	char	   *oprname;
! 	char	   *oprkind;		/*----------
! 								 * 	b = binary,
! 								 *	l = left unary
! 								 *	r = right unary
! 								 *----------
! 								 */
  	char	   *oprcode;		/* operator function name */
  	char	   *oprleft;		/* left operand type */
  	char	   *oprright;		/* right operand type */
Index: src/include/catalog/pg_type.h
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/include/catalog/pg_type.h,v
retrieving revision 1.102
diff -c -r1.102 pg_type.h
*** src/include/catalog/pg_type.h	2001/03/22 04:00:41	1.102
--- src/include/catalog/pg_type.h	2001/03/23 04:37:46
***************
*** 77,84 ****
  	 * be a "real" array type; some ordinary fixed-length types can also
  	 * be subscripted (e.g., oidvector). Variable-length types can *not*
  	 * be turned into pseudo-arrays like that. Hence, the way to determine
! 	 * whether a type is a "true" array type is typelem != 0 and typlen <
! 	 * 0.
  	 */
  	Oid			typelem;
  	regproc		typinput;
--- 77,85 ----
  	 * be a "real" array type; some ordinary fixed-length types can also
  	 * be subscripted (e.g., oidvector). Variable-length types can *not*
  	 * be turned into pseudo-arrays like that. Hence, the way to determine
! 	 * whether a type is a "true" array type is if:
! 	 *
! 	 *	typelem != 0 and typlen < 0.
  	 */
  	Oid			typelem;
  	regproc		typinput;
Index: src/include/nodes/parsenodes.h
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/include/nodes/parsenodes.h,v
retrieving revision 1.125
diff -c -r1.125 parsenodes.h
*** src/include/nodes/parsenodes.h	2001/03/22 04:00:51	1.125
--- src/include/nodes/parsenodes.h	2001/03/23 04:37:47
***************
*** 116,125 ****
  typedef struct AlterTableStmt
  {
  	NodeTag		type;
! 	char		subtype;		/* A = add column, T = alter column, D =
! 								 * drop column, C = add constraint, X =
! 								 * drop constraint, E = add toast table, U
! 								 * = change owner */
  	char	   *relname;		/* table to work on */
  	InhOption	inhOpt;			/* recursively act on children? */
  	char	   *name;			/* column or constraint name to act on, or
--- 116,131 ----
  typedef struct AlterTableStmt
  {
  	NodeTag		type;
! 	char		subtype;		/*------------
! 								 * 	A = add column
! 								 *	T = alter column
! 								 *	D = drop column
! 								 *	C = add constraint
! 								 *	X = drop constraint
! 								 *	E = add toast table,
! 								 *	U = change owner
! 								 *------------
! 								 */
  	char	   *relname;		/* table to work on */
  	InhOption	inhOpt;			/* recursively act on children? */
  	char	   *name;			/* column or constraint name to act on, or
Index: src/interfaces/ecpg/lib/connect.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/ecpg/lib/connect.c,v
retrieving revision 1.8
diff -c -r1.8 connect.c
*** src/interfaces/ecpg/lib/connect.c	2001/03/22 04:01:17	1.8
--- src/interfaces/ecpg/lib/connect.c	2001/03/23 04:37:47
***************
*** 307,316 ****
  		if (strncmp(dbname + offset, "postgresql://", strlen("postgresql://")) == 0)
  		{
  
! 			/*
  			 * new style:
! 			 * <tcp|unix>:postgresql://server[:port|:/unixsocket/path:][/db
! 			 * name][?options]
  			 */
  			offset += strlen("postgresql://");
  
--- 307,317 ----
  		if (strncmp(dbname + offset, "postgresql://", strlen("postgresql://")) == 0)
  		{
  
! 			/*------
  			 * new style:
! 			 * 	<tcp|unix>:postgresql://server[:port|:/unixsocket/path:]
! 			 *	[/db name][?options]
! 			 *------
  			 */
  			offset += strlen("postgresql://");
  
Index: src/interfaces/libpq/fe-connect.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.162
diff -c -r1.162 fe-connect.c
*** src/interfaces/libpq/fe-connect.c	2001/03/22 06:16:20	1.162
--- src/interfaces/libpq/fe-connect.c	2001/03/23 04:37:48
***************
*** 582,591 ****
  		if (strncmp(conn->dbName + offset, "postgresql://", strlen("postgresql://")) == 0)
  		{
  
! 			/*
  			 * new style:
! 			 * <tcp|unix>:postgresql://server[:port|:/unixsocket/path:][/db
! 			 * name][?options]
  			 */
  			offset += strlen("postgresql://");
  
--- 582,592 ----
  		if (strncmp(conn->dbName + offset, "postgresql://", strlen("postgresql://")) == 0)
  		{
  
! 			/*-------
  			 * new style:
! 			 * 	<tcp|unix>:postgresql://server[:port|:/unixsocket/path:]
! 			 *	[/db name][?options]
! 			 *-------
  			 */
  			offset += strlen("postgresql://");
  
Index: src/interfaces/odbc/info.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/odbc/info.c,v
retrieving revision 1.42
diff -c -r1.42 info.c
*** src/interfaces/odbc/info.c	2001/03/22 04:01:33	1.42
--- src/interfaces/odbc/info.c	2001/03/23 04:37:50
***************
*** 1738,1754 ****
  		set_tuplefield_string(&row->tuple[5], field_type_name);
  
  
! 		/*
  		 * Some Notes about Postgres Data Types:
  		 *
  		 * VARCHAR - the length is stored in the pg_attribute.atttypmod field
  		 * BPCHAR  - the length is also stored as varchar is
  		 *
! 		 * NUMERIC - the scale is stored in atttypmod as follows: precision =
! 		 * ((atttypmod - VARHDRSZ) >> 16) & 0xffff scale	 = (atttypmod
! 		 * - VARHDRSZ) & 0xffff
  		 *
  		 *
  		 */
  		qlog("SQLColumns: table='%s',field_name='%s',type=%d,sqltype=%d,name='%s'\n",
  			 table_name, field_name, field_type, pgtype_to_sqltype, field_type_name);
--- 1738,1755 ----
  		set_tuplefield_string(&row->tuple[5], field_type_name);
  
  
! 		/*----------
  		 * Some Notes about Postgres Data Types:
  		 *
  		 * VARCHAR - the length is stored in the pg_attribute.atttypmod field
  		 * BPCHAR  - the length is also stored as varchar is
  		 *
! 		 * NUMERIC - the scale is stored in atttypmod as follows:
  		 *
+ 		 *	precision =((atttypmod - VARHDRSZ) >> 16) & 0xffff
+ 		 *	scale	 = (atttypmod - VARHDRSZ) & 0xffff
  		 *
+ 		 *----------
  		 */
  		qlog("SQLColumns: table='%s',field_name='%s',type=%d,sqltype=%d,name='%s'\n",
  			 table_name, field_name, field_type, pgtype_to_sqltype, field_type_name);
Index: src/interfaces/odbc/options.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/odbc/options.c,v
retrieving revision 1.25
diff -c -r1.25 options.c
*** src/interfaces/odbc/options.c	2001/03/22 04:01:34	1.25
--- src/interfaces/odbc/options.c	2001/03/23 04:37:50
***************
*** 81,96 ****
  				stmt->options.scroll_concurrency = vParam;
  			break;
  
! 			/*
! 			 * if (globals.lie) { if (conn)
! 			 * conn->stmtOptions.scroll_concurrency = vParam; if (stmt)
! 			 * stmt->options.scroll_concurrency = vParam; } else {
  			 *
! 			 * if (conn) conn->stmtOptions.scroll_concurrency =
! 			 * SQL_CONCUR_READ_ONLY; if (stmt)
! 			 * stmt->options.scroll_concurrency = SQL_CONCUR_READ_ONLY;
! 			 *
! 			 * if (vParam != SQL_CONCUR_READ_ONLY) changed = TRUE; } break;
  			 */
  
  		case SQL_CURSOR_TYPE:
--- 81,107 ----
  				stmt->options.scroll_concurrency = vParam;
  			break;
  
! 			/*----------
! 			 * if (globals.lie)
! 			 * {
! 			 *		if (conn)
! 			 * 			conn->stmtOptions.scroll_concurrency = vParam;
! 			 *		if (stmt)
! 			 * 			stmt->options.scroll_concurrency = vParam;
! 			 *		} else {
! 			 * 			if (conn)
! 			 *				conn->stmtOptions.scroll_concurrency =
! 			 * 					SQL_CONCUR_READ_ONLY;
! 			 *			if (stmt)
! 			 * 				stmt->options.scroll_concurrency =
! 			 *					SQL_CONCUR_READ_ONLY;
  			 *
! 			 * 			if (vParam != SQL_CONCUR_READ_ONLY)
! 			 *				changed = TRUE;
! 			 *		}
! 			 *		break;
! 			 *	}
! 			 *----------
  			 */
  
  		case SQL_CURSOR_TYPE:
Thu, 22 Mar 2001 23:48:47 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: [HACKERS] pgindent run?
> Bruce Momjian wrote:
> > 
> > You don't notice the value of pgindent until you have some code that
> > hasn't been run through it.  For example, ODBC was not run through until
> > this release, and I had a terrible time trying to understand the code
> > because it didn't _look_ like the rest of the code.  Now that pgindent
> > is run, it looks more normal, and I am sure that will encourage more
> > people to get in and make changes.
> > 
> 
> I see now the following comment in interfaces/odbc/statement.c.
> Though it's mine(probably), it's hard for me to read.
> Please tell me how to prevent pgindent from changing
> comments.
> 
>  /*
>   * Basically we don't have to begin a transaction in autocommit mode
>   * because Postgres backend runs in autocomit mode. We issue "BEGIN"
>   * in the following cases. 1) we use declare/fetch and the statement
>   * is SELECT (because declare/fetch must be called in a transaction).
>   * 2) we are in autocommit off state and the statement isn't of type
>   * OTHER.
>   */

Sorry that happened.  It is mentioned in the developer's FAQ that the
dashes prevent wrapping.  I know it is hard to remember even if you know
it, and I would be glad to fix any comments that look bad.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Thu, 22 Mar 2001 23:51:18 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: pgindent run?