Attached is a patch that fixes several issues with collection level XUpdate commands. This proposed patch fixes bug 25892 and issues with accidently trying to running XUpdate variables as XPaths.
The one thing I am not sure about is I wait until all of the commands are executed on all of the documents in the collection before committing the changes back. This could be potentially hard on memory but it guarantees that if the one of the commands blew up it doesn't leave the database in a bad state.
Yep - same here - I have same memory concern as you do.
Patch looks good, and if you don't mind adjusting it, please re-send with these minor tweaks:
* HashMap is more appropriate here.
Hashtable is synchronized, which is not needed here.
And switch to map.entrySet().iterator() instead of Enumeration. * Please add a TODO comment before docsUpdated declaration,
something to the effect of "don't buffer changed documents
in the memory if underlying collection supports transactions"
(idea is that if in future collection supports transactions,
no in memory document map will be needed) * Can you add a test case for this scenario, so that
we can automatically check for regressions in the future?
Probably it can be added into the XUpdateQueryTest.* Don't use tabs!!! :-) 4 space indenting only, please.
And, as I am not huge xupdate user myself, can you clarify what this condition is needed for:
> + if (selector != null && !selector.startsWith("$")) {
(I mean, 'selector.startsWith("$")' part)
Thanks, Vadim
Comments and suggestions welcome.
Todd Byrne
------------------------------------------------------------------------
--- xml-xindice/java/src/org/apache/xindice/core/xupdate/XUpdateImpl.java 2004-02-07 19:50:54.000000000 -0700
+++ /home/byrne/jbproject/xindice-cvs-src/xml-xindice/java/src/org/apache/xindice/core/xupdate/XUpdateImpl.java 2004-08-30 10:30:39.000000000 -0600
@@ -24,6 +24,7 @@
import org.apache.xindice.xml.NodeSource;
import org.apache.xindice.xml.dom.CompressedNode;
import org.apache.xindice.xml.dom.DBNode;
+import org.apache.xindice.core.data.Key;
import org.w3c.dom.Document;
import org.w3c.dom.Node;
@@ -150,6 +151,7 @@
public void execute(Collection col) throws Exception {
int attribIndex = 0;
+ Hashtable docsUpdated = new Hashtable();
for (int i = 0; i < super.query[0].size(); i++) {
int cmdID = ((Integer) super.query[0].elementAt(i)).intValue();
@@ -159,29 +161,32 @@
String selector = (String) attribs.get("select");
// If we found an XPath selector we need to execute the commands.
- if (selector != null) {
+ if (selector != null && !selector.startsWith("$")) {
NodeSet ns = col.queryCollection("XPath", selector, nsMap);
Document lastDoc = null;
while (ns != null && ns.hasMoreNodes()) {
DBNode node = (DBNode) ns.getNextNode();
Document doc = node.getOwnerDocument();
+ NodeSource source = node.getSource();
+ Node contextNode = doc.getDocumentElement();
- if (doc == lastDoc) {
+ if (docsUpdated.containsKey(source.getKey())) {
continue; // We only have to process it once
} else {
- lastDoc = doc;
+ docsUpdated.put(source.getKey(), doc);
}
-
- NodeSource source = node.getSource();
-
- Node contextNode = doc.getDocumentElement();
execute(contextNode);
-
- col.setDocument(source.getKey(), doc);
}
}
}
}
+ // update all documents at once + // this way we don't get any half run xupdate commands.
+ Enumeration keys = docsUpdated.keys();
+ while(keys.hasMoreElements()) {
+ Key docID = (Key)keys.nextElement();
+ col.setDocument(docID, (Document)docsUpdated.get(docID));
+ }
}
public int getModifiedCount() {
