apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nick Kew <n...@webthing.com>
Subject Re: [PATCH] apr_dbd_oracle.c query/select fixes #37664
Date Sat, 10 Jun 2006 08:58:02 GMT
On Saturday 10 June 2006 03:10, Chris Darroch wrote:

>    The pvquery and pvselect functions were looking for true int and double
> arguments in the va_list args.	This is not how other drivers function; for
> example, the pgsql and sqlite3 drivers assume all va_list arguments as
> strings.  This is in keeping with the pquery and pselect functions too. So,
> make the Oracle driver do the same.

Isn't that just the support for different argument formats: %d, %f
in an argument format string?  I thought oracle was setting a precedent
others could follow in that regard, since the original drivers only
supported %s but were intended to expand as time&effort permit.

>    The OCIStmtRelease() function should only be used if Oracle statement
> handles have been prepared with OCIStmtPrepare2(), which is only available
> in Oracle 9.2 and above.  For now, stick with just OCIHandleFree() and put 
> the stuff related to OCIStmtRelease() into unused #ifdef blocks in the
> freeStatement() and freeStatements() APR memory pool callback cleanup
> functions.  Note that when OCIStmtRelease() is used in the future,
> OCIHandleFree() should not be used because OCIStmtRelease() will do that
> for you.

OK.

Out of interest, where did you find that information?

>    The freeStatement() callback cleanup function should return apr_status_t
> to match the return value expected by apr_pool_cleanup_register() and
> friends.

indeedie.

>    The freeStatement() callback cleanup function should also use a private
> Oracle error handle like the freeStatements() one (if and when it uses
> OCIStmtRelease() in the future).  Otherwise, if an error occurs during
> the execution of dbd_oracle_query(), then the call to apr_pool_destroy()
> on the private pool causes freeStatement() to be called, which if it
> then encounters its own error, would then overwrite the previous error data
> in the Oracle handle.  That in turn means the caller can't access the
> original error message that caused apr_dbd_query() to fail.
>
>    In all #ifdef DEBUG blocks, never overwrite sql->status.  Otherwise the
> debugging code changes the error values and messages output, which isn't
> really what you want debugging code to do; instead, it should be
> non-invasive.
>
>    In dbd_oracle_prepare(), make sure to set stmt->nargs to 0 before
> counting arguments in the SQL query.  Otherwise, if the caller passes a
> non-NULL statement pointer (i.e., a previously allocated apr_dbd_prepared_t
> structure), we miscalculate the number of expected arguments to
> p[v]query/select(). This usually causes a crash when the subsequent calls
> to p[v]query/select() are made.

OK, makes good sense on a first reading, but I think I may need to try and
return to it sometime when I'm not being driven mad by building noise from
from outside.

>    Finally, remove some non-functional BLOB/CLOB trial code from pvquery()
> and make it similar to the other three p[v]query/select() functions in
> this regard.

Fairy nuff.  There's a variety of #ifdef placeholder code in there, waiting
for any clarification of that oracle documentation.

Good work!

-- 
Nick Kew

Mime
View raw message