You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openjpa.apache.org by aw...@apache.org on 2006/08/18 02:17:31 UTC

svn commit: r432444 - in /incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc: kernel/ kernel/exps/ sql/

Author: awhite
Date: Thu Aug 17 17:17:30 2006
New Revision: 432444

URL: http://svn.apache.org/viewvc?rev=432444&view=rev
Log:
When a query projects and groups on a relation field, make sure to group on
the same columns we select.  Added Select.groupBy(ClassMapping, ...) API.
Implemented by temporarily putting target Select into "group mode" -- in which 
all select() calls are instead routed to groupBy() calls -- and invoking the 
same JDBCStoreManager.select() logic we use for the 
Select.select(ClassMapping...) call.  Having the Select "fake out" its callers
by translating select() calls into groupBy() calls isn't necessarily pretty, 
but it allows us to re-use all our existing select logic (not just in 
JDBCStoreManager, but in all the class, field, discriminator, version, etc 
mapping strategies) rather than creating duplicate logic in parallel groupBy() 
methods in all these components.


Modified:
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCStoreManager.java
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Aggregate.java
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Concat.java
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Const.java
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Extension.java
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/IndexOf.java
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Math.java
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/PCPath.java
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/SelectConstructor.java
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/StringFunction.java
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/SubQ.java
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Substring.java
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Trim.java
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/UnaryOp.java
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/LogicalUnion.java
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/Select.java
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SelectImpl.java

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCStoreManager.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCStoreManager.java?rev=432444&r1=432443&r2=432444&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCStoreManager.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCStoreManager.java Thu Aug 17 17:17:30 2006
@@ -83,8 +83,8 @@
 public class JDBCStoreManager 
     implements StoreManager, JDBCStore {
 
-    private static final Localizer _loc = Localizer
-        .forPackage(JDBCStoreManager.class);
+    private static final Localizer _loc = Localizer.forPackage
+        (JDBCStoreManager.class);
 
     private StoreContext _ctx = null;
     private JDBCConfiguration _conf = null;
@@ -922,8 +922,8 @@
                 continue;
 
             // try to select with join first
-            jtype = (fms[i].getNullValue() == fms[i].NULL_EXCEPTION) ? sel.EAGER_INNER
-                : sel.EAGER_OUTER;
+            jtype = (fms[i].getNullValue() == fms[i].NULL_EXCEPTION) 
+                ? sel.EAGER_INNER : sel.EAGER_OUTER;
             if (mode != fetch.EAGER_PARALLEL && !fms[i].isEagerSelectToMany()
                 && fms[i].supportsSelect(sel, jtype, sm, this, fetch) > 0
                 && sel.eagerClone(fms[i], jtype, false, 1) != null)
@@ -936,12 +936,14 @@
             // to use a to-many join also.  currently we limit eager
             // outer joins to non-LRS, non-ranged selects that don't already
             // have an eager to-many join
-            if ((hasJoin || mode == fetch.EAGER_JOIN || (mode == fetch.DEFAULT && sm != null))
+            if ((hasJoin || mode == fetch.EAGER_JOIN 
+                || (mode == fetch.DEFAULT && sm != null))
                 && fms[i].isEagerSelectToMany()
                 && !inEagerJoin
                 && !sel.hasEagerJoin(true)
                 && (!sel.getAutoDistinct() || (!sel.isLRS()
-                    && sel.getStartIndex() == 0 && sel.getEndIndex() == Long.MAX_VALUE))
+                && sel.getStartIndex() == 0 
+                && sel.getEndIndex() == Long.MAX_VALUE))
                 && fms[i].supportsSelect(sel, jtype, sm, this, fetch) > 0) {
                 if (sel.eagerClone(fms[i], jtype, true, 1) != null)
                     eagerToMany = fms[i];
@@ -952,9 +954,9 @@
             // finally, try parallel
             if (eager == fetch.EAGER_PARALLEL
                 && (sels = fms[i].supportsSelect(sel, sel.EAGER_PARALLEL, sm,
-                    this, fetch)) != 0)
-                sel.eagerClone(fms[i], Select.EAGER_PARALLEL, fms[i]
-                    .isEagerSelectToMany(), sels);
+                this, fetch)) != 0)
+                sel.eagerClone(fms[i], Select.EAGER_PARALLEL, 
+                    fms[i].isEagerSelectToMany(), sels);
         }
         return eagerToMany;
     }
@@ -993,8 +995,8 @@
         boolean ident, boolean joined) {
         ClassMapping parent = mapping.getJoinablePCSuperclassMapping();
         if (parent == null && !mapping.isMapped())
-            throw new InvalidStateException(_loc.get("virtual-mapping", mapping
-                .getDescribedType()));
+            throw new InvalidStateException(_loc.get("virtual-mapping", mapping.
+                getDescribedType()));
 
         int seld = -1;
         int pseld = -1;
@@ -1011,9 +1013,9 @@
             }
 
             // if no instance or not initialized and not exact oid, select type
-            if ((sm == null || (sm.getPCState() == PCState.TRANSIENT && (!(sm
-                .getObjectId() instanceof OpenJPAId) || ((OpenJPAId) sm
-                .getObjectId()).hasSubclasses())))
+            if ((sm == null || (sm.getPCState() == PCState.TRANSIENT 
+                && (!(sm.getObjectId() instanceof OpenJPAId) 
+                || ((OpenJPAId) sm.getObjectId()).hasSubclasses())))
                 && mapping.getDiscriminator().select(sel, orig))
                 seld = 1;
 
@@ -1346,7 +1348,8 @@
      */
     private class CancelPreparedStatement extends DelegatingPreparedStatement {
 
-        public CancelPreparedStatement(PreparedStatement stmnt, Connection conn) {
+        public CancelPreparedStatement(PreparedStatement stmnt, 
+            Connection conn) {
             super(stmnt, conn);
         }
 

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Aggregate.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Aggregate.java?rev=432444&r1=432443&r2=432444&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Aggregate.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Aggregate.java Thu Aug 17 17:17:30 2006
@@ -121,7 +121,7 @@
 
     public void groupBy(Select sel, JDBCStore store, Object[] params,
         JDBCFetchConfiguration fetch) {
-        sel.groupBy(newSQLBuffer(sel, store, params, fetch), false);
+        sel.groupBy(newSQLBuffer(sel, store, params, fetch));
     }
 
     public void orderBy(Select sel, JDBCStore store, Object[] params,

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Concat.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Concat.java?rev=432444&r1=432443&r2=432444&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Concat.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Concat.java Thu Aug 17 17:17:30 2006
@@ -113,7 +113,7 @@
 
     public void groupBy(Select sel, JDBCStore store, Object[] params,
         JDBCFetchConfiguration fetch) {
-        sel.groupBy(newSQLBuffer(sel, store, params, fetch), false);
+        sel.groupBy(newSQLBuffer(sel, store, params, fetch));
     }
 
     public void orderBy(Select sel, JDBCStore store, Object[] params,

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Const.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Const.java?rev=432444&r1=432443&r2=432444&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Const.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Const.java Thu Aug 17 17:17:30 2006
@@ -135,7 +135,7 @@
 
     public void groupBy(Select sel, JDBCStore store, Object[] params,
         JDBCFetchConfiguration fetch) {
-        sel.groupBy(newSQLBuffer(sel, store, params, fetch), false);
+        sel.groupBy(newSQLBuffer(sel, store, params, fetch));
     }
 
     public void orderBy(Select sel, JDBCStore store, Object[] params,

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Extension.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Extension.java?rev=432444&r1=432443&r2=432444&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Extension.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Extension.java Thu Aug 17 17:17:30 2006
@@ -135,7 +135,7 @@
 
     public void groupBy(Select sel, JDBCStore store, Object[] params,
         JDBCFetchConfiguration fetch) {
-        sel.groupBy(newSQLBuffer(sel, store, params, fetch), false);
+        sel.groupBy(newSQLBuffer(sel, store, params, fetch));
     }
 
     public void orderBy(Select sel, JDBCStore store, Object[] params,

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/IndexOf.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/IndexOf.java?rev=432444&r1=432443&r2=432444&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/IndexOf.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/IndexOf.java Thu Aug 17 17:17:30 2006
@@ -99,7 +99,7 @@
 
     public void groupBy(Select sel, JDBCStore store, Object[] params,
         JDBCFetchConfiguration fetch) {
-        sel.groupBy(newSQLBuffer(sel, store, params, fetch), false);
+        sel.groupBy(newSQLBuffer(sel, store, params, fetch));
     }
 
     public void orderBy(Select sel, JDBCStore store, Object[] params,

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Math.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Math.java?rev=432444&r1=432443&r2=432444&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Math.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Math.java Thu Aug 17 17:17:30 2006
@@ -110,7 +110,7 @@
 
     public void groupBy(Select sel, JDBCStore store, Object[] params,
         JDBCFetchConfiguration fetch) {
-        sel.groupBy(newSQLBuffer(sel, store, params, fetch), false);
+        sel.groupBy(newSQLBuffer(sel, store, params, fetch));
     }
 
     public void orderBy(Select sel, JDBCStore store, Object[] params,

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/PCPath.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/PCPath.java?rev=432444&r1=432443&r2=432444&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/PCPath.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/PCPath.java Thu Aug 17 17:17:30 2006
@@ -502,7 +502,14 @@
 
     public void groupBy(Select sel, JDBCStore store, Object[] params,
         JDBCFetchConfiguration fetch) {
-        sel.groupBy(getColumns(), sel.outer(_joins), false);
+        ClassMapping mapping = getClassMapping();
+        if (mapping == null || !_joinedRel)
+            sel.groupBy(getColumns(), sel.outer(_joins));
+        else {
+            int subs = (_type == UNBOUND_VAR) ? sel.SUBS_JOINABLE
+                : sel.SUBS_ANY_JOINABLE;
+            sel.groupBy(mapping, subs, store, fetch, sel.outer(_joins));
+        }
     }
 
     public void orderBy(Select sel, JDBCStore store, Object[] params,

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/SelectConstructor.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/SelectConstructor.java?rev=432444&r1=432443&r2=432444&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/SelectConstructor.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/SelectConstructor.java Thu Aug 17 17:17:30 2006
@@ -233,7 +233,6 @@
             // this ensures that we have all our joins cached
             if (resultVal instanceof PCPath)
                 ((PCPath) resultVal).joinRelation();
-
             joins = sel.and(joins, resultVal.getJoins());
         }
 
@@ -242,6 +241,11 @@
         for (int i = 0; i < exps.grouping.length; i++) {
             groupVal = (Val) exps.grouping[i];
             groupVal.initialize(sel, store, false);
+
+            // have to join through to related type for pc object groupings;
+            // this ensures that we have all our joins cached
+            if (groupVal instanceof PCPath)
+                ((PCPath) groupVal).joinRelation();
             joins = sel.and(joins, groupVal.getJoins());
         }
 
@@ -313,21 +317,15 @@
                 val.select(sel, store, params, pks, fetch);
             }
 
-            // make sure grouping and having columns are selected since it
-            // is required by most DBs.  put them last so they don't affect
-            // result processing
+            // make sure having columns are selected since it is required by 
+            // some DBs.  put them last so they don't affect result processing
             if (exps.having != null && inner != null)
                 ((Exp) exps.having).selectColumns(inner, store, params, true,
                     fetch);
-            for (int i = 0; i < exps.grouping.length; i++) {
-                val = (Val) exps.grouping[i];
-                if (inner != null)
-                    val.selectColumns(inner, store, params, true, fetch);
-                val.select(sel, store, params, true, fetch);
-            }
         }
 
-        // select order data last so it doesn't affect result processing
+        // select ordering columns, since it is required by some DBs.  put them
+        // last so they don't affect result processing
         for (int i = 0; i < exps.ordering.length; i++) {
             val = (Val) exps.ordering[i];
             if (inner != null)

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/StringFunction.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/StringFunction.java?rev=432444&r1=432443&r2=432444&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/StringFunction.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/StringFunction.java Thu Aug 17 17:17:30 2006
@@ -87,7 +87,7 @@
 
     public void groupBy(Select sel, JDBCStore store, Object[] params,
         JDBCFetchConfiguration fetch) {
-        sel.groupBy(newSQLBuffer(sel, store, params, fetch), false);
+        sel.groupBy(newSQLBuffer(sel, store, params, fetch));
     }
 
     public void orderBy(Select sel, JDBCStore store, Object[] params,

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/SubQ.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/SubQ.java?rev=432444&r1=432443&r2=432444&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/SubQ.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/SubQ.java Thu Aug 17 17:17:30 2006
@@ -133,7 +133,7 @@
 
     public void groupBy(Select sel, JDBCStore store, Object[] params,
         JDBCFetchConfiguration fetch) {
-        sel.groupBy(newSQLBuffer(sel, store, params, fetch), false);
+        sel.groupBy(newSQLBuffer(sel, store, params, fetch));
     }
 
     public void orderBy(Select sel, JDBCStore store, Object[] params,

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Substring.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Substring.java?rev=432444&r1=432443&r2=432444&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Substring.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Substring.java Thu Aug 17 17:17:30 2006
@@ -95,7 +95,7 @@
 
     public void groupBy(Select sel, JDBCStore store, Object[] params,
         JDBCFetchConfiguration fetch) {
-        sel.groupBy(newSQLBuffer(sel, store, params, fetch), false);
+        sel.groupBy(newSQLBuffer(sel, store, params, fetch));
     }
 
     public void orderBy(Select sel, JDBCStore store, Object[] params,

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Trim.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Trim.java?rev=432444&r1=432443&r2=432444&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Trim.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Trim.java Thu Aug 17 17:17:30 2006
@@ -109,7 +109,7 @@
 
     public void groupBy(Select sel, JDBCStore store, Object[] params,
         JDBCFetchConfiguration fetch) {
-        sel.groupBy(newSQLBuffer(sel, store, params, fetch), false);
+        sel.groupBy(newSQLBuffer(sel, store, params, fetch));
     }
 
     public void orderBy(Select sel, JDBCStore store, Object[] params,

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/UnaryOp.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/UnaryOp.java?rev=432444&r1=432443&r2=432444&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/UnaryOp.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/UnaryOp.java Thu Aug 17 17:17:30 2006
@@ -99,7 +99,7 @@
 
     public void groupBy(Select sel, JDBCStore store, Object[] params,
         JDBCFetchConfiguration fetch) {
-        sel.groupBy(newSQLBuffer(sel, store, params, fetch), false);
+        sel.groupBy(newSQLBuffer(sel, store, params, fetch));
     }
 
     public void orderBy(Select sel, JDBCStore store, Object[] params,

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/LogicalUnion.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/LogicalUnion.java?rev=432444&r1=432443&r2=432444&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/LogicalUnion.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/LogicalUnion.java Thu Aug 17 17:17:30 2006
@@ -745,36 +745,46 @@
             sel.having(sql, joins);
         }
 
-        public boolean groupBy(SQLBuffer sql, boolean select) {
-            return sel.groupBy(sql, null, select);
+        public void groupBy(SQLBuffer sql) {
+            sel.groupBy(sql);
         }
 
-        public boolean groupBy(SQLBuffer sql, Joins joins, boolean select) {
-            return sel.groupBy(sql, joins, select);
+        public void groupBy(SQLBuffer sql, Joins joins) {
+            sel.groupBy(sql, joins);
         }
 
-        public boolean groupBy(String sql, boolean select) {
-            return sel.groupBy(sql, null, select);
+        public void groupBy(String sql) {
+            sel.groupBy(sql);
         }
 
-        public boolean groupBy(String sql, Joins joins, boolean select) {
-            return sel.groupBy(sql, joins, select);
+        public void groupBy(String sql, Joins joins) {
+            sel.groupBy(sql, joins);
         }
 
-        public boolean groupBy(Column col, boolean select) {
-            return sel.groupBy(col, null, select);
+        public void groupBy(Column col) {
+            sel.groupBy(col);
         }
 
-        public boolean groupBy(Column col, Joins joins, boolean select) {
-            return sel.groupBy(col, joins, select);
+        public void groupBy(Column col, Joins joins) {
+            sel.groupBy(col, joins);
         }
 
-        public int groupBy(Column[] cols, boolean select) {
-            return sel.groupBy(cols, null, select);
+        public void groupBy(Column[] cols) {
+            sel.groupBy(cols);
         }
 
-        public int groupBy(Column[] cols, Joins joins, boolean select) {
-            return sel.groupBy(cols, joins, select);
+        public void groupBy(Column[] cols, Joins joins) {
+            sel.groupBy(cols, joins);
+        }
+
+        public void groupBy(ClassMapping mapping, int subclasses, 
+            JDBCStore store, JDBCFetchConfiguration fetch) {
+            sel.groupBy(mapping, subclasses, store, fetch);
+        }
+
+        public void groupBy(ClassMapping mapping, int subclasses, 
+            JDBCStore store, JDBCFetchConfiguration fetch, Joins joins) {
+            sel.groupBy(mapping, subclasses, store, fetch, joins);
         }
 
         public SelectExecutor whereClone(int sels) {

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/Select.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/Select.java?rev=432444&r1=432443&r2=432444&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/Select.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/Select.java Thu Aug 17 17:17:30 2006
@@ -561,51 +561,57 @@
 
     /**
      * Group by the given column.
-     * Optionally selects grouping data if not already selected.
      */
-    public boolean groupBy(Column col, boolean sel);
+    public void groupBy(Column col);
 
     /**
      * Group by the given column.
-     * Optionally selects grouping data if not already selected.
      */
-    public boolean groupBy(Column col, Joins joins, boolean sel);
+    public void groupBy(Column col, Joins joins);
 
     /**
      * Group by the given columns.
-     * Optionally selects grouping data if not already selected.
      */
-    public int groupBy(Column[] cols, boolean sel);
+    public void groupBy(Column[] cols);
 
     /**
      * Group by the given columns.
-     * Optionally selects grouping data if not already selected.
      */
-    public int groupBy(Column[] cols, Joins joins, boolean sel);
+    public void groupBy(Column[] cols, Joins joins);
 
     /**
      * Add a GROUP BY clause.
-     * Optionally selects grouping data if not already selected.
      */
-    public boolean groupBy(SQLBuffer sql, boolean sel);
+    public void groupBy(SQLBuffer sql);
 
     /**
      * Add a GROUP BY clause.
-     * Optionally selects grouping data if not already selected.
      */
-    public boolean groupBy(SQLBuffer sql, Joins joins, boolean sel);
+    public void groupBy(SQLBuffer sql, Joins joins);
 
     /**
      * Add a GROUP BY clause.
-     * Optionally selects grouping data if not already selected.
      */
-    public boolean groupBy(String sql, boolean sel);
+    public void groupBy(String sql);
 
     /**
      * Add a GROUP BY clause.
-     * Optionally selects grouping data if not already selected.
      */
-    public boolean groupBy(String sql, Joins joins, boolean sel);
+    public void groupBy(String sql, Joins joins);
+
+    /**
+     * Group by the columns of the given mapping, possibly including subclasses.
+     * Assumes EAGER_NONE.
+     */
+    public void groupBy(ClassMapping mapping, int subclasses, JDBCStore store, 
+        JDBCFetchConfiguration fetch);
+
+    /**
+     * Group by the columns of the given mapping, possibly including subclasses.
+     * Assumes EAGER_NONE.
+     */
+    public void groupBy(ClassMapping mapping, int subclasses, JDBCStore store, 
+        JDBCFetchConfiguration fetch, Joins joins);
 
     /**
      * Return a SELECT with the same joins and where conditions as this one.

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SelectImpl.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SelectImpl.java?rev=432444&r1=432443&r2=432444&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SelectImpl.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SelectImpl.java Thu Aug 17 17:17:30 2006
@@ -37,6 +37,7 @@
 
 import org.apache.commons.collections.iterators.EmptyIterator;
 import org.apache.openjpa.jdbc.conf.JDBCConfiguration;
+import org.apache.openjpa.jdbc.kernel.EagerFetchModes;
 import org.apache.openjpa.jdbc.kernel.JDBCFetchConfiguration;
 import org.apache.openjpa.jdbc.kernel.JDBCLockManager;
 import org.apache.openjpa.jdbc.kernel.JDBCStore;
@@ -79,6 +80,7 @@
     private static final int EAGER_TO_ONE = 2 << 9;
     private static final int EAGER_TO_MANY = 2 << 10;
     private static final int RECORD_ORDERED = 2 << 11;
+    private static final int GROUPING = 2 << 12;
 
     private static final String[] TABLE_ALIASES = new String[16];
     private static final String[] ORDER_ALIASES = new String[16];
@@ -588,7 +590,10 @@
     }
 
     public boolean select(SQLBuffer sql, Object id, Joins joins) {
-        return select((Object) sql, id, joins);
+        if (!isGrouping())
+            return select((Object) sql, id, joins);
+        groupBy(sql, joins);
+        return false;
     }
 
     /**
@@ -627,7 +632,10 @@
     }
 
     public boolean select(String sql, Object id, Joins joins) {
-        return select((Object) sql, id, joins);
+        if (!isGrouping())
+            return select((Object) sql, id, joins);
+        groupBy(sql, joins);
+        return true;
     }
 
     public void selectPlaceholder(String sql) {
@@ -658,7 +666,10 @@
     }
 
     public boolean select(Column col, Joins joins) {
-        return select(col, getJoins(joins, true), false);
+        if (!isGrouping())
+            return select(col, getJoins(joins, true), false);
+        groupBy(col, joins);
+        return false;
     }
 
     public int select(Column[] cols) {
@@ -668,6 +679,10 @@
     public int select(Column[] cols, Joins joins) {
         if (cols == null || cols.length == 0)
             return 0;
+        if (isGrouping()) {
+            groupBy(cols, joins);
+            return 0;
+        }
         PathJoins pj = getJoins(joins, true);
         int seld = 0;
         for (int i = 0; i < cols.length; i++)
@@ -758,7 +773,10 @@
     }
 
     public boolean selectIdentifier(Column col, Joins joins) {
-        return select(col, getJoins(joins, true), true);
+        if (!isGrouping())
+            return select(col, getJoins(joins, true), true);
+        groupBy(col, joins);
+        return false;
     }
 
     public int selectIdentifier(Column[] cols) {
@@ -768,6 +786,10 @@
     public int selectIdentifier(Column[] cols, Joins joins) {
         if (cols == null || cols.length == 0)
             return 0;
+        if (isGrouping()) {
+            groupBy(cols, joins);
+            return 0;
+        }
         PathJoins pj = getJoins(joins, true);
         int seld = 0;
         for (int i = 0; i < cols.length; i++)
@@ -814,8 +836,13 @@
             return primaryKeyOperation(sup, sel, asc, joins, aliasOrder);
         }
 
-        PathJoins pj = getJoins(joins, false);
         Column[] cols = mapping.getPrimaryKeyColumns();
+        if (isGrouping()) {
+            groupBy(cols, joins);
+            return 0;
+        }
+
+        PathJoins pj = getJoins(joins, false);
         int seld = 0;
         for (int i = 0; i < cols.length; i++)
             if (columnOperation(cols[i], sel, asc, pj, aliasOrder))
@@ -1285,53 +1312,37 @@
         _having.append(sql);
     }
 
-    public boolean groupBy(SQLBuffer sql, boolean sel) {
-        return groupBy(sql, (Joins) null, sel);
+    public void groupBy(SQLBuffer sql) {
+        groupBy(sql, (Joins) null);
     }
 
-    public boolean groupBy(SQLBuffer sql, Joins joins, boolean sel) {
+    public void groupBy(SQLBuffer sql, Joins joins) {
         getJoins(joins, true);
         if (_grouping == null)
             _grouping = new SQLBuffer(_dict);
         else
             _grouping.append(", ");
         _grouping.append(sql);
-
-        if (!sel)
-            return false;
-        int idx = _selects.indexOfAlias(sql);
-        if (idx != -1)
-            return false;
-        _selects.setAlias(nullId(), sql, false);
-        return true;
     }
 
-    public boolean groupBy(String sql, boolean sel) {
-        return groupBy(sql, (Joins) null, sel);
+    public void groupBy(String sql) {
+        groupBy(sql, (Joins) null);
     }
 
-    public boolean groupBy(String sql, Joins joins, boolean sel) {
+    public void groupBy(String sql, Joins joins) {
         getJoins(joins, true);
         if (_grouping == null)
             _grouping = new SQLBuffer(_dict);
         else
             _grouping.append(", ");
         _grouping.append(sql);
-
-        if (!sel)
-            return false;
-        int idx = _selects.indexOfAlias(sql);
-        if (idx != -1)
-            return false;
-        _selects.setAlias(nullId(), sql, false);
-        return true;
     }
 
-    public boolean groupBy(Column col, boolean sel) {
-        return groupBy(col, null, sel);
+    public void groupBy(Column col) {
+        groupBy(col, null);
     }
 
-    public boolean groupBy(Column col, Joins joins, boolean sel) {
+    public void groupBy(Column col, Joins joins) {
         if (_grouping == null)
             _grouping = new SQLBuffer(_dict);
         else
@@ -1339,30 +1350,55 @@
 
         PathJoins pj = getJoins(joins, true);
         _grouping.append(getColumnAlias(col, pj));
-        return sel && select(col, pj, false);
     }
 
-    public int groupBy(Column[] cols, boolean sel) {
-        return groupBy(cols, null, sel);
+    public void groupBy(Column[] cols) {
+        groupBy(cols, null);
     }
 
-    public int groupBy(Column[] cols, Joins joins, boolean sel) {
+    public void groupBy(Column[] cols, Joins joins) {
         if (_grouping == null)
             _grouping = new SQLBuffer(_dict);
         else
             _grouping.append(", ");
 
         PathJoins pj = getJoins(joins, true);
-        int seld = 0;
         for (int i = 0; i < cols.length; i++) {
             if (i > 0)
                 _grouping.append(", ");
             _grouping.append(getColumnAlias(cols[i], pj));
+        }
+    }
 
-            if (sel && select(cols[i], pj, false))
-                seld |= 2 << i;
+    public void groupBy(ClassMapping mapping, int subclasses, JDBCStore store,
+        JDBCFetchConfiguration fetch) {
+        groupBy(mapping, subclasses, store, fetch, null);
+    }
+
+    public void groupBy(ClassMapping mapping, int subclasses, JDBCStore store,
+        JDBCFetchConfiguration fetch, Joins joins) {
+        // we implement this by putting ourselves into grouping mode, where
+        // all select invocations are re-routed to group-by invocations instead.
+        // this allows us to utilize the same select APIs of the store manager
+        // and all the mapping strategies, rather than having to create 
+        // equivalent APIs and duplicate logic for grouping
+        boolean wasGrouping = isGrouping();
+        _flags |= GROUPING;
+        try {
+            select(mapping, subclasses, store, fetch, 
+                EagerFetchModes.EAGER_NONE, joins);
+        } finally {
+            if (!wasGrouping)
+                _flags &= ~GROUPING;
         }
-        return seld;
+    }
+
+    /**
+     * Whether we're in group mode, where any select is changed to a group-by
+     * call.
+     */
+    boolean isGrouping() {
+        return (_flags & GROUPING) != 0;
     }
 
     /**