Michael Meskes írta:
> On Mon, Aug 03, 2009 at 06:59:30PM +0200, Boszormenyi Zoltan wrote:
>   
>>> Why is this messing with the core grammar?
>>>       
>> ...
>>     
>
> Zoltan, could you please explain why you unrolled FORWARD and BACKWARD? I 
> tried
> applying the rest of your patch, without this unrolling but didn't get any
> shift/reduce problem. Might have been that I missed something, so could you 
> please try again?
>   

Without a re-quoted explanation, please, compare
your modified patch with the attached one. I rolled
FORWARD and BACKWARD back into fetch_direction
in the core grammar, deleting the newly introduced FETCH
and MOVE rules from the core and ECPG grammar and
again I got this during the ECPG grammar compilation:

...
"/usr/bin/perl" ./parse.pl . < ../../../../src/backend/parser/gram.y >
preproc.y
/usr/bin/bison -d  -o preproc.c preproc.y
preproc.y: conflicts: 2 shift/reduce
preproc.y: expected 0 shift/reduce conflicts
make[4]: *** [preproc.c] Error 1
make[4]: Leaving directory
`/home/zozo/Schönig-számlák/leoni/2/pgsql/src/interfaces/ecpg/preproc'
...

FYI:

$ rpm -q bison flex
bison-2.3-5.fc9.x86_64
flex-2.5.35-2.fc9.x86_64

> Tom, AFAICT we only need one core grammar change, moving the cursor name to
> it's own rule that only resolves back to name. This rule should be eliminated
> by bison during the build process anyway, so I see no problem adding it. It
> does make the ecpg changes way smaller though. Is this okay with you?
>
> Zoltan, two more things about this patch need to be cleared:
> - I don't think your code is able to handle varchars.
>   

I will test that, thanks.

> - There is no test. Please add this to some of our test cases or write a new 
> one.
>   

I will write some regression tests, of course.

> Some variable handling commands look suspicious to me, a test case might
> alleviate my concerns.
>   

Best regards,
Zoltán Böszörményi

-- 
Bible has answers for everything. Proof:
"But let your communication be, Yea, yea; Nay, nay: for whatsoever is more
than these cometh of evil." (Matthew 5:37) - basics of digital technology.
"May your kingdom come" - superficial description of plate tectonics

----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
http://www.postgresql.at/

diff -dcrpN pgsql.orig/src/backend/parser/gram.y pgsql/src/backend/parser/gram.y
*** pgsql.orig/src/backend/parser/gram.y	2009-08-03 10:38:28.000000000 +0200
--- pgsql/src/backend/parser/gram.y	2009-08-08 17:26:00.000000000 +0200
*************** static TypeName *TableFuncTypeName(List 
*** 253,259 ****
  
  %type <str>		relation_name copy_file_name
  				database_name access_method_clause access_method attr_name
! 				index_name name file_name cluster_index_specification
  
  %type <list>	func_name handler_name qual_Op qual_all_Op subquery_Op
  				opt_class opt_validator validator_clause
--- 253,259 ----
  
  %type <str>		relation_name copy_file_name
  				database_name access_method_clause access_method attr_name
! 				index_name name cursor_name file_name cluster_index_specification
  
  %type <list>	func_name handler_name qual_Op qual_all_Op subquery_Op
  				opt_class opt_validator validator_clause
*************** reloption_elem:	
*** 1915,1921 ****
   *****************************************************************************/
  
  ClosePortalStmt:
! 			CLOSE name
  				{
  					ClosePortalStmt *n = makeNode(ClosePortalStmt);
  					n->portalname = $2;
--- 1915,1921 ----
   *****************************************************************************/
  
  ClosePortalStmt:
! 			CLOSE cursor_name
  				{
  					ClosePortalStmt *n = makeNode(ClosePortalStmt);
  					n->portalname = $2;
*************** comment_text:
*** 4082,4095 ****
   *
   *****************************************************************************/
  
! FetchStmt:	FETCH fetch_direction from_in name
  				{
  					FetchStmt *n = (FetchStmt *) $2;
  					n->portalname = $4;
  					n->ismove = FALSE;
  					$$ = (Node *)n;
  				}
! 			| FETCH name
  				{
  					FetchStmt *n = makeNode(FetchStmt);
  					n->direction = FETCH_FORWARD;
--- 4082,4095 ----
   *
   *****************************************************************************/
  
! FetchStmt:	FETCH fetch_direction from_in cursor_name
  				{
  					FetchStmt *n = (FetchStmt *) $2;
  					n->portalname = $4;
  					n->ismove = FALSE;
  					$$ = (Node *)n;
  				}
! 			| FETCH cursor_name
  				{
  					FetchStmt *n = makeNode(FetchStmt);
  					n->direction = FETCH_FORWARD;
*************** FetchStmt:	FETCH fetch_direction from_in
*** 4098,4111 ****
  					n->ismove = FALSE;
  					$$ = (Node *)n;
  				}
! 			| MOVE fetch_direction from_in name
  				{
  					FetchStmt *n = (FetchStmt *) $2;
  					n->portalname = $4;
  					n->ismove = TRUE;
  					$$ = (Node *)n;
  				}
! 			| MOVE name
  				{
  					FetchStmt *n = makeNode(FetchStmt);
  					n->direction = FETCH_FORWARD;
--- 4098,4111 ----
  					n->ismove = FALSE;
  					$$ = (Node *)n;
  				}
! 			| MOVE fetch_direction from_in cursor_name
  				{
  					FetchStmt *n = (FetchStmt *) $2;
  					n->portalname = $4;
  					n->ismove = TRUE;
  					$$ = (Node *)n;
  				}
! 			| MOVE cursor_name
  				{
  					FetchStmt *n = makeNode(FetchStmt);
  					n->direction = FETCH_FORWARD;
*************** set_target_list:
*** 6847,6853 ****
   *				CURSOR STATEMENTS
   *
   *****************************************************************************/
! DeclareCursorStmt: DECLARE name cursor_options CURSOR opt_hold FOR SelectStmt
  				{
  					DeclareCursorStmt *n = makeNode(DeclareCursorStmt);
  					n->portalname = $2;
--- 6847,6853 ----
   *				CURSOR STATEMENTS
   *
   *****************************************************************************/
! DeclareCursorStmt: DECLARE cursor_name cursor_options CURSOR opt_hold FOR SelectStmt
  				{
  					DeclareCursorStmt *n = makeNode(DeclareCursorStmt);
  					n->portalname = $2;
*************** DeclareCursorStmt: DECLARE name cursor_o
*** 6858,6863 ****
--- 6858,6866 ----
  				}
  		;
  
+ cursor_name:	name						{ $$ = $1; }
+ 		;
+ 
  cursor_options: /*EMPTY*/					{ $$ = 0; }
  			| cursor_options NO SCROLL		{ $$ = $1 | CURSOR_OPT_NO_SCROLL; }
  			| cursor_options SCROLL			{ $$ = $1 | CURSOR_OPT_SCROLL; }
diff -dcrpN pgsql.orig/src/interfaces/ecpg/preproc/ecpg.addons pgsql/src/interfaces/ecpg/preproc/ecpg.addons
*** pgsql.orig/src/interfaces/ecpg/preproc/ecpg.addons	2009-01-30 17:28:46.000000000 +0100
--- pgsql/src/interfaces/ecpg/preproc/ecpg.addons	2009-08-08 17:28:02.000000000 +0200
*************** ECPG: fetch_directionBACKWARDSignedIcons
*** 221,226 ****
--- 221,235 ----
  			free($2);
  			$2 = make_str("$0");
  		}
+ ECPG: cursor_namename rule
+ 	| char_civar
+ 		{
+ 			char *curname = mm_alloc(strlen($1) + 2);
+ 			sprintf(curname, ":%s", $1);
+ 			free($1);
+ 			$1 = curname;
+ 			$$ = $1;
+ 		}
  ECPG: PrepareStmtPREPAREprepared_nameprep_type_clauseASPreparableStmt block
  	{
  		$$.name = $2;
*************** ECPG: PrepareStmtPREPAREprepared_namepre
*** 235,243 ****
  	}
  ECPG: ExecuteStmtEXECUTEprepared_nameexecute_param_clauseexecute_rest block
  	{ $$ = $2; }
! ECPG: DeclareCursorStmtDECLAREnamecursor_optionsCURSORopt_holdFORSelectStmt block
  	{
  		struct cursor *ptr, *this;
  
  		for (ptr = cur; ptr != NULL; ptr = ptr->next)
  		{
--- 244,253 ----
  	}
  ECPG: ExecuteStmtEXECUTEprepared_nameexecute_param_clauseexecute_rest block
  	{ $$ = $2; }
! ECPG: DeclareCursorStmtDECLAREcursor_namecursor_optionsCURSORopt_holdFORSelectStmt block
  	{
  		struct cursor *ptr, *this;
+ 		char *cursor_marker = $2[0] == ':' ? make_str("$0") : mm_strdup($2);
  
  		for (ptr = cur; ptr != NULL; ptr = ptr->next)
  		{
*************** ECPG: DeclareCursorStmtDECLAREnamecursor
*** 251,257 ****
  		this->name = $2;
  		this->connection = connection;
  		this->opened = false;
! 		this->command =  cat_str(7, make_str("declare"), mm_strdup($2), $3, make_str("cursor"), $5, make_str("for"), $7);
  		this->argsinsert = argsinsert;
  		this->argsresult = argsresult;
  		argsinsert = argsresult = NULL;
--- 261,267 ----
  		this->name = $2;
  		this->connection = connection;
  		this->opened = false;
! 		this->command =  cat_str(7, make_str("declare"), cursor_marker, $3, make_str("cursor"), $5, make_str("for"), $7);
  		this->argsinsert = argsinsert;
  		this->argsresult = argsresult;
  		argsinsert = argsresult = NULL;
*************** ECPG: DeclareCursorStmtDECLAREnamecursor
*** 262,267 ****
--- 272,282 ----
  		else
  			$$ = cat_str(3, make_str("/*"), mm_strdup(this->command), make_str("*/"));
  	}
+ ECPG: ClosePortalStmtCLOSEcursor_name block
+ 	{
+ 		char *cursor_marker = $2[0] == ':' ? make_str("$0") : $2;
+ 		$$ = cat2_str(make_str("close"), cursor_marker);
+ 	}
  ECPG: opt_hold block
  	{
  		if (compat == ECPG_COMPAT_INFORMIX_SE && autocommit == true)
*************** ECPG: VariableShowStmtSHOWALL block
*** 326,371 ****
  		mmerror(PARSE_ERROR, ET_ERROR, "SHOW ALL is not implemented");
  		$$ = EMPTY;
  	}
! ECPG: FetchStmtFETCHfetch_directionfrom_inname block 
  	{
  		add_additional_variables($4, false);
! 		$$ = cat_str(4, make_str("fetch"), $2, $3, $4);
  	}
! ECPG: FetchStmtFETCHname block
  	{
  		add_additional_variables($2, false);
! 		$$ = cat_str(2, make_str("fetch"), $2);
  	}
! ECPG: FetchStmtMOVEname rule
! 	| FETCH fetch_direction from_in name ecpg_into
  		{
  			add_additional_variables($4, false);
! 			$$ = cat_str(4, make_str("fetch"), $2, $3, $4);
  		}
! 	| FETCH fetch_direction name ecpg_into
  		{
  			add_additional_variables($3, false);
! 			$$ = cat_str(4, make_str("fetch"), $2, make_str("from"), $3);
  		}
! 	| FETCH from_in name ecpg_into
  		{
  			add_additional_variables($3, false);
! 			$$ = cat_str(3, make_str("fetch"), $2, $3);
  		}
! 	| FETCH name ecpg_into
  		{
  			add_additional_variables($2, false);
! 			$$ = cat2_str(make_str("fetch"), $2);
  		}
! 	| FETCH fetch_direction name
  		{
  			add_additional_variables($3, false);
! 			$$ = cat_str(4, make_str("fetch"), $2, make_str("from"), $3);
  		}
! 	| FETCH from_in name
  		{
  			add_additional_variables($3, false);
! 			$$ = cat_str(3, make_str("fetch"), $2, $3);
  		}
  ECPG: SpecialRuleRelationOLD addon
  		if (!QueryIsRule)
--- 341,394 ----
  		mmerror(PARSE_ERROR, ET_ERROR, "SHOW ALL is not implemented");
  		$$ = EMPTY;
  	}
! ECPG: FetchStmtFETCHfetch_directionfrom_incursor_name block
  	{
+ 		char *cursor_marker = $4[0] == ':' ? make_str("$0") : $4;
  		add_additional_variables($4, false);
! 		$$ = cat_str(4, make_str("fetch"), $2, $3, cursor_marker);
  	}
! ECPG: FetchStmtFETCHcursor_name block
  	{
+ 		char *cursor_marker = $2[0] == ':' ? make_str("$0") : $2;
  		add_additional_variables($2, false);
! 		$$ = cat_str(2, make_str("fetch"), cursor_marker);
  	}
! ECPG: FetchStmtMOVEcursor_name rule
! 	| FETCH fetch_direction from_in cursor_name ecpg_into
  		{
+ 			char *cursor_marker = $4[0] == ':' ? make_str("$0") : $4;
  			add_additional_variables($4, false);
! 			$$ = cat_str(4, make_str("fetch"), $2, $3, cursor_marker);
  		}
! 	| FETCH fetch_direction cursor_name ecpg_into
  		{
+ 			char *cursor_marker = $3[0] == ':' ? make_str("$0") : $3;
  			add_additional_variables($3, false);
! 			$$ = cat_str(4, make_str("fetch"), $2, make_str("from"), cursor_marker);
  		}
! 	| FETCH from_in cursor_name ecpg_into
  		{
+ 			char *cursor_marker = $3[0] == ':' ? make_str("$0") : $3;
  			add_additional_variables($3, false);
! 			$$ = cat_str(3, make_str("fetch"), $2, cursor_marker);
  		}
! 	| FETCH cursor_name ecpg_into
  		{
+ 			char *cursor_marker = $2[0] == ':' ? make_str("$0") : $2;
  			add_additional_variables($2, false);
! 			$$ = cat2_str(make_str("fetch"), cursor_marker);
  		}
! 	| FETCH fetch_direction cursor_name
  		{
+ 			char *cursor_marker = $3[0] == ':' ? make_str("$0") : $3;
  			add_additional_variables($3, false);
! 			$$ = cat_str(4, make_str("fetch"), $2, make_str("from"), cursor_marker);
  		}
! 	| FETCH from_in cursor_name
  		{
+ 			char *cursor_marker = $3[0] == ':' ? make_str("$0") : $3;
  			add_additional_variables($3, false);
! 			$$ = cat_str(3, make_str("fetch"), $2, cursor_marker);
  		}
  ECPG: SpecialRuleRelationOLD addon
  		if (!QueryIsRule)
diff -dcrpN pgsql.orig/src/interfaces/ecpg/preproc/ecpg.trailer pgsql/src/interfaces/ecpg/preproc/ecpg.trailer
*** pgsql.orig/src/interfaces/ecpg/preproc/ecpg.trailer	2009-08-07 13:06:28.000000000 +0200
--- pgsql/src/interfaces/ecpg/preproc/ecpg.trailer	2009-08-08 17:23:11.000000000 +0200
*************** prepared_name: name             {
*** 285,293 ****
   * Declare a prepared cursor. The syntax is different from the standard
   * declare statement, so we create a new rule.
   */
! ECPGCursorStmt:  DECLARE name cursor_options CURSOR opt_hold FOR prepared_name
  		{
  			struct cursor *ptr, *this;
  			struct variable *thisquery = (struct variable *)mm_alloc(sizeof(struct variable));
  			const char *con = connection ? connection : "NULL";
  
--- 285,294 ----
   * Declare a prepared cursor. The syntax is different from the standard
   * declare statement, so we create a new rule.
   */
! ECPGCursorStmt:  DECLARE cursor_name cursor_options CURSOR opt_hold FOR prepared_name
  		{
  			struct cursor *ptr, *this;
+ 			char *cursor_marker = $2[0] == ':' ? make_str("$0") : mm_strdup($2);
  			struct variable *thisquery = (struct variable *)mm_alloc(sizeof(struct variable));
  			const char *con = connection ? connection : "NULL";
  
*************** ECPGCursorStmt:  DECLARE name cursor_opt
*** 304,310 ****
  			this->next = cur;
  			this->name = $2;
  			this->connection = connection;
! 			this->command =  cat_str(6, make_str("declare"), mm_strdup($2), $3, make_str("cursor"), $5, make_str("for $1"));
  			this->argsresult = NULL;
  
  			thisquery->type = &ecpg_query;
--- 305,311 ----
  			this->next = cur;
  			this->name = $2;
  			this->connection = connection;
! 			this->command =  cat_str(6, make_str("declare"), cursor_marker, $3, make_str("cursor"), $5, make_str("for $1"));
  			this->argsresult = NULL;
  
  			thisquery->type = &ecpg_query;
*************** ECPGCursorStmt:  DECLARE name cursor_opt
*** 314,319 ****
--- 315,326 ----
  			sprintf(thisquery->name, "ECPGprepared_statement(%s, %s, __LINE__)", con, $7);
  
  			this->argsinsert = NULL;
+ 			if ($2[0] == ':')
+ 			{
+ 				struct variable *var = find_variable($2 + 1);
+ 				remove_variable_from_list(&argsinsert, var);
+ 				add_variable_to_head(&(this->argsinsert), var, &no_indicator);
+ 			}
  			add_variable_to_head(&(this->argsinsert), thisquery, &no_indicator);
  
  			cur = this;
*************** ECPGFree:	SQL_FREE name	{ $$ = $2; }
*** 954,960 ****
  /*
   * open is an open cursor, at the moment this has to be removed
   */
! ECPGOpen: SQL_OPEN name opt_ecpg_using { $$ = $2; };
  
  opt_ecpg_using: /*EMPTY*/	{ $$ = EMPTY; }
  		| ecpg_using		{ $$ = $1; }
--- 961,976 ----
  /*
   * open is an open cursor, at the moment this has to be removed
   */
! ECPGOpen: SQL_OPEN cursor_name opt_ecpg_using
! 		{
! 			if ($2[0] == ':')
! 			{
! 				struct variable *var = find_variable($2 + 1);
! 				remove_variable_from_list(&argsinsert, var);
! 			}
! 			$$ = $2;
! 		}
! 		;
  
  opt_ecpg_using: /*EMPTY*/	{ $$ = EMPTY; }
  		| ecpg_using		{ $$ = $1; }
*************** civarind: cvariable indicator
*** 1779,1784 ****
--- 1795,1807 ----
  		}
  		;
  
+ char_civar: char_variable
+ 		{
+ 			add_variable_to_head(&argsinsert, find_variable($1), &no_indicator);
+ 			$$ = $1;
+ 		}
+ 		;
+ 
  civar: cvariable
  		{
  			add_variable_to_head(&argsinsert, find_variable($1), &no_indicator);
diff -dcrpN pgsql.orig/src/interfaces/ecpg/preproc/ecpg.type pgsql/src/interfaces/ecpg/preproc/ecpg.type
*** pgsql.orig/src/interfaces/ecpg/preproc/ecpg.type	2008-11-14 11:03:33.000000000 +0100
--- pgsql/src/interfaces/ecpg/preproc/ecpg.type	2009-08-08 17:23:11.000000000 +0200
***************
*** 43,48 ****
--- 43,49 ----
  %type <str> c_term
  %type <str> c_thing
  %type <str> char_variable
+ %type <str> char_civar
  %type <str> civar
  %type <str> civarind
  %type <str> ColLabel
diff -dcrpN pgsql.orig/src/interfaces/ecpg/preproc/extern.h pgsql/src/interfaces/ecpg/preproc/extern.h
*** pgsql.orig/src/interfaces/ecpg/preproc/extern.h	2009-07-17 07:50:56.000000000 +0200
--- pgsql/src/interfaces/ecpg/preproc/extern.h	2009-08-08 17:23:11.000000000 +0200
*************** extern struct descriptor *lookup_descrip
*** 91,96 ****
--- 91,97 ----
  extern struct variable *descriptor_variable(const char *name, int input);
  extern void add_variable_to_head(struct arguments **, struct variable *, struct variable *);
  extern void add_variable_to_tail(struct arguments **, struct variable *, struct variable *);
+ extern void remove_variable_from_list(struct arguments ** list, struct variable * var);
  extern void dump_variables(struct arguments *, int);
  extern struct typedefs *get_typedef(char *);
  extern void adjust_array(enum ECPGttype, char **, char **, char *, char *, int, bool);
diff -dcrpN pgsql.orig/src/interfaces/ecpg/preproc/variable.c pgsql/src/interfaces/ecpg/preproc/variable.c
*** pgsql.orig/src/interfaces/ecpg/preproc/variable.c	2009-08-07 13:06:28.000000000 +0200
--- pgsql/src/interfaces/ecpg/preproc/variable.c	2009-08-08 17:23:11.000000000 +0200
*************** add_variable_to_tail(struct arguments **
*** 401,406 ****
--- 401,430 ----
  		*list = new;
  }
  
+ void
+ remove_variable_from_list(struct arguments ** list, struct variable * var)
+ {
+ 	struct arguments *p, *prev = NULL;
+ 	bool found = false;
+ 
+ 	for (p = *list; p; p = p->next)
+ 	{
+ 		if (p->variable == var)
+ 		{
+ 			found = true;
+ 			break;
+ 		}
+ 		prev = p;
+ 	}
+ 	if (found)
+ 	{
+ 		if (prev)
+ 			prev->next = p->next;
+ 		else
+ 			*list = p->next;
+ 	}
+ }
+ 
  /* Dump out a list of all the variable on this list.
     This is a recursive function that works from the end of the list and
     deletes the list as we go on.
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to