Bugs fixed in postgresql.lisp

[SOS S. <su...@gm...>]

I apologize for cross-posting, but I felt that this belonged on the
Developer list.

Anyway, relevant parts of the discussion from the user list:

On Mon, Jul 13, 2009 at 11:26 PM, SOS Sukaeto<su...@gm...> wrote:
> On Sat, Jun 20, 2009 at 3:32 AM, Sam Steingold<sd...@gn...> wrote:
>>> (def-call-out PQexecParams (:return-type PGresult)
>>>  (:arguments (conn PGconn) (command c-string) (nParams int)
>>>              (paramTypes (c-ptr (c-array-max Oid #.MAX-PARAM)) :out
>>> :alloca)
>>>              (paramValues (c-ptr (c-array-max c-string #.MAX-PARAM))
>>>                           :out :alloca)
>>>              (paramLengths (c-ptr (c-array-max int #.MAX-PARAM)) :out
>>> :alloca)
>>>              (paramFormats (c-ptr (c-array-max int #.MAX-PARAM)) :out
>>> :alloca)
>>>              (resultFormat int)))
>>>
>>> is the current signature for PQexecParams.  The arguments paramTypes,
>>> ParamValues, paramLengths and paramFormats are all input parameters.
>>> Does the call work when specifying them as out?
>>
>> looks like the definition is, indeed, wrong.
>> I fixed them in the CVS.
>>
>> Sam.
>>
>
> Actually, this is the main point of my post - I tried out the new
> definition (this one here, just to make sure we're both on the same
> page):
>
> (def-call-out PQexecParams (:return-type PGresult)
>  (:arguments (conn PGconn) (command c-string) (nParams int)
>              (paramTypes (c-array-max Oid #.MAX-PARAM))
>              (paramValues (c-array-max c-string #.MAX-PARAM))
>              (paramLengths (c-array-max int #.MAX-PARAM))
>              (paramFormats (c-array-max int #.MAX-PARAM))
>              (resultFormat int)))
>
> This immediately segfaults.  I've tried passing in literal arrays (e.g
> '#(0) and '#("") ), I've tried pre-allocating arrays with make-array
> and passing those in - I've even tried getting pointers with
> ffi:allocate-deep & ffi:allocate-shallow (this didn't cause the
> segfault, but it *did* complain that the arguments were not the
> correct type.)  You can probably reproduce it yourself: Just enclose a
> call to PQexecParams in a valid with-sql-connection, pass it any kind
> of int array for paramTypes, paramLengths, and paramFormats and any
> type of string array for paramValues.  It'll cause CLISP to segfault
> over a null pointer.
>
> Now, I managed to redefine that call-out to this:
>
> (def-call-out PQexecParams (:return-type PGresult)
>  (:arguments (conn PGconn) (command c-string) (nParams int)
>              (paramTypes (c-ptr (c-array Oid 2)) :in :alloca)
>              (paramValues (c-ptr (c-array c-string 2)) :in :alloca)
>              (paramLengths (c-ptr (c-array int 2)) :in :alloca)
>              (paramFormats (c-ptr (c-array int 2)) :in :alloca)
>              (resultFormat int)))
>
> and successfully executed a query, retrieved the number of rows and
> columns - even retrieved some values out of the query correctly.  Now
> of course, those arrays should be of variable size (anywhere from null
> pointers on to infinity, er, INT_MAX,) not 2.  I wasn't quite sure how
> to do that, though.

On Tue, Jul 14, 2009 at 4:10 PM, <Joe...@t-...> wrote:
> So let's begin with your successful attempt:
>>(def-call-out PQexecParams (:return-type PGresult)
>>  (:arguments (conn PGconn) (command c-string) (nParams int)
>>              (paramTypes (c-ptr (c-array Oid 2)) :in :alloca)
>>              (paramValues (c-ptr (c-array c-string 2)) :in :alloca)
>>              (paramLengths (c-ptr (c-array int 2)) :in :alloca)
>>              (paramFormats (c-ptr (c-array int 2)) :in :alloca)
>>              (resultFormat int)))
>>and successfully executed a query, retrieved the number of rows and
>>columns - even retrieved some values out of the query correctly.
>
> Change all (C-PTR (C-ARRAY X ...)) :in to (C-ARRAY-PTR X)
>
> The caller must ensure that all supplied arrays are (at least) nParams
> elements long. (Longer doesn't matter since the callee can't tell).
>
> We welcome documentation enhancements.  Obviously people still have
> trouble despite extensive IMHO explanations and examples in the CLISP
> implementation notes.
>
> Regards,
>        Jorg Hohle
>

I have tested Jörg's solution - it works, and it is clearly superior
(since we can now pass as few or as many params as we'd like.)

I have a copy of postgresql.lisp patched with Jörg's solution here:
http://tulip.cis.udel.edu/files/postgresql.lisp.  It should be all
ready to be committed in cvs.

Also - on a related noted - I stumbled across the bugs in
postgresql.lisp while working on an abstraction on top of it.  I
relayed to Sam on the user list that I'd be interested in contributing
my work to sql.lisp.  I'm still interested in doing this, but first I
want to make sure that people will be able to use it in an LGPL-ish
way (I've read over the license, but I'm not clear on whether or not
this falls into a "relying on CLISP internals" situation.)

Thanks so much,

                                                          -Jeremy-