Database fixes for ODBC/MS/SQL

[PhotonDemon]

Hi,

I have attached a zip of some file outs of code that accesses MS/SQL through ODBC that I have fixed to prevent some problems. The first of which I think is reported by prakash in his post "ODBC HY104 Error". The attached changes are for VA Smalltalk V8. I have been making similar changes since at least VAST 5.5.2.

There are changes in AbtDbmFieldsApp:

AbtDatabaseDecimalField>>valueAsSQLString:
AbtDatabaseDecimalField>>valueAsSQLStringIn:

There current code sends "5." to MS/SQL which it doesn't like, the fix sends "5.0".

AbtDatabaseTimestampField>>precision
AbtDatabaseTimestampField>>precision:

Are changed to use a precision of at most 23. MS/SQL doesn't like the extra digits when the precision of 26. It is too stupid to just throw away the insignificant digits, even when they are 000. I think there is an MS/SQL option to ignore this error but I can't seem to keep it set.

AbtDatabaseTimestampField>>scale
AbtDatabaseTimestampField>>scale:

Keeps the scale to 3 or less. Fixes a similar problem to the one above.

A change is made to AbtDbmOdbcApp in:

AbtTimestamp>>abtAsOdbcTimestampString

Same problem as timestamp above.

In AbtRunDatabaseQueryPartApp

AbtQueryResultRow>>at:
AbtQueryResultRow>>at:put:

are changed to convert the column name to a string. In some places in the VA code a symbol is used for column names and in other places the column names ar strings. These methods want a string for the column name, I send #asString to the column name parameter so it is a string and I can use symbols without a problem. This really doesn't fix a bug, it just make life easier.

Last, I remove AbtRunDatabaseLogonApp as a prerequisite of AbtRunDatabaseQueryPartApp because I don't think it really is a prerequisite and AbtRunDatabaseLogonApp has CommonGraphics as a prerequisite and that prevents its use in a headless NT service.

These changes are all fairly small and simple. I would love it if they were incorporated into the base VA Smalltalk code so I don't have to make them myself every time I upgrade to a new version.

If I am somehow mistaken about the need for these changes, I will be quite happy to apologize for assuming they are bugs and use whatever circumventions or alternate code so I can dump them.

Lou

[waynej]

Our application works with ODBC to SQL Server without changing the base code but by subclassing it. We have our own subclass of AbtDatabaseTimestampField for perhaps the same effect (precision of 23 and scale of 3). We even have a subclass of AbtDatabaseTimestampField to simulate a DATE column. We also use the year 1900 to indicate a TIME column. We also have a subclass of AbtOdbcVarCharField which allows us to store Null characters by replacing them with another character as they are written and then substituted back when read.

[tc]

Hello,

I'll evaluate the changes and raise this issue in our weekly meeting.

--tc

[PhotonDemon]

Thanks tc,

I'll evaluate the changes and raise this issue in our weekly meeting.

If you have any questions, you can post them here or email me directly.

Lou

[benvandijk]

Hi Lou,

Thanks for sharing your fixes.
We did not experience any problems with the decimal field.
Possibly we use a different MS/SQL version

We did change the AbtTimestamp>>abtAsOdbcTimestampString
in another way by leaving out the microseconds.
MySQL does not like the micorseconds either.

Another interesting fact is that subclasses exist for odbc fields
e.g. AbtOdbcDecimalField and AbtOdbcTimestampField
but they are not used in the AbtOdbcDatabaseManager>> buildTypeDict

[PhotonDemon]

Hi tc,

Is there any news or progress on this topic? Can we expect any of the fixes in v8.0.3? I think it is due this month and I would love to upgrade to it without having to add these changes.

Lou

[tc]

Hello,

I don't think it will be in this release. For each change, a test case has to be developed. That means:
1. Preconditions or setup.
2. The actual test.
3. Expected result.

Also a description of why the change is needed and/or what it fixes. Other platforms need to be tested (DB/2, Oracle) since this is primarily for SQL Server.

AbtTimestamp has had long running issues concerning DB's and any changes would probably get extra scrutiny.

What is the effect of changing the precision? Will this break someone's ODBC Oracle app?

The last factor to consider is availability. Things are done when time permits.

Thanks.

--tc

[PhotonDemon]

Hi tc,

I understand your concerns but how about making the values of interest (like precision, scale an such) class variables. They could be set to their current values and changed by those of us who need different values. The changes needed to do this are few and easily tested. Those of us who don't have a problem with the current values would not be affected. Those of us who need different values can easily change them to meet our needs with a few simple lines of code that can be run when our app is loaded or almost any time for that matter. This way we don't have to keep changing base code.

Thanks, Lou

[benvandijk]

Hi tc and Lou,

We strongly support the suggestion from Lou.
Making things more flexible will help everybody

[tc]

Hello,

I'll raise the issue in our weekly meeting, early next week.

--tc

[jtuchel]

Taylor,

I'll raise the issue in our weekly meeting, early next week.

any news on that?

Joachim

[tc]

Hello,

So, the changes are:

1. Have the methods below pull their values from a class variable and have class side accessors which allows one to change the default values.

AbtDatabaseTimestampField>>precision
AbtDatabaseTimestampField>>precision:
AbtDatabaseTimestampField>>scale
AbtDatabaseTimestampField>>scale:

2. Don't include microseconds if they are zero for:

AbtTimestamp>>abtAsOdbcTimestampString

3. No changes for:

AbtQueryResultRow>>at:
AbtQueryResultRow>>at:put:

AbtDatabaseDecimalField>>valueAsSQLString:
AbtDatabaseDecimalField>>valueAsSQLStringIn:

Number two (and 3) above makes me a little nervous. However, I'd like to include the changes but activate them with a class side switch. For example, 'AbtDatabaseDecimalField>>valueAsSQLString:' would become:

Code: Select all

AbtDatabaseDecimalField>>valueAsSQLString:
self class usingMSSql
   ifTrue: [ self msSQLValueAsSQLString: aNumber ]
   ifFalse: [ self defaultValueAsSQLString: aNumber ]

. . . but perhaps you have a better idea about how to include the change or implement the switch?

--tc

[jtuchel]

Hi Taylor,

isn't there something like #abrAsSQLStringForDatabaseMgr:
Maybe the class side switch should be in the (ODBC) Database Manager?
Maybe there's even a need for a MS specific ODBC Database Manager (how ironic ).

To me that sounds much better than implementing a second path of if-statements for things that have been solved differently before. But maybe I am on the wrong path...

Joachim

[PhotonDemon]

Hi Taylor,

Thanks for all your work on this. The changes for:

1. Have the methods below pull their values from a class variable and have class side accessors which allows one to change the default values. Snip...

sound good, they should be a big help.

The change for:

2. Don't include microseconds if they are zero for:

shouldn't concern you too much as most (if not all) code presented with missing low order significant digits, will assume they are zero. The way I changed the code, if microseconds were present (non zero), they are sent to ODBC, if the SQL database engine on the other side could accept them, things are the same as before the change. If the database is MS/SQL, without the extra zeros "000" it is happy, with non zero values, it will have the same problem as before the change.

As for:

AbtQueryResultRow>>at:
AbtQueryResultRow>>at:put:

I would like you to reconsider the change. It just sends #asString to the aColumnName, which is expected to be a string column name. Other than being slightly slower (because of the extra message send), I don't see how this could be a problem. If a string is sent, the string is used, if a Symbol is sent, its string value is used. I can't point to them right now but I think there are other places in the code where this is done.

Also for:

AbtDatabaseDecimalField>>valueAsSQLString:
AbtDatabaseDecimalField>>valueAsSQLStringIn:

it would be nice if you could reconsider those changes too. All the change does is to add a zero "0" after a value that would end up looking like "5.", so we would get "5.0". Don't ask me why but MS/SQL can't deal with "5." and needs "5.0" otherwise it throws an error. Is it just me or is that dumb? Anyway, I can't image any other SQL having a problem with the "5.0" that the changed could would result in.

Also, please don't forget about:

AbtRunDatabaseQueryPartApp - Remove AbtRunDatabaseLogonApp as a prerequisite because it has CommonGraphics as a prerequisite and prevents use as a headless NT service.

I don't think AbtRunDatabaseLogonApp is actually required as a prerequisite of AbtRunDatabaseQueryPartApp.

Thanks much,

Lou

[tc]

Hello,

Can you post a table definition and the SQL being executed that hits each of the areas being changed? IOW, test cases.

Thanks.

--tc