ecpg - GRANT bug
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);
}
;
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
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 19026Tom 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; }
;
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
Show quoted text
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
Import Notes
Reply to msg id not found: 15307.64910.421696.852199@elsick.csl.co.uk | Resolved by subject fallback
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
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.
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 19026In 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 19026Bruce 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_nameLee 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);
}
}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?
--
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 19026Your 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?
--
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 19026Your 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 19026Lee 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
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
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
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.
[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!
