apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Tom Donovan <donov...@bellatlantic.net>
Subject Re: apr_dbd_odbc patches
Date Sun, 25 Jul 2010 18:48:38 GMT
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-


Mime
View raw message