Refactoring of select statement generation code into Relation.get*Select() methods in order to fix bug 667. Also remove some dead code.

src/helma/objectmodel/db/NodeManager.java

@@ -877,48 +877,32 @@ public final class NodeManager {
      *  loaded later on demand.
      */
     public List getNodeIDs(Node home, Relation rel) throws Exception {
-
+        DbMapping type = rel == null ? null : rel.otherType;
-        if ((rel == null) || (rel.otherType == null) || !rel.otherType.isRelational()) {
+        if (type == null || !type.isRelational()) {
             // this should never be called for embedded nodes
-            throw new RuntimeException("NodeMgr.getNodeIDs called for non-relational node " +
+            throw new RuntimeException("getNodeIDs called for non-relational node " + home);
-                                       home);
+        }
-        } else {
         List retval = new ArrayList();
 
         // if we do a groupby query (creating an intermediate layer of groupby nodes),
         // retrieve the value of that field instead of the primary key
-            String idfield = (rel.groupby == null) ? rel.otherType.getIDField()
+        Connection con = type.getConnection();
-                                                   : rel.groupby;
-            Connection con = rel.otherType.getConnection();
         // set connection to read-only mode
         if (!con.isReadOnly()) con.setReadOnly(true);
 
-            String table = rel.otherType.getTableName();
-
         Statement stmt = null;
         long logTimeStart = logSql ? System.currentTimeMillis() : 0;
         String query = null;
 
         try {
-                StringBuffer b = new StringBuffer("SELECT ");
+            StringBuffer b = rel.getIdSelect();
-
-                if (rel.queryHints != null) {
-                    b.append(rel.queryHints).append(" ");
-                }
-
-                if (idfield.indexOf('(') == -1 && idfield.indexOf('.') == -1) {
-                    b.append(table).append('.');
-                }
-                b.append(idfield).append(" FROM ").append(table);
-
-                rel.appendAdditionalTables(b);
 
             if (home.getSubnodeRelation() != null) {
                 // subnode relation was explicitly set
                 query = b.append(" ").append(home.getSubnodeRelation()).toString();
             } else {
                 // let relation object build the query
-                    rel.buildQuery(b, home, null, " WHERE ", true);
+                rel.buildQuery(b, home, true, false);
                 query = b.toString();
             }
 
@@ -960,7 +944,7 @@ public final class NodeManager {
         } finally {
             if (logSql) {
                 long logTimeStop = System.currentTimeMillis();
-                    logSqlStatement("SQL SELECT_IDS", table,
+                logSqlStatement("SQL SELECT_IDS", type.getTableName(),
                         logTimeStart, logTimeStop, query);
             }
             if (stmt != null) {
@@ -973,7 +957,6 @@ public final class NodeManager {
 
         return retval;
     }
-    }
 
     /**
      *  Loades subnodes via subnode relation. This is similar to getNodeIDs, but it
@@ -986,7 +969,7 @@ public final class NodeManager {
 
         if ((rel == null) || (rel.otherType == null) || !rel.otherType.isRelational()) {
             // this should never be called for embedded nodes
-            throw new RuntimeException("NodeMgr.getNodes called for non-relational node " +
+            throw new RuntimeException("getNodes called for non-relational node " +
                                        home);
         }
 
@@ -1010,7 +993,7 @@ public final class NodeManager {
                 b.append(home.getSubnodeRelation());
             } else {
                 // let relation object build the query
-                rel.buildQuery(b, home, null, " WHERE ", true);
+                rel.buildQuery(b, home, true, false);
             }
 
             query = b.toString();
@@ -1053,152 +1036,6 @@ public final class NodeManager {
         return retval;
     }
     
-    /**
-     * Update a UpdateableSubnodeList retrieving all values having
-     * higher Values according to the updateCriteria's set for this Collection's Relation
-     * The returned Map-Object has two Properties:
-     * addedNodes = an Integer representing the number of Nodes added to this collection
-     * newNodes = an Integer representing the number of Records returned by the Select-Statement
-     * These two values may be different if a max-size is defined for this Collection and a new
-     * node would be outside of this Border because of the ordering of this collection.
-     * @param home the home of this subnode-list
-     * @param rel the relation the home-node has to the nodes contained inside the subnodelist
-     * @return A map having two properties of type String (newNodes (number of nodes retreived by the select-statment), addedNodes (nodes added to the collection))
-     * @throws Exception
-     */
-    /* public int updateSubnodeList(Node home, Relation rel) throws Exception {
-        if ((rel == null) || (rel.otherType == null) || !rel.otherType.isRelational()) {
-            // this should never be called for embedded nodes
-            throw new RuntimeException("NodeMgr.updateSubnodeList called for non-relational node " +
-                                       home);
-        } else {
-            List list = home.getSubnodeList();
-            if (list == null)
-                list = home.createSubnodeList();
-            
-            if (!(list instanceof UpdateableSubnodeList))
-                throw new RuntimeException ("unable to update SubnodeList not marked as updateable (" + rel.propName + ")");
-            
-            UpdateableSubnodeList sublist = (UpdateableSubnodeList) list;
-            
-            // FIXME: grouped subnodes aren't supported yet
-            if (rel.groupby != null)
-                throw new RuntimeException ("update not yet supported on grouped collections");
-
-            String idfield = rel.otherType.getIDField();
-            Connection con = rel.otherType.getConnection();
-            String table = rel.otherType.getTableName();
-
-            Statement stmt = null;
-
-            try {
-                String q = null;
-
-                StringBuffer b = new StringBuffer();
-                if (rel.loadAggressively()) {
-                    b.append (rel.otherType.getSelect(rel));
-                } else {
-                    b.append ("SELECT ");
-                    if (rel.queryHints != null) {
-                        b.append(rel.queryHints).append(" ");
-                    }
-                    b.append(table).append('.')
-                                   .append(idfield).append(" FROM ")
-                                   .append(table);
-
-                    rel.appendAdditionalTables(b);
-                }
-                String updateCriteria = sublist.getUpdateCriteria();
-                if (home.getSubnodeRelation() != null) {
-                    if (updateCriteria != null) {
-                        b.append (" WHERE ");
-                        b.append (sublist.getUpdateCriteria());
-                        b.append (" AND ");
-                        b.append (home.getSubnodeRelation());
-                    } else {
-                        b.append (" WHERE ");
-                        b.append (home.getSubnodeRelation());
-                    }
-                } else {
-                    if (updateCriteria != null) {
-                        b.append (" WHERE ");
-                        b.append (updateCriteria);
-                        rel.buildQuery(b, home, null, " AND ", true);
-                    } else {
-                        rel.buildQuery(b, home, null, " WHERE ", true);
-                    }
-                    q = b.toString();
-                }
-
-                long logTimeStart = logSql ? System.currentTimeMillis() : 0;
-
-                stmt = con.createStatement();
-
-                if (rel.maxSize > 0) {
-                    stmt.setMaxRows(rel.maxSize);
-                }
-
-                ResultSet result = stmt.executeQuery(q);
-
-                if (logSql) {
-                    long logTimeStop = System.currentTimeMillis();
-                    logSqlStatement("SQL SELECT_UPDATE_SUBNODE_LIST", table,
-                                    logTimeStart, logTimeStop, q);
-                }
-
-                // problem: how do we derive a SyntheticKey from a not-yet-persistent Node?
-                // Key k = (rel.groupby != null) ? home.getKey() : null;
-                // int cntr = 0;
-                
-                DbColumn[] columns = rel.loadAggressively() ? rel.otherType.getColumns() : null;
-                List newNodes = new ArrayList(rel.maxSize);
-                while (result.next()) {
-                    String kstr = result.getString(1);
-
-                    // jump over null values - this can happen especially when the selected
-                    // column is a group-by column.
-                    if (kstr == null) {
-                        continue;
-                    }
-
-                    // make the proper key for the object, either a generic DB key or a groupby key
-                    Key key;
-                    if (rel.loadAggressively()) {
-                        Node node = createNode(rel.otherType, result, columns, 0);
-                        if (node == null) {
-                            continue;
-                        }
-                        key = node.getKey();
-                        registerNewNode(node, null);
-                    } else {
-                        key = new DbKey(rel.otherType, kstr);
-                    }
-                    newNodes.add(new NodeHandle(key));
-
-                    // if these are groupby nodes, evict nullNode keys
-                    if (rel.groupby != null) {
-                        Node n = (Node) cache.get(key);
-
-                        if ((n != null) && n.isNullNode()) {
-                            evictKey(key);
-                        }
-                    }
-                }
-                // System.err.println("GOT NEW NODES: " + newNodes);
-                if (!newNodes.isEmpty())
-                    sublist.addAll(newNodes);
-                return newNodes.size();
-            } finally {
-                if (stmt != null) {
-                    try {
-                        stmt.close();
-                    } catch (Exception ignore) {
-                    }
-                }
-            }
-        }
-    } */
-
     protected List collectMissingKeys(SubnodeList list, int start, int length) {
         List retval = null;
         for (int i = start; i < start + length; i++) {
@@ -1360,46 +1197,35 @@ public final class NodeManager {
      * which is defined by Relation rel.
      */
     public int countNodes(Node home, Relation rel) throws Exception {
-        if ((rel == null) || (rel.otherType == null) || !rel.otherType.isRelational()) {
+        DbMapping type = rel == null ? null : rel.otherType;
+        if (type == null || !type.isRelational()) {
             // this should never be called for embedded nodes
-            throw new RuntimeException("NodeMgr.countNodes called for non-relational node " +
+            throw new RuntimeException("countNodes called for non-relational node " + home);
-                                       home);
+        }
-        } else {
         int retval = 0;
-            Connection con = rel.otherType.getConnection();
+        Connection con = type.getConnection();
         // set connection to read-only mode
         if (!con.isReadOnly()) con.setReadOnly(true);
 
-            String table = rel.otherType.getTableName();
         Statement stmt = null;
         long logTimeStart = logSql ? System.currentTimeMillis() : 0;
         String query = null;
 
         try {
-                StringBuffer tables = new StringBuffer(table);
+            StringBuffer b = rel.getCountSelect();
-
-                rel.appendAdditionalTables(tables);
-
-                // NOTE: we explicitly convert tables StringBuffer to a String
-                // before appending to be compatible with JDK 1.3
-                StringBuffer b = new StringBuffer("SELECT count(*) FROM ")
-                        .append(tables.toString());
 
             if (home.getSubnodeRelation() != null) {
                 // use the manually set subnoderelation of the home node
                 query = b.append(" ").append(home.getSubnodeRelation()).toString();
             } else {
                 // let relation object build the query
-                    rel.buildQuery(b, home, null, " WHERE ", false);
+                rel.buildQuery(b, home, false, true);
                 query = b.toString();
             }
 
             stmt = con.createStatement();
-
-
             ResultSet rs = stmt.executeQuery(query);
 
-
             if (!rs.next()) {
                 retval = 0;
             } else {
@@ -1408,7 +1234,7 @@ public final class NodeManager {
         } finally {
             if (logSql) {
                 long logTimeStop = System.currentTimeMillis();
-                    logSqlStatement("SQL SELECT_COUNT", table,
+                logSqlStatement("SQL SELECT_COUNT", type.getTableName(),
                         logTimeStart, logTimeStop, query);
             }
             if (stmt != null) {
@@ -1421,31 +1247,23 @@ public final class NodeManager {
 
         return (rel.maxSize > 0) ? Math.min(rel.maxSize, retval) : retval;
     }
-    }
 
     /**
      *  Similar to getNodeIDs, but returns a List that contains the nodes property names instead of IDs
      */
     public Vector getPropertyNames(Node home, Relation rel)
                             throws Exception {
-        if ((rel == null) || (rel.otherType == null) || !rel.otherType.isRelational()) {
+        DbMapping type = rel == null ? null : rel.otherType;
+        if (type == null || !type.isRelational()) {
             // this should never be called for embedded nodes
-            throw new RuntimeException("NodeMgr.getPropertyNames called for non-relational node " +
+            throw new RuntimeException("getPropertyNames called for non-relational node " + home);
-                                       home);
+        }
-        } else {
         Vector retval = new Vector();
 
-            // if we do a groupby query (creating an intermediate layer of groupby nodes),
-            // retrieve the value of that field instead of the primary key
-            String namefield = (rel.groupby == null) ? rel.accessName : rel.groupby;
         Connection con = rel.otherType.getConnection();
         // set connection to read-only mode
         if (!con.isReadOnly()) con.setReadOnly(true);
 
-            String table = rel.otherType.getTableName();
-            StringBuffer tables = new StringBuffer(table);
-            rel.appendAdditionalTables(tables);
-
         Statement stmt = null;
         long logTimeStart = logSql ? System.currentTimeMillis() : 0;
         String query = null;
@@ -1453,15 +1271,13 @@ public final class NodeManager {
         try {
             // NOTE: we explicitly convert tables StringBuffer to a String
             // before appending to be compatible with JDK 1.3
-                StringBuffer b = new StringBuffer("SELECT ").append(namefield)
+            StringBuffer b = rel.getNamesSelect();
-                                                            .append(" FROM ")
-                                                            .append(tables.toString());
 
             if (home.getSubnodeRelation() != null) {
                 b.append(" ").append(home.getSubnodeRelation());
             } else {
                 // let relation object build the query
-                    rel.buildQuery(b, home, null, " WHERE ", true);
+                rel.buildQuery(b, home, true, false);
             }
 
             stmt = con.createStatement();
@@ -1480,7 +1296,7 @@ public final class NodeManager {
         } finally {
             if (logSql) {
                 long logTimeStop = System.currentTimeMillis();
-                    logSqlStatement("SQL SELECT_ACCESSNAMES", table,
+                logSqlStatement("SQL SELECT_ACCESSNAMES", type.getTableName(),
                         logTimeStart, logTimeStop, query);
             }
 
@@ -1494,7 +1310,6 @@ public final class NodeManager {
 
         return retval;
     }
-    }
 
     ///////////////////////////////////////////////////////////////////////////////////////
     // private getNode methods
@@ -1622,7 +1437,7 @@ public final class NodeManager {
                         b.append(")");
                     }
                 } else {
-                    rel.buildQuery(b, home, dbm, kstr, " WHERE ", false);
+                    rel.buildQuery(b, home, dbm, kstr, false, false);
                 }
 
                 stmt = con.createStatement();

src/helma/objectmodel/db/Relation.java

@@ -873,13 +873,58 @@ public final class Relation {
         return vr;
     }
 
+    public StringBuffer getIdSelect() {
+        StringBuffer buf = new StringBuffer("SELECT ");
+
+        if (queryHints != null) {
+                buf.append(queryHints).append(" ");
+            }
+
+        String table = otherType.getTableName();
+        String idfield = (groupby == null) ? otherType.getIDField() : groupby;
+
+        if (idfield.indexOf('(') == -1 && idfield.indexOf('.') == -1) {
+            buf.append(table).append('.');
+        }
+        buf.append(idfield).append(" FROM ").append(table);
+        appendAdditionalTables(buf);
+
+        return buf;
+    }
+
+    public StringBuffer getCountSelect() {
+        StringBuffer buf = new StringBuffer();
+        if (otherType.isOracle() && maxSize > 0) {
+            buf.append("SELECT * FROM ");
+        } else {
+            buf.append("SELECT count(*) FROM ");
+        }
+
+        buf.append(otherType.getTableName());
+        appendAdditionalTables(buf);
+
+        return buf;
+    }
+
+    public StringBuffer getNamesSelect() {
+        // if we do a groupby query (creating an intermediate layer of groupby nodes),
+        // retrieve the value of that field instead of the primary key
+        String namefield = (groupby == null) ? accessName : groupby;
+        String table = otherType.getTableName();
+        StringBuffer buf = new StringBuffer("SELECT ");
+        buf.append(namefield).append(" FROM ").append(table);
+        appendAdditionalTables(buf);
+
+        return buf;
+    }
+
     /**
      *  Build the second half of an SQL select statement according to this relation
      *  and a local object.
      */
-    public void buildQuery(StringBuffer q, Node home, String kstr, String pre, boolean useOrder)
+    public void buildQuery(StringBuffer q, Node home, boolean useOrder, boolean isCount)
             throws SQLException, ClassNotFoundException {
-        buildQuery(q, home, otherType, kstr, pre, useOrder);
+        buildQuery(q, home, otherType, null, useOrder, isCount);
     }
 
     /**
@@ -887,9 +932,9 @@ public final class Relation {
      *  and a local object.
      */
     public void buildQuery(StringBuffer q, Node home, DbMapping otherDbm, String kstr,
-                           String pre, boolean useOrder)
+                           boolean useOrder, boolean isCount)
             throws SQLException, ClassNotFoundException {
-        String prefix = pre;
+        String prefix = " WHERE ";
         Node nonvirtual = home.getNonVirtualParent();
 
         if (kstr != null && !isComplexReference()) {
@@ -922,11 +967,12 @@ public final class Relation {
         if (maxSize > 0) {
             if (otherType.isOracle()) {
                 // see http://www.oracle.com/technology/oramag/oracle/06-sep/o56asktom.html
+                String selectItem = isCount ? "count(*)" : "*";
                 if (offset > 0) {
-                    q.insert(0, "SELECT * FROM ( SELECT /*+ FIRST_ROWS(n) */ a.*, ROWNUM rnum FROM (");
+                    q.insert(0, "SELECT " + selectItem + " FROM ( SELECT /*+ FIRST_ROWS(n) */ a.*, ROWNUM rnum FROM (");
                     q.append(") a WHERE ROWNUM <= ").append(offset + maxSize).append(") WHERE rnum > ").append(offset);
                 } else {
-                    q.insert(0, "SELECT /*+ FIRST_ROWS(n) */ * FROM (");
+                    q.insert(0, "SELECT /*+ FIRST_ROWS(n) */ " + selectItem + " FROM (");
                     q.append(") WHERE ROWNUM <= ").append(maxSize);
                 }
             } else {