Re: libpq and prepared statements progress for 8.0

On Wed, 15 Sep 2004 13:50:43 -0400, Tom Lane wrote:

> > The describe statement part could be much trickier but DBD::Pg
> doesn't really
> > need that for basic functionality.
>
> Doesn't it? I thought the problem was that they couldn't find out what
> datatypes the parameters got resolved as. That may not be important
> if they are happy with always shipping text strings, but if they want
> to
> move over to using binary transmission of parameter values then they
> gotta know the parameter types.

I just finished reading through this thread in the archives, and just
wanted to jump in here for a moment, if I could, to ask a couple of
questions to get a few things straight, at least in my mind.

To get initial PREPARE support, I don't think that the other DBD::Pg
developers mind sending strings to the PostgreSQL server in a SQL
PREPARE statement. I certainly don't. It will be great when the binary
API is there in libpq to exploit, but until it is, strings are fine.
This is how DBD::Oracle works, for what it's worth (it sends VARCHAR
bound parameters to the server, and the server figures out the
appropriate data type).

The question, in my mind, is it currently possible to do this without
specifying the type of every placeholder? As Greg Stark points out, it
would be nice to have an API to get this information from the server,
but it's not really important in the short run if the server just takes
care of unknowns. I'm told that it does, but no one has been able to
help us with the syntax, AFAIK.

What we want, I think, is what Greg Stark asks about here:

http://archives.postgresql.org/pgsql-hackers/2004-09/msg00399.php

And Greg Sabino Mullane was asking about the appropriate syntax here:

http://archives.postgresql.org/pgsql-hackers/2004-09/msg00372.php

If we could just get this figured out and know that we can rely on the
server to determine the appropriate syntax for this, we'll have what we
need to get PREPAREd statements working in the next release of DBD::Pg.
Something like this (although this doesn't seem to work):

PREPARE st(unknown) AS INSERT INTO foobar(a) VALUES ($1::unknown);

> > From what I'm reading now it seems the "read every record before
> > returning" behaviour is rooted in the libpq interface.
>
> Right. Again that's probably something that could be handled by
> exposing more/different API, but no one has stepped up to design it.

I think that would be great, but I'm hoping it's not required to solve
our immediate problem.

Regards and thanks for the help,

David

David Wheeler <david(at)kineticode(dot)com> writes:
> To get initial PREPARE support, I don't think that the other DBD::Pg
> developers mind sending strings to the PostgreSQL server in a SQL
> PREPARE statement.

> The question, in my mind, is it currently possible to do this without
> specifying the type of every placeholder?

Well, that's the problem: you can't. The SQL PREPARE command doesn't
have any provision for dealing with unspecified parameters. I think
this is reasonable since if it could, it would still have no way to tell
you what it resolved the parameter types as. The shortcoming here is
really in libpq and not in the backend: the protocol-level Parse
operation *is* designed to handle this scenario, but libpq isn't letting
you get at it.

regards, tom lane

On Sep 17, 2004, at 1:32 PM, Tom Lane wrote:

> Well, that's the problem: you can't. The SQL PREPARE command doesn't
> have any provision for dealing with unspecified parameters. I think
> this is reasonable since if it could, it would still have no way to
> tell
> you what it resolved the parameter types as. The shortcoming here is
> really in libpq and not in the backend: the protocol-level Parse
> operation *is* designed to handle this scenario, but libpq isn't
> letting
> you get at it.

Great, thank you, Tom, that's exactly the answer I was looking for. And
if I understand a previous post from you, this functionality should be
added to libpq, but you haven't had time and aren't sure what the API
should look like, anyway, is that correct?

Is this the task labeled "llow libpq to check parameterized data types"
here?:

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

Regards,

David

David Wheeler <david(at)kineticode(dot)com> writes:
> if I understand a previous post from you, this functionality should be
> added to libpq, but you haven't had time and aren't sure what the API
> should look like, anyway, is that correct?

Right. Proposals welcome.

> Is this the task labeled "llow libpq to check parameterized data types"
> here?:
> http://candle.pha.pa.us/cgi-bin/pgopenitems

I think that was something else, but memory is fuzzy.

regards, tom lane

On Sep 17, 2004, at 1:54 PM, Tom Lane wrote:

> David Wheeler <david(at)kineticode(dot)com> writes:
>> if I understand a previous post from you, this functionality should be
>> added to libpq, but you haven't had time and aren't sure what the API
>> should look like, anyway, is that correct?
>
> Right. Proposals welcome.

Wish I spoke C! But I don't think it makes sense to support PREPARE in
DBD::Pg without this functionality. :-(

>> Is this the task labeled "allow libpq to check parameterized data
>> types"
>> here?:
>> http://candle.pha.pa.us/cgi-bin/pgopenitems
>
> I think that was something else, but memory is fuzzy.

I'm less sure, since I was the one who asked Jan Wieck about this at
OSCON, for which I believe that this is the relevant discussion:

http://archives.postgresql.org/pgsql-hackers/2004-08/msg00136.php
http://archives.postgresql.org/pgsql-hackers/2004-08/msg00130.php

But maybe Jan and I misunderstood each other?

Regards,

David

David Wheeler <david(at)kineticode(dot)com> writes:
>>> Is this the task labeled "allow libpq to check parameterized data
>>> types" here?:
>>> http://candle.pha.pa.us/cgi-bin/pgopenitems
>>
>> I think that was something else, but memory is fuzzy.

> I'm less sure, since I was the one who asked Jan Wieck about this at
> OSCON, for which I believe that this is the relevant discussion:

Okay, here we have Bruce saying he'll add it to open items:

http://archives.postgresql.org/pgsql-hackers/2004-08/msg00147.php

so I guess it is the same thing. Good, that will give us some political
cover for squeezing it into 8.0 ;-). Assuming that anyone steps up and
does the work, that is.

regards, tom lane

On Sep 17, 2004, at 2:21 PM, Tom Lane wrote:

> Okay, here we have Bruce saying he'll add it to open items:
>
> http://archives.postgresql.org/pgsql-hackers/2004-08/msg00147.php
>
> so I guess it is the same thing. Good, that will give us some
> political
> cover for squeezing it into 8.0 ;-).

Ah, okay, so Jan and I weren't misunderstanding each other, then. :-)
Thanks Tom.

> Assuming that anyone steps up and
> does the work, that is.

So...any volunteers? I suspect that other driver projects would be
happy to have it, too.

Cheers,

David

Tom Lane wrote:
> David Wheeler <david(at)kineticode(dot)com> writes:
> > if I understand a previous post from you, this functionality should be
> > added to libpq, but you haven't had time and aren't sure what the API
> > should look like, anyway, is that correct?
>
> Right. Proposals welcome.
>
> > Is this the task labeled "llow libpq to check parameterized data types"
> > here?:
> > http://candle.pha.pa.us/cgi-bin/pgopenitems
>
> I think that was something else, but memory is fuzzy.

Yep, that is it.

--
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

Tom Lane wrote:
> David Wheeler <david(at)kineticode(dot)com> writes:
>
>>To get initial PREPARE support, I don't think that the other DBD::Pg
>>developers mind sending strings to the PostgreSQL server in a SQL
>>PREPARE statement.
>
>
>>The question, in my mind, is it currently possible to do this without
>>specifying the type of every placeholder?
>
>
> Well, that's the problem: you can't. The SQL PREPARE command doesn't
> have any provision for dealing with unspecified parameters. I think
> this is reasonable since if it could, it would still have no way to tell
> you what it resolved the parameter types as.

But it sounds like the client doesn't care about the type info anyway;
it'd pass all the parameters as text and let the backend sort out the
types when doing implicit conversions in the EXECUTE parse step:

template1=> prepare s1(int) as select $1;
PREPARE
template1=> execute s1('12');
?column?
----------
12
(1 row)

(now replace "int" with "unknown"..)

-O

Oliver Jowett wrote:

> template1=> prepare s1(int) as select $1;
> PREPARE
> template1=> execute s1('12');
> ?column?
> ----------
> 12
> (1 row)
>
> (now replace "int" with "unknown"..)

Ok, bad example since the backend probably can't infer a type for the
PREPARE in this case. A better example:

> template1=> prepare s1(int) as select typname from pg_type where oid = $1;
> PREPARE
> template1=> execute s1('16');
> typname
> ---------
> bool
> (1 row)

-O

On Sep 17, 2004, at 6:23 PM, Oliver Jowett wrote:

>> template1=> prepare s1(int) as select typname from pg_type where oid
>> = $1;
>> PREPARE
>> template1=> execute s1('16');
>> typname ---------
>> bool
>> (1 row)

You're still telling it the type via that int.

Regards,

David

David Wheeler wrote:
> On Sep 17, 2004, at 6:23 PM, Oliver Jowett wrote:
>
>>> template1=> prepare s1(int) as select typname from pg_type where oid
>>> = $1;
>>> PREPARE
>>> template1=> execute s1('16');
>>> typname ---------
>>> bool
>>> (1 row)
>
>
> You're still telling it the type via that int.

Well, obviously. I haven't modified the backend code to accept 'unknown'
in PREPARE..

My point was the client does *not* need to know the type inferred by the
PREPARE in the 'unknown' case to make use of the resulting statement. It
can pass all parameters as text and use the type inference that happens
on EXECUTE -- as is happening in the EXECUTE quoted above.

-O

On Sep 18, 2004, at 1:09 PM, Oliver Jowett wrote:

> Well, obviously. I haven't modified the backend code to accept
> 'unknown' in PREPARE..

Right, and that's what we're looking for.

> My point was the client does *not* need to know the type inferred by
> the PREPARE in the 'unknown' case to make use of the resulting
> statement. It can pass all parameters as text and use the type
> inference that happens on EXECUTE -- as is happening in the EXECUTE
> quoted above.

Yes, that's fine, but it's PREPARE that's at issue here, not EXECUTE.

Regards,

David

David Wheeler wrote:
> On Sep 18, 2004, at 1:09 PM, Oliver Jowett wrote:
>
>> Well, obviously. I haven't modified the backend code to accept
>> 'unknown' in PREPARE..
>
>
> Right, and that's what we're looking for.
>
>> My point was the client does *not* need to know the type inferred by
>> the PREPARE in the 'unknown' case to make use of the resulting
>> statement. It can pass all parameters as text and use the type
>> inference that happens on EXECUTE -- as is happening in the EXECUTE
>> quoted above.
>
>
> Yes, that's fine, but it's PREPARE that's at issue here, not EXECUTE.

I think you misunderstand what I'm saying.

Tom reckons that PREPARE (at the SQL level) taking unknown types is not
useful as there is no feedback mechanism along the lines of the V3
protocol Describe messages to let the client find out what types were
inferred by the PREPARE.

I am saying this doesn't matter as the client can still use the
resulting statement just fine without knowing the types. So allowing
'unknown' in PREPARE *is* useful.

Not that I'm volunteering to implement it, though -- the JDBC driver
does not need this functionality and I'm way too short on time anyway :(

-O

Oliver Jowett <oliver(at)opencloud(dot)com> writes:
> Tom reckons that PREPARE (at the SQL level) taking unknown types is not
> useful as there is no feedback mechanism along the lines of the V3
> protocol Describe messages to let the client find out what types were
> inferred by the PREPARE.

> I am saying this doesn't matter as the client can still use the
> resulting statement just fine without knowing the types. So allowing
> 'unknown' in PREPARE *is* useful.

Well, that was not quite my point, but I guess I wasn't clear. My
reasoning was more like this:
1. What we have now doesn't do what DBD::Pg needs.
2. We can fix it with some-small-amount-of-work in libpq (to add some API),
or with some-probably-also-small-amount-of-work in the backend (to
kluge up SQL PREPARE to allow "unknown").
3. The libpq-side solution is more generally useful, because it can support
feedback about the resolved datatypes.
4. Therefore, we should fix it in libpq.

Note that point 3 is not dependent on whether DBD::Pg in particular
needs this functionality --- somebody out there certainly will.

regards, tom lane

At 2004-09-17 14:28:36 -0700, david(at)kineticode(dot)com wrote:
>
> > Assuming that anyone steps up and does the work, that is.
>
> So...any volunteers?

OK, how about adding a PQprepare (PQcreatePrepared?) function like this?

PGresult *
PQprepare(PGconn *conn,
const char *stmtName,
const char *query,
int nParams,
const Oid *paramTypes)
{
...

PQprepare would construct a Parse message to create a prepared statement
named stmtName from the given query, with nParams types pre-declared. It
could be called by DBD::Pg with nParams == 0 to let the server infer all
of the parameter types.

I suppose an asynchronous equivalent would also be needed.
(Yes, I'm volunteering to write both functions.)

-- ams

On Sep 19, 2004, at 9:13 PM, Abhijit Menon-Sen wrote:

> OK, how about adding a PQprepare (PQcreatePrepared?) function like
> this?
>
> PGresult *
> PQprepare(PGconn *conn,
> const char *stmtName,
> const char *query,
> int nParams,
> const Oid *paramTypes)
> {
> ...
>
> PQprepare would construct a Parse message to create a prepared
> statement
> named stmtName from the given query, with nParams types pre-declared.
> It
> could be called by DBD::Pg with nParams == 0 to let the server infer
> all
> of the parameter types.

Sounds damn good to me, Abhihit, thanks!

> I suppose an asynchronous equivalent would also be needed.
> (Yes, I'm volunteering to write both functions.)

Woot! :-)

Regards,

David

Abhijit Menon-Sen <ams(at)oryx(dot)com> writes:

> OK, how about adding a PQprepare (PQcreatePrepared?) function like this?
>
> PGresult *
> PQprepare(PGconn *conn,
> const char *stmtName,
> const char *query,
> int nParams,
> const Oid *paramTypes)
> {
> ...
>
> PQprepare would construct a Parse message to create a prepared statement
> named stmtName from the given query, with nParams types pre-declared. It
> could be called by DBD::Pg with nParams == 0 to let the server infer all
> of the parameter types.

You have to have a parameter for specifying the portalName. Since using the
"unnamed" portal is no longer really an option.

That means you also need to add a new Execute method that takes a portalName
instead of a command.

I've been fooling around with this. But don't let that stop you, I haven't
said anything precisely because I figure my odds on getting this finished to
be quite low. And in any case I don't know if it'll be up to snuff for
inclusion.

I have a couple questions:

1) I expect any driver to automatically gensym up all the statement names and
portal names. I wonder whether libpq shouldn't be doing the same thing.
This goes back to the same issue of libpq trying to satisfy two masters. A
C programmer would be happy with libpq gensymming the statement names and
portal names, but a driver author would be just as happy doing it himself.

2) How is it that the error handling in the existing PQsendQueryGuts works at
all?! It just aborts wherever it's at and jumps to a function that
consumes all input. It has no idea where to stop consuming since the SYNC
may or may not have been sent. If there's a network burp wouldn't it cause
messages to get left over to confuse matters for the next call?

I wonder whether it would make sense to have SYNC messages come with an
integer parameter attached which gets echoed back by the server. Then libpq
error handling can just issue its own SYNC and wait until it gets back a SYNC
response with the correct value, rather than have to engineer the sending code
to keep careful track of whether the SYNC has been sent or not.

--
greg

At 2004-09-20 01:25:56 -0400, gsstark(at)mit(dot)edu wrote:
>
> That means you also need to add a new Execute method that takes a
> portalName instead of a command.

Yes, thanks. How about these functions, then?

PGresult *
PQprepare(PGconn *conn,
const char *stmtName,
const char *query,
int nParams,
const Oid *paramTypes);

PGresult *
PQbind(PGconn *conn,
const char *stmtName,
const char *portalName,
int nParams,
const char *const *paramValues,
const int *paramLengths,
int nFormats,
const int *paramFormats,
int nResults,
const int *resultFormats);

PGresult *
PQexecute(PGconn *conn,
const char *portalName,
int nRows);

-- ams

Abhijit Menon-Sen <ams(at)oryx(dot)com> writes:

> At 2004-09-20 01:25:56 -0400, gsstark(at)mit(dot)edu wrote:
> >
> > That means you also need to add a new Execute method that takes a
> > portalName instead of a command.

Oh, it occurs to me I missed a step in my earlier logic. I assumed we would
want a separate Bind and execute call. Do we? I think we do, but the only
advantages are pretty thin:

1) It could be convenient to have an application call bind but have another
layer call execute. I've never seen this happen.

2) If we ever want to implement a Describe call then it would have to happen
between Bind and execute. Personally I find it annoying that you can't call
describe on a statement, only a portal, but that's the way it is now.

> Yes, thanks. How about these functions, then?
>
> PGresult *
> PQprepare(PGconn *conn,
> const char *stmtName,
> const char *query,
> int nParams,
> const Oid *paramTypes);
>
> PGresult *
> PQbind(PGconn *conn,
> const char *stmtName,
> const char *portalName,
> int nParams,
> const char *const *paramValues,
> const int *paramLengths,
> int nFormats,
> const int *paramFormats,

> int nResults,
> const int *resultFormats);

resultFormat is just a single integer in the protocol. You don't get to
specify different formats for different columns.

> PGresult *
> PQexecute(PGconn *conn,
> const char *portalName,
> int nRows);

What's nRows? None of the existing PQexec* take an nRows parameter.

--
greg

At 2004-09-20 02:16:50 -0400, gsstark(at)mit(dot)edu wrote:
>
> Personally I find it annoying that you can't call describe on a
> statement, only a portal, but that's the way it is now.

No, it isn't. Describe accepts both prepared statement and portal names.
In the former case, it returns ParameterDescription message as well as
the RowDescriptions it returns for the latter.

> resultFormat is just a single integer in the protocol. You don't get
> to specify different formats for different columns.

Yes, you do. Bind accepts 0 (use the default text format), 1 (use this
format for all results), or as many result format specifiers as there
are results.

> What's nRows? None of the existing PQexec* take an nRows parameter.

Execute can be told to return no more than n rows of results. If there
are more rows available, the server returns PortalSuspended and awaits
another Execute message. The default of 0 imposes no limit.

Please read protocol-flow.html and protocol-message-formats.html.

-- ams

There was some previous discussion of whether DBD:pg should continue
using libpq or implement the wire protocol in Perl, and whether ODBC
should move to using libpq.

I think we should favor libpq usage wherever possible and only
re-implement it in the native language when required, like for jdbc/java.
I think having all interfaces take advantage of libpq improvements and
features is a major win. If we need to add things to libpq to make it
easier, fine, but that is minor work compared to maintaining separate
wire protocol for each interface language.

---------------------------------------------------------------------------

Tom Lane wrote:
> Oliver Jowett <oliver(at)opencloud(dot)com> writes:
> > Tom reckons that PREPARE (at the SQL level) taking unknown types is not
> > useful as there is no feedback mechanism along the lines of the V3
> > protocol Describe messages to let the client find out what types were
> > inferred by the PREPARE.
>
> > I am saying this doesn't matter as the client can still use the
> > resulting statement just fine without knowing the types. So allowing
> > 'unknown' in PREPARE *is* useful.
>
> Well, that was not quite my point, but I guess I wasn't clear. My
> reasoning was more like this:
> 1. What we have now doesn't do what DBD::Pg needs.
> 2. We can fix it with some-small-amount-of-work in libpq (to add some API),
> or with some-probably-also-small-amount-of-work in the backend (to
> kluge up SQL PREPARE to allow "unknown").
> 3. The libpq-side solution is more generally useful, because it can support
> feedback about the resolved datatypes.
> 4. Therefore, we should fix it in libpq.
>
> Note that point 3 is not dependent on whether DBD::Pg in particular
> needs this functionality --- somebody out there certainly will.
>
> regards, tom lane
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org
>

--
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

(I apologise in advance if anyone receives multiple copies of this post.
I posted from the wrong address earlier.)

At 2004-09-20 02:16:50 -0400, gsstark(at)mit(dot)edu wrote:
>
> I assumed we would want a separate Bind and execute call. Do we?

Yes, we do. (See below.)

> Personally I find it annoying that you can't call describe on a
> statement, only a portal, but that's the way it is now.

No, it isn't. Describe accepts both prepared statement and portal names.
In the former case, it returns ParameterDescription message as well as
the RowDescriptions it returns for the latter.

> resultFormat is just a single integer in the protocol. You don't get
> to specify different formats for different columns.

Yes, you do. Bind accepts 0 (use the default text format), 1 (use this
format for all results), or as many result format specifiers as there
are results.

> What's nRows? None of the existing PQexec* take an nRows parameter.

Execute can be told to return no more than n rows of results. If there
are more rows available, the server returns PortalSuspended and awaits
another Execute message. The default of 0 imposes no limit.

Because it's sometimes required to call Execute without Binding values
again, libpq needs separate bind/execute functions to support this.

Please read protocol-flow.html and protocol-message-formats.html.

-- ams

Abhijit Menon-Sen <ams(at)oryx(dot)com> writes:

> (I apologise in advance if anyone receives multiple copies of this post.
> I posted from the wrong address earlier.)
>
> At 2004-09-20 02:16:50 -0400, gsstark(at)mit(dot)edu wrote:
> >
> > I assumed we would want a separate Bind and execute call. Do we?

I forgot the main reason I assumed this. Namely that drivers have to provide
this interface. If we don't provide it in libpq then they have to emulate it
by storing the parameters until execute time.

> No, it isn't. Describe accepts both prepared statement and portal names.
> In the former case, it returns ParameterDescription message as well as
> the RowDescriptions it returns for the latter.

I see this now. I didn't realize you got back both sets of information when
describing a statement. Does Binding get any useful feedback? Does it tell you
at that time if the text input can't be parsed as the appropriate types for
example?

> Yes, you do. Bind accepts 0 (use the default text format), 1 (use this
> format for all results), or as many result format specifiers as there
> are results.

Ooh, I was just looking at the existing code. This is indeed in the protocol
specs.

> > What's nRows? None of the existing PQexec* take an nRows parameter.
>
> Execute can be told to return no more than n rows of results. If there
> are more rows available, the server returns PortalSuspended and awaits
> another Execute message. The default of 0 imposes no limit.

So this is the strongest argument for separating bind and execute. If you
didn't you would still need a second call executeContinue or something.

--
greg

Abhijit Menon-Sen <ams(at)oryx(dot)com> writes:
> Execute can be told to return no more than n rows of results. If there
> are more rows available, the server returns PortalSuspended and awaits
> another Execute message. The default of 0 imposes no limit.

> Because it's sometimes required to call Execute without Binding values
> again, libpq needs separate bind/execute functions to support this.

I don't really think so. Allowing access to the limited-row-count
version of Execute would fundamentally break the PGresult abstraction,
which thinks of a query result as a monolithic entity.

There has been talk from time to time of developing a new API (possibly
a whole new library?) that would allow streaming access, but I would
strongly urge you not to try to solve that problem at the same time;
if only because there is zero chance of such a patch being accepted
within the 8.0 cycle.

In my mind the existing PQexecPrepared operation is all you need to
support binding and execution of prepared statements. What you should
be concerned with right now is providing an API for Parse + Describe
Statement, to substitute for the existing approach of setting up
statement objects via PQexec("PREPARE foo ...").

regards, tom lane

At 2004-09-20 10:20:03 -0400, tgl(at)sss(dot)pgh(dot)pa(dot)us wrote:
>
> What you should be concerned with right now is providing an API for
> Parse + Describe Statement, to substitute for the existing approach
> of setting up statement objects via PQexec("PREPARE foo ...").

Fair enough. That's why my original proposal was to add only a PQprepare
function, which should be a patch small enough to write, test, and maybe
apply before 8.0:

> PGresult *
> PQprepare(PGconn *conn,
> const char *stmtName,
> const char *query,
> int nParams,
> const Oid *paramTypes);

Should I go ahead and do that?

-- ams

Abhijit Menon-Sen <ams(at)oryx(dot)com> writes:
>> PGresult *
>> PQprepare(PGconn *conn,
>> const char *stmtName,
>> const char *query,
>> int nParams,
>> const Oid *paramTypes);

> Should I go ahead and do that?

(1) What about preparing an unnamed statement ... will you allow
stmtName = NULL to mean that? (Actually it might be that stmtName = ""
will cover this, I'm too lazy to check the backend code right now.)
You do need to allow access to the unnamed statement because of
Oliver Jowett's recent efficiency hacks.

(2) What about discovering the actually resolved parameter types?

regards, tom lane

At 2004-09-20 11:02:50 -0400, tgl(at)sss(dot)pgh(dot)pa(dot)us wrote:
>
> (1) What about preparing an unnamed statement ...

stmtName == "" will work.

> (2) What about discovering the actually resolved parameter types?

I'm not sure what to do about that.

I could extend PGresult to hold ParameterDescription information, then
provide accessor functions à la PQnfields/PQfformat. But that would be
a fairly intrusive change. And I shudder to even think about trying to
reuse the existing PGresult fields/accessors to do the job.

Do you have any suggestions?

-- ams

On Sep 20, 2004, at 12:34 AM, Bruce Momjian wrote:

> I think we should favor libpq usage wherever possible and only
> re-implement it in the native language when required, like for
> jdbc/java.
> I think having all interfaces take advantage of libpq improvements and
> features is a major win. If we need to add things to libpq to make it
> easier, fine, but that is minor work compared to maintaining separate
> wire protocol for each interface language.

I don't normally post "me too" posts, but I think that what Bruce says
here is extremely important. The more drivers can rely on a single,
well-developed, and stable API to create a variety of drivers, the less
work *everyone* has to do. I think that this kind of pragmatic
componentization (to coin a phrase) can only be to the benefit of
PostgreSQL.

Regards,

David

PS: And for those who really want a Pure Perl implementation of a
PostgreSQL driver in Perl, I suggest you take a look at helping out
with DBD::PgPP:

http://search.cpan.org/dist/DBD-PgPP/

David Wheeler <david(at)kineticode(dot)com> writes:
> On Sep 20, 2004, at 12:34 AM, Bruce Momjian wrote:
>> I think we should favor libpq usage wherever possible and only
>> re-implement it in the native language when required, like for
>> jdbc/java.

> I don't normally post "me too" posts, but I think that what Bruce says
> here is extremely important.

Allow me to state a contrary position ;-)

The first problem with this approach is that it requires libpq to be all
things to all people. We've already had some discussion in this thread
about the tension between supporting application programs written in C,
which want one set of features, and drivers, which need some other ones.
After awhile you end up with a bloated, probably buggy library. We're
already some way down that path, and I don't care to go much further.

The second problem is the one someone already pointed out, that you
*need* multiple implementations in order to keep the protocol definition
honest.

I don't necessarily disagree about the immediate issues. I think it
would be a win to reimplement the ODBC driver atop libpq (if it's a
comfortable fit --- but not if we have to add warts to libpq to make
it work). And I don't feel any strong need to redo DBD::Pg as a
native-Perl driver. But I disagree that either of those decisions
should be taken on the basis of an "everyone should use libpq"
philosophy. Rather they should be taken on the basis of what makes
sense for each of those projects individually.

regards, tom lane

Abhijit Menon-Sen <ams(at)oryx(dot)com> writes:
> At 2004-09-20 11:02:50 -0400, tgl(at)sss(dot)pgh(dot)pa(dot)us wrote:
>> (2) What about discovering the actually resolved parameter types?

> Do you have any suggestions?

It depends on whether you think that PQprepare should bundle the
Describe Statement operation or not. You can make a good argument
either way: (a) for apps or drivers that don't need to find out the
actual arg types, bundling in the Describe would be a waste of cycles;
but (b) if you need the Describe then it's a loss to have to spend an
extra network round trip to get the results. Neither of these overheads
is compellingly large, though.

If you want it unbundled then a separate function is easy enough,
perhaps
PGresult * PQdescribeStatement(PGconn *conn,
const char *stmtName,
int *numParams,
Oid **paramTypes);
where *paramTypes receives a pointer to a malloc'd array (caller
to free after use); or null on failure. The PGresult would just
be used to convey success/failure.

If you want it bundled, perhaps add output parameters defined
similarly to the above to PQprepare.

It'd be possible to handle both cases in PQprepare: add the out
parameters, but say that passing NULL for them indicates the Describe
step is not wanted. I dunno if that's too complicated.

You had mentioned wanting to support async operation. We couldn't very
reasonably support async operation with separate output parameters ---
it would be a lot cleaner if the param type list were embedded in the
PGresult instead, so that PQgetResult would be sufficient. I understand
your distaste for multiplexing the use of the PGresult fields, but still
I think it would be most sensible to define PQnfields() and PQftype()
as the way to extract the information from the PGresult if we go that
way. We could invent a new PQresultStatus, say PGRES_PREPARE_OK,
as a way to distinguish a PGresult of this kind from the normal
query-result object.

If you prefer this last case then I think the cleanest approach is just
to automatically bundle the Describe operation into PQprepare. You
could imagine adding a boolean to PQprepare's param list to specify
whether you care about getting valid parameter type info back or not,
but I think that looks way too much like a wart.

If you need more options, I can probably think of some ;-)

regards, tom lane

In article <12594(dot)1095699940(at)sss(dot)pgh(dot)pa(dot)us>,
Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:

> David Wheeler <david(at)kineticode(dot)com> writes:
>> On Sep 20, 2004, at 12:34 AM, Bruce Momjian wrote:
>>> I think we should favor libpq usage wherever possible and only
>>> re-implement it in the native language when required, like for
>>> jdbc/java.

>> I don't normally post "me too" posts, but I think that what Bruce says
>> here is extremely important.

> Allow me to state a contrary position ;-)

> The first problem with this approach is that it requires libpq to be all
> things to all people. We've already had some discussion in this thread
> about the tension between supporting application programs written in C,
> which want one set of features, and drivers, which need some other ones.
> After awhile you end up with a bloated, probably buggy library. We're
> already some way down that path, and I don't care to go much further.

I don't think that's what David meant, although he said so :-)

What we should have is a C API especially for use by driver authors;
probably this API is so far away from the rest of libpq that it should
not be part of it.

This API could make life easier for driver authours, resulting in more
and better drivers for more languages.

> The second problem is the one someone already pointed out, that you
> *need* multiple implementations in order to keep the protocol definition
> honest.

Nobody forces a driver author to use that API, and there are driver
authors who *cannot* use it, e.g. Java. This means there will be more
than one implementation anyways.

Harald Fuchs wrote:

>>The first problem with this approach is that it requires libpq to be all
>>things to all people. We've already had some discussion in this thread
>>about the tension between supporting application programs written in C,
>>which want one set of features, and drivers, which need some other ones.
>>After awhile you end up with a bloated, probably buggy library. We're
>>already some way down that path, and I don't care to go much further.
>>
>>
>
>I don't think that's what David meant, although he said so :-)
>
>What we should have is a C API especially for use by driver authors;
>probably this API is so far away from the rest of libpq that it should
>not be part of it.
>
OLE DB is based on libpq. While the proposed function would be very nice
to have (and, in fact, needed for some obscure semantics of the OLE DB
protocol that no one really uses), at the moment there are NO major
features missing from OLE DB that cannot be provided using the existing
code. This may be a result of libpq going some way down bloat av., as
Tom said, but personally I don't see the need for a separate API.

I have not delved too deeply into the ODBC sources, so I can't attest to
the feasibility of using libpq there.

>This API could make life easier for driver authours, resulting in more
>and better drivers for more languages.
>
I'm really interested in what this would provide. It could be that I'm
missing something painfully obvious here, but why are driver developers
in such a different situation than end users?

Don't get me wrong. Having an API to fill data from the server directly
into user's buffers would be nice. However, as OLE DB transfers data in
binary, as most data types require conversion, and as some of the OLD DB
"accessors" are really weird, I doubt a sane API can be written that I'd
use anyways.

Likewise, having an API that does gradual delivery of data would be
nice. However, things really can be achieved using the asynchronous
libpq mechanism, and proper cursors can achieve most of the rest.

In short, I may be missing something painfully simple here, but I don't
see the real need for a driver oriented backend communication library.

Shachar

--
Shachar Shemesh
Lingnu Open Source Consulting ltd.
http://www.lingnu.com/

At 2004-09-20 13:24:47 -0400, tgl(at)sss(dot)pgh(dot)pa(dot)us wrote:
>
> It depends on whether you think that PQprepare should bundle the
> Describe Statement operation or not.

I decided against bundling the two operations together. Here's a patch
to add PQprepare() and PQsendPrepare() in a fairly self-contained way.
Also attached is a test program à la testlibpq3.c that I used to test
the change. This should be all that's needed for DBD::Pg to prepare a
statement without pre-specifying types.

(I'll post a separate patch for PQdescribe() later. It's a little more
involved, and I need to fix a couple of bugs I found during testing.)

Any thoughts? Does this look good enough for 8.0?

-- ams

On Oct 5, 2004, at 9:59 AM, Abhijit Menon-Sen wrote:

> I decided against bundling the two operations together. Here's a patch
> to add PQprepare() and PQsendPrepare() in a fairly self-contained way.
> Also attached is a test program à la testlibpq3.c that I used to test
> the change. This should be all that's needed for DBD::Pg to prepare a
> statement without pre-specifying types.

Ah, fantastic news, Abhijit! This is very exciting for DBD::Pg
development, as well as other dynamic language libraries that plan to
use prepare(), I expect. I very much look forward to Beta 4 hoping that
this patch makes it in.

Thanks for doing this!

Regards,

David

Abhijit Menon-Sen <ams(at)oryx(dot)com> writes:
> I decided against bundling the two operations together. Here's a patch
> to add PQprepare() and PQsendPrepare() in a fairly self-contained way.
> Any thoughts? Does this look good enough for 8.0?

Seems OK as far as it goes, but a complete patch would require
documentation additions. Also you missed adding entry points in the
.dll files, and possibly other minor things. Searching for all
references to one of the existing entry points such as PQexecPrepared
will probably help you identify what you need to do.

regards, tom lane

At 2004-10-05 17:48:27 -0400, tgl(at)sss(dot)pgh(dot)pa(dot)us wrote:
>
> Searching for all references to one of the existing entry points such
> as PQexecPrepared will probably help you identify what you need to do.

OK. I've attached two additional patches below.

I don't really understand how the *.def files work, so I'm just guessing
about what needs to be changed. Thanks to Josh Berkus and Kris Jurka for
looking over the documentation patch, which just adds descriptions of
the two new functions.

I apologise in advance if I've missed anything.

-- ams

Abhijit Menon-Sen <ams(at)oryx(dot)com> writes:

> --- fe-protocol3.c.1~ 2004-10-05 18:59:55.293092244 +0530
> +++ fe-protocol3.c 2004-10-05 19:17:48.154807848 +0530
> @@ -220,6 +220,11 @@ pqParseInput3(PGconn *conn)
> conn->asyncStatus = PGASYNC_READY;
> break;
> case '1': /* Parse Complete */
> + if (conn->result == NULL)
> + conn->result = PQmakeEmptyPGresult(conn,
> + PGRES_COMMAND_OK);
> + conn->asyncStatus = PGASYNC_READY;
> + break;
> case '2': /* Bind Complete */
> case '3': /* Close Complete */
> /* Nothing to do for these message types */

So why is this part of the patch ok? Isn't it going to make libpq get confused
every time a PQExecPrepared sends a v3.0 prepare message? It will mark the
connection as PGASYNC_READY as soon as the prepare response is parsed instead
of waiting for the responses from the bind and execute messages that have
already been sent.

This is more or less where I got stuck on my attempt at the same thing. It
seems like to handle "bundled" calls like PQExecPrepared libpq would have to
keep track of from what call various messages arose. That seems like it would
complicate things quite a bit.

--
greg

(I apologise for the delayed response.)

At 2004-10-07 01:23:56 -0400, gsstark(at)mit(dot)edu wrote:
>
> So why is this part of the patch ok? Isn't it going to make libpq get
> confused every time a PQExecPrepared sends a v3.0 prepare message?

I thought about that for a while, but I couldn't find anything that is
actually broken or confused by the patch. I could be missing something
obvious, though, so I'd appreciate another set of eyes looking at it.

Does anyone have any ideas?

-- ams

On Oct 14, 2004, at 6:50 PM, Abhijit Menon-Sen wrote:

> I thought about that for a while, but I couldn't find anything that is
> actually broken or confused by the patch. I could be missing something
> obvious, though, so I'd appreciate another set of eyes looking at it.
>
> Does anyone have any ideas?

Not I, but I still have my fingers crossed that this will go in in time
for 8.0. I think that Tom said it looked good when you first posted the
patch.

So, core hackers, is it going in or not? The dynamic language driver
developers will thank you for it!

Many thanks,

David

It was just added to CVS!

---------------------------------------------------------------------------

David Wheeler wrote:
> On Oct 14, 2004, at 6:50 PM, Abhijit Menon-Sen wrote:
>
> > I thought about that for a while, but I couldn't find anything that is
> > actually broken or confused by the patch. I could be missing something
> > obvious, though, so I'd appreciate another set of eyes looking at it.
> >
> > Does anyone have any ideas?
>
> Not I, but I still have my fingers crossed that this will go in in time
> for 8.0. I think that Tom said it looked good when you first posted the
> patch.
>
> So, core hackers, is it going in or not? The dynamic language driver
> developers will thank you for it!
>
> Many thanks,
>
> David
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/docs/faqs/FAQ.html
>

--
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

On Oct 18, 2004, at 3:12 PM, Bruce Momjian wrote:

> It was just added to CVS!

Awesome!

Abhijit++
Bruce++
Tom++

Regards,

David

On Mon, Oct 18, 2004 at 06:12:29PM -0400, Bruce Momjian wrote:
>
> It was just added to CVS!

Woohooo!!!!!

Big, BIG kudos to all involved :) :)

Cheers,
D(BD::Pg)
--
David Fetter david(at)fetter(dot)org http://fetter.org/
phone: +1 510 893 6100 mobile: +1 415 235 3778

Remember to vote!