Hi, I've been using Torque in my application for over a year now and I've just discovered that every time an OM object is saved, the record is first SELECT'ed. I can't fathom a reason why this would be necessary or deemed a good practice. It may be acceptable when updating single records, but when doUpdate() is used to affect multiple records, the performance cost is totally unacceptable.
Another problem is that doUpdate doesn't report back how many records were updated. There's a check in the code which logs an error if a single record was expected and MORE than 1 record was updated, but there's no warning if NO records were updated. This can lead to insidious bugs in Torque if the user messes with the "new" property of their OM objects. If the user calls setNew(false) on an OM object that has not been saved yet, the update will silently do nothing. Yes, that is a user error, but I think it might also be possible for Torque itself to erroneously setNew(false) and cause this to happen. Our DBA consultants have been complaining for a long time about the number of queries filling the SQL cache in our Oracle DB. The problem is that Torque uses Statements instead of PreparedStatements for all queries generated by Criteria. Thus every SQL statement is unique and requires parsing by the SQL engine. At a minimum, any time you perform retrieveByPK, a PreparedStatment should be used so that a distinct query isn't generated every time. Random queries generated by Criteria are less frequent and concern me less. But, the other problem with generating SQL strings instead of using PreparedStatements is that it requires conversion of every Java type to a string representation which is error-prone and inefficient. We have run into a problem when we use a VARCHAR field with a numeric value. We have a legacy table where the primary key is a VARCHAR2 column, but in practice the values are always numbers. Thus in the code we want to use Number objects to enforce the data integrity. The problem is that Torque will blindly create a WHERE clause of the form "order_number = 1234" instead of "order_number = '1234'" because it does not take into consideration the actual data type of the column. The result is that queries must do FULL TABLE scans even though we're selecting a single record by the primary key value. (When you supply a number constant for a VARCHAR2 field, the SQL engine has to convert every single VARCHAR2 key value to a NUMBER in order to make the comparison properly.) For now we've worked around the problem by converting the Number to a String in the Peer classes that setup the Criteria, but this is not the optimal solution. Using PreparedStatements for all SELECTs would solve this problem entirely, and give the added benefit of handling other SQL types. Another improvement would be if the Torque objects only updated the columns that were actually modified. A simple BitSet could be used to keep track of the dirty properties, and then the Criteria for update could be generated using only the dirty columns. This can easily be added to the Peer template. I don't understand what the point is in using Village for Torque. It seems to me that Torque has to bend over backwards to make things work with Village and would be better served by using the JDBC API directly. Village appears to be a good tool for developers who were previously doing things directly with JDBC, but not for the developers who have graduated to Torque. Torque is very inefficient when doing selects because it has to convert every Village Record into a Torque object. It would be much faster to simply use the ResultSet directly. The only valuable part of Vlillage that I can see is the "Value" class which handles conversion to/from Java native types to SQL native types automatically. This can easily be extracted into a more reusable form. I don't think we should drag along the whole Village API just for that. The Torque code would be much easier to maintain if it didn't use yet-another API. Besides, it's very difficult for developers to even find info on Village on the Net. I will be submitting a patch to fix the doUpdate method, since I consider that the biggest problem. I may also start working on updating the doPSSelectMethods so that they can be used instead of the existing implementation. - Chris __________________________________________________________________ The NEW Netscape 7.0 browser is now available. Upgrade now! http://channels.netscape.com/ns/browsers/download.jsp Get your own FREE, personal Netscape Mail account today at http://webmail.netscape.com/
