Re: Limit usage of tcop/dest.h

Hackers,

The "Debug" destreceiver has a pretty bad name. In fact for PL/php it
collides with a PHP symbol. Does anyone mind if I rename it to, say,
DestDebug?

--
Alvaro Herrera http://www.advogato.org/person/alvherre
"El sabio habla porque tiene algo que decir;
el tonto, porque tiene que decir algo" (Platon).

Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> writes:
> The "Debug" destreceiver has a pretty bad name. In fact for PL/php it
> collides with a PHP symbol. Does anyone mind if I rename it to, say,
> DestDebug?

If you do that, you should rename all the values of the enum
(DestNone, etc). For consistency, and because the same charge
could be leveled at None and Remote.

regards, tom lane

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

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)

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)

Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> 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

Tom Lane wrote:
> Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> 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(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Bruce Momjian wrote:
> Tom Lane wrote:
> > Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> 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)

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)

Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> 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