Allow diagnostics to be retrieved on SQL_NO_DATA

[kadler]

Fixes #136

[v-chojas]

herror->defer_extract = ( ret_code == SQL_SUCCESS_WITH_INFO ? defer_type : defer_type >> 1 ) & 1;

I'm not sure if inverting that check would work for other cases. I believe we added this code specifically to align behaviour with the Windows DM (in most cases), and changing that could cause the behaviour to diverge again.

[lurcher]

On 06/04/2023 18:19, v-chojas wrote:

herror->defer_extract = ( ret_code == SQL_SUCCESS_WITH_INFO ?
defer_type : defer_type >> 1 ) & 1;

I'm not sure if inverting that check would work for other cases. I believe we added this code specifically to align behaviour with the Windows DM (in most cases), and changing that could cause the behaviour to diverge again.

— Reply to this email directly, view it on GitHub https://github.com/lurcher/unixODBC/pull/137#issuecomment-1499381966, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABYK62MQHZE52WBYOF3UYMDW733ILANCNFSM6AAAAAAWVVHLEI. You are receiving this because you modified the open/close state.Message ID: @.***>

I must confess I had not paid as much attention to this defer logic as I should have done. If I understand correctly (and I suspect I don't) what its trying to do, I feel a better description of the logic and the intention may be better for those to come.

[kadler]

I'm not sure if inverting that check would work for other cases. I believe we added this code specifically to align behaviour with the Windows DM (in most cases), and changing that could cause the behaviour to diverge again.

Yeah, I wasn't too sure about that part of the change. I just figured that the deferral was there for non-error cases where applications are less likely to check diagnostic records. I agree with @lurcher that a better description of what the logic is there for would be beneficial.

[lurcher]

On 06/04/2023 19:02, Kevin Adler wrote:

I'm not sure if inverting that check would work for other cases. I
believe we added this code specifically to align behaviour with
the Windows DM (in most cases), and changing that could cause the
behaviour to diverge again.

Yeah, I wasn't too sure about that part of the change. I just figured that the deferral was there for non-error cases where applications are less likely to check diagnostic records. I agree with @lurcher https://github.com/lurcher that a better description of what the logic is there for would be beneficial.

— Reply to this email directly, view it on GitHub https://github.com/lurcher/unixODBC/pull/137#issuecomment-1499431142, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABYK62LHZ3VWKZNVC3BINMTW74AMLANCNFSM6AAAAAAWVVHLEI. You are receiving this because you were mentioned.Message ID: @.***>

I have been thinking a bit more. Just what situation would SQL_NO_DATA happen with error/info messages? Is that not saying a searched update or delete was executed and no rows were matched AND something happened while nothing was happening that we want to tell you about. Would SQL_SUCCESS_WITH_INFO have not been the correct return in that situation? Your "SQL0100 - Row not found for UPDATE" seems entirely superfluous given that is what SQL_NO_DATA indicates.

I guess what I am asking is what was lost of value in the behavior before the change?

[kadler]

TBH, I'm not sure the benefit of returning these diagnostic records myself for the same reason you mention. As I mentioned in the #136, it was a customer that reported the behavior difference compared to Windows and I don't have any knowledge of why our driver returns these records other than "because it can".

As to returning SQL_SUCCESS_WITH_INFO instead, that goes against the documented behavior of SQLExecDirect:

If SQLExecDirect executes a searched update, insert, or delete statement that does not affect any rows at the data source, the call to SQLExecDirect returns SQL_NO_DATA.

I checked other drivers on Windows to see what they returned in this case and neither MariaDB, Postgres, or MSSQL drivers provide any diagnostic records in this case. All return SQL_NO_DATA from SQLExecDirect except MariaDB, which returns SQL_SUCCESS for some strange reason.

I've told our service team to inform the customer that there's nothing we can do from the drivers side to fix the issue and pointed them to #136. I'm not sure if they will chime in here or not, but if they respond to our service team then I may be able to pass that information along.

[lurcher]

On 06/04/2023 20:05, Kevin Adler wrote:

TBH, I'm not sure the benefit of returning these diagnostic records myself for the same reason you mention. As I mentioned in the #136 https://github.com/lurcher/unixODBC/issues/136, it was a customer that reported the behavior difference compared to Windows and I don't have any knowledge of why our driver returns these records other than "because it can".

As to returning |SQL_SUCCESS_WITH_INFO| instead, that goes against the documented behavior of SQLExecDirect:

If SQLExecDirect executes a searched update, insert, or delete
statement that does not affect any rows at the data source, the
call to SQLExecDirect returns SQL_NO_DATA.

Yep, but just to play the "reading the book" game,

"When SQLExecDirect returns either SQL_ERROR or SQL_SUCCESS_WITH_INFO, an associated SQLSTATE value can be obtained by calling SQLGetDiagRec"

I would read that as if neither SQL_ERROR or SQL_SUCCESS_WITH_INFO is returned, an associated SQLSTATE value can not be obtained by calling *SQLGetDiagRec***

Also from the same page

01000 General warning Driver-specific informational message. (Function returns SQL_SUCCESS_WITH_INFO.)

Which to me indicates that it should be returning SQL_SUCCESS_WITH_INFO.

But reality always gets in the way of the spec.

I would just want to understand what the significance of the deferred errors are and what should be done there. Its a holiday weekend here so I will do my best to ignore this for a couple of days, but in the absence of more info I will try and understand the logic behind it on Tuesday.

[kadler]

Yeah, the bigger the spec the more likely there is for ambiguity and stuff to fall in the cracks.

I did a bit of further digging to see if I could find more clarity here and found this page: https://learn.microsoft.com/en-us/sql/odbc/reference/develop-app/return-codes-odbc

SQL_NO_DATA - No more data was available. The application calls SQLGetDiagRec or SQLGetDiagField to retrieve additional information. One or more driver-defined status records in class 02xxx may be returned.

So it does seem that the spec supports this, though apparently our driver should be returning a class 02xxx SQLSTATE instead of a class 01xxx. Class 02xxx does actually match with our data sources' SQLSTATE definitions so I'll add that as a todo for myself.

However, unless you found this page you could be very well mistaken for believing that only SQL_SUCCESS_WITH_INFO and SQL_ERROR return diagnostic records since that seems to be pretty common throughout the rest of the doc. Heck, even pages within the diagnostics section refer to just these return codes, eg. https://learn.microsoft.com/en-us/sql/odbc/reference/develop-app/using-sqlgetdiagrec-and-sqlgetdiagfield

For example, the following code prompts the user for an SQL statement and executes it. If any diagnostic information was returned, it calls SQLGetDiagField to get the number of status records and SQLGetDiagRec to get the SQLSTATE, native error code, and diagnostic message from those records.

(emphasis added)

It then shows an example where diagnostic records are retrieved only when SQL_SUCCESS_WITH_INFO or SQL_ERROR is returned...

So I think it's correct to say that the ODBC spec was written with the assumption that SQL_SUCCESS_WITH_INFO and SQL_ERROR are the only return codes for which it expects data sources to provide diagnostic records, but technically any return code other than SQL_INVALID_HANDLE could return diagnostic records.

[lurcher]

On 07/04/2023 16:04, Kevin Adler wrote:

Yeah, the bigger the spec the more likely there is for ambiguity and stuff to fall in the cracks.

Yep, its ever as clear as mud. The real problem is caused by the ODBC 2 and 3 error functions behaving differently, SQLError just queues the errors and SQLGetDiag only has errors available until the next API call. That's the origin of unixODBC saving the errors to allow SQLError to be called by a ODBC 2 application when connecting to a ODBC 3 driver. If it wasn't for that it would have been a lot simpler.

[kadler]

The real problem is caused by the ODBC 2 and 3 error functions behaving differently, SQLError just queues the errors and SQLGetDiag only has errors available until the next API call.

Ohhhhh! I was wondering why the driver manager was doing all this work and not just passing things through to the driver. That makes sense now.

[lurcher]

On 07/04/2023 18:42, Kevin Adler wrote:

The real problem is caused by the ODBC 2 and 3 error functions
behaving differently, SQLError just queues the
errors and SQLGetDiag only has errors available until the next API
call.

Ohhhhh! I was wondering why the driver manager was doing all this work and not just passing things through to the driver. That makes sense now.

— Reply to this email directly, view it on GitHub https://github.com/lurcher/unixODBC/pull/137#issuecomment-1500498475, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABYK62PRM3FWJE2TT4HXNH3XABGXJANCNFSM6AAAAAAWVVHLEI. You are receiving this because you were mentioned.Message ID: @.***>

TBH, SQLError has not worked in a ODBC 2 way with regards to queuing up errors for many years now, USE_OLD_ODBC2_ERROR_CLEARING would be need to be enabled to allow this.

My worry is to avoid an API call into the driver on the end of every SQLFetch that returns SQL_NO_DATA. I think at the moment the change is ok in that respect, but as to how it matches the MS DM I am not sure ATM.

--

Nick