Hi,

    Just in case you'd like to see what I was talking about, I am attaching
my patch to src/interfaces/libpq/fe-exec.c to prevent utility functions
called from SPI from locking up the client.

Jerry
Thu, 4 Mar 1999 12:37:34 +0900
"Gerald L. Gay" <glgay@pass.korea.army.mil>
Re: [HACKERS] libpq and SPI 
Applied.


[Charset iso-8859-1 unsupported, filtering to ASCII...]
> Hi,
> 
>     Just in case you'd like to see what I was talking about, I am attaching
> my patch to src/interfaces/libpq/fe-exec.c to prevent utility functions
> called from SPI from locking up the client.
> 
> Jerry
> 

[Attachment, skipping...]


-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Sun, 14 Mar 1999 11:42:13 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] libpq and SPI
Bruce Momjian <maillist@candle.pha.pa.us> writes:
> Applied.
>
>> Just in case you'd like to see what I was talking about, I am attaching
>> my patch to src/interfaces/libpq/fe-exec.c to prevent utility functions
>> called from SPI from locking up the client.

Uh, I didn't actually believe that that patch was a good idea.  Hacking
libpq to survive a protocol violation committed by the backend is *not*
a solution; the correct answer is to fix the backend.  Otherwise we will
have to discover similar workarounds for other clients that do not
use libpq (ODBC, for example).

Please reverse out that patch until someone can find some time to look
at the issue.  (I will, if no one else does, but it would probably be
more efficient for someone who already knows something about SPI to
fix it...)

			regards, tom lane

Sun, 14 Mar 1999 12:31:17 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: [HACKERS] libpq and SPI 
Tom Lane wrote:
> Please reverse out that patch until someone can find some time to look
> at the issue.  (I will, if no one else does, but it would probably be
> more efficient for someone who already knows something about SPI to
> fix it...)

What is the problem? I'll research a SPI patch.

:) Clark

Sun, 14 Mar 1999 17:42:47 +0000
Clark Evans <clark.evans@manhattanproject.com>
Re: [HACKERS] libpq and SPI
Reversed out.  Attached is the patch.

> Bruce Momjian <maillist@candle.pha.pa.us> writes:
> > Applied.
> >
> >> Just in case you'd like to see what I was talking about, I am attaching
> >> my patch to src/interfaces/libpq/fe-exec.c to prevent utility functions
> >> called from SPI from locking up the client.
> 
> Uh, I didn't actually believe that that patch was a good idea.  Hacking
> libpq to survive a protocol violation committed by the backend is *not*
> a solution; the correct answer is to fix the backend.  Otherwise we will
> have to discover similar workarounds for other clients that do not
> use libpq (ODBC, for example).
> 
> Please reverse out that patch until someone can find some time to look
> at the issue.  (I will, if no one else does, but it would probably be
> more efficient for someone who already knows something about SPI to
> fix it...)
> 
> 			regards, tom lane
> 


-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

--- fe-exec.c.orig	Mon Feb 22 05:03:04 1999
+++ fe-exec.c	Fri Mar  5 06:03:43 1999
@@ -338,6 +338,7 @@
 parseInput(PGconn *conn)
 {
 	char		id;
+        static int      pendingT = 0;
 
 	/*
 	 * Loop to parse successive complete messages available in the buffer.
@@ -406,7 +407,16 @@
 														   PGRES_COMMAND_OK);
 					if (pqGets(conn->result->cmdStatus, CMDSTATUS_LEN, conn))
 						return;
-					conn->asyncStatus = PGASYNC_READY;
+                                        if (pendingT) {
+                                            /* Check the returned message */
+                                            /* if it's a SELECT or FETCH in a pendingT case, */
+                                            /* then it probably means no rows returned. */
+                                            /* We clear pendingT in that case. */
+                                            if ((strncmp(conn->result->cmdStatus, "SELECT", 6) == 0) ||
+                                                (strncmp(conn->result->cmdStatus, "FETCH",  5) == 0))
+                                                pendingT = 0;
+                                        }
+					if (!pendingT) conn->asyncStatus = PGASYNC_READY;
 					break;
 				case 'E':		/* error return */
 					if (pqGets(conn->errorMessage, ERROR_MSG_LENGTH, conn))
@@ -416,10 +426,11 @@
 					/* and build an error result holding the error message */
 					conn->result = PQmakeEmptyPGresult(conn,
 													   PGRES_FATAL_ERROR);
-					conn->asyncStatus = PGASYNC_READY;
+					if (!pendingT) conn->asyncStatus = PGASYNC_READY;
 					break;
 				case 'Z':		/* backend is ready for new query */
 					conn->asyncStatus = PGASYNC_IDLE;
+                                        pendingT = 0;
 					break;
 				case 'I':		/* empty query */
 					/* read and throw away the closing '\0' */
@@ -434,7 +445,7 @@
 					if (conn->result == NULL)
 						conn->result = PQmakeEmptyPGresult(conn,
 														   PGRES_EMPTY_QUERY);
-					conn->asyncStatus = PGASYNC_READY;
+					if (!pendingT) conn->asyncStatus = PGASYNC_READY;
 					break;
 				case 'K':		/* secret key data from the backend */
 
@@ -455,11 +466,15 @@
 					break;
 				case 'T':		/* row descriptions (start of query
 								 * results) */
+                                        if (pendingT) {
+                                            DONOTICE(conn, "Got second 'T' message!\n");
+                                        }
 					if (conn->result == NULL)
 					{
 						/* First 'T' in a query sequence */
 						if (getRowDescriptions(conn))
 							return;
+                                                pendingT = 1;
 					}
 					else
 					{
@@ -471,11 +486,13 @@
 						 * We stop parsing until the application accepts
 						 * the current result.
 						 */
+                                                pendingT = 0;
 						conn->asyncStatus = PGASYNC_READY;
 						return;
 					}
 					break;
 				case 'D':		/* ASCII data tuple */
+                                        pendingT = 0;
 					if (conn->result != NULL)
 					{
 						/* Read another tuple of a normal query response */
@@ -493,6 +510,7 @@
 					}
 					break;
 				case 'B':		/* Binary data tuple */
+                                        pendingT = 0;
 					if (conn->result != NULL)
 					{
 						/* Read another tuple of a normal query response */
@@ -510,12 +528,15 @@
 					}
 					break;
 				case 'G':		/* Start Copy In */
+                                        pendingT = 0;
 					conn->asyncStatus = PGASYNC_COPY_IN;
 					break;
 				case 'H':		/* Start Copy Out */
+                                        pendingT = 0;
 					conn->asyncStatus = PGASYNC_COPY_OUT;
 					break;
 				default:
+                                        pendingT = 0;
 					sprintf(conn->errorMessage,
 					"unknown protocol character '%c' read from backend.  "
 					"(The protocol character is the first character the "
Sun, 14 Mar 1999 13:10:32 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] libpq and SPI
> What is the problem? I'll research a SPI patch.

Check the hackers thread (last month I think) titled "libpq and SPI";
the problem occurs when one submits a utility statement rather than
a plannable query via SPI.  Apparently what is happening is that the
backend emits a 'T' message before it invokes the called statement
and then emits a 'D' message afterwards --- so if the called statement
causes a 'C' message to come out, libpq gets unhappy.  This seems
to be clearly a violation of the FE/BE protocol to me, so I don't think
it's libpq's fault.

Reasonable fixes might be to postpone the sending of 'T' till after
the invoked statement is executed, or to modify the traffic cop so
that a utility statement invoked from SPI doesn't send 'C'.

I know a little bit about the parts of the backend that communicate with
the frontend, but nothing about SPI, so I'm not well prepared to solve
the problem by myself.

			regards, tom lane

Sun, 14 Mar 1999 13:20:59 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: [HACKERS] libpq and SPI 
> Tom Lane wrote:
> > Please reverse out that patch until someone can find some time to look
> > at the issue.  (I will, if no one else does, but it would probably be
> > more efficient for someone who already knows something about SPI to
> > fix it...)
> 
> What is the problem? I'll research a SPI patch.
> 
> :) Clark
> 

I think someone already supplied such a patch.  Update via cvs and try
it.


-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Sun, 14 Mar 1999 13:49:00 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] libpq and SPI
Hello all,

> -----Original Message-----
> From: owner-pgsql-hackers@postgreSQL.org
> [mailto:owner-pgsql-hackers@postgreSQL.org]On Behalf Of Tom Lane
> Sent: Monday, March 15, 1999 3:21 AM
> To: Clark Evans
> Cc: pgsql-hackers@postgreSQL.org
> Subject: Re: [HACKERS] libpq and SPI 
> 
> 
> > What is the problem? I'll research a SPI patch.
> 
> Check the hackers thread (last month I think) titled "libpq and SPI";
> the problem occurs when one submits a utility statement rather than
> a plannable query via SPI.  Apparently what is happening is that the
> backend emits a 'T' message before it invokes the called statement
> and then emits a 'D' message afterwards --- so if the called statement
> causes a 'C' message to come out, libpq gets unhappy.  This seems
> to be clearly a violation of the FE/BE protocol to me, so I don't think
> it's libpq's fault.
> 
> Reasonable fixes might be to postpone the sending of 'T' till after
> the invoked statement is executed, or to modify the traffic cop so
> that a utility statement invoked from SPI doesn't send 'C'.
> 
> I know a little bit about the parts of the backend that communicate with
> the frontend, but nothing about SPI, so I'm not well prepared to solve
> the problem by myself.
>

Probably it's not the problem of SPI.
Specific utility commands(CREATE USER/ALTER USER/DROP USER
/CREATE DATABASE/DROP DATABASE) break FE/BE protocol 
when they are executed inside the PostgreSQL function call.

They call pg_exec_query() (tcop/postgres.c) in their implementation. 
In many cases the destination of pg_exec_query() is "Remote" 
which means that results are sent to frontend process.  But the 
results shouldn't be sent to the frontend directly inside the execution 
of PostgreSQL function.

ProcessUtility() (tcop/utility.c) function processes utility commands 
and takes the destination of command results as its second parameter. 
So should we call pg_exec_query_dest(query_string,  the destination 
parameter of ProcessUtility(), FALSE) instead of pg_exec_query(),
shouldn't we ? 

Thanks.

Hiroshi Inoue
Inoue@tpf.co.jp

Mon, 15 Mar 1999 19:00:10 +0900
"Hiroshi Inoue" <Inoue@tpf.co.jp>
RE: [HACKERS] libpq and SPI 
>
> > What is the problem? I'll research a SPI patch.
>
> Check the hackers thread (last month I think) titled "libpq and SPI";
> the problem occurs when one submits a utility statement rather than
> a plannable query via SPI.  Apparently what is happening is that the
> backend emits a 'T' message before it invokes the called statement
> and then emits a 'D' message afterwards --- so if the called statement
> causes a 'C' message to come out, libpq gets unhappy.  This seems
> to be clearly a violation of the FE/BE protocol to me, so I don't think
> it's libpq's fault.
>
> Reasonable fixes might be to postpone the sending of 'T' till after
> the invoked statement is executed, or to modify the traffic cop so
> that a utility statement invoked from SPI doesn't send 'C'.
>
> I know a little bit about the parts of the backend that communicate with
> the frontend, but nothing about SPI, so I'm not well prepared to solve
> the problem by myself.

    Another  major  problem  of  SPI  on utilities must get fixed
    prior.

    SPI  cannot  save  a  prepared  plan  for  utilities  because
    copyObject()  doesn't handle the utility part of Query. So it
    get's NULL and invoking such a  saved  plan  will  cause  the
    backend to crash.


Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.                                  #
#======================================== jwieck@debis.com (Jan Wieck) #



Mon, 15 Mar 1999 17:04:21 +0100 (MET)
jwieck@debis.com (Jan Wieck)
Re: [HACKERS] libpq and SPI

Are you ready to install the blinking map in the main site?


-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Mon, 15 Mar 1999 11:53:56 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
developers globe
>
>
> Are you ready to install the blinking map in the main site?

    Not yet.

    I  still have problems with the incompatibilities between NS4
    and IE4.  It's terrible.

    I have the whole map inside a centered table entry.

    If I don't put  the  main  image  (the  map  itself)  into  a
    diversion,  I  can  get  the  x,y location of the image after
    loading in NS via the undocumented non-standard properties  x
    and y. But then I'm unable to get the position in IE4.

    OTOH,  if I put the image into a diversion with a style, then
    I can get the position in both browsers. But this  time,  NS4
    does  very strange things if I resize the browser window (the
    image jumps out of table into the upper left  corner  of  the
    window :-}).

    I'm  still  playing  around  with  it locally. Hope to have a
    solution later today.


Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.                                  #
#======================================== jwieck@debis.com (Jan Wieck) #



Mon, 15 Mar 1999 18:21:51 +0100 (MET)
jwieck@debis.com (Jan Wieck)
Re: developers globe
Hello all,

> -----Original Message-----
> From: owner-pgsql-hackers@postgreSQL.org
> [mailto:owner-pgsql-hackers@postgreSQL.org]On Behalf Of Hiroshi Inoue
> Sent: Monday, March 15, 1999 7:00 PM
> To: Tom Lane; Clark Evans
> Cc: pgsql-hackers@postgreSQL.org
> Subject: RE: [HACKERS] libpq and SPI
>
> >
> >
> > > What is the problem? I'll research a SPI patch.
> >
> > Check the hackers thread (last month I think) titled "libpq and SPI";
> > the problem occurs when one submits a utility statement rather than
> > a plannable query via SPI.  Apparently what is happening is that the
> > backend emits a 'T' message before it invokes the called statement
> > and then emits a 'D' message afterwards --- so if the called statement
> > causes a 'C' message to come out, libpq gets unhappy.  This seems
> > to be clearly a violation of the FE/BE protocol to me, so I don't think
> > it's libpq's fault.
> >
> > Reasonable fixes might be to postpone the sending of 'T' till after
> > the invoked statement is executed, or to modify the traffic cop so
> > that a utility statement invoked from SPI doesn't send 'C'.
> >
> > I know a little bit about the parts of the backend that communicate with
> > the frontend, but nothing about SPI, so I'm not well prepared to solve
> > the problem by myself.
> >
>
> Probably it's not the problem of SPI.
> Specific utility commands(CREATE USER/ALTER USER/DROP USER
> /CREATE DATABASE/DROP DATABASE) break FE/BE protocol
> when they are executed inside the PostgreSQL function call.
>

Here is a patch.
I have changed to call pg_exec_query_dest() instead of pg_exec_query().

Thanks.

Hiroshi Inoue
Inoue@tpf.co.jp


*** backend/tcop/utility.c.orig	Thu Feb 18 17:01:27 1999
--- backend/tcop/utility.c	Tue Mar 16 09:25:07 1999
***************
*** 57,65 ****
  #include "utils/syscache.h"
  #endif

! void		DefineUser(CreateUserStmt *stmt);
! void		AlterUser(AlterUserStmt *stmt);
! void		RemoveUser(char *username);

  /* ----------------
   *		CHECK_IF_ABORTED() is used to avoid doing unnecessary
--- 57,65 ----
  #include "utils/syscache.h"
  #endif

! void		DefineUser(CreateUserStmt *stmt, CommandDest);
! void		AlterUser(AlterUserStmt *stmt, CommandDest);
! void		RemoveUser(char *username, CommandDest);

  /* ----------------
   *		CHECK_IF_ABORTED() is used to avoid doing unnecessary
***************
*** 558,564 ****

  				PS_SET_STATUS(commandTag = "CREATEDB");
  				CHECK_IF_ABORTED();
! 				createdb(stmt->dbname, stmt->dbpath, stmt->encoding);
  			}
  			break;

--- 558,564 ----

  				PS_SET_STATUS(commandTag = "CREATEDB");
  				CHECK_IF_ABORTED();
! 				createdb(stmt->dbname, stmt->dbpath, stmt->encoding, dest);
  			}
  			break;

***************
*** 568,574 ****

  				PS_SET_STATUS(commandTag = "DESTROYDB");
  				CHECK_IF_ABORTED();
! 				destroydb(stmt->dbname);
  			}
  			break;

--- 568,574 ----

  				PS_SET_STATUS(commandTag = "DESTROYDB");
  				CHECK_IF_ABORTED();
! 				destroydb(stmt->dbname, dest);
  			}
  			break;

***************
*** 748,754 ****
  			PS_SET_STATUS(commandTag = "CREATE USER");
  			CHECK_IF_ABORTED();

! 			DefineUser((CreateUserStmt *) parsetree);
  			break;

  		case T_AlterUserStmt:
--- 748,754 ----
  			PS_SET_STATUS(commandTag = "CREATE USER");
  			CHECK_IF_ABORTED();

! 			DefineUser((CreateUserStmt *) parsetree, dest);
  			break;

  		case T_AlterUserStmt:
***************
*** 755,761 ****
  			PS_SET_STATUS(commandTag = "ALTER USER");
  			CHECK_IF_ABORTED();

! 			AlterUser((AlterUserStmt *) parsetree);
  			break;

  		case T_DropUserStmt:
--- 755,761 ----
  			PS_SET_STATUS(commandTag = "ALTER USER");
  			CHECK_IF_ABORTED();

! 			AlterUser((AlterUserStmt *) parsetree, dest);
  			break;

  		case T_DropUserStmt:
***************
*** 762,768 ****
  			PS_SET_STATUS(commandTag = "DROP USER");
  			CHECK_IF_ABORTED();

! 			RemoveUser(((DropUserStmt *) parsetree)->user);
  			break;

  		case T_LockStmt:
--- 762,768 ----
  			PS_SET_STATUS(commandTag = "DROP USER");
  			CHECK_IF_ABORTED();

! 			RemoveUser(((DropUserStmt *) parsetree)->user, dest);
  			break;

  		case T_LockStmt:
*** backend/commands/user.c.orig	Thu Feb 18 17:00:38 1999
--- backend/commands/user.c	Tue Mar 16 09:50:09 1999
***************
*** 46,52 ****
   */
  static
  void
! UpdatePgPwdFile(char *sql)
  {

  	char	*filename,
--- 46,52 ----
   */
  static
  void
! UpdatePgPwdFile(char *sql, CommandDest dest)
  {

  	char	*filename,
***************
*** 71,77 ****
  	snprintf(sql, SQL_LENGTH,
  			"copy %s to '%s' using delimiters %s",
  			ShadowRelationName, tempname, CRYPT_PWD_FILE_SEPCHAR);
! 	pg_exec_query(sql);
  	rename(tempname, filename);
  	pfree((void *) tempname);

--- 71,77 ----
  	snprintf(sql, SQL_LENGTH,
  			"copy %s to '%s' using delimiters %s",
  			ShadowRelationName, tempname, CRYPT_PWD_FILE_SEPCHAR);
! 	pg_exec_query_dest(sql, dest, false);
  	rename(tempname, filename);
  	pfree((void *) tempname);

***************
*** 92,98 ****
   *---------------------------------------------------------------------
   */
  void
! DefineUser(CreateUserStmt *stmt)
  {

  	char					*pg_shadow,
--- 92,98 ----
   *---------------------------------------------------------------------
   */
  void
! DefineUser(CreateUserStmt *stmt, CommandDest dest)
  {

  	char					*pg_shadow,
***************
*** 175,187 ****
  			stmt->password ? stmt->password : "''",
  			stmt->validUntil ? stmt->validUntil : "");

! 	pg_exec_query(sql);

  	/*
  	 * Add the stuff here for groups.
  	 */

! 	UpdatePgPwdFile(sql);

  	/*
  	 * This goes after the UpdatePgPwdFile to be certain that two backends
--- 175,187 ----
  			stmt->password ? stmt->password : "''",
  			stmt->validUntil ? stmt->validUntil : "");

! 	pg_exec_query_dest(sql, dest, false);

  	/*
  	 * Add the stuff here for groups.
  	 */

! 	UpdatePgPwdFile(sql, dest);

  	/*
  	 * This goes after the UpdatePgPwdFile to be certain that two backends
***************
*** 196,202 ****


  extern void
! AlterUser(AlterUserStmt *stmt)
  {

  	char			*pg_shadow,
--- 196,202 ----


  extern void
! AlterUser(AlterUserStmt *stmt, CommandDest dest)
  {

  	char			*pg_shadow,
***************
*** 282,292 ****

  	snprintf(sql, SQL_LENGTH, "%s where usename = '%s'", sql, stmt->user);

! 	pg_exec_query(sql);

  	/* do the pg_group stuff here */

! 	UpdatePgPwdFile(sql);

  	UnlockRelation(pg_shadow_rel, AccessExclusiveLock);
  	heap_close(pg_shadow_rel);
--- 282,292 ----

  	snprintf(sql, SQL_LENGTH, "%s where usename = '%s'", sql, stmt->user);

! 	pg_exec_query_dest(sql, dest, false);

  	/* do the pg_group stuff here */

! 	UpdatePgPwdFile(sql, dest);

  	UnlockRelation(pg_shadow_rel, AccessExclusiveLock);
  	heap_close(pg_shadow_rel);
***************
*** 297,303 ****


  extern void
! RemoveUser(char *user)
  {

  	char	   *pg_shadow;
--- 297,303 ----


  extern void
! RemoveUser(char *user, CommandDest dest)
  {

  	char	   *pg_shadow;
***************
*** 390,396 ****
  		elog(NOTICE, "Dropping database %s", dbase[ndbase]);
  		snprintf(sql, SQL_LENGTH, "drop database %s", dbase[ndbase]);
  		pfree((void *) dbase[ndbase]);
! 		pg_exec_query(sql);
  	}
  	if (dbase)
  		pfree((void *) dbase);
--- 390,396 ----
  		elog(NOTICE, "Dropping database %s", dbase[ndbase]);
  		snprintf(sql, SQL_LENGTH, "drop database %s", dbase[ndbase]);
  		pfree((void *) dbase[ndbase]);
! 		pg_exec_query_dest(sql, dest, false);
  	}
  	if (dbase)
  		pfree((void *) dbase);
***************
*** 418,426 ****
  	 */
  	snprintf(sql, SQL_LENGTH,
  			"delete from %s where usename = '%s'", ShadowRelationName, user);
! 	pg_exec_query(sql);

! 	UpdatePgPwdFile(sql);

  	UnlockRelation(pg_shadow_rel, AccessExclusiveLock);
  	heap_close(pg_shadow_rel);
--- 418,426 ----
  	 */
  	snprintf(sql, SQL_LENGTH,
  			"delete from %s where usename = '%s'", ShadowRelationName, user);
! 	pg_exec_query_dest(sql, dest, false);

! 	UpdatePgPwdFile(sql, dest);

  	UnlockRelation(pg_shadow_rel, AccessExclusiveLock);
  	heap_close(pg_shadow_rel);
*** backend/commands/dbcommands.c.orig	Thu Feb 18 17:00:36 1999
--- backend/commands/dbcommands.c	Tue Mar 16 09:36:33 1999
***************
*** 24,30 ****
  #include "catalog/catname.h"
  #include "catalog/pg_database.h"
  #include "catalog/pg_shadow.h"
- #include "commands/dbcommands.h"
  #include "fmgr.h"
  #include "miscadmin.h"			/* for DataDir */
  #include "storage/bufmgr.h"
--- 24,29 ----
***************
*** 31,36 ****
--- 30,36 ----
  #include "storage/fd.h"
  #include "storage/lmgr.h"
  #include "tcop/tcopprot.h"
+ #include "commands/dbcommands.h"
  #include "utils/rel.h"
  #include "utils/syscache.h"

***************
*** 42,48 ****
  static void stop_vacuum(char *dbpath, char *dbname);

  void
! createdb(char *dbname, char *dbpath, int encoding)
  {
  	Oid			db_id;
  	int4		user_id;
--- 42,48 ----
  static void stop_vacuum(char *dbpath, char *dbname);

  void
! createdb(char *dbname, char *dbpath, int encoding, CommandDest dest)
  {
  	Oid			db_id;
  	int4		user_id;
***************
*** 87,97 ****
  			"insert into pg_database (datname, datdba, encoding, datpath)"
  			" values ('%s', '%d', '%d', '%s');", dbname, user_id, encoding,
loc);

! 	pg_exec_query(buf);
  }

  void
! destroydb(char *dbname)
  {
  	int4		user_id;
  	Oid			db_id;
--- 87,97 ----
  			"insert into pg_database (datname, datdba, encoding, datpath)"
  			" values ('%s', '%d', '%d', '%s');", dbname, user_id, encoding,
loc);

! 	pg_exec_query_dest(buf, dest, false);
  }

  void
! destroydb(char *dbname, CommandDest dest)
  {
  	int4		user_id;
  	Oid			db_id;
***************
*** 123,129 ****
  	 */
  	snprintf(buf, 512,
  			"delete from pg_database where pg_database.oid = \'%d\'::oid",
db_id);
! 	pg_exec_query(buf);

  	/*
  	 * remove the data directory. If the DELETE above failed, this will
--- 123,129 ----
  	 */
  	snprintf(buf, 512,
  			"delete from pg_database where pg_database.oid = \'%d\'::oid",
db_id);
! 	pg_exec_query_dest(buf ,dest, false);

  	/*
  	 * remove the data directory. If the DELETE above failed, this will
*** include/commands/user.h.orig	Thu Feb 18 17:01:49 1999
--- include/commands/user.h	Tue Mar 16 09:23:01 1999
***************
*** 10,17 ****
  #ifndef USER_H
  #define USER_H

! extern void DefineUser(CreateUserStmt *stmt);
! extern void AlterUser(AlterUserStmt *stmt);
! extern void RemoveUser(char *user);

  #endif	 /* USER_H */
--- 10,17 ----
  #ifndef USER_H
  #define USER_H

! extern void DefineUser(CreateUserStmt *stmt, CommandDest);
! extern void AlterUser(AlterUserStmt *stmt, CommandDest);
! extern void RemoveUser(char *user, CommandDest);

  #endif	 /* USER_H */
*** include/commands/dbcommands.h.orig	Thu Feb 18 17:01:48 1999
--- include/commands/dbcommands.h	Tue Mar 16 09:23:52 1999
***************
*** 19,25 ****
   */
  #define SIGKILLDAEMON1	SIGTERM

! extern void createdb(char *dbname, char *dbpath, int encoding);
! extern void destroydb(char *dbname);

  #endif	 /* DBCOMMANDS_H */
--- 19,25 ----
   */
  #define SIGKILLDAEMON1	SIGTERM

! extern void createdb(char *dbname, char *dbpath, int encoding,
CommandDest);
! extern void destroydb(char *dbname, CommandDest);

  #endif	 /* DBCOMMANDS_H */






Tue, 16 Mar 1999 11:57:37 +0900
"Hiroshi Inoue" <Inoue@tpf.co.jp>
RE: [HACKERS] libpq and SPI 

Applied.


---------------------------------------------------------------------------
Hello all,

> -----Original Message-----
> From: owner-pgsql-hackers@postgreSQL.org
> [mailto:owner-pgsql-hackers@postgreSQL.org]On Behalf Of Hiroshi Inoue
> Sent: Monday, March 15, 1999 7:00 PM
> To: Tom Lane; Clark Evans
> Cc: pgsql-hackers@postgreSQL.org
> Subject: RE: [HACKERS] libpq and SPI
>
> >
> >
> > > What is the problem? I'll research a SPI patch.
> >
> > Check the hackers thread (last month I think) titled "libpq and SPI";
> > the problem occurs when one submits a utility statement rather than
> > a plannable query via SPI.  Apparently what is happening is that the
> > backend emits a 'T' message before it invokes the called statement
> > and then emits a 'D' message afterwards --- so if the called statement
> > causes a 'C' message to come out, libpq gets unhappy.  This seems
> > to be clearly a violation of the FE/BE protocol to me, so I don't think
> > it's libpq's fault.
> >
> > Reasonable fixes might be to postpone the sending of 'T' till after
> > the invoked statement is executed, or to modify the traffic cop so
> > that a utility statement invoked from SPI doesn't send 'C'.
> >
> > I know a little bit about the parts of the backend that communicate with
> > the frontend, but nothing about SPI, so I'm not well prepared to solve
> > the problem by myself.
> >
>
> Probably it's not the problem of SPI.
> Specific utility commands(CREATE USER/ALTER USER/DROP USER
> /CREATE DATABASE/DROP DATABASE) break FE/BE protocol
> when they are executed inside the PostgreSQL function call.
>

Here is a patch.
I have changed to call pg_exec_query_dest() instead of pg_exec_query().

Thanks.

Hiroshi Inoue
Inoue@tpf.co.jp


*** backend/tcop/utility.c.orig	Thu Feb 18 17:01:27 1999
--- backend/tcop/utility.c	Tue Mar 16 09:25:07 1999
***************
*** 57,65 ****
  #include "utils/syscache.h"
  #endif

! void		DefineUser(CreateUserStmt *stmt);
! void		AlterUser(AlterUserStmt *stmt);
! void		RemoveUser(char *username);

  /* ----------------
   *		CHECK_IF_ABORTED() is used to avoid doing unnecessary
--- 57,65 ----
  #include "utils/syscache.h"
  #endif

! void		DefineUser(CreateUserStmt *stmt, CommandDest);
! void		AlterUser(AlterUserStmt *stmt, CommandDest);
! void		RemoveUser(char *username, CommandDest);

  /* ----------------
   *		CHECK_IF_ABORTED() is used to avoid doing unnecessary
***************
*** 558,564 ****

  				PS_SET_STATUS(commandTag = "CREATEDB");
  				CHECK_IF_ABORTED();
! 				createdb(stmt->dbname, stmt->dbpath, stmt->encoding);
  			}
  			break;

--- 558,564 ----

  				PS_SET_STATUS(commandTag = "CREATEDB");
  				CHECK_IF_ABORTED();
! 				createdb(stmt->dbname, stmt->dbpath, stmt->encoding, dest);
  			}
  			break;

***************
*** 568,574 ****

  				PS_SET_STATUS(commandTag = "DESTROYDB");
  				CHECK_IF_ABORTED();
! 				destroydb(stmt->dbname);
  			}
  			break;

--- 568,574 ----

  				PS_SET_STATUS(commandTag = "DESTROYDB");
  				CHECK_IF_ABORTED();
! 				destroydb(stmt->dbname, dest);
  			}
  			break;

***************
*** 748,754 ****
  			PS_SET_STATUS(commandTag = "CREATE USER");
  			CHECK_IF_ABORTED();

! 			DefineUser((CreateUserStmt *) parsetree);
  			break;

  		case T_AlterUserStmt:
--- 748,754 ----
  			PS_SET_STATUS(commandTag = "CREATE USER");
  			CHECK_IF_ABORTED();

! 			DefineUser((CreateUserStmt *) parsetree, dest);
  			break;

  		case T_AlterUserStmt:
***************
*** 755,761 ****
  			PS_SET_STATUS(commandTag = "ALTER USER");
  			CHECK_IF_ABORTED();

! 			AlterUser((AlterUserStmt *) parsetree);
  			break;

  		case T_DropUserStmt:
--- 755,761 ----
  			PS_SET_STATUS(commandTag = "ALTER USER");
  			CHECK_IF_ABORTED();

! 			AlterUser((AlterUserStmt *) parsetree, dest);
  			break;

  		case T_DropUserStmt:
***************
*** 762,768 ****
  			PS_SET_STATUS(commandTag = "DROP USER");
  			CHECK_IF_ABORTED();

! 			RemoveUser(((DropUserStmt *) parsetree)->user);
  			break;

  		case T_LockStmt:
--- 762,768 ----
  			PS_SET_STATUS(commandTag = "DROP USER");
  			CHECK_IF_ABORTED();

! 			RemoveUser(((DropUserStmt *) parsetree)->user, dest);
  			break;

  		case T_LockStmt:
*** backend/commands/user.c.orig	Thu Feb 18 17:00:38 1999
--- backend/commands/user.c	Tue Mar 16 09:50:09 1999
***************
*** 46,52 ****
   */
  static
  void
! UpdatePgPwdFile(char *sql)
  {

  	char	*filename,
--- 46,52 ----
   */
  static
  void
! UpdatePgPwdFile(char *sql, CommandDest dest)
  {

  	char	*filename,
***************
*** 71,77 ****
  	snprintf(sql, SQL_LENGTH,
  			"copy %s to '%s' using delimiters %s",
  			ShadowRelationName, tempname, CRYPT_PWD_FILE_SEPCHAR);
! 	pg_exec_query(sql);
  	rename(tempname, filename);
  	pfree((void *) tempname);

--- 71,77 ----
  	snprintf(sql, SQL_LENGTH,
  			"copy %s to '%s' using delimiters %s",
  			ShadowRelationName, tempname, CRYPT_PWD_FILE_SEPCHAR);
! 	pg_exec_query_dest(sql, dest, false);
  	rename(tempname, filename);
  	pfree((void *) tempname);

***************
*** 92,98 ****
   *---------------------------------------------------------------------
   */
  void
! DefineUser(CreateUserStmt *stmt)
  {

  	char					*pg_shadow,
--- 92,98 ----
   *---------------------------------------------------------------------
   */
  void
! DefineUser(CreateUserStmt *stmt, CommandDest dest)
  {

  	char					*pg_shadow,
***************
*** 175,187 ****
  			stmt->password ? stmt->password : "''",
  			stmt->validUntil ? stmt->validUntil : "");

! 	pg_exec_query(sql);

  	/*
  	 * Add the stuff here for groups.
  	 */

! 	UpdatePgPwdFile(sql);

  	/*
  	 * This goes after the UpdatePgPwdFile to be certain that two backends
--- 175,187 ----
  			stmt->password ? stmt->password : "''",
  			stmt->validUntil ? stmt->validUntil : "");

! 	pg_exec_query_dest(sql, dest, false);

  	/*
  	 * Add the stuff here for groups.
  	 */

! 	UpdatePgPwdFile(sql, dest);

  	/*
  	 * This goes after the UpdatePgPwdFile to be certain that two backends
***************
*** 196,202 ****


  extern void
! AlterUser(AlterUserStmt *stmt)
  {

  	char			*pg_shadow,
--- 196,202 ----


  extern void
! AlterUser(AlterUserStmt *stmt, CommandDest dest)
  {

  	char			*pg_shadow,
***************
*** 282,292 ****

  	snprintf(sql, SQL_LENGTH, "%s where usename = '%s'", sql, stmt->user);

! 	pg_exec_query(sql);

  	/* do the pg_group stuff here */

! 	UpdatePgPwdFile(sql);

  	UnlockRelation(pg_shadow_rel, AccessExclusiveLock);
  	heap_close(pg_shadow_rel);
--- 282,292 ----

  	snprintf(sql, SQL_LENGTH, "%s where usename = '%s'", sql, stmt->user);

! 	pg_exec_query_dest(sql, dest, false);

  	/* do the pg_group stuff here */

! 	UpdatePgPwdFile(sql, dest);

  	UnlockRelation(pg_shadow_rel, AccessExclusiveLock);
  	heap_close(pg_shadow_rel);
***************
*** 297,303 ****


  extern void
! RemoveUser(char *user)
  {

  	char	   *pg_shadow;
--- 297,303 ----


  extern void
! RemoveUser(char *user, CommandDest dest)
  {

  	char	   *pg_shadow;
***************
*** 390,396 ****
  		elog(NOTICE, "Dropping database %s", dbase[ndbase]);
  		snprintf(sql, SQL_LENGTH, "drop database %s", dbase[ndbase]);
  		pfree((void *) dbase[ndbase]);
! 		pg_exec_query(sql);
  	}
  	if (dbase)
  		pfree((void *) dbase);
--- 390,396 ----
  		elog(NOTICE, "Dropping database %s", dbase[ndbase]);
  		snprintf(sql, SQL_LENGTH, "drop database %s", dbase[ndbase]);
  		pfree((void *) dbase[ndbase]);
! 		pg_exec_query_dest(sql, dest, false);
  	}
  	if (dbase)
  		pfree((void *) dbase);
***************
*** 418,426 ****
  	 */
  	snprintf(sql, SQL_LENGTH,
  			"delete from %s where usename = '%s'", ShadowRelationName, user);
! 	pg_exec_query(sql);

! 	UpdatePgPwdFile(sql);

  	UnlockRelation(pg_shadow_rel, AccessExclusiveLock);
  	heap_close(pg_shadow_rel);
--- 418,426 ----
  	 */
  	snprintf(sql, SQL_LENGTH,
  			"delete from %s where usename = '%s'", ShadowRelationName, user);
! 	pg_exec_query_dest(sql, dest, false);

! 	UpdatePgPwdFile(sql, dest);

  	UnlockRelation(pg_shadow_rel, AccessExclusiveLock);
  	heap_close(pg_shadow_rel);
*** backend/commands/dbcommands.c.orig	Thu Feb 18 17:00:36 1999
--- backend/commands/dbcommands.c	Tue Mar 16 09:36:33 1999
***************
*** 24,30 ****
  #include "catalog/catname.h"
  #include "catalog/pg_database.h"
  #include "catalog/pg_shadow.h"
- #include "commands/dbcommands.h"
  #include "fmgr.h"
  #include "miscadmin.h"			/* for DataDir */
  #include "storage/bufmgr.h"
--- 24,29 ----
***************
*** 31,36 ****
--- 30,36 ----
  #include "storage/fd.h"
  #include "storage/lmgr.h"
  #include "tcop/tcopprot.h"
+ #include "commands/dbcommands.h"
  #include "utils/rel.h"
  #include "utils/syscache.h"

***************
*** 42,48 ****
  static void stop_vacuum(char *dbpath, char *dbname);

  void
! createdb(char *dbname, char *dbpath, int encoding)
  {
  	Oid			db_id;
  	int4		user_id;
--- 42,48 ----
  static void stop_vacuum(char *dbpath, char *dbname);

  void
! createdb(char *dbname, char *dbpath, int encoding, CommandDest dest)
  {
  	Oid			db_id;
  	int4		user_id;
***************
*** 87,97 ****
  			"insert into pg_database (datname, datdba, encoding, datpath)"
  			" values ('%s', '%d', '%d', '%s');", dbname, user_id, encoding,
loc);

! 	pg_exec_query(buf);
  }

  void
! destroydb(char *dbname)
  {
  	int4		user_id;
  	Oid			db_id;
--- 87,97 ----
  			"insert into pg_database (datname, datdba, encoding, datpath)"
  			" values ('%s', '%d', '%d', '%s');", dbname, user_id, encoding,
loc);

! 	pg_exec_query_dest(buf, dest, false);
  }

  void
! destroydb(char *dbname, CommandDest dest)
  {
  	int4		user_id;
  	Oid			db_id;
***************
*** 123,129 ****
  	 */
  	snprintf(buf, 512,
  			"delete from pg_database where pg_database.oid = \'%d\'::oid",
db_id);
! 	pg_exec_query(buf);

  	/*
  	 * remove the data directory. If the DELETE above failed, this will
--- 123,129 ----
  	 */
  	snprintf(buf, 512,
  			"delete from pg_database where pg_database.oid = \'%d\'::oid",
db_id);
! 	pg_exec_query_dest(buf ,dest, false);

  	/*
  	 * remove the data directory. If the DELETE above failed, this will
*** include/commands/user.h.orig	Thu Feb 18 17:01:49 1999
--- include/commands/user.h	Tue Mar 16 09:23:01 1999
***************
*** 10,17 ****
  #ifndef USER_H
  #define USER_H

! extern void DefineUser(CreateUserStmt *stmt);
! extern void AlterUser(AlterUserStmt *stmt);
! extern void RemoveUser(char *user);

  #endif	 /* USER_H */
--- 10,17 ----
  #ifndef USER_H
  #define USER_H

! extern void DefineUser(CreateUserStmt *stmt, CommandDest);
! extern void AlterUser(AlterUserStmt *stmt, CommandDest);
! extern void RemoveUser(char *user, CommandDest);

  #endif	 /* USER_H */
*** include/commands/dbcommands.h.orig	Thu Feb 18 17:01:48 1999
--- include/commands/dbcommands.h	Tue Mar 16 09:23:52 1999
***************
*** 19,25 ****
   */
  #define SIGKILLDAEMON1	SIGTERM

! extern void createdb(char *dbname, char *dbpath, int encoding);
! extern void destroydb(char *dbname);

  #endif	 /* DBCOMMANDS_H */
--- 19,25 ----
   */
  #define SIGKILLDAEMON1	SIGTERM

! extern void createdb(char *dbname, char *dbpath, int encoding,
CommandDest);
! extern void destroydb(char *dbname, CommandDest);

  #endif	 /* DBCOMMANDS_H */








-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Mon, 15 Mar 1999 22:24:25 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] libpq and SPI

Is this fixed?


> > What is the problem? I'll research a SPI patch.
> 
> Check the hackers thread (last month I think) titled "libpq and SPI";
> the problem occurs when one submits a utility statement rather than
> a plannable query via SPI.  Apparently what is happening is that the
> backend emits a 'T' message before it invokes the called statement
> and then emits a 'D' message afterwards --- so if the called statement
> causes a 'C' message to come out, libpq gets unhappy.  This seems
> to be clearly a violation of the FE/BE protocol to me, so I don't think
> it's libpq's fault.
> 
> Reasonable fixes might be to postpone the sending of 'T' till after
> the invoked statement is executed, or to modify the traffic cop so
> that a utility statement invoked from SPI doesn't send 'C'.
> 
> I know a little bit about the parts of the backend that communicate with
> the frontend, but nothing about SPI, so I'm not well prepared to solve
> the problem by myself.
> 
> 			regards, tom lane
> 
> 


-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Sun, 9 May 1999 16:10:52 -0400 (EDT)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] libpq and SPI