Re: [HACKERS] libpq and SPI

Applied.

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

> -----Original Message-----
> From: owner-pgsql-hackers(at)postgreSQL(dot)org
> [mailto:owner-pgsql-hackers(at)postgreSQL(dot)org]On Behalf Of Hiroshi Inoue
> Sent: Monday, March 15, 1999 7:00 PM
> To: Tom Lane; Clark Evans
> Cc: pgsql-hackers(at)postgreSQL(dot)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(at)tpf(dot)co(dot)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(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026