On Thu, 10 Nov 2005, Scott Eade wrote:

Thomas Fischer wrote:

I took a quick glance at the issue, and when I uncommented the test case
you checked in and resolved an issue of visibility (use
"Criteria.getNewCriterion()" instead of "new Criteria.Criterion"), the test
case did not produce an error.


This is interesting.  What JVM are you using?  I am using J2SE 1.4.2_06
from Sun for Win32.

I could reproduce the error at a second machine. It uses 1.4.2-05 for linux.

Then I looked what the test case does, and it uses Criteria.equals(Object)
to test whether the two criteria are equal. This is not enough, since
Criteria.equals does not compare aliases and asColumns. There are two
possibilities to remedy this:
- The first is to change criteria.equals, which I did not want to do
between release candidates, but if somebody else is of another opinion
here,  we need to discuss it.


If equals() is performing its function correctly then this is IMO a bug
and most bugs warrant fixing - even between release candidates (though
preferably not between the last rc and the final release).

ok, now that I can reproduce the error, I see that Criteria.equals() is not the problem (it is not strict enough in my opinion, but this has nothing to do with the serialisation problem.)

...
As I said earlier, this has been broken (at least for me) for a while, I
wouldn't hold off 3.2 for this issue as nobody spoke up earlier or felt
that it was worth investing their own time to fix (me included).

If we want to solve this problem, we can as easily fix it before the release as after. So let's do it now, if we can.

According to Bloch (Effective Java Item 54) the default serialized form
of an inner class is ill-defined.  This could lead to varying behaviour
between different JVMs.  The answer may be to add custom serialization
code to Criterion.

I could not get hold of Bloch's book in short time, but according to what I have found on the internet, the problem with inner classes only occurs for non-static inner classes. So I thought maybe one can fix this by making Criterion a static inner class (diff appended), but this does not solve the problem (same behaviour as before).

I am puzzled at the moment and need to dissect the problem further to understand it. Not sure whether it will happen before the week-end, though

   Thomas
Index: 
/home/tfischer/workspace/db-torque/runtime/src/java/org/apache/torque/util/Criteria.java
===================================================================
--- 
/home/tfischer/workspace/db-torque/runtime/src/java/org/apache/torque/util/Criteria.java
    (revision 279957)
+++ 
/home/tfischer/workspace/db-torque/runtime/src/java/org/apache/torque/util/Criteria.java
    (working copy)
@@ -401,7 +401,7 @@
     public Criterion getNewCriterion(String column, Object value,
             SqlEnum comparison)
     {
-        return new Criterion(column, value, comparison);
+        return new Criterion(this, column, value, comparison);
     }
 
     /**
@@ -416,7 +416,7 @@
     public Criterion getNewCriterion(String table, String column,
             Object value, SqlEnum comparison)
     {
-        return new Criterion(table, column, value, comparison);
+        return new Criterion(this, table, column, value, comparison);
     }
 
     /**
@@ -973,7 +973,7 @@
      */
     public Criteria add(String column, Object value, SqlEnum comparison)
     {
-        super.put(column, new Criterion(column, value, comparison));
+        super.put(column, new Criterion(this, column, value, comparison));
         return this;
     }
 
@@ -1036,7 +1036,7 @@
         sb.append('.');
         sb.append(column);
         super.put(sb.toString(),
-                new Criterion(table, column, value, comparison));
+                new Criterion(this,table, column, value, comparison));
         return this;
     }
 
@@ -1963,7 +1963,7 @@
     public Criteria and(String column, Object value, SqlEnum comparison)
     {
         Criterion oc = getCriterion(column);
-        Criterion nc = new Criterion(column, value, comparison);
+        Criterion nc = new Criterion(this, column, value, comparison);
 
         if (oc == null)
         {
@@ -2033,7 +2033,7 @@
         sb.append(column);
 
         Criterion oc = getCriterion(table, column);
-        Criterion nc = new Criterion(table, column, value, comparison);
+        Criterion nc = new Criterion(this, table, column, value, comparison);
 
         if (oc == null)
         {
@@ -2528,7 +2528,7 @@
     public Criteria or(String column, Object value, SqlEnum comparison)
     {
         Criterion oc = getCriterion(column);
-        Criterion nc = new Criterion(column, value, comparison);
+        Criterion nc = new Criterion(this, column, value, comparison);
 
         if (oc == null)
         {
@@ -2596,7 +2596,7 @@
         sb.append(column);
 
         Criterion oc = getCriterion(table, column);
-        Criterion nc = new Criterion(table, column, value, comparison);
+        Criterion nc = new Criterion(this, table, column, value, comparison);
         if (oc == null)
         {
             super.put(sb.toString(), nc);
@@ -3003,7 +3003,7 @@
      * This is an inner class that describes an object in the
      * criteria.
      */
-    public final class Criterion implements Serializable
+    public final static class Criterion implements Serializable
     {
         public static final String AND = " AND ";
         public static final String OR = " OR ";
@@ -3034,12 +3034,15 @@
          */
         private List clauses = new ArrayList();
         private List conjunctions = new ArrayList();
+        
+        private Criteria criteria;
 
         /**
          * Creates a new instance, initializing a couple members.
          */
-        private Criterion(Object val, SqlEnum comp)
+        private Criterion(Criteria criteria, Object val, SqlEnum comp)
         {
+            this.criteria = criteria;
             this.value = val;
             this.comparison = comp;
         }
@@ -3052,9 +3055,9 @@
          * @param val An Object with the value for the Criteria.
          * @param comp A String with the comparison value.
          */
-        Criterion(String table, String column, Object val, SqlEnum comp)
+        Criterion(Criteria criteria, String table, String column, Object val, 
SqlEnum comp)
         {
-            this(val, comp);
+            this(criteria, val, comp);
             this.table = (table == null ? "" : table);
             this.column = (column == null ? "" : column);
         }
@@ -3067,9 +3070,9 @@
          * @param val An Object with the value for the Criteria.
          * @param comp A String with the comparison value.
          */
-        Criterion(String tableColumn, Object val, SqlEnum comp)
+        Criterion(Criteria criteria, String tableColumn, Object val, SqlEnum 
comp)
         {
-            this(val, comp);
+            this(criteria, val, comp);
             int dot = tableColumn.lastIndexOf('.');
             if (dot == -1)
             {
@@ -3090,9 +3093,9 @@
          * @param column A String with the name of the column.
          * @param val An Object with the value for the Criteria.
          */
-        Criterion(String table, String column, Object val)
+        Criterion(Criteria criteria, String table, String column, Object val)
         {
-            this(table, column, val, EQUAL);
+            this(criteria, table, column, val, EQUAL);
         }
 
         /**
@@ -3102,9 +3105,9 @@
          * column.
          * @param val An Object with the value for the Criteria.
          */
-        Criterion(String tableColumn, Object val)
+        Criterion(Criteria criteria, String tableColumn, Object val)
         {
-            this(tableColumn, val, EQUAL);
+            this(criteria, tableColumn, val, EQUAL);
         }
 
         /**
@@ -3172,7 +3175,7 @@
                 // debugging.
                 try
                 {
-                    db = Torque.getDB(getDbName());
+                    db = Torque.getDB(criteria.getDbName());
                 }
                 catch (Exception e)
                 {
@@ -3378,7 +3381,7 @@
                         Object item = Array.get(value, i);
 
                         inClause.add(SqlExpression.processInValue(item,
-                                             ignoreCase,
+                                             criteria.isIgnoreCase(),
                                              db));
                     }
 
@@ -3389,7 +3392,7 @@
                 }
                 else
                 {
-                    if (ignoreCase)
+                    if (criteria.isIgnoreCase())
                     {
                         sb.append(db.ignoreCase(field))
                                 .append(comparison)
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to