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
