PATCH: ODBC application discarding sqlstate in get_diagnos function

[Andrew Thompson-15]

Hi,

I've been using the ODBC application against a unixodbc/mysql setup for
a while now and its been bothering me that ANY SQL error always returns
{port_exit,could_not_access_column_count} from the erlang side. I
decided to dig into the code and figure out why and this is what I
discovered:

In the get_diagnos function of odbcserver.c every time SQLGetDiagRec is
called diagnos.sqlState is passed in as the target to write the returned
SQLSTATE into. However, when you've run out of diagnostic information
and SQLGetDiagRec() returns something other than SQL_SUCCESS, the value
of diagnos.sqlState *still* gets changed (at least in my case) to 00000,
which is, of course, the INFO SQLSTATE. This causes the check in
db_query against INFO to succeed when it should be failing and since in
my case the error message didn't begin with 'error' it falls through
encode_result which is where the EXIT_COLS comes in. Attached is a patch
to resolve this problem by only updating diagnos.sqlState when
SQLGetDiagRec returns success.

I don't know if all ODBC implementations behave this way, but I'd assume
that the SQLSTATE from a failed SQLGetDiagRec can safely be ignored in
all cases.

Thanks,

Andrew

--- c_src/odbcserver.c.orig 2009-10-09 13:11:01.000000000 -0400
+++ c_src/odbcserver.c 2009-10-09 13:20:21.000000000 -0400
@@ -2451,6 +2451,7 @@
     SQLSMALLINT errmsg_buffer_size, record_nr, errmsg_size;
     int acc_errmsg_size;
     byte *current_errmsg_pos;
+    SQLCHAR current_sql_state[SQL_STATE_SIZE];
 
     diagnos.error_msg[0] = 0;
     
@@ -2463,7 +2464,7 @@
     /* Foreach diagnostic record in the current set of diagnostic records
        the error message is obtained */
     for(record_nr = 1; ;record_nr++) {    
- if(SQLGetDiagRec(handleType, handle, record_nr, diagnos.sqlState,
+ if(SQLGetDiagRec(handleType, handle, record_nr, current_sql_state,
  &nativeError, current_errmsg_pos,
  (SQLSMALLINT)errmsg_buffer_size, &errmsg_size)
    != SQL_SUCCESS) {
@@ -2471,6 +2472,9 @@
       
     break;
  } else {
+    /* update the sqlstate in the diagnos record, because the SQLGetDiagRec
+       call succeeded */
+    memcpy(diagnos.sqlState, current_sql_state, SQL_STATE_SIZE);
     errmsg_buffer_size = errmsg_buffer_size - errmsg_size;
     acc_errmsg_size = acc_errmsg_size + errmsg_size;
     current_errmsg_pos = current_errmsg_pos + errmsg_size;



________________________________________________________________
erlang-patches mailing list. See http://www.erlang.org/faq.html
erlang-patches (at) erlang.org

[Juhani Ränkimies-2]

I've had similar problems on windows with mssql2008. The behaviour
wasn't identical, but with luck your patch fixes it in my setup too.

I'll definitely have a look. It'll take a while though, as setting up
a build env in windows is laborious and I've got my hands full
already.

Thanks,
-juhani


On Fri, Oct 9, 2009 at 8:47 PM, Andrew Thompson <andrew@...> wrote:

> Hi,
>
> I've been using the ODBC application against a unixodbc/mysql setup for
> a while now and its been bothering me that ANY SQL error always returns
> {port_exit,could_not_access_column_count} from the erlang side. I
> decided to dig into the code and figure out why and this is what I
> discovered:
>
> In the get_diagnos function of odbcserver.c every time SQLGetDiagRec is
> called diagnos.sqlState is passed in as the target to write the returned
> SQLSTATE into. However, when you've run out of diagnostic information
> and SQLGetDiagRec() returns something other than SQL_SUCCESS, the value
> of diagnos.sqlState *still* gets changed (at least in my case) to 00000,
> which is, of course, the INFO SQLSTATE. This causes the check in
> db_query against INFO to succeed when it should be failing and since in
> my case the error message didn't begin with 'error' it falls through
> encode_result which is where the EXIT_COLS comes in. Attached is a patch
> to resolve this problem by only updating diagnos.sqlState when
> SQLGetDiagRec returns success.
>
> I don't know if all ODBC implementations behave this way, but I'd assume
> that the SQLSTATE from a failed SQLGetDiagRec can safely be ignored in
> all cases.
>
> Thanks,
>
> Andrew
>
>
> ________________________________________________________________
> erlang-patches mailing list. See http://www.erlang.org/faq.html
> erlang-patches (at) erlang.org

________________________________________________________________
erlang-patches mailing list. See http://www.erlang.org/faq.html
erlang-patches (at) erlang.org

[Andrew Thompson-15]

On Sat, Oct 10, 2009 at 10:30:48AM +0300, Juhani R?nkimies wrote:
> I've had similar problems on windows with mssql2008. The behaviour
> wasn't identical, but with luck your patch fixes it in my setup too.
>
> I'll definitely have a look. It'll take a while though, as setting up
> a build env in windows is laborious and I've got my hands full
> already.
>
If it doesn't, I can try to resurrect my windows build setup and do some
investigating. What behaviour are you seeing?

Andrew

________________________________________________________________
erlang-patches mailing list. See http://www.erlang.org/faq.html
erlang-patches (at) erlang.org

[Juhani Ränkimies-2]

On Sat, Oct 10, 2009 at 7:45 PM, Andrew Thompson <andrew@...> wrote:

>>
> If it doesn't, I can try to resurrect my windows build setup and do some
> investigating. What behaviour are you seeing?
>

Sometimes when an update/insert/delete is successfully executed in the
db, I get {error,"No SQL-driver information available."}.

My assumption (with no factual support) is that odbc app gets confused
if the db sends additional information (SQLSTATE?) about the execution
of the statement, like nulls ignored by aggregate functions or info
about type conversions that were made.

-juhani

________________________________________________________________
erlang-patches mailing list. See http://www.erlang.org/faq.html
erlang-patches (at) erlang.org

[Andrew Thompson-15]

On Sun, Oct 11, 2009 at 10:40:10AM +0300, Juhani R?nkimies wrote:
> On Sat, Oct 10, 2009 at 7:45 PM, Andrew Thompson <andrew@...> wrote:
>
 
> Sometimes when an update/insert/delete is successfully executed in the
> db, I get {error,"No SQL-driver information available."}.
>
> My assumption (with no factual support) is that odbc app gets confused
> if the db sends additional information (SQLSTATE?) about the execution
> of the statement, like nulls ignored by aggregate functions or info
> about type conversions that were made.
>

Ah, that does sound like a similar issue although I'm not sure my patch
will solve it. Does it happen consistantly or is it random? Are you
using parameterized queries?

Andrew

________________________________________________________________
erlang-patches mailing list. See http://www.erlang.org/faq.html
erlang-patches (at) erlang.org

[Juhani Ränkimies-2]

On 10/11/09, Andrew Thompson <andrew@...> wrote:
>
> Ah, that does sound like a similar issue although I'm not sure my patch
> will solve it. Does it happen consistantly or is it random? Are you
> using parameterized queries?

I haven't worked with that part of the system in 5-6 months and i'm no
longer sure if it was consistent or not. I'm on a trip and can't check
it now. I'll try to find time to reproduce it on weekend.

Juhani

________________________________________________________________
erlang-patches mailing list. See http://www.erlang.org/faq.html
erlang-patches (at) erlang.org

[Juhani Ränkimies-2]

On Mon, Oct 12, 2009 at 11:33 PM, Juhani Ränkimies <juhani@...> wrote:
> On 10/11/09, Andrew Thompson <andrew@...> wrote:
>>
>> Ah, that does sound like a similar issue although I'm not sure my patch
>> will solve it. Does it happen consistantly or is it random? Are you
>> using parameterized queries?
>
> it now. I'll try to find time to reproduce it on weekend.
>

Yes, I'm using param_query.

I wasn't able to reproduce the problem at this time.
I couldn't use the environment where I observed the problem and the
test setup has at least these differences:
- newer version of erlang
- newer versions of sql server and drivers
- it's is not patched to support unicode strings
- my tests were light, so if it occurs rarely I might not have caught it

next week i'll drop my workaround for {error,"No SQL-driver
information available."} from couple of places and see what happens.

> Juhani
>

________________________________________________________________
erlang-patches mailing list. See http://www.erlang.org/faq.html
erlang-patches (at) erlang.org

[Andrew Thompson-15]

Any feedback from the erlang team on this? I'm willing to work on the
patch if its not satisfactory.

Andrew

________________________________________________________________
erlang-patches mailing list. See http://www.erlang.org/faq.html
erlang-patches (at) erlang.org

[Dan Gudmundsson]

Thanks, I have applied it.
/Dan

On Tue, Nov 10, 2009 at 6:27 PM, Andrew Thompson <andrew@...> wrote:

> Any feedback from the erlang team on this? I'm willing to work on the
> patch if its not satisfactory.
>
> Andrew
>
> ________________________________________________________________
> erlang-patches mailing list. See http://www.erlang.org/faq.html
> erlang-patches (at) erlang.org
>
>

________________________________________________________________
erlang-patches mailing list. See http://www.erlang.org/faq.html
erlang-patches (at) erlang.org

[Andrew Thompson-15]

On Wed, Nov 18, 2009 at 09:12:39AM +0100, Dan Gudmundsson wrote:
> Thanks, I have applied it.
> /Dan
>
Thank you very much.

Andrew

________________________________________________________________
erlang-patches mailing list. See http://www.erlang.org/faq.html
erlang-patches (at) erlang.org