eXist-db / exist

eXist Native XML Database and Application Platform
https://exist-db.org
GNU Lesser General Public License v2.1
428 stars 179 forks source link

updating/inserting/deleting comment()'s before the root element #628

Open mgr34 opened 9 years ago

mgr34 commented 9 years ago

notes:

  1. filling issue at the behest of Jörn from eXist solutions
  2. _original mailing list submission_

Not able to update (or insert) comments before the root element. Take for example

db/test.xml

<?xml version="1.0" encoding="UTF-8"?>
<!-- to be replaced -->
<document>
    <p>Contains stuff.</p>
</document>

db/update.xq

xquery version "3.1";

let $doc := doc('/db/test.xml')
let $comment := $doc/comment()
return 
    update replace $comment with comment { "replaced" }

Responds with:

exerr:ERROR The root element of a document can not be replaced with 'update replace'. Please consider removing the document or use 'update value' to just replace the children of the root. [at line 6, column 5, source: String]

expected db/test.xml:

<?xml version="1.0" encoding="UTF-8"?>
<!-- replaced -->
<document>
    <p>Contains stuff.</p>
</document>

I am currently working with the develop branch from Apr 15. (4a94ebd). On Debian 7.8. I suspect this is likely the case in eXist 2.2, but cannot confirm.

joewiz commented 9 years ago

Possibly related to #623?

mgr34 commented 9 years ago

@joewiz I made that original stackoverflow post so it is related there. But I think otherwise this issue may be unrelated.

For example if you look at the particular error message displayed it looks at the parentNode and throws an error if its null, But in this case that makes sense. But comments and processing instructions would still be valid?

parent = (ElementImpl) node.getParentStoredNode();
 if (parent == null)
                    {throw new EXistException("The root element of a document can not be replaced with 'xu:replace'. " +
                        "Please consider removing the document or use 'xu:update' to just replace the children of the root.");}

My interpretation could be wrong. I didn't spend that much time actually trying to digest what was happening.


EDIT:

actually, digging down further it looks like it won't let me update any comment()'s value


switch (node.getNodeType()) {
                    case Node.ELEMENT_NODE:
                        if (modifications == 0) {modifications = 1;}
                        temp = children.item(0);
                        parent.replaceChild(transaction, temp, node);
                        break;
                    case Node.TEXT_NODE:
                        temp = children.item(0);
                        text = new TextImpl(temp.getNodeValue());
                        modifications = 1;
                        text.setOwnerDocument(doc);
                        parent.updateChild(transaction, node, text);
                        break;
                    case Node.ATTRIBUTE_NODE:
                        final AttrImpl attr = (AttrImpl) node;
                        temp = children.item(0);
                        attribute = new AttrImpl(attr.getQName(), temp.getNodeValue(), broker.getBrokerPool().getSymbols());
                        attribute.setOwnerDocument(doc);
                        parent.updateChild(transaction, node, attribute);
                        break;
                    default:
                        throw new EXistException("unsupported node-type");
                }

Moving the comment to the position of first-child of the root node I am instead thrown the error referencing unsupported node-type. hmm.... I guess I could try deleting and inserting a new comment, but that might be a digression right now.

joewiz commented 9 years ago

After discussing this with @wolfgangmm in Oxford, I understand the XQuery Update code would need to be drastically reworked to allow update expressions to modify comment nodes outside the root element. I propose this be closed and labelled "won't fix."

adamretter commented 9 years ago

@joewiz I do think this could be fixed, but it would require some rearchitecting of the XQuery Update code. I do consider this to be a bug, simply saying its a lot of work does not change that IMHO. We should leave the ticket open as it serves as a reference.

joewiz commented 9 years ago

@adamretter Fair enough. Given the choice between adding this feature and adding official XQuery Update Facility support to eXist, what would you say is the best use of resources?

joewiz commented 9 years ago

(Implicit in my statement was that XQUF would work on persistent as well as in-memory nodes.)

adamretter commented 9 years ago

XQUF is the way to go, maintaining something non-standard and broken is bad. However, adding XQUF is much more work of course... so their is a quality vs time/resources trade off.

Also if people wanted to apply XQUF inline, then we would need some parts of XQuery Scripting too.