Re: [PATCHES] Limit usage of tcop/dest.h
Merlin Moncure wrote: Tom Lane wrote: I thought renaming them was a better idea, actually. Here is a patch for that. I will apply this to HEAD later today. I'm getting compiler error (win32) which I think is related to this patch: Oops, certainly. Sorry, fixed. I looked for other cases of symbols in EXEC_BACKEND and didn't find one. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] Limit usage of tcop/dest.h
Tom Lane wrote: I thought renaming them was a better idea, actually. Here is a patch for that. I will apply this to HEAD later today. -- I'm getting compiler error (win32) which I think is related to this patch: postmaster.c: In function `SubPostmasterMain': postmaster.c:3189: error: `None' undeclared (first use in this function) postmaster.c:3189: error: (Each undeclared identifier is reported only once postmaster.c:3189: error: for each function it appears in.) make[2]: *** [postmaster.o] Error 1 make[2]: Leaving directory `/postgres/pgsql/src/backend/postmaster' make[1]: *** [postmaster-recursive] Error 2 Merlin ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
[PATCHES] Limit usage of tcop/dest.h
Hi, I just found out that tcop/dest.h is included in executor/spi.h, and it contains many things that aren't needed for compiling SPI programs/ libraries. By way of example I compiled the whole of contrib with the attached patch and it works fine. Notice that the only thing I'm doing is taking the forward declaration of Portal into a separate file, portalforw.h -- that's the only definition that's really needed by SPI programs. (It also allows PL/php to compile without having to patch PHP nor PostgreSQL sources). Note that since tcop/dest.h now includes portalforw.h, anybody who currently needs the Portal definition is still getting it. The only thing I'm doing is un-export the rest of tcop/dest.h from executor/spi.h. So instead of changing the names of the CommandDest enum, I'm hiding it from external view. Note that executor/spi.h does not follow the convention that #includes should be alphabetically ordered. I did not change that in this patch in order to show that this change is really minimal. Does anybody object to committing this patch to current CVS HEAD? (Comments about a better position/name for the new file are welcome.) -- Alvaro Herrera http://www.amazon.com/gp/registry/DXLWNGRJD34J The only difference is that Saddam would kill you on private, where the Americans will kill you in public (Mohammad Saleh, 39, a building contractor) Index: executor/spi.h === RCS file: /home/alvherre/Code/cvs/pgsql/src/include/executor/spi.h,v retrieving revision 1.53 diff -c -r1.53 spi.h *** executor/spi.h 15 Oct 2005 02:49:44 - 1.53 --- executor/spi.h 2 Nov 2005 10:32:33 - *** *** 28,34 #include tcop/pquery.h #include tcop/tcopprot.h #include tcop/utility.h ! #include tcop/dest.h #include nodes/params.h #include utils/builtins.h #include utils/datum.h --- 28,34 #include tcop/pquery.h #include tcop/tcopprot.h #include tcop/utility.h ! #include executor/portalforw.h #include nodes/params.h #include utils/builtins.h #include utils/datum.h Index: tcop/dest.h === RCS file: /home/alvherre/Code/cvs/pgsql/src/include/tcop/dest.h,v retrieving revision 1.47 diff -c -r1.47 dest.h *** tcop/dest.h 15 Oct 2005 02:49:46 - 1.47 --- tcop/dest.h 2 Nov 2005 10:18:40 - *** *** 62,67 --- 62,68 #define DEST_H #include executor/tuptable.h + #include executor/portalforw.h /* buffer size to use for command completion tags */ *** *** 117,126 extern DestReceiver *None_Receiver; /* permanent receiver for None */ - /* This is a forward reference to utils/portal.h */ - - typedef struct PortalData *Portal; - /* The primary destination management functions */ extern void BeginCommand(const char *commandTag, CommandDest dest); --- 118,123 /*- * * portalforw.h * * $PostgreSQL$ * *- */ #ifndef PORTALFORW_H #define PORTALFORW_H /* This is a forward reference to utils/portal.h */ typedef struct PortalData *Portal; #endif /* PORTALFORW_H */ ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] Limit usage of tcop/dest.h
Alvaro Herrera said: Hi, (It also allows PL/php to compile without having to patch PHP nor PostgreSQL sources). That will make some people I know happy ;-) cheers andrew ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] Limit usage of tcop/dest.h
Andrew Dunstan wrote: Alvaro Herrera said: Hi, (It also allows PL/php to compile without having to patch PHP nor PostgreSQL sources). That will make some people I know happy ;-) Yeah -- the current PL/php build system is a crock (not sure what that is, but it sounds nice and it appears on Pg sources) :-) so I'm currently modifying it to work using PGXS. They won't need Pg sources at all really (and conversely, only PHP headers and the shared lib, not the whole sources). -- Alvaro Herrera http://www.amazon.com/gp/registry/CTMLCN8V17R4 La realidad se compone de muchos sueños, todos ellos diferentes, pero en cierto aspecto, parecidos... (Yo, hablando de sueños eróticos) ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] Limit usage of tcop/dest.h
Alvaro Herrera [EMAIL PROTECTED] writes: So instead of changing the names of the CommandDest enum, I'm hiding it from external view. I thought renaming them was a better idea, actually. A whole separate include file to have one forward typedef seems pretty silly. Nor am I convinced that you won't break some people's code by removing the rest of dest.h from spi.h. Finally, for anyone who *does* need to include dest.h, this doesn't address the underlying problem of risk of conflict of names. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] Limit usage of tcop/dest.h
Tom Lane wrote: Alvaro Herrera [EMAIL PROTECTED] writes: So instead of changing the names of the CommandDest enum, I'm hiding it from external view. I thought renaming them was a better idea, actually. A whole separate include file to have one forward typedef seems pretty silly. Nor am I convinced that you won't break some people's code by removing the rest of dest.h from spi.h. Finally, for anyone who *does* need to include dest.h, this doesn't address the underlying problem of risk of conflict of names. Does the change make building PL/PHP easier? -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] Limit usage of tcop/dest.h
Bruce Momjian wrote: Tom Lane wrote: Alvaro Herrera [EMAIL PROTECTED] writes: So instead of changing the names of the CommandDest enum, I'm hiding it from external view. I thought renaming them was a better idea, actually. A whole separate include file to have one forward typedef seems pretty silly. Nor am I convinced that you won't break some people's code by removing the rest of dest.h from spi.h. Finally, for anyone who *does* need to include dest.h, this doesn't address the underlying problem of risk of conflict of names. Does the change make building PL/PHP easier? Yes, the point of these changes is to make PL/php much easier. Either one will do -- renaming the enum elements is what I'm doing now, so we don't have to change include file. (Mind you, I still believe that that particular declaration does not belong in that file, but that's a different discussion.) (We will still need some hack in order to build PL/php against 8.0, but that's another problem.) -- Alvaro Herrera http://www.amazon.com/gp/registry/DXLWNGRJD34J Nunca se desea ardientemente lo que solo se desea por razón (F. Alexandre) ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] Limit usage of tcop/dest.h
Tom Lane wrote:
I thought renaming them was a better idea, actually.
Here is a patch for that. I will apply this to HEAD later today.
--
Alvaro Herrera Valdivia, Chile ICBM: S 39º 49' 17.7, W 73º 14' 26.8
The eagle never lost so much time, as
when he submitted to learn of the crow. (William Blake)
Index: src/backend/access/common/printtup.c
===
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/common/printtup.c,v
retrieving revision 1.92
diff -c -r1.92 printtup.c
*** src/backend/access/common/printtup.c15 Oct 2005 02:49:08 -
1.92
--- src/backend/access/common/printtup.c2 Nov 2005 12:12:58 -
***
*** 94,101
self-portal = portal;
! /* Send T message automatically if Remote, but not if RemoteExecute */
! self-sendDescrip = (dest == Remote);
self-attrinfo = NULL;
self-nattrs = 0;
--- 94,104
self-portal = portal;
! /*
!* Send T message automatically if DestRemote, but not if
!* DestRemoteExecute
!*/
! self-sendDescrip = (dest == DestRemote);
self-attrinfo = NULL;
self-nattrs = 0;
Index: src/backend/commands/async.c
===
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/commands/async.c,v
retrieving revision 1.126
diff -c -r1.126 async.c
*** src/backend/commands/async.c15 Oct 2005 02:49:15 - 1.126
--- src/backend/commands/async.c2 Nov 2005 12:14:08 -
***
*** 994,1000
static void
NotifyMyFrontEnd(char *relname, int32 listenerPID)
{
! if (whereToSendOutput == Remote)
{
StringInfoData buf;
--- 994,1000
static void
NotifyMyFrontEnd(char *relname, int32 listenerPID)
{
! if (whereToSendOutput == DestRemote)
{
StringInfoData buf;
Index: src/backend/commands/copy.c
===
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/commands/copy.c,v
retrieving revision 1.253
diff -c -r1.253 copy.c
*** src/backend/commands/copy.c 15 Oct 2005 02:49:15 - 1.253
--- src/backend/commands/copy.c 2 Nov 2005 12:14:42 -
***
*** 966,972
}
if (pipe)
{
! if (whereToSendOutput == Remote)
ReceiveCopyBegin(cstate);
else
cstate-copy_file = stdin;
--- 966,972
}
if (pipe)
{
! if (whereToSendOutput == DestRemote)
ReceiveCopyBegin(cstate);
else
cstate-copy_file = stdin;
***
*** 1017,1023
}
if (pipe)
{
! if (whereToSendOutput == Remote)
cstate-fe_copy = true;
else
cstate-copy_file = stdout;
--- 1017,1023
}
if (pipe)
{
! if (whereToSendOutput == DestRemote)
cstate-fe_copy = true;
else
cstate-copy_file = stdout;
Index: src/backend/commands/portalcmds.c
===
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/commands/portalcmds.c,v
retrieving revision 1.43
diff -c -r1.43 portalcmds.c
*** src/backend/commands/portalcmds.c 15 Oct 2005 02:49:15 - 1.43
--- src/backend/commands/portalcmds.c 2 Nov 2005 12:49:43 -
***
*** 190,196
return; /* keep compiler happy
*/
}
! /* Adjust dest if needed. MOVE wants destination None */
if (stmt-ismove)
dest = None_Receiver;
--- 190,196
return; /* keep compiler happy
*/
}
! /* Adjust dest if needed. MOVE wants destination DestNone */
if (stmt-ismove)
dest = None_Receiver;
***
*** 369,375
ExecutorRewind(queryDesc);
/* Change the destination to output to the tuplestore */
! queryDesc-dest = CreateDestReceiver(Tuplestore, portal);
/* Fetch the result set into the tuplestore */
ExecutorRun(queryDesc, ForwardScanDirection, 0L);
--- 369,375
ExecutorRewind(queryDesc);
/* Change the destination to output to the tuplestore */
! queryDesc-dest = CreateDestReceiver(DestTuplestore, portal);
/* Fetch the
Re: [PATCHES] Limit usage of tcop/dest.h
Alvaro Herrera [EMAIL PROTECTED] writes: Tom Lane wrote: I thought renaming them was a better idea, actually. Here is a patch for that. I will apply this to HEAD later today. Looks ok in a quick eyeball pass. regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
