Thread: Re: [HACKERS] libpq and SPI

Re: [HACKERS] libpq and SPI

From
"Gerald L. Gay"
Date:
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


Re: [HACKERS] libpq and SPI

From
Bruce Momjian
Date:
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,
Pennsylvania19026
 


Re: [HACKERS] libpq and SPI

From
Tom Lane
Date:
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


Re: [HACKERS] libpq and SPI

From
Clark Evans
Date:
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


Re: [HACKERS] libpq and SPI

From
Bruce Momjian
Date:
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,
Pennsylvania19026 
--- 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':        /*
emptyquery */                    /* 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                         *
thecurrent 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
characterthe " 

Re: [HACKERS] libpq and SPI

From
Tom Lane
Date:
> 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


Re: [HACKERS] libpq and SPI

From
Bruce Momjian
Date:
> 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,
Pennsylvania19026
 


RE: [HACKERS] libpq and SPI

From
"Hiroshi Inoue"
Date:
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


Re: [HACKERS] libpq and SPI

From
jwieck@debis.com (Jan Wieck)
Date:
>
> > 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) #

developers globe

From
Bruce Momjian
Date:
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,
Pennsylvania19026
 


Re: developers globe

From
jwieck@debis.com (Jan Wieck)
Date:
>
>
> 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) #

RE: [HACKERS] libpq and SPI

From
"Hiroshi Inoue"
Date:
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 */







Re: [HACKERS] libpq and SPI

From
Bruce Momjian
Date:
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,
Pennsylvania19026
 


Re: [HACKERS] libpq and SPI

From
Bruce Momjian
Date:
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,
Pennsylvania19026