Thread: libpq Describe Extension [WAS: Bytea and perl]

Hi,

On Mar 25 08:47, John DeSoi wrote:
> I have not looked at libpq in any detail, but it should have access
> to the type of all the parameters in the prepared statement. The
> Describe (F) statement in the frontend/backend protocol identifies
> the type of each parameter.

I've prepared a patch for the Describe <-> ParameterDescription
messaging which is available via current extended query protocol.

Usage (and implementation) is explained in the patch's documentation
related part. (Also I tried to place informative comments in the code
too.)

But I've a problem with ereport() error calls caused by erronous
target_type entries. After an error in exec_describe_statement_message()
(or exec_describe_portal_message()) it leaves block with ereport() call
and client side stalls in PGASYNC_BUSY state while backend stalls in
PostgresMain() by calling ReadCommand(). To summerize, an error
returning pqDescribe() call causes both side to stall.

I'd be so appreciated to hear your thoughts about the patch and above
problem.


Regards.

Hi,

[Sending this message (first) to -hackers for discussion about the
extension and followed implementation.]

On Apr 01 09:39, Volkan YAZICI wrote:
> I've prepared a patch for the Describe <-> ParameterDescription
> messaging which is available via current extended query protocol.

Here's a written from scratch patch for the above mentioned extension.
It adds PQdescribePrepared() and PQdescribePortal() functions to the
libpq. New functions work as follows:

1. Issue a PQdescribePrepared() call.
2. First PQgetResult() will return a PGresult with input parameter
   types of the prepared statement. (You can use PQftype() on this
   PGresult to extract information.)
3. Second PQgetResult() will return another PGresult which holds the
   column information for the will be returned tuples. (All PQf*()
   functions can be used on this result.)

(A PQdescribePortal() call will just skip the 2nd step in the above
list.)

Patch passes regression tests and there're two examples attached for
PQdescribePrepared() and PQdescribePortal() usage.

To mention about the followed implementation, it needed some hack on
pqParseInput3() code to make it understand if a received message is
a reponse to a Describe ('D') query or to another tuple returning
query. To summarize problem, there're two possible forms of a 'D'
response:

 1. Description of a prepared statement: t, T, Z
 2. Description of a portal: T, Z

The problem is, AFAICS, it's not possible to distinguish between a tuple
returning query (T, ..., C, Z or T, E) and a description of a portal (T,
Z). Therefore, I've created a global flag (parsing_row_desc) which is
turned on when we receive a 'T' and turned off if we receive a 'C' or
'E'. It's a kind of ugly method but the only solution I could come up
with.


Regards.

Volkan YAZICI wrote:
> To mention about the followed implementation, it needed some hack on
> pqParseInput3() code to make it understand if a received message is
> a reponse to a Describe ('D') query or to another tuple returning
> query. To summarize problem, there're two possible forms of a 'D'
> response:
> 
>  1. Description of a prepared statement: t, T, Z
>  2. Description of a portal: T, Z
> 
> The problem is, AFAICS, it's not possible to distinguish between a tuple
> returning query (T, ..., C, Z or T, E) and a description of a portal (T,
> Z). Therefore, I've created a global flag (parsing_row_desc) which is
> turned on when we receive a 'T' and turned off if we receive a 'C' or
> 'E'. It's a kind of ugly method but the only solution I could come up
> with.

The problem with this solution is that it is not thread-safe.  Perhaps
you can use a per-PGconn boolean?

--  Bruce Momjian   http://candle.pha.pa.us EnterpriseDB    http://www.enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Volkan YAZICI wrote:
>> The problem is, AFAICS, it's not possible to distinguish between a tuple
>> returning query (T, ..., C, Z or T, E) and a description of a portal (T,
>> Z). Therefore, I've created a global flag (parsing_row_desc) which is
>> turned on when we receive a 'T' and turned off if we receive a 'C' or
>> 'E'. It's a kind of ugly method but the only solution I could come up
>> with.

> The problem with this solution is that it is not thread-safe.  Perhaps
> you can use a per-PGconn boolean?

The whole thing sounds like brute force to me.  Shouldn't you be adding
states to enum PGQueryClass, if you need to track what sort of Describe
you're doing?
        regards, tom lane

On Jun 16 08:21, Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Volkan YAZICI wrote:
> >> The problem is, AFAICS, it's not possible to distinguish between a tuple
> >> returning query (T, ..., C, Z or T, E) and a description of a portal (T,
> >> Z). Therefore, I've created a global flag (parsing_row_desc) which is
> >> turned on when we receive a 'T' and turned off if we receive a 'C' or
> >> 'E'. It's a kind of ugly method but the only solution I could come up
> >> with.
>
> > The problem with this solution is that it is not thread-safe.  Perhaps
> > you can use a per-PGconn boolean?

Ie replaced the static flag with a conn->queryclass value using
PGQueryClass as Tom suggested. Also updated patch to be compatible with
exports.txt stuff.

> The whole thing sounds like brute force to me.  Shouldn't you be adding
> states to enum PGQueryClass, if you need to track what sort of Describe
> you're doing?

I totally agree with the followed ugly style. But IMHO the recursive
parsing (that is followed in pqParseInputN()) of received data is the main
problem behind this. I think, it will even get harder everytime somebody
try to to add another type of message parsing capability to that loop.
For instance, isn't pollution of PGQueryClass with state variables (like
PGQUERY_PREPARE or PGQUERY_DESCRIBE) one of the proofs of this.

While playing with pqParseInputN loops, I feel like coding Lisp recursions
using C syntax; it's quite ridiculous.


Regards.

On Sat, Jun 24, 2006 at 02:45:33PM +0300, Volkan YAZICI wrote:
> I totally agree with the followed ugly style. But IMHO the recursive
> parsing (that is followed in pqParseInputN()) of received data is the main
> problem behind this. I think, it will even get harder everytime somebody
> try to to add another type of message parsing capability to that loop.
> For instance, isn't pollution of PGQueryClass with state variables (like
> PGQUERY_PREPARE or PGQUERY_DESCRIBE) one of the proofs of this.

What's the alternative? pqParseInputN() work using state machines, but
they're not recursive. You're trying to parse messages where you don't
know beforehand if you have enough data. Moreover, each message could
be quite large, you don't want to have to store all of them without
parsing what you can. You're also not allowed to wait for more data to
appear.

However, it seems to me you could simplify quite a bit of coding by
adding a pqHaveNBytes function that returns true if there are that many
bytes available. Then right after you know the number of attributes,
you can do a pqHaveNBytes(4*nattr) and skip the checking within the
loop.

Have a nice day,
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> From each according to his ability. To each according to his ability to litigate.

Volkan YAZICI <yazicivo@ttnet.net.tr> writes:
> [ patch to add PQdescribePrepared and PQdescribePortal ]

After looking this over, I don't see the point of PQdescribePortal,
at least not without adding other functionality to libpq.  There is
no functionality currently exposed by libpq that allows creating a
portal (that is, sending a Bind message) without also executing the
portal.  And the execution always returns the portal description.
So I don't see why you'd use it.

PQdescribePrepared is useful though, as it plugs the feature omission
mentioned in the description of PQprepare, namely, you can't find out
what datatype was inferred for a parameter that you didn't specify a
type for.

My inclination is to add PQdescribePrepared, but leave out
PQdescribePortal until such time as we decide to add functions
to libpq that support separate Bind and Execute operations.
(That might be never, seeing that no one's gotten around to it
since 7.4...)

The patch is missing an asynchronous version of PQdescribePrepared.
I'm not real sure what to call it --- the naming conventions we've
used in libpq are not as orthogonal as one could wish.
PQsendDescribePrepared is the best I can manage; anyone have a better
idea?

Also, we could take a completely different tack, which is to not invent
new functions but instead fold this functionality into PQprepare and
PQsendPrepare.  What Volkan's done with this patch is to define the
successful result of PQdescribePrepared as being a PGresult in which
only the number of columns and their datatypes (PQnfields and PQftype)
are meaningful.  We could perfectly well use that convention in the
PGresults returned by PQprepare/PQsendPrepare.  The upside of this
method is that it wouldn't require an extra network round trip to get
the information (because we'd just include the Describe Statement
request in the original Prepare packet).  The downside is that we'd
always spend the time to perform Describe Statement, even if the
application doesn't need it.  However I'd expect that time to be
pretty minimal in comparison to the other costs of a Prepare.

I'm leaning slightly to the fold-it-into-PQprepare way, but am by
no means set on that.  Comments anyone?
        regards, tom lane

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


> I'm leaning slightly to the fold-it-into-PQprepare way, but am by
> no means set on that.  Comments anyone?

As a heavy user of libpq via DBD::Pg, +1 to folding in.

- --
Greg Sabino Mullane greg@turnstep.com  greg@endpoint.com
End Point Corporation
PGP Key: 0x14964AC8 200608101212
http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8

-----BEGIN PGP SIGNATURE-----

iD8DBQFE21r4vJuQZxSWSsgRAh9VAJ9YBooLrf27LfOXo5JYheASXb1ytwCfbRKv
sUhZ6HAsL7Stbatoxhdp4GY=
=ttAV
-----END PGP SIGNATURE-----

"Greg Sabino Mullane" <greg@turnstep.com> writes:
>> I'm leaning slightly to the fold-it-into-PQprepare way, but am by
>> no means set on that.  Comments anyone?

> As a heavy user of libpq via DBD::Pg, +1 to folding in.

Another thought: I looked into the protocol description and was reminded
that Describe Statement actually returns both ParameterDescription and
RowDescription, ie, both the list of parameter datatypes and the list
of column names and types that will be returned by the eventual
execution of the statement.  So another theory about how this ought to
work is that PQprepare's result PGresult ought to carry the column
name/type info where PQfname and PQftype can get them, and then we'd
have to have two new PGresult-inspection functions to pull out the
separately stored parameter-datatype info.  This seems much cleaner than
overloading the meaning of PQftype, but OTOH it's yet a few more cycles
added to the execution cost of PQprepare.  Anyone have a need to get the
result type info during PQprepare?
        regards, tom lane

On Thu, Aug 10, 2006 at 12:31:52PM -0400, Tom Lane wrote:
> "Greg Sabino Mullane" <greg@turnstep.com> writes:
> >> I'm leaning slightly to the fold-it-into-PQprepare way, but am by
> >> no means set on that.  Comments anyone?
> 
> > As a heavy user of libpq via DBD::Pg, +1 to folding in.
> 
> Another thought: I looked into the protocol description and was
> reminded that Describe Statement actually returns both
> ParameterDescription and RowDescription, ie, both the list of
> parameter datatypes and the list of column names and types that will
> be returned by the eventual execution of the statement.  So another
> theory about how this ought to work is that PQprepare's result
> PGresult ought to carry the column name/type info where PQfname and
> PQftype can get them, and then we'd have to have two new
> PGresult-inspection functions to pull out the separately stored
> parameter-datatype info.  This seems much cleaner than overloading
> the meaning of PQftype, but OTOH it's yet a few more cycles added to
> the execution cost of PQprepare.  Anyone have a need to get the
> result type info during PQprepare?

It could be handy.  Perhaps a different version (or different options
to) PQprepare for those who do?

Cheers,
D
-- 
David Fetter <david@fetter.org> http://fetter.org/
phone: +1 415 235 3778        AIM: dfetter666                             Skype: davidfetter

Remember to vote!

On Aug 10 11:35, Tom Lane wrote:
> Volkan YAZICI <yazicivo@ttnet.net.tr> writes:
> > [ patch to add PQdescribePrepared and PQdescribePortal ]
> 
> After looking this over, I don't see the point of PQdescribePortal,
> at least not without adding other functionality to libpq.  There is
> no functionality currently exposed by libpq that allows creating a
> portal (that is, sending a Bind message) without also executing the
> portal.  And the execution always returns the portal description.
> So I don't see why you'd use it.
> 
> PQdescribePrepared is useful though, as it plugs the feature omission
> mentioned in the description of PQprepare, namely, you can't find out
> what datatype was inferred for a parameter that you didn't specify a
> type for.
> 
> My inclination is to add PQdescribePrepared, but leave out
> PQdescribePortal until such time as we decide to add functions
> to libpq that support separate Bind and Execute operations.
> (That might be never, seeing that no one's gotten around to it
> since 7.4...)

My intention while implementing PQdescribePortal() was to gather
information about a portal created by an explicit DECLARE ... CURSOR
query. In case of connections are persistenly established with some pool
mechanism, it can be handy to be able to learn will be returned row
descriptions from an existing portal.

> The patch is missing an asynchronous version of PQdescribePrepared.
> I'm not real sure what to call it --- the naming conventions we've
> used in libpq are not as orthogonal as one could wish.
> PQsendDescribePrepared is the best I can manage; anyone have a better
> idea?
> 
> Also, we could take a completely different tack, which is to not invent
> new functions but instead fold this functionality into PQprepare and
> PQsendPrepare.  What Volkan's done with this patch is to define the
> successful result of PQdescribePrepared as being a PGresult in which
> only the number of columns and their datatypes (PQnfields and PQftype)
> are meaningful.  We could perfectly well use that convention in the
> PGresults returned by PQprepare/PQsendPrepare.  The upside of this
> method is that it wouldn't require an extra network round trip to get
> the information (because we'd just include the Describe Statement
> request in the original Prepare packet).  The downside is that we'd
> always spend the time to perform Describe Statement, even if the
> application doesn't need it.  However I'd expect that time to be
> pretty minimal in comparison to the other costs of a Prepare.
> 
> I'm leaning slightly to the fold-it-into-PQprepare way, but am by
> no means set on that. Comments anyone?

IMHO, it isn't the only use case of Description messages for prepared
queries to learn the infered types just after a PQprepare() call. I
think it would be quite handy to be able to gather information about a
prepared stmt in later phases of an application. For instance one might
need to get the parameter and row types of a prepared query that he/she
isn't created. If we'd place Describe message facility into PQprepare(),
then we'll just lose that functionality of the feature.

OTOH, moving Describe data processing into the PQprepare() is fairly
conventional for introducing a new functionality at the same time
keeping the API consistent without raising any compatibility problems.
But AFAICS, that's not possible without giving over one of the features
of Describe messages for prepared statements: parameter types
information or row types information. Because, if we consider placing
Describe facility into PQprepare(), client would have to issue two
distinct PQgetResult() calls; one for parameter types and another one
for row types.

On Aug 10 12:31, Tom Lane wrote:
> So another theory about how this ought to work is that PQprepare's
> result PGresult ought to carry the column name/type info where PQfname
> and PQftype can get them, and then we'd have to have two new
> PGresult-inspection functions to pull out the separately stored
> parameter-datatype info.

Yes, that's another feasible approach to the solution. But this one too
has its own PITAs as the one mentioned above.

> This seems much cleaner than overloading the meaning of PQftype, but
> OTOH it's yet a few more cycles added to the execution cost of
> PQprepare.

I think, placing Describe facility into PQprepare() will just obfuscate
the problem. In every approach we tried to place Describe into
PQprepare(), we needed to introduce new functions or broke compatibility
with the exisiting versions. ISTM, Describe features having their own
functions is the only fair solution I could come up with.

> Anyone have a need to get the result type info during PQprepare?

I don't think so. And if one would ever need such an information, can
reach it quite easily via PQdescribePrepared().


Regards.

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


> think it would be quite handy to be able to gather information
> about a prepared stmt in later phases of an application. For
> instance one might need to get the parameter and row types of
> a prepared query that he/she isn't created.

Prepared statements are not visible nor survivable outside of your
session, so this doesn't really make sense. If your application needs
the information, it can get it at prepare time.

>> Anyone have a need to get the result type info during PQprepare?

> I don't think so. And if one would ever need such an information, can
> reach it quite easily via PQdescribePrepared().

That's a good point, however, along with your other arguments. :) I
could live with either way.

- --
Greg Sabino Mullane greg@turnstep.com
End Point Corporation
PGP Key: 0x14964AC8 200608110849
http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8
-----BEGIN PGP SIGNATURE-----

iD8DBQFE3Hz8vJuQZxSWSsgRAuJuAJ4z/LmnoLOXIoZcdSh0VFYCdBDMlwCfd3HW
8YOwN30Jb8jHGx/OOjWzPaQ=
=9tRi
-----END PGP SIGNATURE-----

On Aug 11 12:51, Greg Sabino Mullane wrote:
> > think it would be quite handy to be able to gather information about
> > a prepared stmt in later phases of an application. For instance one
> > might need to get the parameter and row types of a prepared query
> > that he/she isn't created.
> 
> Prepared statements are not visible nor survivable outside of your
> session, so this doesn't really make sense. If your application needs
> the information, it can get it at prepare time.

What about persistent connections? Actually, I can give lots of corner
cases to support my idea but they're not that often used. I think, as
long as we'll break compatibility, placing Describe facility in the
PQprepare() is not the way to go.

> >> Anyone have a need to get the result type info during PQprepare?
> 
> > I don't think so. And if one would ever need such an information, can
> > reach it quite easily via PQdescribePrepared().
> 
> That's a good point, however, along with your other arguments. :) I
> could live with either way.

I'm just declined to break current PQprepare() or to introduce new
PGresult-processor functions for a feature (IMHO) that needs its own
function. But the general use case is the main fact that'll say the last
word.


Regards.

Volkan YAZICI <yazicivo@ttnet.net.tr> writes:
> On Aug 16 11:37, Tom Lane wrote:
>> I think this viewpoint has pretty much carried the day, so the
>> PQdescribe functions should remain separate.  However, it still seems
>> to me that it'd be a shame if PQdescribePrepared couldn't return the
>> statement's output column types, seeing that the backend is going to
>> pass that info to us anyway.

> I think you have a misunderstanding about the patch I previously sent.
> When you issue a PQdescribePrepared() call, in the first PQgetResult()
> call returned PGresult will have the input parameter types of the
> prepared statement. And in the second PQgetResult() call, returned
> PGresult will hold statement's output column types.

[ raised eyebrow... ]  You're right, I didn't understand that, and now
that I do I find it completely unacceptable.  We need exactly one
PGresult per operation, or things just get too weird for clients to
manage, particularly when considering async behavior.  What you suggest
is a *huge* violation of the principle of least surprise.  Adding a
couple more PGresult accessor functions seems far saner.

> Another possibility can be like this:

> PGresult *PQdescribePrepared(PGconn *conn,
>                              const char *stmt,
>                              Oid **argtypes);

No, because that doesn't work at all for the async case.
        regards, tom lane

On Aug 16 11:37, Tom Lane wrote:
> Volkan YAZICI <yazicivo@ttnet.net.tr> writes:
> > On Aug 11 12:51, Greg Sabino Mullane wrote:
> >> Prepared statements are not visible nor survivable outside of your
> >> session, so this doesn't really make sense. If your application needs
> >> the information, it can get it at prepare time.
> 
> > What about persistent connections? Actually, I can give lots of corner
> > cases to support my idea but they're not that often used. I think, as
> > long as we'll break compatibility, placing Describe facility in the
> > PQprepare() is not the way to go.
> 
> I think this viewpoint has pretty much carried the day, so the
> PQdescribe functions should remain separate.  However, it still seems
> to me that it'd be a shame if PQdescribePrepared couldn't return the
> statement's output column types, seeing that the backend is going to
> pass that info to us anyway.

I think you have a misunderstanding about the patch I previously sent.
When you issue a PQdescribePrepared() call, in the first PQgetResult()
call returned PGresult will have the input parameter types of the
prepared statement. And in the second PQgetResult() call, returned
PGresult will hold statement's output column types.

> So I propose storing the parameter type
> info in a new section of a PGresult struct, and adding new PGresult
> accessor functions PQnparams, PQparamtype (or maybe PQptype to follow
> the existing PQftype precedent more closely) to fetch the parameter type
> info.  The existing functions PQnfields etc will fetch output-column
> info.  Aside from being more functional, this definition maintains the
> principle of least surprise, in that the interpretation of a PGresult
> from Describe isn't fundamentally different from a PGresult from a
> regular query.

Another possibility can be like this:

PGresult *PQdescribePrepared(PGconn *conn,                            const char *stmt,                            Oid
**argtypes);

A PQdescribePrepared() call will immediatly return a PGresult
(previosly, we were just returning a boolean value that shows the result
of the command send status) result that holds statement's output column
types and argtypes will get altered to point to an Oid array that has
input parameter type information. (By assigning NULL value to argtypes,
user will decide to receive or not receive input parameter types.)

> We also need async versions PQsendDescribePrepared and
> PQsendDescribePortal, as I mentioned before.

If you decided on the method to use I'm volunteered to modify existing
patch. Waiting for your comments.


Regards.

Volkan YAZICI <yazicivo@ttnet.net.tr> writes:
> On Aug 11 12:51, Greg Sabino Mullane wrote:
>> Prepared statements are not visible nor survivable outside of your
>> session, so this doesn't really make sense. If your application needs
>> the information, it can get it at prepare time.

> What about persistent connections? Actually, I can give lots of corner
> cases to support my idea but they're not that often used. I think, as
> long as we'll break compatibility, placing Describe facility in the
> PQprepare() is not the way to go.

I think this viewpoint has pretty much carried the day, so the
PQdescribe functions should remain separate.  However, it still seems
to me that it'd be a shame if PQdescribePrepared couldn't return the
statement's output column types, seeing that the backend is going to
pass that info to us anyway.  So I propose storing the parameter type
info in a new section of a PGresult struct, and adding new PGresult
accessor functions PQnparams, PQparamtype (or maybe PQptype to follow
the existing PQftype precedent more closely) to fetch the parameter type
info.  The existing functions PQnfields etc will fetch output-column
info.  Aside from being more functional, this definition maintains the
principle of least surprise, in that the interpretation of a PGresult
from Describe isn't fundamentally different from a PGresult from a
regular query.

We also need async versions PQsendDescribePrepared and
PQsendDescribePortal, as I mentioned before.

Anyone have different suggestions for the names of these functions?
        regards, tom lane

Volkan YAZICI <yazicivo@ttnet.net.tr> writes:
> I've prepared a new patch that adds below commands to the libpq:

>   /* Accessor functions for PGresParamDesc field of PGresult. */
>   int PQnparams(const PGresult *res)
>   int PQparamType(const PGresult *res, int param_num)

>   /* Async functions. */
>   int PQsendDescribePrepared(PGconn *conn, const char *stmt)
>   int PQsendDescribePortal(PGconn *conn, const char *portal)

>   /* Synchronous ones. */
>   PGresult *PQdescribePrepared(PGconn *conn, const char *stmt)
>   PGresult *PQdescribePortal(PGconn *conn, const char *portal)

Applied with some small revisions to make it fit in better with the
existing libpq code (I don't think it desirable to have
copied-and-pasted versions of PQsendQueryStart, for instance).
I added some documentation also.
        regards, tom lane