John, I agree that it is a bit of a hack, but I do find the output of Criteria.toString() to be useful for debugging purposes and I fear that a "correct" fix will require a fair bit of work and will be some time off.
Would you be happy if I mark the new methods as protected? Scott -- Scott Eade Backstage Technologies Pty. Ltd. http://www.backstagetech.com.au > From: John McNally <[EMAIL PROTECTED]> > > -1. This patch creates two new public methods that serve no real > purpose other than to be used as part of other methods. And all this is > done just so Criteria.toString() can print out a partial sql string > which is even less useful than it was. It does not solve the problem > that createQueryString is modifying the Criteria object and is only > going to add complexity that makes fixing that more difficult. That last > statement may be incorrect as I don't know what the better fix is. But > I can't see how having to code around these two new public methods is > going to help. > > john mcnally > > On Thu, 2002-11-28 at 08:32, [EMAIL PROTECTED] wrote: >> mpoeschl 2002/11/28 08:32:37 >> >> Modified: src/java/org/apache/torque/util BasePeer.java Criteria.java >> src/test/org/apache/torque/util CriteriaTest.java >> Log: >> TRQS73: Criteria.toString() modifies the Criteria object >> TRQS61: more concise name on Criteria.setSingleRecord >> >> Revision Changes Path >> 1.54 +70 -9 >> jakarta-turbine-torque/src/java/org/apache/torque/util/BasePeer.java >> >> Index: BasePeer.java >> =================================================================== >> RCS file: >> /home/cvs/jakarta-turbine-torque/src/java/org/apache/torque/util/BasePeer.jav >> a,v >> retrieving revision 1.53 >> retrieving revision 1.54 >> diff -u -r1.53 -r1.54 >> --- BasePeer.java 11 Nov 2002 12:09:24 -0000 1.53 >> +++ BasePeer.java 28 Nov 2002 16:32:36 -0000 1.54 >> @@ -902,7 +902,21 @@ >> } >> >> /** >> - * Method to create an SQL query based on values in a Criteria. >> + * Method to create an SQL query for display only based on values in a >> + * Criteria. >> + * >> + * @param criteria A Criteria. >> + * @exception TorqueException Trouble creating the query string. >> + */ >> + public static String createQueryDisplayString(Criteria criteria) >> + throws TorqueException >> + { >> + return createQuery(criteria).toString(); >> + } >> + >> + /** >> + * Method to create an SQL query for actual execution based on values >> in a >> + * Criteria. >> * >> * @param criteria A Criteria. >> * @exception TorqueException Trouble creating the query string. >> @@ -910,6 +924,43 @@ >> public static String createQueryString(Criteria criteria) >> throws TorqueException >> { >> + Query query = createQuery(criteria); >> + DB db = Torque.getDB(criteria.getDbName()); >> + >> + // Limit the number of rows returned. >> + int limit = criteria.getLimit(); >> + int offset = criteria.getOffset(); >> + if (offset > 0 && db.supportsNativeOffset()) >> + { >> + // Now set the criteria's limit and offset to return the >> + // full resultset since the results are limited on the >> + // server. >> + criteria.setLimit(-1); >> + criteria.setOffset(0); >> + } >> + else if (limit > 0 && db.supportsNativeLimit()) >> + { >> + // Now set the criteria's limit to return the full >> + // resultset since the results are limited on the server. >> + criteria.setLimit(-1); >> + } >> + >> + String sql = query.toString(); >> + category.debug(sql); >> + return sql; >> + } >> + >> + /** >> + * Method to create an SQL query based on values in a Criteria. Note >> that >> + * final manipulation of the limit and offset are performed when the >> query >> + * is actually executed. >> + * >> + * @param criteria A Criteria. >> + * @exception TorqueException Trouble creating the query string. >> + */ >> + public static Query createQuery(Criteria criteria) >> + throws TorqueException >> + { >> Query query = new Query(); >> DB db = Torque.getDB(criteria.getDbName()); >> DatabaseMap dbMap = Torque.getDatabaseMap(criteria.getDbName()); >> @@ -1206,19 +1257,31 @@ >> break; >> } >> >> + // The following is now done in createQueryString() to enable >> this >> + // method to be used as part of Criteria.toString() without >> altering >> + // the criteria itself. The commented code is retained here >> to >> + // make it easier to understand how the criteria is built into >> a >> + // query. >> + >> // Now set the criteria's limit and offset to return the >> // full resultset since the results are limited on the >> // server. >> - criteria.setLimit(-1); >> - criteria.setOffset(0); >> + //criteria.setLimit(-1); >> + //criteria.setOffset(0); >> } >> else if (limit > 0 && db.supportsNativeLimit()) >> { >> limitString = String.valueOf(limit); >> >> + // The following is now done in createQueryString() to enable >> this >> + // method to be used as part of Criteria.toString() without >> altering >> + // the criteria itself. The commented code is retained here >> to >> + // make it easier to understand how the criteria is built into >> a >> + // query. >> + >> // Now set the criteria's limit to return the full >> // resultset since the results are limited on the server. >> - criteria.setLimit(-1); >> + //criteria.setLimit(-1); >> } >> >> if (limitString != null) >> @@ -1233,9 +1296,7 @@ >> } >> } >> >> - String sql = query.toString(); >> - category.debug(sql); >> - return sql; >> + return query; >> } >> >> /** >> @@ -1858,7 +1919,7 @@ >> * @param stmt A String with the sql statement to execute. >> * @param dbName Name of database to connect to. >> * @return The number of rows affected. >> - * @exception TorqueException, a generic exception. >> + * @exception TorqueException a generic exception. >> */ >> public static int executeStatement(String stmt, String dbName) >> throws TorqueException >> >> >> >> 1.35 +11 -9 >> jakarta-turbine-torque/src/java/org/apache/torque/util/Criteria.java >> >> Index: Criteria.java >> =================================================================== >> RCS file: >> /home/cvs/jakarta-turbine-torque/src/java/org/apache/torque/util/Criteria.jav >> a,v >> retrieving revision 1.34 >> retrieving revision 1.35 >> diff -u -r1.34 -r1.35 >> --- Criteria.java 27 Nov 2002 17:24:30 -0000 1.34 >> +++ Criteria.java 28 Nov 2002 16:32:36 -0000 1.35 >> @@ -1638,9 +1638,16 @@ >> } >> >> /** >> - * Set single record? >> + * Set single record? Set this to <code>true</code> if you expect the >> query >> + * to result in only a single result record (the default behaviour is >> to >> + * throw a TorqueException if multiple records are returned when the >> query >> + * is executed). This should be used in situations where returning >> multiple >> + * rows would indicate an error of some sort. If your query might >> return >> + * multiple records but you are only interested in the first one then >> you >> + * should be using setLimit(1). >> * >> - * @param b True if a single record should be returned. >> + * @param b set to <code>true</code> if you expect the query to select >> just >> + * one record. >> * @return A modified Criteria object. >> */ >> public Criteria setSingleRecord(boolean b) >> @@ -1865,17 +1872,12 @@ >> .append(super.get(key).toString()).append(": "); >> } >> >> - /* createQueryString modifies the Criteria object, so we should >> not >> - call it during toString(). Commenting this out instead of >> removing >> - it because a better fix would fix createQueryString to not >> modify >> - the Criteria >> try >> { >> sb.append("\nCurrent Query SQL (may not be complete or >> applicable): ") >> - .append(BasePeer.createQueryString(this)); >> + .append(BasePeer.createQueryDisplayString(this)); >> } >> catch (Exception exc) {} >> - */ >> >> return sb.toString(); >> } >> >> >> >> 1.15 +58 -1 >> jakarta-turbine-torque/src/test/org/apache/torque/util/CriteriaTest.java >> >> Index: CriteriaTest.java >> =================================================================== >> RCS file: >> /home/cvs/jakarta-turbine-torque/src/test/org/apache/torque/util/CriteriaTest >> .java,v >> retrieving revision 1.14 >> retrieving revision 1.15 >> diff -u -r1.14 -r1.15 >> --- CriteriaTest.java 11 Nov 2002 12:09:24 -0000 1.14 >> +++ CriteriaTest.java 28 Nov 2002 16:32:36 -0000 1.15 >> @@ -359,4 +359,61 @@ >> >> } >> >> + /** >> + * This test case has been written to try out the fix applied to >> resolve >> + * TRQS73 - i.e. ensuring that Criteria.toString() does not alter any >> limit >> + * or offset that may be stored in the Criteria object. This testcase >> + * could actually pass without the fix if the database in use does not >> + * support native limits and offsets. >> + */ >> + public void testCriteriaToStringOffset() >> + { >> + Criteria c = new Criteria() >> + .add("TABLE.DATE_COLUMN", Criteria.CURRENT_DATE) >> + .setOffset(3) >> + .setLimit(5); >> + >> + String toStringExpect = "Criteria:: >> TABLE.DATE_COLUMN<=>TABLE.DATE_COLUMN=CURRENT_DATE: " >> + + "\nCurrent Query SQL (may not be complete or >> applicable): " >> + + "SELECT FROM TABLE WHERE TABLE.DATE_COLUMN=CURRENT_DATE >> LIMIT 3, 5"; >> + >> + String cString = c.toString(); >> + //System.out.println(cString); >> + assertEquals(cString, toStringExpect); >> + >> + // Note that this is intentially the same as above as the >> behaviour is >> + // only observed on subsequent invocations of toString(). >> + cString = c.toString(); >> + //System.out.println(cString); >> + assertEquals(cString, toStringExpect); >> + } >> + >> + /** >> + * This test case has been written to try out the fix applied to >> resolve >> + * TRQS73 - i.e. ensuring that Criteria.toString() does not alter any >> limit >> + * or offset that may be stored in the Criteria object. This testcase >> + * could actually pass without the fix if the database in use does not >> + * support native limits and offsets. >> + */ >> + public void testCriteriaToStringLimit() >> + { >> + Criteria c = new Criteria() >> + .add("TABLE.DATE_COLUMN", Criteria.CURRENT_DATE) >> + .setLimit(5); >> + >> + String toStringExpect = "Criteria:: >> TABLE.DATE_COLUMN<=>TABLE.DATE_COLUMN=CURRENT_DATE: " >> + + "\nCurrent Query SQL (may not be complete or >> applicable): " >> + + "SELECT FROM TABLE WHERE TABLE.DATE_COLUMN=CURRENT_DATE >> LIMIT 5"; >> + >> + String cString = c.toString(); >> + //System.out.println(cString); >> + assertEquals(cString, toStringExpect); >> + >> + // Note that this is intentially the same as above as the >> behaviour is >> + // only observed on subsequent invocations of toString(). >> + cString = c.toString(); >> + //System.out.println(cString); >> + assertEquals(cString, toStringExpect); >> + } >> + >> } >> >> >> >> >> -- >> To unsubscribe, e-mail: >> <mailto:[EMAIL PROTECTED]> >> For additional commands, e-mail: >> <mailto:[EMAIL PROTECTED]> > > > > -- > To unsubscribe, e-mail: > <mailto:[EMAIL PROTECTED]> > For additional commands, e-mail: > <mailto:[EMAIL PROTECTED]> > -- To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]> For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
