I've noticed general buggyness with ecpg on one of my source files for
a while now but it only got really annoying after setting up overnight
build on Linux (output corrupt code), Solaris (output correct code),
AIX (crashed) and HPUX (crashed).

After comparing the output from ecpg on Linux and Solaris the
following type of statement was the root of the crash:

 EXEC SQL GRANT ALL ON exampletable TO PUBLIC;

When the parser code was rebuilding the query to pass onto the server
it was trying to include an extra, non-existent, parameter...

The bug is present in 7.1.2, 7.1.3 and the current CVS sources. The
following patch (against CVS version) corrects this bug:

./interfaces/ecpg/preproc/preproc.y
*** ./interfaces/ecpg/preproc/preproc.y.orig	Fri Oct 12 16:22:05 2001
--- ./interfaces/ecpg/preproc/preproc.y	Fri Oct 12 16:22:09 2001
***************
*** 1693,1699 ****
  
  GrantStmt:  GRANT privileges ON opt_table relation_name_list TO grantee_list opt_with_grant
  				{
! 					$$ = cat_str(8, make_str("grant"), $2, make_str("on"), $4, $5, make_str("to"), $7);
  				}
  		;
  
--- 1693,1699 ----
  
  GrantStmt:  GRANT privileges ON opt_table relation_name_list TO grantee_list opt_with_grant
  				{
! 					$$ = cat_str(7, make_str("grant"), $2, make_str("on"), $4, $5, make_str("to"), $7);
  				}
  		;

Fri, 12 Oct 2001 16:33:00 +0100 (BST)
Lee Kindness <lkindness@csl.co.uk>
ecpg - GRANT bug
Lee Kindness <lkindness@csl.co.uk> writes:
> ***************
> *** 1693,1699 ****
  
>   GrantStmt:  GRANT privileges ON opt_table relation_name_list TO grantee_list opt_with_grant
>   				{
> ! 					$$ = cat_str(8, make_str("grant"), $2, make_str("on"), $4, $5, make_str("to"), $7);
>   				}
>   		;
  
> --- 1693,1699 ----
  
>   GrantStmt:  GRANT privileges ON opt_table relation_name_list TO grantee_list opt_with_grant
>   				{
> ! 					$$ = cat_str(7, make_str("grant"), $2, make_str("on"), $4, $5, make_str("to"), $7);
>   				}
>   		;


Uh, isn't the correct fix

! 					$$ = cat_str(8, make_str("grant"), $2, make_str("on"), $4, $5, make_str("to"), $7, $8);

ISTM your patch loses the opt_with_grant clause.  (Of course the backend
doesn't currently accept that clause anyway, but that's no reason for
ecpg to drop it.)

			regards, tom lane

Fri, 12 Oct 2001 13:18:49 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: ecpg - GRANT bug 

Your patch has been added to the PostgreSQL unapplied patches list at:

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

I will try to apply it within the next 48 hours.

---------------------------------------------------------------------------


> I've noticed general buggyness with ecpg on one of my source files for
> a while now but it only got really annoying after setting up overnight
> build on Linux (output corrupt code), Solaris (output correct code),
> AIX (crashed) and HPUX (crashed).
> 
> After comparing the output from ecpg on Linux and Solaris the
> following type of statement was the root of the crash:
> 
>  EXEC SQL GRANT ALL ON exampletable TO PUBLIC;
> 
> When the parser code was rebuilding the query to pass onto the server
> it was trying to include an extra, non-existent, parameter...
> 
> The bug is present in 7.1.2, 7.1.3 and the current CVS sources. The
> following patch (against CVS version) corrects this bug:
> 
> ./interfaces/ecpg/preproc/preproc.y
> *** ./interfaces/ecpg/preproc/preproc.y.orig	Fri Oct 12 16:22:05 2001
> --- ./interfaces/ecpg/preproc/preproc.y	Fri Oct 12 16:22:09 2001
> ***************
> *** 1693,1699 ****
>   
>   GrantStmt:  GRANT privileges ON opt_table relation_name_list TO grantee_list opt_with_grant
>   				{
> ! 					$$ = cat_str(8, make_str("grant"), $2, make_str("on"), $4, $5, make_str("to"), $7);
>   				}
>   		;
>   
> --- 1693,1699 ----
>   
>   GrantStmt:  GRANT privileges ON opt_table relation_name_list TO grantee_list opt_with_grant
>   				{
> ! 					$$ = cat_str(7, make_str("grant"), $2, make_str("on"), $4, $5, make_str("to"), $7);
>   				}
>   		;
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
>     (send "unregister YourEmailAddressHere" to majordomo@postgresql.org)
> 

-- 
  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

Sat, 13 Oct 2001 00:14:46 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: ecpg - GRANT bug
Tom Lane writes:
 > Uh, isn't the correct fix
 > ! $$ = cat_str(8, make_str("grant"), $2, make_str("on"), $4, $5,
 > make_str("to"), $7, $8);
 > ISTM your patch loses the opt_with_grant clause.  (Of course the
 > backend doesn't currently accept that clause anyway, but that's no
 > reason for ecpg to drop it.)

My patch doesn't loose the option, it's never been passed on anyway:

 opt_with_grant:  WITH GRANT OPTION
				{
					mmerror(ET_ERROR, "WITH GRANT OPTION is not supported.  Only relation owners can set privileges");
				 }
		| /*EMPTY*/ 
		;

The existing code in ecpg/preproc/preproc.y to handle the WITH option
simply throws an error and aborts the processing... The patch below
prevents the segfault and also passes on the WITH option to the
backend, probably a better fix.

Regards, Lee.

Index: interfaces/ecpg/preproc/preproc.y
===================================================================
RCS file: /projects/cvsroot/pgsql/src/interfaces/ecpg/preproc/preproc.y,v
retrieving revision 1.159
diff -c -r1.159 preproc.y
*** interfaces/ecpg/preproc/preproc.y	2001/10/14 12:07:57	1.159
--- interfaces/ecpg/preproc/preproc.y	2001/10/15 09:06:29
***************
*** 1693,1699 ****
  
  GrantStmt:  GRANT privileges ON opt_table relation_name_list TO grantee_list opt_with_grant
  				{
! 					$$ = cat_str(7, make_str("grant"), $2, make_str("on"), $4, $5, make_str("to"), $7);
  				}
  		;
  
--- 1693,1699 ----
  
  GrantStmt:  GRANT privileges ON opt_table relation_name_list TO grantee_list opt_with_grant
  				{
! 					$$ = cat_str(8, make_str("grant"), $2, make_str("on"), $4, $5, make_str("to"), $7, $8);
  				}
  		;
  
***************
*** 1769,1779 ****
  		| grantee_list ',' grantee 	{ $$ = cat_str(3, $1, make_str(","), $3); }
  		;
  
! opt_with_grant:  WITH GRANT OPTION
! 				{
! 					mmerror(ET_ERROR, "WITH GRANT OPTION is not supported.  Only relation owners can set privileges");
! 				 }
! 		| /*EMPTY*/ 
  		;
  
  
--- 1769,1776 ----
  		| grantee_list ',' grantee 	{ $$ = cat_str(3, $1, make_str(","), $3); }
  		;
  
! opt_with_grant:  WITH GRANT OPTION { $$ = make_str("with grant option"); }
! 		| /*EMPTY*/ { $$ = EMPTY; }
  		;
  
  



Mon, 15 Oct 2001 10:09:03 +0100 (BST)
Lee Kindness <lkindness@csl.co.uk>
ecpg - GRANT bug 
On Tue, 16 Oct 2001, Lee Kindness wrote:

> And the patch below corrects a pet peeve I have with ecpg, all errors
> and warnings are output with a line number one less than reality...

I think this patch is wrong. Wouldn't it be better to make the line number
in yylineno be correct? Also, there are users of the line number in pcg.l
which you didn't change.

Looking at it, I don't see why the line number is off. It is initialized
to 1 at the begining and whenever a new file is included. In the generated
code, it is incrimented whenever a '\n' is found. Strange...

Take care,

Bill

> Lee.
>
> *** ./interfaces/ecpg/preproc/preproc.y.orig	Tue Oct 16 10:19:27 2001
> --- ./interfaces/ecpg/preproc/preproc.y	Tue Oct 16 10:19:49 2001
> ***************
> *** 36,49 ****
>       switch(type)
>       {
>   	case ET_NOTICE:
> ! 		fprintf(stderr, "%s:%d: WARNING: %s\n", input_filename, yylineno, error);
>   		break;
>   	case ET_ERROR:
> ! 		fprintf(stderr, "%s:%d: ERROR: %s\n", input_filename, yylineno, error);
>   		ret_value = PARSE_ERROR;
>   		break;
>   	case ET_FATAL:
> ! 		fprintf(stderr, "%s:%d: ERROR: %s\n", input_filename, yylineno, error);
>   		exit(PARSE_ERROR);
>       }
>   }
> --- 36,52 ----
>       switch(type)
>       {
>   	case ET_NOTICE:
> ! 		fprintf(stderr, "%s:%d: WARNING: %s\n", input_filename,
> ! 			yylineno + 1, error);
>   		break;
>   	case ET_ERROR:
> ! 		fprintf(stderr, "%s:%d: ERROR: %s\n", input_filename,
> ! 			yylineno + 1, error);
>   		ret_value = PARSE_ERROR;
>   		break;
>   	case ET_FATAL:
> ! 		fprintf(stderr, "%s:%d: ERROR: %s\n", input_filename,
> ! 			yylineno + 1, error);
>   		exit(PARSE_ERROR);
>       }
>   }
>
> ---------------------------(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
>


Mon, 15 Oct 2001 05:15:13 -0700 (PDT)
Bill Studenmund <wrstuden@netbsd.org>
Re: ecpg - GRANT bug
Lee Kindness <lkindness@csl.co.uk> writes:
> The existing code in ecpg/preproc/preproc.y to handle the WITH option
> simply throws an error and aborts the processing... The patch below
> prevents the segfault and also passes on the WITH option to the
> backend, probably a better fix.

I agree.  It shouldn't be ecpg's business to throw errors on behalf of
the backend, especially not "not yet implemented" kinds of errors.
That just causes ecpg to be more tightly coupled to a particular backend
version than it needs to be.

			regards, tom lane

Mon, 15 Oct 2001 10:10:40 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: ecpg - GRANT bug 
Tom Lane writes:
 > Lee Kindness <lkindness@csl.co.uk> writes:
 > > The existing code in ecpg/preproc/preproc.y to handle the WITH option
 > > simply throws an error and aborts the processing... The patch below
 > > prevents the segfault and also passes on the WITH option to the
 > > backend, probably a better fix.
 > I agree.  It shouldn't be ecpg's business to throw errors on behalf of
 > the backend, especially not "not yet implemented" kinds of errors.
 > That just causes ecpg to be more tightly coupled to a particular backend
 > version than it needs to be.

In which case a number of other cases should be weeded out of
parser.y and passed onto the backend:

 CREATE TABLE:    GLOBAL TEMPORARY option.
 CREATE FUNCTION: IN/OUT/INOUT options (note there's a bug in parser.y
                  there anyway, it would pass on 'oinut' for INOUT).
 COMMIT:          AND [NO] CHAIN options? Where do these come from,
                  it's not ANSI (i'd probably leave this one).

Perhaps an ET_NOTICE should still be output however...

Let me known if you want a patch for these cases too.

Regards, Lee Kindness.
 

Mon, 15 Oct 2001 15:32:27 +0100 (BST)
Lee Kindness <lkindness@csl.co.uk>
Re: ecpg - GRANT bug 

Patch applied.  Thanks.

---------------------------------------------------------------------------


> Tom Lane writes:
>  > Uh, isn't the correct fix
>  > ! $$ = cat_str(8, make_str("grant"), $2, make_str("on"), $4, $5,
>  > make_str("to"), $7, $8);
>  > ISTM your patch loses the opt_with_grant clause.  (Of course the
>  > backend doesn't currently accept that clause anyway, but that's no
>  > reason for ecpg to drop it.)
> 
> My patch doesn't loose the option, it's never been passed on anyway:
> 
>  opt_with_grant:  WITH GRANT OPTION
> 				{
> 					mmerror(ET_ERROR, "WITH GRANT OPTION is not supported.  Only relation owners can set privileges");
> 				 }
> 		| /*EMPTY*/ 
> 		;
> 
> The existing code in ecpg/preproc/preproc.y to handle the WITH option
> simply throws an error and aborts the processing... The patch below
> prevents the segfault and also passes on the WITH option to the
> backend, probably a better fix.
> 
> Regards, Lee.
> 
> Index: interfaces/ecpg/preproc/preproc.y
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/interfaces/ecpg/preproc/preproc.y,v
> retrieving revision 1.159
> diff -c -r1.159 preproc.y
> *** interfaces/ecpg/preproc/preproc.y	2001/10/14 12:07:57	1.159
> --- interfaces/ecpg/preproc/preproc.y	2001/10/15 09:06:29
> ***************
> *** 1693,1699 ****
>   
>   GrantStmt:  GRANT privileges ON opt_table relation_name_list TO grantee_list opt_with_grant
>   				{
> ! 					$$ = cat_str(7, make_str("grant"), $2, make_str("on"), $4, $5, make_str("to"), $7);
>   				}
>   		;
>   
> --- 1693,1699 ----
>   
>   GrantStmt:  GRANT privileges ON opt_table relation_name_list TO grantee_list opt_with_grant
>   				{
> ! 					$$ = cat_str(8, make_str("grant"), $2, make_str("on"), $4, $5, make_str("to"), $7, $8);
>   				}
>   		;
>   
> ***************
> *** 1769,1779 ****
>   		| grantee_list ',' grantee 	{ $$ = cat_str(3, $1, make_str(","), $3); }
>   		;
>   
> ! opt_with_grant:  WITH GRANT OPTION
> ! 				{
> ! 					mmerror(ET_ERROR, "WITH GRANT OPTION is not supported.  Only relation owners can set privileges");
> ! 				 }
> ! 		| /*EMPTY*/ 
>   		;
>   
>   
> --- 1769,1776 ----
>   		| grantee_list ',' grantee 	{ $$ = cat_str(3, $1, make_str(","), $3); }
>   		;
>   
> ! opt_with_grant:  WITH GRANT OPTION { $$ = make_str("with grant option"); }
> ! 		| /*EMPTY*/ { $$ = EMPTY; }
>   		;
>   
>   
> 
> 
> 

-- 
  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

Mon, 15 Oct 2001 14:16:23 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: ecpg - GRANT bug
> In which case a number of other cases should be weeded out of
> parser.y and passed onto the backend:
> 
>  CREATE TABLE:    GLOBAL TEMPORARY option.
>  CREATE FUNCTION: IN/OUT/INOUT options (note there's a bug in parser.y
>                   there anyway, it would pass on 'oinut' for INOUT).
>  COMMIT:          AND [NO] CHAIN options? Where do these come from,
>                   it's not ANSI (i'd probably leave this one).
> 
> Perhaps an ET_NOTICE should still be output however...
> 
> Let me known if you want a patch for these cases too.

Sure, send them on 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

Mon, 15 Oct 2001 14:16:38 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: ecpg - GRANT bug
Bruce Momjian writes:
 > Lee Kindness writes:
 > > In which case a number of other cases should be weeded out of
 > > parser.y and passed onto the backend:
 > > [ snip ]
 > > Let me known if you want a patch for these cases too.
 > Sure, send them on over.

Patch below, it changes:

 1. A number of mmerror(ET_ERROR) to mmerror(ET_NOTICE), passing on
    the (currently) unsupported options to the backend with warning.

 2. Standardises warning messages in such cases.

 3. Corrects typo in passing of 'CREATE FUNCTION/INOUT' parameter.

Patch:

? interfaces/ecpg/preproc/ecpg
Index: interfaces/ecpg/preproc/preproc.y
===================================================================
RCS file: /projects/cvsroot/pgsql/src/interfaces/ecpg/preproc/preproc.y,v
retrieving revision 1.161
diff -c -r1.161 preproc.y
*** interfaces/ecpg/preproc/preproc.y	2001/10/15 20:15:09	1.161
--- interfaces/ecpg/preproc/preproc.y	2001/10/16 09:15:53
***************
*** 1074,1084 ****
  		| LOCAL TEMPORARY	{ $$ = make_str("local temporary"); }
  		| LOCAL TEMP		{ $$ = make_str("local temp"); }
  		| GLOBAL TEMPORARY	{
! 					  mmerror(ET_ERROR, "GLOBAL TEMPORARY TABLE is not currently supported");
  					  $$ = make_str("global temporary");
  					}
  		| GLOBAL TEMP		{
! 					  mmerror(ET_ERROR, "GLOBAL TEMPORARY TABLE is not currently supported");
  					  $$ = make_str("global temp");
  					}
  		| /*EMPTY*/		{ $$ = EMPTY; }
--- 1074,1084 ----
  		| LOCAL TEMPORARY	{ $$ = make_str("local temporary"); }
  		| LOCAL TEMP		{ $$ = make_str("local temp"); }
  		| GLOBAL TEMPORARY	{
! 					  mmerror(ET_NOTICE, "Currently unsupported CREATE TABLE/GLOBAL TEMPORARY will be passed to backend");
  					  $$ = make_str("global temporary");
  					}
  		| GLOBAL TEMP		{
! 					  mmerror(ET_NOTICE, "Currently unsupported CREATE TABLE/GLOBAL TEMP will be passed to backend");
  					  $$ = make_str("global temp");
  					}
  		| /*EMPTY*/		{ $$ = EMPTY; }
***************
*** 1103,1110 ****
  				{
  					if (strlen($4) > 0)
  					{
! 						sprintf(errortext, "CREATE TABLE/COLLATE %s not yet implemented; clause ignored", $4);
! 						mmerror(ET_NOTICE, errortext);
  					}
  					$$ = cat_str(4, $1, $2, $3, $4);
  				}
--- 1103,1110 ----
  				{
  					if (strlen($4) > 0)
  					{
!  						sprintf(errortext, "Currently unsupported CREATE TABLE/COLLATE %s will be passed to backend", $4);
!  						mmerror(ET_NOTICE, errortext);
  					}
  					$$ = cat_str(4, $1, $2, $3, $4);
  				}
***************
*** 1219,1225 ****
  		}
  		| MATCH PARTIAL		
  		{
! 			mmerror(ET_NOTICE, "FOREIGN KEY/MATCH PARTIAL not yet implemented");
  			$$ = make_str("match partial");
  		}
  		| /*EMPTY*/
--- 1219,1225 ----
  		}
  		| MATCH PARTIAL		
  		{
! 			mmerror(ET_NOTICE, "Currently unsupported FOREIGN KEY/MATCH PARTIAL will be passed to backend");
  			$$ = make_str("match partial");
  		}
  		| /*EMPTY*/
***************
*** 1614,1620 ****
  		| BACKWARD	{ $$ = make_str("backward"); }
  		| RELATIVE      { $$ = make_str("relative"); }
                  | ABSOLUTE	{
! 					mmerror(ET_NOTICE, "FETCH/ABSOLUTE not supported, backend will use RELATIVE");
  					$$ = make_str("absolute");
  				}
  		;
--- 1614,1620 ----
  		| BACKWARD	{ $$ = make_str("backward"); }
  		| RELATIVE      { $$ = make_str("relative"); }
                  | ABSOLUTE	{
! 					mmerror(ET_NOTICE, "Currently unsupported FETCH/ABSOLUTE will be passed to backend, backend will use RELATIVE");
  					$$ = make_str("absolute");
  				}
  		;
***************
*** 1769,1775 ****
  		| grantee_list ',' grantee 	{ $$ = cat_str(3, $1, make_str(","), $3); }
  		;
  
! opt_with_grant:  WITH GRANT OPTION { $$ = make_str("with grant option"); }
  		| /*EMPTY*/ { $$ = EMPTY; }
  		;
  
--- 1769,1779 ----
  		| grantee_list ',' grantee 	{ $$ = cat_str(3, $1, make_str(","), $3); }
  		;
  
! opt_with_grant:  WITH GRANT OPTION
!                                 {
! 					mmerror(ET_NOTICE, "Currently unsupported GRANT/WITH GRANT OPTION will be passed to backend");
! 					$$ = make_str("with grant option");
! 				}
  		| /*EMPTY*/ { $$ = EMPTY; }
  		;
  
***************
*** 1919,1932 ****
  
  opt_arg:  IN    { $$ = make_str("in"); }
  	| OUT	{ 
! 		  mmerror(ET_ERROR, "CREATE FUNCTION/OUT parameters are not supported");
  
  	 	  $$ = make_str("out");
  		}
  	| INOUT	{ 
! 		  mmerror(ET_ERROR, "CREATE FUNCTION/INOUT parameters are not supported");
  
! 	 	  $$ = make_str("oinut");
  		}
  	;
  
--- 1923,1936 ----
  
  opt_arg:  IN    { $$ = make_str("in"); }
  	| OUT	{ 
! 		  mmerror(ET_NOTICE, "Currently unsupported CREATE FUNCTION/OUT will be passed to backend");
  
  	 	  $$ = make_str("out");
  		}
  	| INOUT	{ 
! 		  mmerror(ET_NOTICE, "Currently unsupported CREATE FUNCTION/INOUT will be passed to backend");
  
! 	 	  $$ = make_str("inout");
  		}
  	;
  
***************
*** 2164,2170 ****
  
  opt_chain: AND NO CHAIN 	{ $$ = make_str("and no chain"); }
  	| AND CHAIN		{
! 				  mmerror(ET_ERROR, "COMMIT/CHAIN not yet supported");
  
  				  $$ = make_str("and chain");
  				}
--- 2168,2174 ----
  
  opt_chain: AND NO CHAIN 	{ $$ = make_str("and no chain"); }
  	| AND CHAIN		{
! 				  mmerror(ET_NOTICE, "Currently unsupported COMMIT/CHAIN will be passed to backend");
  
  				  $$ = make_str("and chain");
  				}
***************
*** 2609,2620 ****
  			}
                         | GLOBAL TEMPORARY opt_table relation_name
                          {
! 				mmerror(ET_ERROR, "GLOBAL TEMPORARY TABLE is not currently supported");
  				$$ = cat_str(3, make_str("global temporary"), $3, $4);
                          }
                         | GLOBAL TEMP opt_table relation_name
                          {
! 				mmerror(ET_ERROR, "GLOBAL TEMPORARY TABLE is not currently supported");
  				$$ = cat_str(3, make_str("global temp"), $3, $4);
                          }
                         | TABLE relation_name
--- 2613,2624 ----
  			}
                         | GLOBAL TEMPORARY opt_table relation_name
                          {
! 				mmerror(ET_NOTICE, "Currently unsupported CREATE TABLE/GLOBAL TEMPORARY will be passed to backend");
  				$$ = cat_str(3, make_str("global temporary"), $3, $4);
                          }
                         | GLOBAL TEMP opt_table relation_name
                          {
! 				mmerror(ET_NOTICE, "Currently unsupported CREATE TABLE/GLOBAL TEMP will be passed to backend");
  				$$ = cat_str(3, make_str("global temp"), $3, $4);
                          }
                         | TABLE relation_name

Tue, 16 Oct 2001 10:16:38 +0100 (BST)
Lee Kindness <lkindness@csl.co.uk>
Re: ecpg - GRANT bug
Lee Kindness writes:
 > Patch below, it changes:
 >  1. A number of mmerror(ET_ERROR) to mmerror(ET_NOTICE), passing on
 >     the (currently) unsupported options to the backend with warning.
 >  2. Standardises warning messages in such cases.
 >  3. Corrects typo in passing of 'CREATE FUNCTION/INOUT' parameter.

And the patch below corrects a pet peeve I have with ecpg, all errors
and warnings are output with a line number one less than reality...

Lee.

*** ./interfaces/ecpg/preproc/preproc.y.orig	Tue Oct 16 10:19:27 2001
--- ./interfaces/ecpg/preproc/preproc.y	Tue Oct 16 10:19:49 2001
***************
*** 36,49 ****
      switch(type)
      {
  	case ET_NOTICE: 
! 		fprintf(stderr, "%s:%d: WARNING: %s\n", input_filename, yylineno, error); 
  		break;
  	case ET_ERROR:
! 		fprintf(stderr, "%s:%d: ERROR: %s\n", input_filename, yylineno, error);
  		ret_value = PARSE_ERROR;
  		break;
  	case ET_FATAL:
! 		fprintf(stderr, "%s:%d: ERROR: %s\n", input_filename, yylineno, error);
  		exit(PARSE_ERROR);
      }
  }
--- 36,52 ----
      switch(type)
      {
  	case ET_NOTICE: 
! 		fprintf(stderr, "%s:%d: WARNING: %s\n", input_filename,
! 			yylineno + 1, error); 
  		break;
  	case ET_ERROR:
! 		fprintf(stderr, "%s:%d: ERROR: %s\n", input_filename,
! 			yylineno + 1, error);
  		ret_value = PARSE_ERROR;
  		break;
  	case ET_FATAL:
! 		fprintf(stderr, "%s:%d: ERROR: %s\n", input_filename,
! 			yylineno + 1, error);
  		exit(PARSE_ERROR);
      }
  }

Tue, 16 Oct 2001 10:27:42 +0100 (BST)
Lee Kindness <lkindness@csl.co.uk>
Re: ecpg - GRANT bug

Your patch has been added to the PostgreSQL unapplied patches list at:

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

I will try to apply it within the next 48 hours.

---------------------------------------------------------------------------


> Tom Lane writes:
>  > Uh, isn't the correct fix
>  > ! $$ = cat_str(8, make_str("grant"), $2, make_str("on"), $4, $5,
>  > make_str("to"), $7, $8);
>  > ISTM your patch loses the opt_with_grant clause.  (Of course the
>  > backend doesn't currently accept that clause anyway, but that's no
>  > reason for ecpg to drop it.)
> 
> My patch doesn't loose the option, it's never been passed on anyway:
> 
>  opt_with_grant:  WITH GRANT OPTION
> 				{
> 					mmerror(ET_ERROR, "WITH GRANT OPTION is not supported.  Only relation owners can set privileges");
> 				 }
> 		| /*EMPTY*/ 
> 		;
> 
> The existing code in ecpg/preproc/preproc.y to handle the WITH option
> simply throws an error and aborts the processing... The patch below
> prevents the segfault and also passes on the WITH option to the
> backend, probably a better fix.
> 
> Regards, Lee.
> 
> Index: interfaces/ecpg/preproc/preproc.y
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/interfaces/ecpg/preproc/preproc.y,v
> retrieving revision 1.159
> diff -c -r1.159 preproc.y
> *** interfaces/ecpg/preproc/preproc.y	2001/10/14 12:07:57	1.159
> --- interfaces/ecpg/preproc/preproc.y	2001/10/15 09:06:29
> ***************
> *** 1693,1699 ****
>   
>   GrantStmt:  GRANT privileges ON opt_table relation_name_list TO grantee_list opt_with_grant
>   				{
> ! 					$$ = cat_str(7, make_str("grant"), $2, make_str("on"), $4, $5, make_str("to"), $7);
>   				}
>   		;
>   
> --- 1693,1699 ----
>   
>   GrantStmt:  GRANT privileges ON opt_table relation_name_list TO grantee_list opt_with_grant
>   				{
> ! 					$$ = cat_str(8, make_str("grant"), $2, make_str("on"), $4, $5, make_str("to"), $7, $8);
>   				}
>   		;
>   
> ***************
> *** 1769,1779 ****
>   		| grantee_list ',' grantee 	{ $$ = cat_str(3, $1, make_str(","), $3); }
>   		;
>   
> ! opt_with_grant:  WITH GRANT OPTION
> ! 				{
> ! 					mmerror(ET_ERROR, "WITH GRANT OPTION is not supported.  Only relation owners can set privileges");
> ! 				 }
> ! 		| /*EMPTY*/ 
>   		;
>   
>   
> --- 1769,1776 ----
>   		| grantee_list ',' grantee 	{ $$ = cat_str(3, $1, make_str(","), $3); }
>   		;
>   
> ! opt_with_grant:  WITH GRANT OPTION { $$ = make_str("with grant option"); }
> ! 		| /*EMPTY*/ { $$ = EMPTY; }
>   		;
>   
>   
> 
> 
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
> 
> http://www.postgresql.org/users-lounge/docs/faq.html
> 

-- 
  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, 16 Oct 2001 09:55:59 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: ecpg - GRANT bug

Your patch has been added to the PostgreSQL unapplied patches list at:

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

I will try to apply it within the next 48 hours.

---------------------------------------------------------------------------


> Bruce Momjian writes:
>  > Lee Kindness writes:
>  > > In which case a number of other cases should be weeded out of
>  > > parser.y and passed onto the backend:
>  > > [ snip ]
>  > > Let me known if you want a patch for these cases too.
>  > Sure, send them on over.
> 
> Patch below, it changes:
> 
>  1. A number of mmerror(ET_ERROR) to mmerror(ET_NOTICE), passing on
>     the (currently) unsupported options to the backend with warning.
> 
>  2. Standardises warning messages in such cases.
> 
>  3. Corrects typo in passing of 'CREATE FUNCTION/INOUT' parameter.
> 
> Patch:
> 
> ? interfaces/ecpg/preproc/ecpg
> Index: interfaces/ecpg/preproc/preproc.y
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/interfaces/ecpg/preproc/preproc.y,v
> retrieving revision 1.161
> diff -c -r1.161 preproc.y
> *** interfaces/ecpg/preproc/preproc.y	2001/10/15 20:15:09	1.161
> --- interfaces/ecpg/preproc/preproc.y	2001/10/16 09:15:53
> ***************
> *** 1074,1084 ****
>   		| LOCAL TEMPORARY	{ $$ = make_str("local temporary"); }
>   		| LOCAL TEMP		{ $$ = make_str("local temp"); }
>   		| GLOBAL TEMPORARY	{
> ! 					  mmerror(ET_ERROR, "GLOBAL TEMPORARY TABLE is not currently supported");
>   					  $$ = make_str("global temporary");
>   					}
>   		| GLOBAL TEMP		{
> ! 					  mmerror(ET_ERROR, "GLOBAL TEMPORARY TABLE is not currently supported");
>   					  $$ = make_str("global temp");
>   					}
>   		| /*EMPTY*/		{ $$ = EMPTY; }
> --- 1074,1084 ----
>   		| LOCAL TEMPORARY	{ $$ = make_str("local temporary"); }
>   		| LOCAL TEMP		{ $$ = make_str("local temp"); }
>   		| GLOBAL TEMPORARY	{
> ! 					  mmerror(ET_NOTICE, "Currently unsupported CREATE TABLE/GLOBAL TEMPORARY will be passed to backend");
>   					  $$ = make_str("global temporary");
>   					}
>   		| GLOBAL TEMP		{
> ! 					  mmerror(ET_NOTICE, "Currently unsupported CREATE TABLE/GLOBAL TEMP will be passed to backend");
>   					  $$ = make_str("global temp");
>   					}
>   		| /*EMPTY*/		{ $$ = EMPTY; }
> ***************
> *** 1103,1110 ****
>   				{
>   					if (strlen($4) > 0)
>   					{
> ! 						sprintf(errortext, "CREATE TABLE/COLLATE %s not yet implemented; clause ignored", $4);
> ! 						mmerror(ET_NOTICE, errortext);
>   					}
>   					$$ = cat_str(4, $1, $2, $3, $4);
>   				}
> --- 1103,1110 ----
>   				{
>   					if (strlen($4) > 0)
>   					{
> !  						sprintf(errortext, "Currently unsupported CREATE TABLE/COLLATE %s will be passed to backend", $4);
> !  						mmerror(ET_NOTICE, errortext);
>   					}
>   					$$ = cat_str(4, $1, $2, $3, $4);
>   				}
> ***************
> *** 1219,1225 ****
>   		}
>   		| MATCH PARTIAL		
>   		{
> ! 			mmerror(ET_NOTICE, "FOREIGN KEY/MATCH PARTIAL not yet implemented");
>   			$$ = make_str("match partial");
>   		}
>   		| /*EMPTY*/
> --- 1219,1225 ----
>   		}
>   		| MATCH PARTIAL		
>   		{
> ! 			mmerror(ET_NOTICE, "Currently unsupported FOREIGN KEY/MATCH PARTIAL will be passed to backend");
>   			$$ = make_str("match partial");
>   		}
>   		| /*EMPTY*/
> ***************
> *** 1614,1620 ****
>   		| BACKWARD	{ $$ = make_str("backward"); }
>   		| RELATIVE      { $$ = make_str("relative"); }
>                   | ABSOLUTE	{
> ! 					mmerror(ET_NOTICE, "FETCH/ABSOLUTE not supported, backend will use RELATIVE");
>   					$$ = make_str("absolute");
>   				}
>   		;
> --- 1614,1620 ----
>   		| BACKWARD	{ $$ = make_str("backward"); }
>   		| RELATIVE      { $$ = make_str("relative"); }
>                   | ABSOLUTE	{
> ! 					mmerror(ET_NOTICE, "Currently unsupported FETCH/ABSOLUTE will be passed to backend, backend will use RELATIVE");
>   					$$ = make_str("absolute");
>   				}
>   		;
> ***************
> *** 1769,1775 ****
>   		| grantee_list ',' grantee 	{ $$ = cat_str(3, $1, make_str(","), $3); }
>   		;
>   
> ! opt_with_grant:  WITH GRANT OPTION { $$ = make_str("with grant option"); }
>   		| /*EMPTY*/ { $$ = EMPTY; }
>   		;
>   
> --- 1769,1779 ----
>   		| grantee_list ',' grantee 	{ $$ = cat_str(3, $1, make_str(","), $3); }
>   		;
>   
> ! opt_with_grant:  WITH GRANT OPTION
> !                                 {
> ! 					mmerror(ET_NOTICE, "Currently unsupported GRANT/WITH GRANT OPTION will be passed to backend");
> ! 					$$ = make_str("with grant option");
> ! 				}
>   		| /*EMPTY*/ { $$ = EMPTY; }
>   		;
>   
> ***************
> *** 1919,1932 ****
>   
>   opt_arg:  IN    { $$ = make_str("in"); }
>   	| OUT	{ 
> ! 		  mmerror(ET_ERROR, "CREATE FUNCTION/OUT parameters are not supported");
>   
>   	 	  $$ = make_str("out");
>   		}
>   	| INOUT	{ 
> ! 		  mmerror(ET_ERROR, "CREATE FUNCTION/INOUT parameters are not supported");
>   
> ! 	 	  $$ = make_str("oinut");
>   		}
>   	;
>   
> --- 1923,1936 ----
>   
>   opt_arg:  IN    { $$ = make_str("in"); }
>   	| OUT	{ 
> ! 		  mmerror(ET_NOTICE, "Currently unsupported CREATE FUNCTION/OUT will be passed to backend");
>   
>   	 	  $$ = make_str("out");
>   		}
>   	| INOUT	{ 
> ! 		  mmerror(ET_NOTICE, "Currently unsupported CREATE FUNCTION/INOUT will be passed to backend");
>   
> ! 	 	  $$ = make_str("inout");
>   		}
>   	;
>   
> ***************
> *** 2164,2170 ****
>   
>   opt_chain: AND NO CHAIN 	{ $$ = make_str("and no chain"); }
>   	| AND CHAIN		{
> ! 				  mmerror(ET_ERROR, "COMMIT/CHAIN not yet supported");
>   
>   				  $$ = make_str("and chain");
>   				}
> --- 2168,2174 ----
>   
>   opt_chain: AND NO CHAIN 	{ $$ = make_str("and no chain"); }
>   	| AND CHAIN		{
> ! 				  mmerror(ET_NOTICE, "Currently unsupported COMMIT/CHAIN will be passed to backend");
>   
>   				  $$ = make_str("and chain");
>   				}
> ***************
> *** 2609,2620 ****
>   			}
>                          | GLOBAL TEMPORARY opt_table relation_name
>                           {
> ! 				mmerror(ET_ERROR, "GLOBAL TEMPORARY TABLE is not currently supported");
>   				$$ = cat_str(3, make_str("global temporary"), $3, $4);
>                           }
>                          | GLOBAL TEMP opt_table relation_name
>                           {
> ! 				mmerror(ET_ERROR, "GLOBAL TEMPORARY TABLE is not currently supported");
>   				$$ = cat_str(3, make_str("global temp"), $3, $4);
>                           }
>                          | TABLE relation_name
> --- 2613,2624 ----
>   			}
>                          | GLOBAL TEMPORARY opt_table relation_name
>                           {
> ! 				mmerror(ET_NOTICE, "Currently unsupported CREATE TABLE/GLOBAL TEMPORARY will be passed to backend");
>   				$$ = cat_str(3, make_str("global temporary"), $3, $4);
>                           }
>                          | GLOBAL TEMP opt_table relation_name
>                           {
> ! 				mmerror(ET_NOTICE, "Currently unsupported CREATE TABLE/GLOBAL TEMP will be passed to backend");
>   				$$ = cat_str(3, make_str("global temp"), $3, $4);
>                           }
>                          | TABLE relation_name
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
> 
> http://www.postgresql.org/users-lounge/docs/faq.html
> 

-- 
  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, 16 Oct 2001 09:56:31 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: ecpg - GRANT bug

Your patch has been added to the PostgreSQL unapplied patches list at:

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

I will try to apply it within the next 48 hours.

---------------------------------------------------------------------------


> Lee Kindness writes:
>  > Patch below, it changes:
>  >  1. A number of mmerror(ET_ERROR) to mmerror(ET_NOTICE), passing on
>  >     the (currently) unsupported options to the backend with warning.
>  >  2. Standardises warning messages in such cases.
>  >  3. Corrects typo in passing of 'CREATE FUNCTION/INOUT' parameter.
> 
> And the patch below corrects a pet peeve I have with ecpg, all errors
> and warnings are output with a line number one less than reality...
> 
> Lee.
> 
> *** ./interfaces/ecpg/preproc/preproc.y.orig	Tue Oct 16 10:19:27 2001
> --- ./interfaces/ecpg/preproc/preproc.y	Tue Oct 16 10:19:49 2001
> ***************
> *** 36,49 ****
>       switch(type)
>       {
>   	case ET_NOTICE: 
> ! 		fprintf(stderr, "%s:%d: WARNING: %s\n", input_filename, yylineno, error); 
>   		break;
>   	case ET_ERROR:
> ! 		fprintf(stderr, "%s:%d: ERROR: %s\n", input_filename, yylineno, error);
>   		ret_value = PARSE_ERROR;
>   		break;
>   	case ET_FATAL:
> ! 		fprintf(stderr, "%s:%d: ERROR: %s\n", input_filename, yylineno, error);
>   		exit(PARSE_ERROR);
>       }
>   }
> --- 36,52 ----
>       switch(type)
>       {
>   	case ET_NOTICE: 
> ! 		fprintf(stderr, "%s:%d: WARNING: %s\n", input_filename,
> ! 			yylineno + 1, error); 
>   		break;
>   	case ET_ERROR:
> ! 		fprintf(stderr, "%s:%d: ERROR: %s\n", input_filename,
> ! 			yylineno + 1, error);
>   		ret_value = PARSE_ERROR;
>   		break;
>   	case ET_FATAL:
> ! 		fprintf(stderr, "%s:%d: ERROR: %s\n", input_filename,
> ! 			yylineno + 1, error);
>   		exit(PARSE_ERROR);
>       }
>   }
> 
> ---------------------------(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
> 

-- 
  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, 16 Oct 2001 09:56:37 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: ecpg - GRANT bug
Lee Kindness <lkindness@csl.co.uk> writes:
> And the patch below corrects a pet peeve I have with ecpg, all errors
> and warnings are output with a line number one less than reality...

Hmm.  yylineno *should* be the right thing.  I think you are patching
a symptom rather than fixing the correct cause.  Perhaps look into the
way that the line number counter is initialized?

			regards, tom lane

Tue, 16 Oct 2001 11:38:48 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: ecpg - GRANT bug 

I agree we need to find out why the line number is off rather than
covering up the problem.  Patch rejected.

---------------------------------------------------------------------------

> Lee Kindness writes:
>  > Patch below, it changes:
>  >  1. A number of mmerror(ET_ERROR) to mmerror(ET_NOTICE), passing on
>  >     the (currently) unsupported options to the backend with warning.
>  >  2. Standardises warning messages in such cases.
>  >  3. Corrects typo in passing of 'CREATE FUNCTION/INOUT' parameter.
> 
> And the patch below corrects a pet peeve I have with ecpg, all errors
> and warnings are output with a line number one less than reality...
> 
> Lee.
> 
> *** ./interfaces/ecpg/preproc/preproc.y.orig	Tue Oct 16 10:19:27 2001
> --- ./interfaces/ecpg/preproc/preproc.y	Tue Oct 16 10:19:49 2001
> ***************
> *** 36,49 ****
>       switch(type)
>       {
>   	case ET_NOTICE: 
> ! 		fprintf(stderr, "%s:%d: WARNING: %s\n", input_filename, yylineno, error); 
>   		break;
>   	case ET_ERROR:
> ! 		fprintf(stderr, "%s:%d: ERROR: %s\n", input_filename, yylineno, error);
>   		ret_value = PARSE_ERROR;
>   		break;
>   	case ET_FATAL:
> ! 		fprintf(stderr, "%s:%d: ERROR: %s\n", input_filename, yylineno, error);
>   		exit(PARSE_ERROR);
>       }
>   }
> --- 36,52 ----
>       switch(type)
>       {
>   	case ET_NOTICE: 
> ! 		fprintf(stderr, "%s:%d: WARNING: %s\n", input_filename,
> ! 			yylineno + 1, error); 
>   		break;
>   	case ET_ERROR:
> ! 		fprintf(stderr, "%s:%d: ERROR: %s\n", input_filename,
> ! 			yylineno + 1, error);
>   		ret_value = PARSE_ERROR;
>   		break;
>   	case ET_FATAL:
> ! 		fprintf(stderr, "%s:%d: ERROR: %s\n", input_filename,
> ! 			yylineno + 1, error);
>   		exit(PARSE_ERROR);
>       }
>   }
> 
> ---------------------------(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
> 

-- 
  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, 16 Oct 2001 14:48:33 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: ecpg - GRANT bug
Lee Kindness writes:

>  COMMIT:          AND [NO] CHAIN options? Where do these come from,
>                   it's not ANSI (i'd probably leave this one).

Sure is.

-- 
Peter Eisentraut   peter_e@gmx.net   http://funkturm.homeip.net/~peter


Tue, 16 Oct 2001 22:13:43 +0200 (CEST)
Peter Eisentraut <peter_e@gmx.net>
Re: ecpg - GRANT bug 
Bill Studenmund writes:
 > I think this patch is wrong. Wouldn't it be better to make the line number
 > in yylineno be correct? Also, there are users of the line number in pcg.l
 > which you didn't change.
 > Looking at it, I don't see why the line number is off. It is initialized
 > to 1 at the begining and whenever a new file is included. In the generated
 > code, it is incrimented whenever a '\n' is found. Strange...

The main reason I split the patch from the previous one for ecpg was
for this reason - I didn't think it was the correct patch myself.

However after a serious hunt for the root of the problem I've found
that it is actually working correctly in the 7.2 sources and I was
picking up an ecpg from a 7.1.3ish build (which only contained an ecpg
binary). Appologies for the false hunt!

For the record it was fixed in pgc.l 1.79 (meskes 13-Jun-01).

Regards, Lee.

Wed, 17 Oct 2001 10:00:31 +0100 (BST)
Lee Kindness <lkindness@csl.co.uk>
Re: ecpg - GRANT bug
[Sorry, for the late replies, but I was on the road since Sunday.]

On Mon, Oct 15, 2001 at 10:10:40AM -0400, Tom Lane wrote:
> Lee Kindness <lkindness@csl.co.uk> writes:
> > The existing code in ecpg/preproc/preproc.y to handle the WITH option
> > simply throws an error and aborts the processing... The patch below
> > prevents the segfault and also passes on the WITH option to the
> > backend, probably a better fix.

Yes, that of course is better. Sorry, I simply didn't see this either.

> I agree.  It shouldn't be ecpg's business to throw errors on behalf of
> the backend, especially not "not yet implemented" kinds of errors.

I beg to disagree.

> That just causes ecpg to be more tightly coupled to a particular backend
> version than it needs to be.

Sure, but it also makes sure you get the error message at compile time
rather than at run time. If this is not how ecpg should work, there is no
need to use this complex parser at all. I could simply accept all sql
statements and let the backend decide which one it accepts. 

But for the user this is not a good idea IMO. I don't like running a program
to debug syntax.

Michael
-- 
Michael Meskes
Michael@Fam-Meskes.De
Go SF 49ers! Go Rhein Fire!
Use Debian GNU/Linux! Use PostgreSQL!

Thu, 18 Oct 2001 12:54:41 +0200
Michael Meskes <meskes@postgresql.org>
Re: ecpg - GRANT bug
On Tue, Oct 16, 2001 at 10:16:38AM +0100, Lee Kindness wrote:
> Patch below, it changes:
> ...

I just added this to my sources. Will commit in a few minutes.

Michael

-- 
Michael Meskes
Michael@Fam-Meskes.De
Go SF 49ers! Go Rhein Fire!
Use Debian GNU/Linux! Use PostgreSQL!

Thu, 18 Oct 2001 12:58:31 +0200
Michael Meskes <meskes@postgresql.org>
Re: ecpg - GRANT bug
On Tue, Oct 16, 2001 at 10:27:42AM +0100, Lee Kindness wrote:
> And the patch below corrects a pet peeve I have with ecpg, all errors
> and warnings are output with a line number one less than reality...

I wish I knew where this comes from. I've been trying to track this bug down
for years now, but have yet to find the reason. Okay, didn't check for quite
some time now, but the first time I committed a fix was March 1998. But
somehow I still haven't found all problems it seems.

Michael
-- 
Michael Meskes
Michael@Fam-Meskes.De
Go SF 49ers! Go Rhein Fire!
Use Debian GNU/Linux! Use PostgreSQL!

Thu, 18 Oct 2001 13:00:04 +0200
Michael Meskes <meskes@postgresql.org>
Re: ecpg - GRANT bug
> On Tue, Oct 16, 2001 at 10:16:38AM +0100, Lee Kindness wrote:
> > Patch below, it changes:
> > ...
> 
> I just added this to my sources. Will commit in a few minutes.

Michael, I will let you apply the ecpg patches you desire.

-- 
  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, 18 Oct 2001 10:59:29 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: ecpg - GRANT bug

Michael will apply the required patches.

> Tom Lane writes:
>  > Uh, isn't the correct fix
>  > ! $$ = cat_str(8, make_str("grant"), $2, make_str("on"), $4, $5,
>  > make_str("to"), $7, $8);
>  > ISTM your patch loses the opt_with_grant clause.  (Of course the
>  > backend doesn't currently accept that clause anyway, but that's no
>  > reason for ecpg to drop it.)
> 
> My patch doesn't loose the option, it's never been passed on anyway:
> 
>  opt_with_grant:  WITH GRANT OPTION
> 				{
> 					mmerror(ET_ERROR, "WITH GRANT OPTION is not supported.  Only relation owners can set privileges");
> 				 }
> 		| /*EMPTY*/ 
> 		;
> 
> The existing code in ecpg/preproc/preproc.y to handle the WITH option
> simply throws an error and aborts the processing... The patch below
> prevents the segfault and also passes on the WITH option to the
> backend, probably a better fix.
> 
> Regards, Lee.
> 
> Index: interfaces/ecpg/preproc/preproc.y
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/interfaces/ecpg/preproc/preproc.y,v
> retrieving revision 1.159
> diff -c -r1.159 preproc.y
> *** interfaces/ecpg/preproc/preproc.y	2001/10/14 12:07:57	1.159
> --- interfaces/ecpg/preproc/preproc.y	2001/10/15 09:06:29
> ***************
> *** 1693,1699 ****
>   
>   GrantStmt:  GRANT privileges ON opt_table relation_name_list TO grantee_list opt_with_grant
>   				{
> ! 					$$ = cat_str(7, make_str("grant"), $2, make_str("on"), $4, $5, make_str("to"), $7);
>   				}
>   		;
>   
> --- 1693,1699 ----
>   
>   GrantStmt:  GRANT privileges ON opt_table relation_name_list TO grantee_list opt_with_grant
>   				{
> ! 					$$ = cat_str(8, make_str("grant"), $2, make_str("on"), $4, $5, make_str("to"), $7, $8);
>   				}
>   		;
>   
> ***************
> *** 1769,1779 ****
>   		| grantee_list ',' grantee 	{ $$ = cat_str(3, $1, make_str(","), $3); }
>   		;
>   
> ! opt_with_grant:  WITH GRANT OPTION
> ! 				{
> ! 					mmerror(ET_ERROR, "WITH GRANT OPTION is not supported.  Only relation owners can set privileges");
> ! 				 }
> ! 		| /*EMPTY*/ 
>   		;
>   
>   
> --- 1769,1776 ----
>   		| grantee_list ',' grantee 	{ $$ = cat_str(3, $1, make_str(","), $3); }
>   		;
>   
> ! opt_with_grant:  WITH GRANT OPTION { $$ = make_str("with grant option"); }
> ! 		| /*EMPTY*/ { $$ = EMPTY; }
>   		;
>   
>   
> 
> 
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
> 
> http://www.postgresql.org/users-lounge/docs/faq.html
> 

-- 
  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, 18 Oct 2001 10:59:51 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: ecpg - GRANT bug

Michael will apply the required patches.

> Bruce Momjian writes:
>  > Lee Kindness writes:
>  > > In which case a number of other cases should be weeded out of
>  > > parser.y and passed onto the backend:
>  > > [ snip ]
>  > > Let me known if you want a patch for these cases too.
>  > Sure, send them on over.
> 
> Patch below, it changes:
> 
>  1. A number of mmerror(ET_ERROR) to mmerror(ET_NOTICE), passing on
>     the (currently) unsupported options to the backend with warning.
> 
>  2. Standardises warning messages in such cases.
> 
>  3. Corrects typo in passing of 'CREATE FUNCTION/INOUT' parameter.
> 
> Patch:
> 
> ? interfaces/ecpg/preproc/ecpg
> Index: interfaces/ecpg/preproc/preproc.y
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/interfaces/ecpg/preproc/preproc.y,v
> retrieving revision 1.161
> diff -c -r1.161 preproc.y
> *** interfaces/ecpg/preproc/preproc.y	2001/10/15 20:15:09	1.161
> --- interfaces/ecpg/preproc/preproc.y	2001/10/16 09:15:53
> ***************
> *** 1074,1084 ****
>   		| LOCAL TEMPORARY	{ $$ = make_str("local temporary"); }
>   		| LOCAL TEMP		{ $$ = make_str("local temp"); }
>   		| GLOBAL TEMPORARY	{
> ! 					  mmerror(ET_ERROR, "GLOBAL TEMPORARY TABLE is not currently supported");
>   					  $$ = make_str("global temporary");
>   					}
>   		| GLOBAL TEMP		{
> ! 					  mmerror(ET_ERROR, "GLOBAL TEMPORARY TABLE is not currently supported");
>   					  $$ = make_str("global temp");
>   					}
>   		| /*EMPTY*/		{ $$ = EMPTY; }
> --- 1074,1084 ----
>   		| LOCAL TEMPORARY	{ $$ = make_str("local temporary"); }
>   		| LOCAL TEMP		{ $$ = make_str("local temp"); }
>   		| GLOBAL TEMPORARY	{
> ! 					  mmerror(ET_NOTICE, "Currently unsupported CREATE TABLE/GLOBAL TEMPORARY will be passed to backend");
>   					  $$ = make_str("global temporary");
>   					}
>   		| GLOBAL TEMP		{
> ! 					  mmerror(ET_NOTICE, "Currently unsupported CREATE TABLE/GLOBAL TEMP will be passed to backend");
>   					  $$ = make_str("global temp");
>   					}
>   		| /*EMPTY*/		{ $$ = EMPTY; }
> ***************
> *** 1103,1110 ****
>   				{
>   					if (strlen($4) > 0)
>   					{
> ! 						sprintf(errortext, "CREATE TABLE/COLLATE %s not yet implemented; clause ignored", $4);
> ! 						mmerror(ET_NOTICE, errortext);
>   					}
>   					$$ = cat_str(4, $1, $2, $3, $4);
>   				}
> --- 1103,1110 ----
>   				{
>   					if (strlen($4) > 0)
>   					{
> !  						sprintf(errortext, "Currently unsupported CREATE TABLE/COLLATE %s will be passed to backend", $4);
> !  						mmerror(ET_NOTICE, errortext);
>   					}
>   					$$ = cat_str(4, $1, $2, $3, $4);
>   				}
> ***************
> *** 1219,1225 ****
>   		}
>   		| MATCH PARTIAL		
>   		{
> ! 			mmerror(ET_NOTICE, "FOREIGN KEY/MATCH PARTIAL not yet implemented");
>   			$$ = make_str("match partial");
>   		}
>   		| /*EMPTY*/
> --- 1219,1225 ----
>   		}
>   		| MATCH PARTIAL		
>   		{
> ! 			mmerror(ET_NOTICE, "Currently unsupported FOREIGN KEY/MATCH PARTIAL will be passed to backend");
>   			$$ = make_str("match partial");
>   		}
>   		| /*EMPTY*/
> ***************
> *** 1614,1620 ****
>   		| BACKWARD	{ $$ = make_str("backward"); }
>   		| RELATIVE      { $$ = make_str("relative"); }
>                   | ABSOLUTE	{
> ! 					mmerror(ET_NOTICE, "FETCH/ABSOLUTE not supported, backend will use RELATIVE");
>   					$$ = make_str("absolute");
>   				}
>   		;
> --- 1614,1620 ----
>   		| BACKWARD	{ $$ = make_str("backward"); }
>   		| RELATIVE      { $$ = make_str("relative"); }
>                   | ABSOLUTE	{
> ! 					mmerror(ET_NOTICE, "Currently unsupported FETCH/ABSOLUTE will be passed to backend, backend will use RELATIVE");
>   					$$ = make_str("absolute");
>   				}
>   		;
> ***************
> *** 1769,1775 ****
>   		| grantee_list ',' grantee 	{ $$ = cat_str(3, $1, make_str(","), $3); }
>   		;
>   
> ! opt_with_grant:  WITH GRANT OPTION { $$ = make_str("with grant option"); }
>   		| /*EMPTY*/ { $$ = EMPTY; }
>   		;
>   
> --- 1769,1779 ----
>   		| grantee_list ',' grantee 	{ $$ = cat_str(3, $1, make_str(","), $3); }
>   		;
>   
> ! opt_with_grant:  WITH GRANT OPTION
> !                                 {
> ! 					mmerror(ET_NOTICE, "Currently unsupported GRANT/WITH GRANT OPTION will be passed to backend");
> ! 					$$ = make_str("with grant option");
> ! 				}
>   		| /*EMPTY*/ { $$ = EMPTY; }
>   		;
>   
> ***************
> *** 1919,1932 ****
>   
>   opt_arg:  IN    { $$ = make_str("in"); }
>   	| OUT	{ 
> ! 		  mmerror(ET_ERROR, "CREATE FUNCTION/OUT parameters are not supported");
>   
>   	 	  $$ = make_str("out");
>   		}
>   	| INOUT	{ 
> ! 		  mmerror(ET_ERROR, "CREATE FUNCTION/INOUT parameters are not supported");
>   
> ! 	 	  $$ = make_str("oinut");
>   		}
>   	;
>   
> --- 1923,1936 ----
>   
>   opt_arg:  IN    { $$ = make_str("in"); }
>   	| OUT	{ 
> ! 		  mmerror(ET_NOTICE, "Currently unsupported CREATE FUNCTION/OUT will be passed to backend");
>   
>   	 	  $$ = make_str("out");
>   		}
>   	| INOUT	{ 
> ! 		  mmerror(ET_NOTICE, "Currently unsupported CREATE FUNCTION/INOUT will be passed to backend");
>   
> ! 	 	  $$ = make_str("inout");
>   		}
>   	;
>   
> ***************
> *** 2164,2170 ****
>   
>   opt_chain: AND NO CHAIN 	{ $$ = make_str("and no chain"); }
>   	| AND CHAIN		{
> ! 				  mmerror(ET_ERROR, "COMMIT/CHAIN not yet supported");
>   
>   				  $$ = make_str("and chain");
>   				}
> --- 2168,2174 ----
>   
>   opt_chain: AND NO CHAIN 	{ $$ = make_str("and no chain"); }
>   	| AND CHAIN		{
> ! 				  mmerror(ET_NOTICE, "Currently unsupported COMMIT/CHAIN will be passed to backend");
>   
>   				  $$ = make_str("and chain");
>   				}
> ***************
> *** 2609,2620 ****
>   			}
>                          | GLOBAL TEMPORARY opt_table relation_name
>                           {
> ! 				mmerror(ET_ERROR, "GLOBAL TEMPORARY TABLE is not currently supported");
>   				$$ = cat_str(3, make_str("global temporary"), $3, $4);
>                           }
>                          | GLOBAL TEMP opt_table relation_name
>                           {
> ! 				mmerror(ET_ERROR, "GLOBAL TEMPORARY TABLE is not currently supported");
>   				$$ = cat_str(3, make_str("global temp"), $3, $4);
>                           }
>                          | TABLE relation_name
> --- 2613,2624 ----
>   			}
>                          | GLOBAL TEMPORARY opt_table relation_name
>                           {
> ! 				mmerror(ET_NOTICE, "Currently unsupported CREATE TABLE/GLOBAL TEMPORARY will be passed to backend");
>   				$$ = cat_str(3, make_str("global temporary"), $3, $4);
>                           }
>                          | GLOBAL TEMP opt_table relation_name
>                           {
> ! 				mmerror(ET_NOTICE, "Currently unsupported CREATE TABLE/GLOBAL TEMP will be passed to backend");
>   				$$ = cat_str(3, make_str("global temp"), $3, $4);
>                           }
>                          | TABLE relation_name
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
> 
> http://www.postgresql.org/users-lounge/docs/faq.html
> 

-- 
  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, 18 Oct 2001 10:59:56 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: ecpg - GRANT bug
> > On Tue, Oct 16, 2001 at 10:16:38AM +0100, Lee Kindness wrote:
> > > Patch below, it changes:
> > > ...
> > 
> > I just added this to my sources. Will commit in a few minutes.
> 
> Michael, I will let you apply the ecpg patches you desire.

I have removed all the ecpg patches from the unapplied patches queue.

-- 
  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, 18 Oct 2001 12:28:23 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: ecpg - GRANT bug
Michael Meskes wrote:

> On Tue, Oct 16, 2001 at 10:27:42AM +0100, Lee Kindness wrote:
> > And the patch below corrects a pet peeve I have with ecpg, all errors
> > and warnings are output with a line number one less than reality...
>
> I wish I knew where this comes from. I've been trying to track this bug down
> for years now, but have yet to find the reason. Okay, didn't check for quite
> some time now, but the first time I committed a fix was March 1998. But
> somehow I still haven't found all problems it seems.

I somewhat got the impression that using  C++ style comments (//) are related
to worse the problem. But I must confess I didn't dig deep enough to contribute
anything substancial. Perhaps the problem is a misunderstanding of ecpg and
cpp.

I was confused by the blank lines following or preceding a #line statement
every time I looked at it. This should be not necessary.

While talking about warnings: ecpg warns about NULLIF being not implemented
yet. But actually it works (for me).

Christof



Fri, 19 Oct 2001 09:37:59 +0200
Christof Petig <christof@petig-baender.de>
Re: ecpg - GRANT bug
On Fri, Oct 19, 2001 at 09:37:59AM +0200, Christof Petig wrote:
> I somewhat got the impression that using  C++ style comments (//) are related
> to worse the problem. But I must confess I didn't dig deep enough to contribute
> anything substancial. Perhaps the problem is a misunderstanding of ecpg and
> cpp.

If I find some time I will dig into it, but that looks like a longshot right
now.

> While talking about warnings: ecpg warns about NULLIF being not implemented
> yet. But actually it works (for me).

Fixed.

Michael
-- 
Michael Meskes
Michael@Fam-Meskes.De
Go SF 49ers! Go Rhein Fire!
Use Debian GNU/Linux! Use PostgreSQL!

Fri, 19 Oct 2001 16:36:49 +0200
Michael Meskes <meskes@postgresql.org>
Re: ecpg - GRANT bug

I saw that change from 8 to 7.  I think in the ecpg code that the first
parameter to cat_str is the number of arguments _after_ the first
parameter.

 preproc.c:{ yyval.str = cat_str(3, yyvsp[-2].str, make_str(","), yyvsp[0].str);

so I think the change from 8 -> 7 is correct.


---------------------------------------------------------------------------

> Lee Kindness <lkindness@csl.co.uk> writes:
> > ***************
> > *** 1693,1699 ****
>   
> >   GrantStmt:  GRANT privileges ON opt_table relation_name_list TO grantee_list opt_with_grant
> >   				{
> > ! 					$$ = cat_str(8, make_str("grant"), $2, make_str("on"), $4, $5, make_str("to"), $7);
> >   				}
> >   		;
>   
> > --- 1693,1699 ----
>   
> >   GrantStmt:  GRANT privileges ON opt_table relation_name_list TO grantee_list opt_with_grant
> >   				{
> > ! 					$$ = cat_str(7, make_str("grant"), $2, make_str("on"), $4, $5, make_str("to"), $7);
> >   				}
> >   		;
> 
> 
> Uh, isn't the correct fix
> 
> ! 					$$ = cat_str(8, make_str("grant"), $2, make_str("on"), $4, $5, make_str("to"), $7, $8);
> 
> ISTM your patch loses the opt_with_grant clause.  (Of course the backend
> doesn't currently accept that clause anyway, but that's no reason for
> ecpg to drop it.)
> 
> 			regards, tom lane
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
> 

-- 
  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

Mon, 29 Oct 2001 12:44:09 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: ecpg - GRANT bug
On Mon, Oct 29, 2001 at 12:44:09PM -0500, Bruce Momjian wrote:
> I saw that change from 8 to 7.  I think in the ecpg code that the first
> parameter to cat_str is the number of arguments _after_ the first
> parameter.

It is indeed. 

> so I think the change from 8 -> 7 is correct.

Not exactly. Yes, the cat_str has 7 arguments besides the number, but the
rule suggests it should have 8.

Michael
-- 
Michael Meskes
Michael@Fam-Meskes.De
Go SF 49ers! Go Rhein Fire!
Use Debian GNU/Linux! Use PostgreSQL!

Tue, 30 Oct 2001 07:48:15 +0100
Michael Meskes <meskes@postgresql.org>
Re: ecpg - GRANT bug
> On Mon, Oct 29, 2001 at 12:44:09PM -0500, Bruce Momjian wrote:
> > I saw that change from 8 to 7.  I think in the ecpg code that the first
> > parameter to cat_str is the number of arguments _after_ the first
> > parameter.
> 
> It is indeed. 
> 
> > so I think the change from 8 -> 7 is correct.
> 
> Not exactly. Yes, the cat_str has 7 arguments besides the number, but the
> rule suggests it should have 8.

Sorry, I am lost.  The rule above says to count the number of params
after the first one.  Doesn't that make it 7?

-- 
  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, 30 Oct 2001 01:50:09 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: ecpg - GRANT bug
On Tue, Oct 30, 2001 at 01:50:09AM -0500, Bruce Momjian wrote:
> > Not exactly. Yes, the cat_str has 7 arguments besides the number, but the
> > rule suggests it should have 8.
> 
> Sorry, I am lost.  The rule above says to count the number of params
> after the first one.  Doesn't that make it 7?

Sorry, I meant the bison rule that has 8 argumenents.

Michael
-- 
Michael Meskes
Michael@Fam-Meskes.De
Go SF 49ers! Go Rhein Fire!
Use Debian GNU/Linux! Use PostgreSQL!

Tue, 30 Oct 2001 08:31:53 +0100
Michael Meskes <meskes@postgresql.org>
Re: ecpg - GRANT bug
All in all it doesn't really matter... Michael's already applied a
better fix! For some reason Tom's email has been lost in the system
for weeks!

Lee.

Bruce Momjian writes:
 > > On Mon, Oct 29, 2001 at 12:44:09PM -0500, Bruce Momjian wrote:
 > > > I saw that change from 8 to 7.  I think in the ecpg code that the first
 > > > parameter to cat_str is the number of arguments _after_ the first
 > > > parameter.
 > > 
 > > It is indeed. 
 > > 
 > > > so I think the change from 8 -> 7 is correct.
 > > 
 > > Not exactly. Yes, the cat_str has 7 arguments besides the number, but the
 > > rule suggests it should have 8.
 > 
 > Sorry, I am lost.  The rule above says to count the number of params
 > after the first one.  Doesn't that make it 7?
 > 
 > -- 
 >   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 5: Have you checked our extensive FAQ?
 > 
 > http://www.postgresql.org/users-lounge/docs/faq.html

Tue, 30 Oct 2001 09:15:15 +0000 (GMT)
Lee Kindness <lkindness@csl.co.uk>
Re: ecpg - GRANT bug