Add a DiscriminatorStrategy.hasClassConditions() method rather than having the

DiscriminatorStrategy.getClassConditions() method return null for no conditions
so that we can detect whether the joins to the base class owning the 
discriminator are necessary before making them.  Otherwise, we run the risk of
creating table aliases for base class tables without joining them, which can
result in bad SQL. 



git-svn-id: https://svn.apache.org/repos/asf/incubator/openjpa/trunk@505017 13f79535-47bb-0310-9956-ffa450edef68

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/InstanceofExpression.java

@@ -87,7 +87,7 @@ class InstanceofExpression
                 getClassLoader(), false);
 
             // if not looking for a PC, don't bother with indicator
-            if (mapping == null)
+            if (mapping == null || !discrim.hasClassConditions(mapping, true))
                 discrim = null;
             else {
                 ClassMapping owner = discrim.getClassMapping();
@@ -144,10 +144,7 @@ class InstanceofExpression
             ctx.store.loadSubclasses(istate.discrim.getClassMapping());
             SQLBuffer buf = istate.discrim.getClassConditions(sel,
                 istate.joins, istate.mapping, true);
-            if (buf == null)
-                sql.append("1 = 1");
-            else
-                sql.append(buf);
+            sql.append(buf);
         }
         sel.append(sql, istate.joins);
     }

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/Discriminator.java

@@ -297,6 +297,8 @@ public class Discriminator
         if (_mapping.getJoinablePCSuperclassMapping() == null
             && _mapping.getJoinablePCSubclassMappings().length == 0)
             return false;
+        if (!hasClassConditions(_mapping, subs))
+            return false;
 
         // join down to base class where conditions will be added
         ClassMapping from = _mapping;
@@ -310,12 +312,8 @@ public class Discriminator
             }
         }
 
-        SQLBuffer buf = getClassConditions(sel, joins, _mapping, subs);
-        if (buf != null) {
-            sel.where(buf, joins);
-            return true;
-        }
-        return false;
+        sel.where(getClassConditions(sel, joins, _mapping, subs), joins);
+        return true;
     }
 
     ////////////////////////////////////////
@@ -394,6 +392,10 @@ public class Discriminator
         return assertStrategy().getClass(store, base, result);
     }
 
+    public boolean hasClassConditions(ClassMapping base, boolean subs) {
+        return assertStrategy().hasClassConditions(base, subs);
+    }
+
     public SQLBuffer getClassConditions(Select sel, Joins joins, 
         ClassMapping base, boolean subs) {
         return assertStrategy().getClassConditions(sel, joins, base, subs);

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/DiscriminatorStrategy.java

@@ -58,11 +58,17 @@ public interface DiscriminatorStrategy
     public Class getClass(JDBCStore store, ClassMapping base, Result result)
         throws SQLException, ClassNotFoundException;
 
+    /**
+     * Whether any class conditions are necessary.
+     *
+     * @see #getClassConditions
+     */
+    public boolean hasClassConditions(ClassMapping base, boolean subs);
+
     /**
      * Return SQL to limit the classes selected as much as possible to the
-     * given base class, and optionally its subclasses. Return null if
-     * no conditions needed. The select and joins instances are supplied
-     * in order to get column aliases.
+     * given base class, and optionally its subclasses. The select and joins 
+     * instances are supplied in order to get column aliases.
      */
     public SQLBuffer getClassConditions(Select sel, Joins joins, 
         ClassMapping base, boolean subs);

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/AbstractDiscriminatorStrategy.java

@@ -82,6 +82,10 @@ public abstract class AbstractDiscriminatorStrategy
         return base.getDescribedType();
     }
 
+    public boolean hasClassConditions(ClassMapping base, boolean subs) {
+        return false;
+    }
+
     public SQLBuffer getClassConditions(Select sel, Joins joins, 
         ClassMapping base, boolean subs) {
         return null;

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/InValueDiscriminatorStrategy.java

@@ -113,19 +113,23 @@ public abstract class InValueDiscriminatorStrategy
         return getClass(cls, store);
     }
 
-    public SQLBuffer getClassConditions(Select sel, Joins joins, 
-        ClassMapping base, boolean subclasses) {
+    public boolean hasClassConditions(ClassMapping base, boolean subclasses) {
         // if selecting the first mapped class and all subclasses, no need
         // to limit the query
         if (isFinal || (base.getJoinablePCSuperclassMapping() == null
             && subclasses))
-            return null;
+            return false;
 
         // if no subclasses or superclass, no need for conditions
         ClassMapping[] subs = base.getJoinablePCSubclassMappings();
         if (subs.length == 0 && base.getJoinablePCSuperclassMapping() == null)
-            return null;
+            return false;
 
+        return true;
+    }
+
+    public SQLBuffer getClassConditions(Select sel, Joins joins, 
+        ClassMapping base, boolean subclasses) {
         Column col = disc.getColumns()[0];
         SQLBuffer sql = new SQLBuffer(sel.getConfiguration().
             getDBDictionaryInstance());
@@ -136,6 +140,7 @@ public abstract class InValueDiscriminatorStrategy
         sql.append(alias);
 
         // if not selecting subclasses, limit to just the given class
+        ClassMapping[] subs = base.getJoinablePCSubclassMappings();
         if (!outer && (!subclasses || subs.length == 0))
             return sql.append(" = ").appendValue(getDiscriminatorValue(base),
                 col);

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/SubclassJoinDiscriminatorStrategy.java

@@ -130,16 +130,20 @@ public class SubclassJoinDiscriminatorStrategy
         return derived;
     }
 
-    public SQLBuffer getClassConditions(Select sel, Joins joins, 
-        ClassMapping base, boolean subclasses) {
+    public boolean hasClassConditions(ClassMapping base, boolean subclasses) {
         if (isFinal || subclasses)
-            return null;
+            return false;
         ClassMapping[] subs = base.getJoinablePCSubclassMappings();
         if (subs.length == 0)
-            return null;
+            return false;
+        return true;
+    }
 
+    public SQLBuffer getClassConditions(Select sel, Joins joins, 
+        ClassMapping base, boolean subclasses) {
         // add conditions making sure no subclass tables have records for
         // this instance
+        ClassMapping[] subs = base.getJoinablePCSubclassMappings();
         SQLBuffer buf = null;
         Column[] pks;
         for (int i = 0; i < subs.length; i++) {

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/SuperclassDiscriminatorStrategy.java

@@ -61,6 +61,11 @@ public class SuperclassDiscriminatorStrategy
             getDiscriminator().getClass(store, base, res);
     }
 
+    public boolean hasClassConditions(ClassMapping base, boolean subclasses) {
+        return disc.getClassMapping().getPCSuperclassMapping().
+            getDiscriminator().hasClassConditions(base, subclasses);
+    }
+
     public SQLBuffer getClassConditions(Select sel, Joins joins, 
         ClassMapping base, boolean subclasses) {
         return disc.getClassMapping().getPCSuperclassMapping().

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DBDictionary.java

@@ -1743,7 +1743,8 @@ public class DBDictionary
      * cases where a subselect is required and the database doesn't support
      * subselects), this method should return null.
      */
-    public SQLBuffer toDelete(ClassMapping mapping, Select sel, Object[] params) {
+    public SQLBuffer toDelete(ClassMapping mapping, Select sel, 
+        Object[] params) {
         return toBulkOperation(mapping, sel, null, params, null);
     }