Recently I've been seeing regular but very occasional errors like the 
following while using psql:

test=> BEGIN ;
BEGIN
test=> UPDATE language SET name_native = 'Franšais' WHERE lang_id='fr';
ERROR:  current transaction is aborted, commands ignored until end of 
transaction block

where the UPDATE statement itself is entirely correct and is executed 
correctly when a new transaction is started. Unfortunately I was never able
to reproduce the error and thought it might be some kind of beta flakiness, 
until it turned up in a 7.3 installation too.

The culprit is the following section of psql's tab-complete.c , around line
1248:

/* WHERE */
        /* Simple case of the word before the where being the table name */
        else if (strcasecmp(prev_wd, "WHERE") == 0)
                COMPLETE_WITH_ATTR(prev2_wd);

which is correct for SELECT statements. Where the line contains an UPDATE
statement however, and tab is pressed after WHERE, the word before WHERE is
passed to the backend via a sprintf-generated query with the %s between single
quotes, i.e. in the above case 
  AND c.relname='%s'
is translated to
  AND c.relname=''Franšais''

which is causing a silent error and the transaction failure.

I don't see a simple solution to cater for UPDATE syntax in this context
(you'd need to keep track of whether the statement begins with SELECT
or UPDATE), though it might be a good todo item. 

A quick (but not dirty) fix for this and other current or future potential 
corner cases would be to ensure any statements executed by the tab completion
functions are quoted correctly, so even if the statement does not produce any
results for tab completion, at least it cannot cause mysterious transaction
errors (and associated doubts about PostgreSQL's stability ;-).

A patch for this using PQescapeString (is there another preferred method?) is
attached as a possible solution.


Ian Barwick
[EMAIL PROTECTED]


Index: tab-complete.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/bin/psql/tab-complete.c,v
retrieving revision 1.85
diff -c -r1.85 tab-complete.c
*** tab-complete.c	7 Sep 2003 15:26:54 -0000	1.85
--- tab-complete.c	14 Oct 2003 21:03:58 -0000
***************
*** 789,801 ****
  	 * list of tables (Also cover the analogous backslash command)
  	 */
  	else if (strcasecmp(prev_wd, "COPY") == 0 ||
! 			 strcasecmp(prev_wd, "\\copy") == 0 ||
  			 (strcasecmp(prev2_wd, "COPY") == 0 &&
  			  strcasecmp(prev_wd, "BINARY") == 0))
  		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables);
  	/* If we have COPY|BINARY <sth>, complete it with "TO" or "FROM" */
  	else if (strcasecmp(prev2_wd, "COPY") == 0 ||
! 			 strcasecmp(prev2_wd, "\\copy") == 0 ||
  			 strcasecmp(prev2_wd, "BINARY") == 0)
  	{
  		char	   *list_FROMTO[] = {"FROM", "TO", NULL};
--- 789,801 ----
  	 * list of tables (Also cover the analogous backslash command)
  	 */
  	else if (strcasecmp(prev_wd, "COPY") == 0 ||
! 			 strcasecmp(prev_wd, "\\\\copy") == 0 ||
  			 (strcasecmp(prev2_wd, "COPY") == 0 &&
  			  strcasecmp(prev_wd, "BINARY") == 0))
  		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables);
  	/* If we have COPY|BINARY <sth>, complete it with "TO" or "FROM" */
  	else if (strcasecmp(prev2_wd, "COPY") == 0 ||
! 			 strcasecmp(prev2_wd, "\\\\copy") == 0 ||
  			 strcasecmp(prev2_wd, "BINARY") == 0)
  	{
  		char	   *list_FROMTO[] = {"FROM", "TO", NULL};
***************
*** 1258,1296 ****
  
  /* Backslash commands */
  /* TODO:  \dc \dd \dl */
! 	else if (strcmp(prev_wd, "\\connect") == 0 || strcmp(prev_wd, "\\c") == 0)
  		COMPLETE_WITH_QUERY(Query_for_list_of_databases);
! 	else if (strcmp(prev_wd, "\\d") == 0 || strcmp(prev_wd, "\\d+") == 0)
  		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tisv);
! 	else if (strcmp(prev_wd, "\\da") == 0)
  		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_aggregates);
! 	else if (strcmp(prev_wd, "\\dD") == 0)
  		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_domains);
! 	else if (strcmp(prev_wd, "\\df") == 0 || strcmp(prev_wd, "\\df+") == 0)
  		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_functions);
! 	else if (strcmp(prev_wd, "\\di") == 0 || strcmp(prev_wd, "\\di+") == 0)
  		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_indexes);
! 	else if (strcmp(prev_wd, "\\dn") == 0)
  		COMPLETE_WITH_QUERY(Query_for_list_of_schemas);
! 	else if (strcmp(prev_wd, "\\dp") == 0 || strcmp(prev_wd, "\\z") == 0)
  		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tsv);
! 	else if (strcmp(prev_wd, "\\ds") == 0 || strcmp(prev_wd, "\\ds+") == 0)
  		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_sequences);
! 	else if (strcmp(prev_wd, "\\dS") == 0 || strcmp(prev_wd, "\\dS+") == 0)
  		COMPLETE_WITH_QUERY(Query_for_list_of_system_relations);
! 	else if (strcmp(prev_wd, "\\dt") == 0 || strcmp(prev_wd, "\\dt+") == 0)
  		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables);
! 	else if (strcmp(prev_wd, "\\dT") == 0 || strcmp(prev_wd, "\\dT+") == 0)
  		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_datatypes);
! 	else if (strcmp(prev_wd, "\\du") == 0)
  		COMPLETE_WITH_QUERY(Query_for_list_of_users);
! 	else if (strcmp(prev_wd, "\\dv") == 0 || strcmp(prev_wd, "\\dv+") == 0)
  		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_views);
! 	else if (strcmp(prev_wd, "\\encoding") == 0)
  		COMPLETE_WITH_QUERY(Query_for_list_of_encodings);
! 	else if (strcmp(prev_wd, "\\h") == 0 || strcmp(prev_wd, "\\help") == 0)
  		COMPLETE_WITH_LIST(sql_commands);
! 	else if (strcmp(prev_wd, "\\pset") == 0)
  	{
  		char	   *my_list[] = {"format", "border", "expanded",
  			"null", "fieldsep", "tuples_only", "title", "tableattr", "pager",
--- 1258,1296 ----
  
  /* Backslash commands */
  /* TODO:  \dc \dd \dl */
! 	else if (strcmp(prev_wd, "\\\\connect") == 0 || strcmp(prev_wd, "\\\\c") == 0)
  		COMPLETE_WITH_QUERY(Query_for_list_of_databases);
! 	else if (strcmp(prev_wd, "\\\\d") == 0 || strcmp(prev_wd, "\\\\d+") == 0)
  		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tisv);
! 	else if (strcmp(prev_wd, "\\\\da") == 0)
  		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_aggregates);
! 	else if (strcmp(prev_wd, "\\\\dD") == 0)
  		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_domains);
! 	else if (strcmp(prev_wd, "\\\\df") == 0 || strcmp(prev_wd, "\\\\df+") == 0)
  		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_functions);
! 	else if (strcmp(prev_wd, "\\\\di") == 0 || strcmp(prev_wd, "\\\\di+") == 0)
  		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_indexes);
! 	else if (strcmp(prev_wd, "\\\\dn") == 0)
  		COMPLETE_WITH_QUERY(Query_for_list_of_schemas);
! 	else if (strcmp(prev_wd, "\\\\dp") == 0 || strcmp(prev_wd, "\\\\z") == 0)
  		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tsv);
! 	else if (strcmp(prev_wd, "\\\\ds") == 0 || strcmp(prev_wd, "\\\\ds+") == 0)
  		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_sequences);
! 	else if (strcmp(prev_wd, "\\\\dS") == 0 || strcmp(prev_wd, "\\\\dS+") == 0)
  		COMPLETE_WITH_QUERY(Query_for_list_of_system_relations);
! 	else if (strcmp(prev_wd, "\\\\dt") == 0 || strcmp(prev_wd, "\\\\dt+") == 0)
  		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables);
! 	else if (strcmp(prev_wd, "\\\\dT") == 0 || strcmp(prev_wd, "\\\\dT+") == 0)
  		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_datatypes);
! 	else if (strcmp(prev_wd, "\\\\du") == 0)
  		COMPLETE_WITH_QUERY(Query_for_list_of_users);
! 	else if (strcmp(prev_wd, "\\\\dv") == 0 || strcmp(prev_wd, "\\\\dv+") == 0)
  		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_views);
! 	else if (strcmp(prev_wd, "\\\\encoding") == 0)
  		COMPLETE_WITH_QUERY(Query_for_list_of_encodings);
! 	else if (strcmp(prev_wd, "\\\\h") == 0 || strcmp(prev_wd, "\\\\help") == 0)
  		COMPLETE_WITH_LIST(sql_commands);
! 	else if (strcmp(prev_wd, "\\\\pset") == 0)
  	{
  		char	   *my_list[] = {"format", "border", "expanded",
  			"null", "fieldsep", "tuples_only", "title", "tableattr", "pager",
***************
*** 1298,1310 ****
  
  		COMPLETE_WITH_LIST(my_list);
  	}
! 	else if (strcmp(prev_wd, "\\cd") == 0 ||
! 		 strcmp(prev_wd, "\\e") == 0 || strcmp(prev_wd, "\\edit") == 0 ||
! 			 strcmp(prev_wd, "\\g") == 0 ||
! 			 strcmp(prev_wd, "\\i") == 0 || strcmp(prev_wd, "\\include") == 0 ||
! 		  strcmp(prev_wd, "\\o") == 0 || strcmp(prev_wd, "\\out") == 0 ||
! 			 strcmp(prev_wd, "\\s") == 0 ||
! 		   strcmp(prev_wd, "\\w") == 0 || strcmp(prev_wd, "\\write") == 0
  		)
  		matches = completion_matches(text, filename_completion_function);
  
--- 1298,1310 ----
  
  		COMPLETE_WITH_LIST(my_list);
  	}
! 	else if (strcmp(prev_wd, "\\\\cd") == 0 ||
! 		 strcmp(prev_wd, "\\\\e") == 0 || strcmp(prev_wd, "\\\\edit") == 0 ||
! 			 strcmp(prev_wd, "\\\\g") == 0 ||
! 			 strcmp(prev_wd, "\\\\i") == 0 || strcmp(prev_wd, "\\\\include") == 0 ||
! 		  strcmp(prev_wd, "\\\\o") == 0 || strcmp(prev_wd, "\\\\out") == 0 ||
! 			 strcmp(prev_wd, "\\\\s") == 0 ||
! 		   strcmp(prev_wd, "\\\\w") == 0 || strcmp(prev_wd, "\\\\write") == 0
  		)
  		matches = completion_matches(text, filename_completion_function);
  
***************
*** 1466,1472 ****
  				return NULL;
  			}
  		}
- 
  		result = exec_query(query_buffer);
  	}
  
--- 1466,1471 ----
***************
*** 1618,1624 ****
  				start = 0,
  				end = -1,
  				inquotes = 0;
! 	char	   *s;
  
  	while (skip-- >= 0)
  	{
--- 1617,1623 ----
  				start = 0,
  				end = -1,
  				inquotes = 0;
! 	char	   *s, *r;
  
  	while (skip-- >= 0)
  	{
***************
*** 1672,1678 ****
  	strncpy(s, &rl_line_buffer[start], end - start + 1);
  	s[end - start + 1] = '\0';
  
! 	return s;
  }
  
  
--- 1671,1691 ----
  	strncpy(s, &rl_line_buffer[start], end - start + 1);
  	s[end - start + 1] = '\0';
  
! 	/* escape user input */
! 	r = (char *) malloc(strlen(s) * 2);
! 	if (!r)
! 	{
! 		psql_error("out of memory\n");
! 		if (!pset.cur_cmd_interactive)
! 			exit(EXIT_FAILURE);
! 		else
! 			return NULL;
! 	}
! 
! 	PQescapeString(r, s, strlen(s));
! 	free(s);
! 
! 	return r;
  }
  
  
---------------------------(end of broadcast)---------------------------
TIP 2: you can get off all lists at once with the unregister command
    (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])

Reply via email to