You are viewing a plain text version of this content. The canonical link for it is here.
Posted to torque-dev@db.apache.org by John McNally <jm...@collab.net> on 2002/11/29 20:52:28 UTC
Re: cvs commit:
jakarta-turbine-torque/src/test/org/apache/torque/util CriteriaTest.java
-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, mpoeschl@apache.org 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.java,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.java,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: <ma...@jakarta.apache.org>
> For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: cvs commit:
jakarta-turbine-torque/src/test/org/apache/torque/util CriteriaTest.java
Posted by John McNally <jm...@collab.net>.
yes. i have applied the patch.
john mcnally
On Sat, 2002-11-30 at 14:01, Scott Eade wrote:
> > From: John McNally <jm...@collab.net>
> >
> > I would prefer package private, any reason that's not possible?
> >
> John,
>
> I have added a second patch to TRQS73 which uses the more correct package
> private visibility modifier (ore rather no visibility modifier).
>
> Can you please confirm that this is sufficient for you to revoke your -1.
>
> Thanks,
>
> Scott
> --
> Scott Eade
> Backstage Technologies Pty. Ltd.
> http://www.backstagetech.com.au
>
>
>
> --
> To unsubscribe, e-mail: <ma...@jakarta.apache.org>
> For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: cvs commit:
jakarta-turbine-torque/src/test/org/apache/torque/util CriteriaTest.java
Posted by Scott Eade <se...@backstagetech.com.au>.
> From: John McNally <jm...@collab.net>
>
> I would prefer package private, any reason that's not possible?
>
John,
I have added a second patch to TRQS73 which uses the more correct package
private visibility modifier (ore rather no visibility modifier).
Can you please confirm that this is sufficient for you to revoke your -1.
Thanks,
Scott
--
Scott Eade
Backstage Technologies Pty. Ltd.
http://www.backstagetech.com.au
Re: cvs commit:
jakarta-turbine-torque/src/test/org/apache/torque/util CriteriaTest.java
Posted by John McNally <jm...@collab.net>.
I would prefer package private, any reason that's not possible?
john mcnally
On Fri, 2002-11-29 at 14:06, Scott Eade wrote:
> > From: Scott Eade <se...@backstagetech.com.au>
> >
> > Would you be happy if I mark the new methods as protected?
> s/happy/happier
> or
> s/happy/less irritated
> or
> s/happy/inclined to remove your -1
>
> Scott
> --
> Scott Eade
> Backstage Technologies Pty. Ltd.
> http://www.backstagetech.com.au
>
>
>
> --
> To unsubscribe, e-mail: <ma...@jakarta.apache.org>
> For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: cvs commit:
jakarta-turbine-torque/src/test/org/apache/torque/util CriteriaTest.java
Posted by Scott Eade <se...@backstagetech.com.au>.
> From: Scott Eade <se...@backstagetech.com.au>
>
> Would you be happy if I mark the new methods as protected?
s/happy/happier
or
s/happy/less irritated
or
s/happy/inclined to remove your -1
Scott
--
Scott Eade
Backstage Technologies Pty. Ltd.
http://www.backstagetech.com.au
Re: cvs commit:
jakarta-turbine-torque/src/test/org/apache/torque/util CriteriaTest.java
Posted by Scott Eade <se...@backstagetech.com.au>.
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 <jm...@collab.net>
>
> -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, mpoeschl@apache.org 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:
>> <ma...@jakarta.apache.org>
>> For additional commands, e-mail:
>> <ma...@jakarta.apache.org>
>
>
>
> --
> To unsubscribe, e-mail:
> <ma...@jakarta.apache.org>
> For additional commands, e-mail:
> <ma...@jakarta.apache.org>
>