 really hesitate to make this criticism given all the work people are
doing on the system and all the great features and fixes that are
being added but could we please make it a hard and fast rule that
nothing gets added into the tree until the person adding it has at
least compiled the source, if not a full regression test.  I have
finally jumped in and tried to add full primary key support but I
keep stumbling over simple syntax errors in files I am not working
on but that keep me from fully testing my own changes.  Here's what
I get compiling gram.y[c].

gcc -I../../include -I../../backend   -I/usr/local/include -O2 -pipe  -Wall -Wmissing-prototypes -I.. -Wno-error   -c gram.c -o gram.o
/usr/local/share/bison.simple: In function `yyparse':
/usr/local/share/bison.simple:327: warning: implicit declaration of function `yyerror'
/usr/local/share/bison.simple:387: warning: implicit declaration of function `yylex'
gram.y:2797: `forUpdate' undeclared (first use in this function)
gram.y:2797: (Each undeclared identifier is reported only once
gram.y:2797: for each function it appears in.)
gram.y:2800: syntax error before `->'
gram.y:2800: syntax error before `)'
gram.y:2804: `n' undeclared (first use in this function)
gram.y:2815: case label not within a switch statement
gram.y:2834: break statement not within loop or switch
gram.y:2835: case label not within a switch statement
gram.y:2838: break statement not within loop or switch
gram.y:2839: case label not within a switch statement
gram.y:2843: break statement not within loop or switch
...etc for many more lines.

I assume that line 2800 in gram.y is missing a paren and should have the
following change.

*** ../src.original/./backend/parser/gram.y Sun Jan 17 23:49:29 1999
--- ./backend/parser/gram.y Sun Jan 17 23:59:13 1999
***************
*** 2797,2803 ****
                  first_select>forUpdate = $3;
                  $$ = (Node *)first_select;
                }
!               if ((SelectStmt *)$$)->forUpdate != NULL)
                {
                    SelectStmt *n = (SelectStmt *)$1;
 
--- 2797,2803 ----
                  first_select>forUpdate = $3;
                  $$ = (Node *)first_select;
                }
!               if (((SelectStmt *)$$)->forUpdate != NULL)
                {
                    SelectStmt *n = (SelectStmt *)$1;

But I can't tell what should be done for the missing declaration for
forUpdate.  Can whoever added this please complete it.  It appears
4 times in the file.


Further, backend/libpq/pqcomprim.c is missing a header that causes the
compile to fail.

*** ../src.original/./backend/libpq/pqcomprim.c Sun Jan 17 23:43:29 1999
--- ./backend/libpq/pqcomprim.c Sun Jan 17 23:43:43 1999
***************
*** 1,5 ****
--- 1,6 ----        
  #include <stdlib.h>           
  #include <stdio.h>            
+ #include <errno.h>            
  #include <sys/types.h>
  #include <sys/socket.h>
  #include <netinet/in.h>

Sorry if I seem a little frustrated.  It's only because I am.  This isn't
the first time this has happened while I was trying to work on the code.
I don't think a full compile after extracting your diffs but before
sending them in is so much to ask, is it?  I'm not, after all, asking
for full regression testing.

Anyway, I have (I think) all the changes needed to put full primary key
support in.  As soon as I can test them I'll send them in.  Or maybe I
should send them in now without fully testing them.  (Wry joke.)

-- 
D'Arcy J.M. Cain <darcy@{druid|vex}.net>   |  Democracy is three wolves
http://www.druid.net/darcy/                |  and a sheep voting on
+1 416 424 2871     (DoD#0082)    (eNTP)   |  what's for dinner.

Mon, 18 Jan 1999 00:18:16 -0500 (EST)
"D'Arcy" "J.M." Cain <darcy@druid.net>
Syntax errors in current tree
"D'Arcy J.M. Cain" wrote:
> 
>  really hesitate to make this criticism given all the work people are
> doing on the system and all the great features and fixes that are
> being added but could we please make it a hard and fast rule that
> nothing gets added into the tree until the person adding it has at
> least compiled the source, if not a full regression test.  I have
  ^^^^^^^^^^^^^^^^^^^^^^^^^
This is what I always do!
forUpdate was added by me into gram.y and parsenodes.h:SelectStmt.
I guess that I committed both backend & include dirs
(cvs update inside include dir doesn't show anything for
parsenodes.h)...

> finally jumped in and tried to add full primary key support but I
> keep stumbling over simple syntax errors in files I am not working
> on but that keep me from fully testing my own changes.  Here's what
> I get compiling gram.y[c].
> 
> gcc -I../../include -I../../backend   -I/usr/local/include -O2 -pipe  -Wall -Wmissing-prototypes -I.. -Wno-error   -c gram.c -o gram.o
> /usr/local/share/bison.simple: In function `yyparse':
> /usr/local/share/bison.simple:327: warning: implicit declaration of function `yyerror'
> /usr/local/share/bison.simple:387: warning: implicit declaration of function `yylex'
> gram.y:2797: `forUpdate' undeclared (first use in this function)

Vadim

Mon, 18 Jan 1999 12:41:11 +0700
Vadim Mikheev <vadim@krs.ru>
Re: [HACKERS] Syntax errors in current tree
> "D'Arcy J.M. Cain" wrote:
> > 
> >  really hesitate to make this criticism given all the work people are
> > doing on the system and all the great features and fixes that are
> > being added but could we please make it a hard and fast rule that
> > nothing gets added into the tree until the person adding it has at
> > least compiled the source, if not a full regression test.  I have
>   ^^^^^^^^^^^^^^^^^^^^^^^^^
> This is what I always do!
> forUpdate was added by me into gram.y and parsenodes.h:SelectStmt.
> I guess that I committed both backend & include dirs
> (cvs update inside include dir doesn't show anything for
> parsenodes.h)...

No, it is me.  I merged the Stephan's patches into gram.y because he did
not have ForUpdate support in his patch.  Working on it now.

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@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

Mon, 18 Jan 1999 01:20:44 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] Syntax errors in current tree
>  really hesitate to make this criticism given all the work people are
> doing on the system and all the great features and fixes that are
> being added but could we please make it a hard and fast rule that
> nothing gets added into the tree until the person adding it has at
> least compiled the source, if not a full regression test.  I have
> finally jumped in and tried to add full primary key support but I
> keep stumbling over simple syntax errors in files I am not working
> on but that keep me from fully testing my own changes.  Here's what
> I get compiling gram.y[c].

Fixed.  I assume people test the patches before submission, so I don't
usually compile after each one.  I did add some code in gram.y, and that
was what caused the problem.

Sorry.  Blech (Sound of me falling on my sword.  :-))

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@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

Mon, 18 Jan 1999 01:48:07 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] Syntax errors in current tree
Thus spake Bruce Momjian
> Fixed.  I assume people test the patches before submission, so I don't

Not an unreasonable assumption.

> usually compile after each one.  I did add some code in gram.y, and that
> was what caused the problem.

I don't think that the core maintainers should have to compile each
and every patch before committing it.  However, perhaps a form letter
can go out to each new submitter asking if their patch was tested and
holding their first patch till they respond affirmatively.  Once they
have a history, even of one patch, then accept their submissions as
long as they appear good.

Maybe we need a way to track this.  Anyone know of a good system for
tracking this sort of BASic DATA.  :-)

> Sorry.  Blech (Sound of me falling on my sword.  :-))

That's OK.  I just hope it was lying flat on the ground at the time.  :-)

-- 
D'Arcy J.M. Cain <darcy@{druid|vex}.net>   |  Democracy is three wolves
http://www.druid.net/darcy/                |  and a sheep voting on
+1 416 424 2871     (DoD#0082)    (eNTP)   |  what's for dinner.

Mon, 18 Jan 1999 07:59:48 -0500 (EST)
"D'Arcy" "J.M." Cain <darcy@druid.net>
Re: [HACKERS] Syntax errors in current tree
> Thus spake Bruce Momjian
> > Fixed.  I assume people test the patches before submission, so I don't
> 
> Not an unreasonable assumption.
> 
> > usually compile after each one.  I did add some code in gram.y, and that
> > was what caused the problem.
> 
> I don't think that the core maintainers should have to compile each
> and every patch before committing it.  However, perhaps a form letter
> can go out to each new submitter asking if their patch was tested and
> holding their first patch till they respond affirmatively.  Once they
> have a history, even of one patch, then accept their submissions as
> long as they appear good.
> 
> Maybe we need a way to track this.  Anyone know of a good system for
> tracking this sort of BASic DATA.  :-)

Not really a problem.  Most patch problems are either my mucking with it
to merge it with other changes, or platform-specific problems that the
tester would never have see.  For example, Win32 used EINTR without
including errno.h.


-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@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

Mon, 18 Jan 1999 08:26:40 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] Syntax errors in current tree
Bruce Momjian <maillist@candle.pha.pa.us> writes:
> Not really a problem.  Most patch problems are either my mucking with it
> to merge it with other changes, or platform-specific problems that the
> tester would never have see.  For example, Win32 used EINTR without
> including errno.h.

And, by the same token, whoever does commit a patch might fail to notice
a platform-specific problem that didn't happen to show up on his
platform.  You can't really expect things to be 100% tested before they
hit the CVS tree (that's why we have a release cycle).

My experience so far is that the Postgres CVS sources are pretty stable;
most of the time they work, which can't be said for some other projects
I'm involved in :-(.  Not that we shouldn't strive to do even better,
but an occasional breakage is going to happen.

			regards, tom lane

Mon, 18 Jan 1999 11:07:14 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: [HACKERS] Syntax errors in current tree 
> Bruce Momjian <maillist@candle.pha.pa.us> writes:
> > Not really a problem.  Most patch problems are either my mucking with it
> > to merge it with other changes, or platform-specific problems that the
> > tester would never have see.  For example, Win32 used EINTR without
> > including errno.h.
> 
> And, by the same token, whoever does commit a patch might fail to notice
> a platform-specific problem that didn't happen to show up on his
> platform.  You can't really expect things to be 100% tested before they
> hit the CVS tree (that's why we have a release cycle).
> 
> My experience so far is that the Postgres CVS sources are pretty stable;
> most of the time they work, which can't be said for some other projects
> I'm involved in :-(.  Not that we shouldn't strive to do even better,
> but an occasional breakage is going to happen.

Problem is that I rarely have a solid block of time to apply the
patches.  (I used to.)  It is done in pieces as I can get time during
the day.  This makes for mistakes.

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@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

Mon, 18 Jan 1999 11:36:48 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] Syntax errors in current tree