Return-Path: Delivered-To: apmail-db-ojb-dev-archive@www.apache.org Received: (qmail 7629 invoked from network); 7 Oct 2005 14:34:35 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (209.237.227.199) by minotaur.apache.org with SMTP; 7 Oct 2005 14:34:34 -0000 Received: (qmail 38025 invoked by uid 500); 7 Oct 2005 14:34:34 -0000 Delivered-To: apmail-db-ojb-dev-archive@db.apache.org Received: (qmail 37988 invoked by uid 500); 7 Oct 2005 14:34:33 -0000 Mailing-List: contact ojb-dev-help@db.apache.org; run by ezmlm Precedence: bulk List-Unsubscribe: List-Help: List-Post: List-Id: "OJB Developers List" Reply-To: "OJB Developers List" Delivered-To: mailing list ojb-dev@db.apache.org Received: (qmail 37977 invoked by uid 500); 7 Oct 2005 14:34:33 -0000 Received: (qmail 37965 invoked by uid 99); 7 Oct 2005 14:34:33 -0000 X-ASF-Spam-Status: No, hits=-9.8 required=10.0 tests=ALL_TRUSTED,NO_REAL_NAME X-Spam-Check-By: apache.org Received: from [209.237.227.194] (HELO minotaur.apache.org) (209.237.227.194) by apache.org (qpsmtpd/0.29) with SMTP; Fri, 07 Oct 2005 07:34:32 -0700 Received: (qmail 6624 invoked by uid 1510); 7 Oct 2005 14:34:12 -0000 Date: 7 Oct 2005 14:34:12 -0000 Message-ID: <20051007143412.6623.qmail@minotaur.apache.org> From: arminw@apache.org To: db-ojb-cvs@apache.org Subject: cvs commit: db-ojb/src/java/org/apache/ojb/broker/accesslayer JdbcAccessImpl.java X-Virus-Checked: Checked by ClamAV on apache.org X-Spam-Rating: minotaur.apache.org 1.6.2 0/1000/N arminw 2005/10/07 07:34:12 Modified: src/java/org/apache/ojb/broker/accesslayer/sql SqlDeleteByPkStatement.java SqlExistStatement.java SqlGeneratorDefaultImpl.java SqlInsertStatement.java SqlSelectByPkStatement.java SqlUpdateFieldsStatement.java SqlUpdateStatement.java src/java/org/apache/ojb/broker/accesslayer JdbcAccessImpl.java Log: code optimization, cache sql-string of SqlStatement Revision Changes Path 1.9 +16 -11 db-ojb/src/java/org/apache/ojb/broker/accesslayer/sql/SqlDeleteByPkStatement.java Index: SqlDeleteByPkStatement.java =================================================================== RCS file: /home/cvs/db-ojb/src/java/org/apache/ojb/broker/accesslayer/sql/SqlDeleteByPkStatement.java,v retrieving revision 1.8 retrieving revision 1.9 diff -u -r1.8 -r1.9 --- SqlDeleteByPkStatement.java 22 Nov 2004 20:55:23 -0000 1.8 +++ SqlDeleteByPkStatement.java 7 Oct 2005 14:34:11 -0000 1.9 @@ -32,8 +32,9 @@ */ public class SqlDeleteByPkStatement extends SqlPkStatement { + private String sql; - /** + /** * Constructor for SqlDeleteByPkStatement. * @param aPlatform * @param aLogger @@ -49,15 +50,19 @@ */ public String getStatement() { - StringBuffer stmt = new StringBuffer(1024); - ClassDescriptor cld = getClassDescriptor(); - - stmt.append("DELETE FROM "); - appendTable(cld,stmt); - appendWhereClause(cld, true, stmt); //use Locking - - return stmt.toString(); - } + if(sql == null) + { + StringBuffer stmt = new StringBuffer(1024); + ClassDescriptor cld = getClassDescriptor(); + + stmt.append("DELETE FROM "); + appendTable(cld,stmt); + appendWhereClause(cld, true, stmt); //use Locking + + sql = stmt.toString(); + } + return sql; + } /** * Generate a where clause for a prepared Statement. 1.7 +22 -16 db-ojb/src/java/org/apache/ojb/broker/accesslayer/sql/SqlExistStatement.java Index: SqlExistStatement.java =================================================================== RCS file: /home/cvs/db-ojb/src/java/org/apache/ojb/broker/accesslayer/sql/SqlExistStatement.java,v retrieving revision 1.6 retrieving revision 1.7 diff -u -r1.6 -r1.7 --- SqlExistStatement.java 22 Nov 2004 20:55:23 -0000 1.6 +++ SqlExistStatement.java 7 Oct 2005 14:34:11 -0000 1.7 @@ -33,6 +33,8 @@ private static final String SELECT = "SELECT "; private static final String FROM = " FROM "; + private String sql; + public SqlExistStatement(Platform aPlatform, Logger aLogger, ClassDescriptor aCld) { super(aPlatform, aLogger, aCld); @@ -43,22 +45,26 @@ */ public String getStatement() { - StringBuffer stmt = new StringBuffer(128); - ClassDescriptor cld = getClassDescriptor(); - - FieldDescriptor[] fieldDescriptors = cld.getPkFields(); - if (fieldDescriptors == null || fieldDescriptors.length == 0) + if(sql == null) { - throw new OJBRuntimeException("No PK fields defined in metadata for " + cld.getClassNameOfObject()); - } - FieldDescriptor field = fieldDescriptors[0]; + StringBuffer stmt = new StringBuffer(128); + ClassDescriptor cld = getClassDescriptor(); - stmt.append(SELECT); - appendField(field, stmt); - stmt.append(FROM); - appendTable(cld, stmt); - appendWhereClause(cld, false, stmt); - - return stmt.toString(); + FieldDescriptor[] fieldDescriptors = cld.getPkFields(); + if (fieldDescriptors == null || fieldDescriptors.length == 0) + { + throw new OJBRuntimeException("No PK fields defined in metadata for " + cld.getClassNameOfObject()); + } + FieldDescriptor field = fieldDescriptors[0]; + + stmt.append(SELECT); + appendField(field, stmt); + stmt.append(FROM); + appendTable(cld, stmt); + appendWhereClause(cld, false, stmt); + + sql = stmt.toString(); + } + return sql; } } 1.36 +65 -18 db-ojb/src/java/org/apache/ojb/broker/accesslayer/sql/SqlGeneratorDefaultImpl.java Index: SqlGeneratorDefaultImpl.java =================================================================== RCS file: /home/cvs/db-ojb/src/java/org/apache/ojb/broker/accesslayer/sql/SqlGeneratorDefaultImpl.java,v retrieving revision 1.35 retrieving revision 1.36 diff -u -r1.35 -r1.36 --- SqlGeneratorDefaultImpl.java 3 Oct 2005 17:35:24 -0000 1.35 +++ SqlGeneratorDefaultImpl.java 7 Oct 2005 14:34:11 -0000 1.36 @@ -16,8 +16,9 @@ */ import java.util.Enumeration; -import java.util.HashMap; +import java.util.Map; +import org.apache.commons.collections.map.ReferenceIdentityMap; import org.apache.ojb.broker.metadata.ClassDescriptor; import org.apache.ojb.broker.metadata.FieldDescriptor; import org.apache.ojb.broker.metadata.ProcedureDescriptor; @@ -48,8 +49,17 @@ { private Logger m_logger = LoggerFactory.getLogger(SqlGeneratorDefaultImpl.class); private Platform m_platform; + /* + arminw: + TODO: In ClassDescriptor we need support for "field change event" listener if we allow + to change metadata at runtime. + Further on we have to deal with weak references to allow GC of outdated Metadata classes + (key and value of map have to be weak references, because the SqlForClass indirectly + refer the key in some cases). + Field changes are not reflected in this implementation! + */ /** Cache for {@link SqlForClass} instances, keyed per class descriptor. */ - private HashMap sqlForClass = new HashMap(); + private Map sqlForClass = new ReferenceIdentityMap(ReferenceIdentityMap.WEAK, ReferenceIdentityMap.WEAK); public SqlGeneratorDefaultImpl(Platform platform) { @@ -127,9 +137,8 @@ */ public SqlStatement getPreparedDeleteStatement(ClassDescriptor cld) { - SqlStatement sql; SqlForClass sfc = getSqlForClass(cld); - sql = sfc.getDeleteSql(); + SqlStatement sql = sfc.getDeleteSql(); if(sql == null) { @@ -322,25 +331,52 @@ public SqlStatement getPreparedUpdateStatement(ClassDescriptor cld, FieldDescriptor[] fields) { - SqlStatement result; - // TODO: Check this! Should we check if all fields non-PK fields or should we assume - // that only non-PK fields will be specified? - if(fields.length == cld.getNonPkRwFields().length) + SqlStatement result = null; + /* + TODO: Check this! Should we check that all specified fields are non-PK fields or should we assume + that only non-PK fields will be specified? + + if stored procedures are used we can't use changed fields information + TODO: Is it possible to use stored procedures handling with different numbers of fields?? + */ + if(cld.getUpdateProcedure() != null || fields.length == cld.getNonPkRwFields().length) { result = getPreparedUpdateStatement(cld); } else { - ProcedureDescriptor pd = cld.getUpdateProcedure(); - if(pd == null) - { - result = new SqlUpdateFieldsStatement(m_platform, m_logger, cld, fields); + SqlForClass sfc = getSqlForClass(cld); + /* + arminw: + to avoid generation of new SqlUpdateFieldsStatement for each update statement, we use + a minor optimisation and compare the current fields with the last used SqlUpdateFieldsStatement + TODO: Would it be overkill to cache each combination of update fields or is the current solution sufficient? + */ + SqlUpdateFieldsStatement tmp = sfc.getLastUsedUpdateFieldsSql(); + if(tmp != null) + { + FieldDescriptor[] previousFieldsToUpdate = tmp.getFieldsToUpdate(); + int length = fields.length; + if(previousFieldsToUpdate != null && length == previousFieldsToUpdate.length) + { + for(int i = 0; i < length; i++) + { + FieldDescriptor current = fields[i]; + if(current == previousFieldsToUpdate[i] && (i+1 == length)) + { + result = tmp; + } + else + { + break; + } + } + } } - else + if(result == null) { - // when procedures are used we can't use changed fields information - // TODO: Is it possible to use stored procedures handling with different numbers of fields?? - result = new SqlProcedureStatement(pd, m_logger); + result = new SqlUpdateFieldsStatement(m_platform, m_logger, cld, fields); + sfc.setLastUsedUpdateFieldsSql((SqlUpdateFieldsStatement) result); } } if(m_logger.isDebugEnabled()) m_logger.debug("SQL:" + result.getStatement()); @@ -589,11 +625,12 @@ * This class serves as a cache for sql-Statements * used for persistence operations. */ - public static class SqlForClass + final static class SqlForClass { private SqlStatement deleteSql; private SqlStatement insertSql; private SqlStatement updateSql; + private SqlUpdateFieldsStatement lastUsedUpdateFieldsSql; private SelectStatement selectByPKSql; private SqlStatement selectExists; @@ -622,6 +659,16 @@ return updateSql; } + public SqlUpdateFieldsStatement getLastUsedUpdateFieldsSql() + { + return lastUsedUpdateFieldsSql; + } + + public void setLastUsedUpdateFieldsSql(SqlUpdateFieldsStatement previousUpdateFieldsSql) + { + this.lastUsedUpdateFieldsSql = previousUpdateFieldsSql; + } + public void setUpdateSql(SqlStatement updateSql) { this.updateSql = updateSql; 1.10 +17 -12 db-ojb/src/java/org/apache/ojb/broker/accesslayer/sql/SqlInsertStatement.java Index: SqlInsertStatement.java =================================================================== RCS file: /home/cvs/db-ojb/src/java/org/apache/ojb/broker/accesslayer/sql/SqlInsertStatement.java,v retrieving revision 1.9 retrieving revision 1.10 diff -u -r1.9 -r1.10 --- SqlInsertStatement.java 22 Nov 2004 20:55:23 -0000 1.9 +++ SqlInsertStatement.java 7 Oct 2005 14:34:11 -0000 1.10 @@ -31,8 +31,9 @@ */ public class SqlInsertStatement extends SqlPkStatement { + private String sql; - /** + /** * Constructor for SqlInsertStatement. * @param aPlatform * @param aLogger @@ -48,18 +49,22 @@ */ public String getStatement() { - StringBuffer stmt = new StringBuffer(1024); - ClassDescriptor cld = getClassDescriptor(); + if(sql == null) + { + StringBuffer stmt = new StringBuffer(1024); + ClassDescriptor cld = getClassDescriptor(); - stmt.append("INSERT INTO "); - appendTable(cld, stmt); - stmt.append(" ("); - appendListOfColumns(cld, stmt); - stmt.append(")"); - appendListOfValues(cld, stmt); + stmt.append("INSERT INTO "); + appendTable(cld, stmt); + stmt.append(" ("); + appendListOfColumns(cld, stmt); + stmt.append(")"); + appendListOfValues(cld, stmt); - return stmt.toString(); - } + sql = stmt.toString(); + } + return sql; + } private List appendListOfColumns(ClassDescriptor cld, StringBuffer buf) { 1.12 +14 -4 db-ojb/src/java/org/apache/ojb/broker/accesslayer/sql/SqlSelectByPkStatement.java Index: SqlSelectByPkStatement.java =================================================================== RCS file: /home/cvs/db-ojb/src/java/org/apache/ojb/broker/accesslayer/sql/SqlSelectByPkStatement.java,v retrieving revision 1.11 retrieving revision 1.12 diff -u -r1.11 -r1.12 --- SqlSelectByPkStatement.java 18 Sep 2005 13:26:14 -0000 1.11 +++ SqlSelectByPkStatement.java 7 Oct 2005 14:34:11 -0000 1.12 @@ -32,6 +32,8 @@ public class SqlSelectByPkStatement extends SqlSelectStatement { + private String sql; + /** * Constructor for SqlSelectByPkStatement. * @param cld @@ -45,7 +47,7 @@ /** * Build a Pk-Query base on the ClassDescriptor. * @param cld - * @return + * @return a select by PK query */ private static Query buildQuery(ClassDescriptor cld) { @@ -57,7 +59,15 @@ crit.addEqualTo(pkFields[i].getAttributeName(), null); } // TODO: should use broker.getQueryFactory().newQuery() - Query query = QueryFactory.newQuery(cld.getClassOfObject(), crit); - return query; + return QueryFactory.newQuery(cld.getClassOfObject(), crit); + } + + public String getStatement() + { + if(sql == null) + { + sql = super.getStatement(); + } + return sql; } } 1.3 +1 -1 db-ojb/src/java/org/apache/ojb/broker/accesslayer/sql/SqlUpdateFieldsStatement.java 1.9 +17 -12 db-ojb/src/java/org/apache/ojb/broker/accesslayer/sql/SqlUpdateStatement.java Index: SqlUpdateStatement.java =================================================================== RCS file: /home/cvs/db-ojb/src/java/org/apache/ojb/broker/accesslayer/sql/SqlUpdateStatement.java,v retrieving revision 1.8 retrieving revision 1.9 diff -u -r1.8 -r1.9 --- SqlUpdateStatement.java 22 Nov 2004 20:55:23 -0000 1.8 +++ SqlUpdateStatement.java 7 Oct 2005 14:34:11 -0000 1.9 @@ -28,8 +28,9 @@ */ public class SqlUpdateStatement extends SqlPkStatement { + protected String sql; - /** + /** * Constructor for SqlUpdateStatement. * @param aPlatform * @param aLogger @@ -75,15 +76,19 @@ */ public String getStatement() { - StringBuffer stmt = new StringBuffer(1024); - ClassDescriptor cld = getClassDescriptor(); - - stmt.append("UPDATE "); - appendTable(cld, stmt); - appendSetClause(stmt); - appendWhereClause(cld, true, stmt); //use Locking - - return stmt.toString(); - } + if(sql == null) + { + StringBuffer stmt = new StringBuffer(1024); + ClassDescriptor cld = getClassDescriptor(); + + stmt.append("UPDATE "); + appendTable(cld, stmt); + appendSetClause(stmt); + appendWhereClause(cld, true, stmt); //use Locking + + sql = stmt.toString(); + } + return sql; + } } 1.40 +79 -62 db-ojb/src/java/org/apache/ojb/broker/accesslayer/JdbcAccessImpl.java Index: JdbcAccessImpl.java =================================================================== RCS file: /home/cvs/db-ojb/src/java/org/apache/ojb/broker/accesslayer/JdbcAccessImpl.java,v retrieving revision 1.39 retrieving revision 1.40 diff -u -r1.39 -r1.40 --- JdbcAccessImpl.java 3 Oct 2005 17:35:25 -0000 1.39 +++ JdbcAccessImpl.java 7 Oct 2005 14:34:12 -0000 1.40 @@ -113,8 +113,14 @@ } BatchManager bm = broker.serviceBatchManager(); + /* + arminw: + if optimistic locking is used and the jdbc-driver dosn't support + proper return values, we have to execute the batch and use the + normal statement execution + */ boolean batchExecution = cld.isLocking() && !bm.getBatchSupportOptimisticLocking(); - if(isInBatchMode() && !batchExecution) + if(!batchExecution && isInBatchMode(cld)) { BatchInfo info = new BatchInfo(sql, cld, BatchInfo.TYPE_OBJECT_DELETE); try @@ -128,12 +134,9 @@ } else { - /* - arminw: - if optimistic locking is used and the jdbc-driver dosn't support - proper return values, we have to execute the batch and use the - normal statement execution - */ + // arminw: if an object with optimistic locking enabled, can't be handled + // in batch mode, it's recommended to execute already enqueued batch entries + // to avoid dependency issues if(batchExecution) bm.executeBatch(); PreparedStatement stmt = null; @@ -197,7 +200,9 @@ { values = broker.serviceBrokerHelper().getAllRwValues(cld, obj); } - if(isInBatchMode()) + // if DB Identity columns are used for PK values, we can't use batch + // updates for object insert + if(isInBatchMode(cld) && !cld.useIdentityColumnField()) { BatchInfo info = new BatchInfo(sql, cld, BatchInfo.TYPE_OBJECT_INSERT); try @@ -256,7 +261,7 @@ final StatementManager sm = broker.serviceStatementManager(); final boolean isStoredProcedure = SqlHelper.isStoredProcedure(sql); int result = Statement.SUCCESS_NO_INFO; - if(isInBatchMode()) + if(isInBatchMode(null)) { BatchInfo info = new BatchInfo(sql); try @@ -346,8 +351,14 @@ } BatchManager bm = broker.serviceBatchManager(); + /* + arminw: + if optimistic locking is used and the jdbc-driver dosn't support + proper return values, we have to execute the batch and use the + normal statement execution + */ boolean batchExecution = cld.isLocking() && !bm.getBatchSupportOptimisticLocking(); - if(isInBatchMode() && !batchExecution) + if(isInBatchMode(cld) && !batchExecution) { BatchInfo info = new BatchInfo(sql, cld, BatchInfo.TYPE_OBJECT_UPDATE); try @@ -361,12 +372,9 @@ } else { - /* - arminw: - if optimistic locking is used and the jdbc-driver dosn't support - proper return values, we have to execute the batch and use the - normal statement execution - */ + // arminw: if an object with optimistic locking enabled, can't be handled + // in batch mode, it's recommended to execute already enqueued batch entries + // to avoid dependency issues if(batchExecution) bm.executeBatch(); PreparedStatement stmt = null; @@ -1094,51 +1102,60 @@ return scrollable; } - /** - * A class is batchable when
- * - PB is in tx
- * - batch mode is enabled
- * - class allows batch mode
- *

- * Additionally this method checks if this class use optimistic locking and if the - * {@link org.apache.ojb.broker.accesslayer.batch.BatchManager#getBatchSupportOptimisticLocking} - * flag was set. If both was true the batch - * will be executed and false will be returned. Except when in class-descriptor - * {@link org.apache.ojb.broker.metadata.ClassDescriptor#setBatchable(boolean)} false - * was set, in that case the user is responsible to execute the batch by himself. - *

- */ - protected boolean isBatchableAndCheckOptimisticLockingExecute(ClassDescriptor cld) throws SQLException - { - boolean result = false; - // first we check if we are in batch mode - if(cld.isBatchable() && isInBatchMode()) - { - // if object use optimistic locking and we don't want - // include those objects in batch - if(!broker.serviceBatchManager().getBatchSupportOptimisticLocking() && cld.isLocking()) - { - // if object is batchable we automatic handle the - // batch and execute batch, else user is responsible for doing this - result = false; - broker.serviceBatchManager().executeBatch(); - } - else - { - result = true; - } - } - return result; - } - - protected boolean needsBatchExecuteForOptimisticLocking(ClassDescriptor cld) - { - return cld.isLocking() && !broker.serviceBatchManager().getBatchSupportOptimisticLocking(); - } - - protected boolean isInBatchMode() - { - return broker.isInTransaction() && broker.serviceBatchManager().isBatchMode(); +// /** +// * A class is batchable when
+// * - PB is in tx
+// * - batch mode is enabled
+// * - class allows batch mode
+// *

+// * Additionally this method checks if this class use optimistic locking and if the +// * {@link org.apache.ojb.broker.accesslayer.batch.BatchManager#getBatchSupportOptimisticLocking} +// * flag was set. If both was true the batch +// * will be executed and false will be returned. Except when in class-descriptor +// * {@link org.apache.ojb.broker.metadata.ClassDescriptor#setBatchable(boolean)} false +// * was set, in that case the user is responsible to execute the batch by himself. +// *

+// */ +// protected boolean isBatchableAndCheckOptimisticLockingExecute(ClassDescriptor cld) throws SQLException +// { +// boolean result = false; +// // first we check if we are in batch mode +// if(cld.isBatchable() && isInBatchMode(cld)) +// { +// // if object use optimistic locking and we don't want +// // include those objects in batch +// if(!broker.serviceBatchManager().getBatchSupportOptimisticLocking() && cld.isLocking()) +// { +// // if object is batchable we automatic handle the +// // batch and execute batch, else user is responsible for doing this +// result = false; +// broker.serviceBatchManager().executeBatch(); +// } +// else +// { +// result = true; +// } +// } +// return result; +// } +// +// protected boolean needsBatchExecuteForOptimisticLocking(ClassDescriptor cld) +// { +// return cld.isLocking() && !broker.serviceBatchManager().getBatchSupportOptimisticLocking(); +// } + + /** + * Returns true if batch mode is enabled and allowed for + * the specified {@link org.apache.ojb.broker.metadata.ClassDescriptor}. + * + * @param cld The current used class or null if not available. + * @return True if batch mode can be used. + */ + protected boolean isInBatchMode(ClassDescriptor cld) + { + return broker.isInTransaction() + && broker.serviceBatchManager().isBatchMode() + && (cld == null || cld.isBatchable()); } /** --------------------------------------------------------------------- To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org For additional commands, e-mail: ojb-dev-help@db.apache.org