On 7/19/2010 12:14 PM, Kappa wrote:
After doing some testing and then comparing the odbc driver to the other
drivers (pgsql, oracle, etc). I noticed
what appears to be a bug in the ODBC driver. For getting binary data (in
odbc_datum_get), it is using the
/void *data/ argument differently than the other drivers--it is copying
over the contents pointed to by the data pointer
instead of assigning the pointer address into the location of the data
pointer. The first patch below shows the change
that fixes it to be like the other drivers. (Oracle and the other
drivers all do something like: '/*(char**)data = (char*)entry;/".)
I tested this and it seems to fix it to work like the other drivers (at
least the pgsql driver--that is the one I have tested
for comparison).
The second patch below fixes it to (silently) return zero rows if that
is what is selected within the odbc_pbquery function.
It does not seem proper to me to report a (noisy) error if no rows are
selected--that could be the point of the query after all.
*** apr_dbd_odbc.c.orig 2010-07-19 10:52:17.000000000 -0500
--- apr_dbd_odbc.c 2010-07-19 11:01:54.000000000 -0500
***************
*** 1323,1329 ****
return APR_ENOENT; /* SQL NULL value */
if (len < 0)
! strcpy(data, p);
else
memcpy(data, p, len);
--- 1323,1329 ----
return APR_ENOENT; /* SQL NULL value */
if (len < 0)
! *(char**)data = (char *)p;
else
memcpy(data, p, len);
***************
*** 1557,1565 ****
if (SQL_SUCCEEDED(rc)) {
rc = SQLExecute(statement->stmt);
! CHECK_ERROR(handle, "SQLExecute", rc, SQL_HANDLE_STMT,
statement->stmt);
}
if (SQL_SUCCEEDED(rc)) {
SQLLEN rowcount;
--- 1557,1569 ----
if (SQL_SUCCEEDED(rc)) {
rc = SQLExecute(statement->stmt);
! if (rc == SQL_NO_DATA) {
! *nrows = (int) 0;
! } else {
! CHECK_ERROR(handle, "SQLExecute", rc, SQL_HANDLE_STMT,
statement->stmt);
}
+ }
if (SQL_SUCCEEDED(rc)) {
SQLLEN rowcount;
------------------------------------------end
patch--------------------------
Brian Dunford-Shore
--
ψ (λ _κ )
PsiLambda LLC
2430 Tesson Ferry Road #203
Saint Louis, Missouri 63128-2702
Hi Brian,
Thanks for your suggestions about the ODBC driver.
You are right that the ODBC driver delivers binary data differently from the other dbd drivers for
some datatypes: TEXT, STRING, TIME, DATE, etc. It isn't clear which driver is correct.
The User Manual at
http://apr.apache.org/docs/apr/trunk/group___a_p_r___util___d_b_d.html#g67e57ef4eb7952df79ceaa6e92767d41
says: "data - pointer to data, allocated by the caller"
which implies that the caller manages the memory for binary data retrieved with
apr_dbd_datum_get.
Does this mean - for datatypes like STRING and TEXT -
A) the caller allocates memory for the data, and the data remains accessible until the caller
releases it? This is what the ODBC driver does.
-or-
B) the caller allocates only the pointer, not the data, and the data becomes inaccessible after the
next call to apr_dbd_get_row? This is what the other drivers do.
I think A) is probably the better choice, else the caller manages the data lifetime for some
datatypes but not for others with apr_dbd_datum_get. That just seems prone to error.
We do, however, need to make the dbd drivers consistent. Does anyone else have an opinion on which
way is correct here?
re: "It does not seem proper to me to report a (noisy) error if no rows are
selected"
The error shouldn't get reported on stderr unless the debugging environment variable
'apr_dbd_odbc_log' is set. It is still necessary to create and save some string for the SQL_NO_DATA
result, in case the apr_dbd_error function gets called next.
Are you finding that the calls to the check_error function are a performance problem? or do you
just not want to see any SQL_NO_DATA messages when you have 'apr_dbd_odbc_log' set?
Regards,
-tom-
