Re: [GENERAL] Re: copy to/from stdout using libpgtcl

g(dot)hintermayer(at)inode(dot)at wrote:
>has anyone ever successfully done copy to/from stdout with the
>tcl-extension for postgreSQL.
>I'm currently using 7.0 and always getting a seg fault when I try to
>read from the database connection after issueing a "COPY table TO
>stdout;" (I'm using the connection handle, *not* the result handle).
>Maybe this is fixed in a later release.
>The README file in src/interfaces/libpgtcl tells me, that this should
>work, but unforunately it doesn't.

Yes, it seems broken. It is a bug in libpgtcl. Are you running Tcl >= 8.3.2?
That's when the Tcl team changed the data structure for channel
callbacks. The change itself was designed to be backward compatible, but I
suspect a related change made the code more sensitive to errors in the
structure (NULL pointers where functions are required). Either that, or
nobody has tried to use libpgtcl with COPY in a long time.

First, I have to say I can't think of a good reason to use PostgreSQL's
COPY command from a Tcl application. I think it should only be used with
psql for importing data from another source into PostgreSQL, or for
exporting PostgreSQL data into another database (but why would anyone do
that?) If it was me, I would stick with SELECT and INSERT and be "SQL
Compliant".

OK, editorial is over. Try applying the patch below to fix
src/interfaces/libpgtcl/pgtclId.c
and let us know if it works. I did little testing on it, but my test did
segfault before and ran fine (copy in and copy out) after the patch. This
is for PostgreSQL-7.1.2 - since you are running older 7.0, I don't know if
this will work, but I suspect it will.

PS It's the absence of PgWatchProc which kills it. I didn't upgrade it
to the "V2" channel type structure, so it should be compatible with older
Tcl's. But aside from gets and puts, I doubt any other file operations
would work on the handle during a copy.
========================================================================

--- src/interfaces/libpgtcl/pgtclId.c.orig Fri Feb 9 21:31:29 2001
+++ src/interfaces/libpgtcl/pgtclId.c Sat Aug 11 16:55:14 2001
@@ -138,17 +138,32 @@

#endif

+/*
+ * The WatchProc and GetHandleProc are no-ops but must be present.
+ */
+static void
+PgWatchProc(ClientData instanceData, int mask)
+{
+}
+static int
+PgGetHandleProc(ClientData instanceData, int direction,
+ ClientData *handlePtr)
+{
+ return TCL_ERROR;
+}
+
Tcl_ChannelType Pg_ConnType = {
"pgsql", /* channel type */
NULL, /* blockmodeproc */
PgDelConnectionId, /* closeproc */
PgInputProc, /* inputproc */
PgOutputProc, /* outputproc */
-
- /*
- * Note the additional stuff can be left NULL, or is initialized
- * during a PgSetConnectionId
- */
+ NULL, /* SeekProc, Not used */
+ NULL, /* SetOptionProc, Not used */
+ NULL, /* GetOptionProc, Not used */
+ PgWatchProc, /* WatchProc, must be defined */
+ PgGetHandleProc, /* GetHandleProc, must be defined */
+ NULL /* Close2Proc, Not used */
};

/*
========================================================================

Is this safe to put into the main code? It looks fine.

> g(dot)hintermayer(at)inode(dot)at wrote:
> >has anyone ever successfully done copy to/from stdout with the
> >tcl-extension for postgreSQL.
> >I'm currently using 7.0 and always getting a seg fault when I try to
> >read from the database connection after issueing a "COPY table TO
> >stdout;" (I'm using the connection handle, *not* the result handle).
> >Maybe this is fixed in a later release.
> >The README file in src/interfaces/libpgtcl tells me, that this should
> >work, but unforunately it doesn't.
>
> Yes, it seems broken. It is a bug in libpgtcl. Are you running Tcl >= 8.3.2?
> That's when the Tcl team changed the data structure for channel
> callbacks. The change itself was designed to be backward compatible, but I
> suspect a related change made the code more sensitive to errors in the
> structure (NULL pointers where functions are required). Either that, or
> nobody has tried to use libpgtcl with COPY in a long time.
>
> First, I have to say I can't think of a good reason to use PostgreSQL's
> COPY command from a Tcl application. I think it should only be used with
> psql for importing data from another source into PostgreSQL, or for
> exporting PostgreSQL data into another database (but why would anyone do
> that?) If it was me, I would stick with SELECT and INSERT and be "SQL
> Compliant".
>
> OK, editorial is over. Try applying the patch below to fix
> src/interfaces/libpgtcl/pgtclId.c
> and let us know if it works. I did little testing on it, but my test did
> segfault before and ran fine (copy in and copy out) after the patch. This
> is for PostgreSQL-7.1.2 - since you are running older 7.0, I don't know if
> this will work, but I suspect it will.
>
> PS It's the absence of PgWatchProc which kills it. I didn't upgrade it
> to the "V2" channel type structure, so it should be compatible with older
> Tcl's. But aside from gets and puts, I doubt any other file operations
> would work on the handle during a copy.
> ========================================================================
>
> --- src/interfaces/libpgtcl/pgtclId.c.orig Fri Feb 9 21:31:29 2001
> +++ src/interfaces/libpgtcl/pgtclId.c Sat Aug 11 16:55:14 2001
> @@ -138,17 +138,32 @@
>
> #endif
>
> +/*
> + * The WatchProc and GetHandleProc are no-ops but must be present.
> + */
> +static void
> +PgWatchProc(ClientData instanceData, int mask)
> +{
> +}
> +static int
> +PgGetHandleProc(ClientData instanceData, int direction,
> + ClientData *handlePtr)
> +{
> + return TCL_ERROR;
> +}
> +
> Tcl_ChannelType Pg_ConnType = {
> "pgsql", /* channel type */
> NULL, /* blockmodeproc */
> PgDelConnectionId, /* closeproc */
> PgInputProc, /* inputproc */
> PgOutputProc, /* outputproc */
> -
> - /*
> - * Note the additional stuff can be left NULL, or is initialized
> - * during a PgSetConnectionId
> - */
> + NULL, /* SeekProc, Not used */
> + NULL, /* SetOptionProc, Not used */
> + NULL, /* GetOptionProc, Not used */
> + PgWatchProc, /* WatchProc, must be defined */
> + PgGetHandleProc, /* GetHandleProc, must be defined */
> + NULL /* Close2Proc, Not used */
> };
>
> /*
> ========================================================================
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
> http://www.postgresql.org/search.mpl
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(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

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> Is this safe to put into the main code? It looks fine.

It looks to me like it'll break compatibility with all pre-8.3 Tcls.
Need some #ifdef's in there, no?

regards, tom lane

g(dot)hintermayer(at)inode(dot)at wrote:
>...
>> PS It's the absence of PgWatchProc which kills it. I didn't upgrade it
>> to the "V2" channel type structure, so it should be compatible with older
>> Tcl's. But aside from gets and puts, I doubt any other file operations
>> would work on the handle during a copy.
>
>I'd expect also eof to work, and fileevent readable/writable.

eof is probably OK, except that once the copy is over the channel
becomes unreadable, so you probably only get one shot at [eof].
fileevent is really iffy. Probably no, since watchProc is
just dummy procedure and I think it is needed to make event
notification work.

tgl(at)sss(dot)pgh(dot)pa(dot)us wrote:
>Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
>> Is this safe to put into the main code? It looks fine.

Looks fine to me too, but I suppose you want to know if it works and
doesn't break anything else...
The original poster may be able to answer this with more confidence.

>It looks to me like it'll break compatibility with all pre-8.3 Tcls.
>Need some #ifdef's in there, no?

No, I don't think so. It still uses the older "Tcl v1 channel type"
structure, not the newer one introduced in Tcl 8.3.2. Should be
OK back through 8.0, although I base this only on an eyeball-check
of the Tcl 8.0.5 include files.

Your patch has been added to the PostgreSQL unapplied patches list at:

http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

> g(dot)hintermayer(at)inode(dot)at wrote:
> >has anyone ever successfully done copy to/from stdout with the
> >tcl-extension for postgreSQL.
> >I'm currently using 7.0 and always getting a seg fault when I try to
> >read from the database connection after issueing a "COPY table TO
> >stdout;" (I'm using the connection handle, *not* the result handle).
> >Maybe this is fixed in a later release.
> >The README file in src/interfaces/libpgtcl tells me, that this should
> >work, but unforunately it doesn't.
>
> Yes, it seems broken. It is a bug in libpgtcl. Are you running Tcl >= 8.3.2?
> That's when the Tcl team changed the data structure for channel
> callbacks. The change itself was designed to be backward compatible, but I
> suspect a related change made the code more sensitive to errors in the
> structure (NULL pointers where functions are required). Either that, or
> nobody has tried to use libpgtcl with COPY in a long time.
>
> First, I have to say I can't think of a good reason to use PostgreSQL's
> COPY command from a Tcl application. I think it should only be used with
> psql for importing data from another source into PostgreSQL, or for
> exporting PostgreSQL data into another database (but why would anyone do
> that?) If it was me, I would stick with SELECT and INSERT and be "SQL
> Compliant".
>
> OK, editorial is over. Try applying the patch below to fix
> src/interfaces/libpgtcl/pgtclId.c
> and let us know if it works. I did little testing on it, but my test did
> segfault before and ran fine (copy in and copy out) after the patch. This
> is for PostgreSQL-7.1.2 - since you are running older 7.0, I don't know if
> this will work, but I suspect it will.
>
> PS It's the absence of PgWatchProc which kills it. I didn't upgrade it
> to the "V2" channel type structure, so it should be compatible with older
> Tcl's. But aside from gets and puts, I doubt any other file operations
> would work on the handle during a copy.
> ========================================================================
>
> --- src/interfaces/libpgtcl/pgtclId.c.orig Fri Feb 9 21:31:29 2001
> +++ src/interfaces/libpgtcl/pgtclId.c Sat Aug 11 16:55:14 2001
> @@ -138,17 +138,32 @@
>
> #endif
>
> +/*
> + * The WatchProc and GetHandleProc are no-ops but must be present.
> + */
> +static void
> +PgWatchProc(ClientData instanceData, int mask)
> +{
> +}
> +static int
> +PgGetHandleProc(ClientData instanceData, int direction,
> + ClientData *handlePtr)
> +{
> + return TCL_ERROR;
> +}
> +
> Tcl_ChannelType Pg_ConnType = {
> "pgsql", /* channel type */
> NULL, /* blockmodeproc */
> PgDelConnectionId, /* closeproc */
> PgInputProc, /* inputproc */
> PgOutputProc, /* outputproc */
> -
> - /*
> - * Note the additional stuff can be left NULL, or is initialized
> - * during a PgSetConnectionId
> - */
> + NULL, /* SeekProc, Not used */
> + NULL, /* SetOptionProc, Not used */
> + NULL, /* GetOptionProc, Not used */
> + PgWatchProc, /* WatchProc, must be defined */
> + PgGetHandleProc, /* GetHandleProc, must be defined */
> + NULL /* Close2Proc, Not used */
> };
>
> /*
> ========================================================================
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
> http://www.postgresql.org/search.mpl
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(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

> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > Is this safe to put into the main code? It looks fine.
>
> It looks to me like it'll break compatibility with all pre-8.3 Tcls.
> Need some #ifdef's in there, no?

I will test with earlier TCL's but if a conditional test is required,
can I have a new patch?

--
Bruce Momjian | http://candle.pha.pa.us
pgman(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

Patch is in the queue. Seems portability was already tested.

> tgl(at)sss(dot)pgh(dot)pa(dot)us wrote:
> >Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> >> Is this safe to put into the main code? It looks fine.
>
> Looks fine to me too, but I suppose you want to know if it works and
> doesn't break anything else...
> The original poster may be able to answer this with more confidence.
>
> >It looks to me like it'll break compatibility with all pre-8.3 Tcls.
> >Need some #ifdef's in there, no?
>
> No, I don't think so. It still uses the older "Tcl v1 channel type"
> structure, not the newer one introduced in Tcl 8.3.2. Should be
> OK back through 8.0, although I base this only on an eyeball-check
> of the Tcl 8.0.5 include files.
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/users-lounge/docs/faq.html
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(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

Patch applied. Thanks.

> g(dot)hintermayer(at)inode(dot)at wrote:
> >has anyone ever successfully done copy to/from stdout with the
> >tcl-extension for postgreSQL.
> >I'm currently using 7.0 and always getting a seg fault when I try to
> >read from the database connection after issueing a "COPY table TO
> >stdout;" (I'm using the connection handle, *not* the result handle).
> >Maybe this is fixed in a later release.
> >The README file in src/interfaces/libpgtcl tells me, that this should
> >work, but unforunately it doesn't.
>
> Yes, it seems broken. It is a bug in libpgtcl. Are you running Tcl >= 8.3.2?
> That's when the Tcl team changed the data structure for channel
> callbacks. The change itself was designed to be backward compatible, but I
> suspect a related change made the code more sensitive to errors in the
> structure (NULL pointers where functions are required). Either that, or
> nobody has tried to use libpgtcl with COPY in a long time.
>
> First, I have to say I can't think of a good reason to use PostgreSQL's
> COPY command from a Tcl application. I think it should only be used with
> psql for importing data from another source into PostgreSQL, or for
> exporting PostgreSQL data into another database (but why would anyone do
> that?) If it was me, I would stick with SELECT and INSERT and be "SQL
> Compliant".
>
> OK, editorial is over. Try applying the patch below to fix
> src/interfaces/libpgtcl/pgtclId.c
> and let us know if it works. I did little testing on it, but my test did
> segfault before and ran fine (copy in and copy out) after the patch. This
> is for PostgreSQL-7.1.2 - since you are running older 7.0, I don't know if
> this will work, but I suspect it will.
>
> PS It's the absence of PgWatchProc which kills it. I didn't upgrade it
> to the "V2" channel type structure, so it should be compatible with older
> Tcl's. But aside from gets and puts, I doubt any other file operations
> would work on the handle during a copy.
> ========================================================================
>
> --- src/interfaces/libpgtcl/pgtclId.c.orig Fri Feb 9 21:31:29 2001
> +++ src/interfaces/libpgtcl/pgtclId.c Sat Aug 11 16:55:14 2001
> @@ -138,17 +138,32 @@
>
> #endif
>
> +/*
> + * The WatchProc and GetHandleProc are no-ops but must be present.
> + */
> +static void
> +PgWatchProc(ClientData instanceData, int mask)
> +{
> +}
> +static int
> +PgGetHandleProc(ClientData instanceData, int direction,
> + ClientData *handlePtr)
> +{
> + return TCL_ERROR;
> +}
> +
> Tcl_ChannelType Pg_ConnType = {
> "pgsql", /* channel type */
> NULL, /* blockmodeproc */
> PgDelConnectionId, /* closeproc */
> PgInputProc, /* inputproc */
> PgOutputProc, /* outputproc */
> -
> - /*
> - * Note the additional stuff can be left NULL, or is initialized
> - * during a PgSetConnectionId
> - */
> + NULL, /* SeekProc, Not used */
> + NULL, /* SetOptionProc, Not used */
> + NULL, /* GetOptionProc, Not used */
> + PgWatchProc, /* WatchProc, must be defined */
> + PgGetHandleProc, /* GetHandleProc, must be defined */
> + NULL /* Close2Proc, Not used */
> };
>
> /*
> ========================================================================
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
> http://www.postgresql.org/search.mpl
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(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