Satheesh Bandaram wrote:
I have briefly looked at the proposed changes. Some initial comments:
Wow, that was fast ;)
Thanks for the feedback. I think all 4 points you made are good ones, and I
will look into them more.
> 1. I suspect the logic to reject XML columns at the toplevel is more
> complicated than is needed.
I agree that the logic to reject XML columns seems a bit convoluted. I hadn't
noticed the rejectParameters() method that you mentioned, but it sounds like it
could offer a better alternative. I'll definitely follow up there.
2. Is it possible to consolidate some of the new nodes into existing nodes or
with each other?
Yes, it's possible. And I thought about that when I was writing those classes.
But to consolidate would mean adding "if" logic into a single class and
calling different binding methods based on the node type. Which is of course
doable and simple--but it doesn't seem quite as "clean" to me as separate nodes.
Nonetheless, if this is the preferred way to go about it, I can certainly make
that change.
3. How do you query XML documents with namespace tags?
Not sure what you mean there. Can you explain a bit more?
Also, I think you have to turn on namespace processing tags for
Xalan to get the correct results
Good catch. Yes, I'll have to turn namespace processing on. There's a feature
(http://xml.org/sax/features/namespaces) for that which can be set on the
parser, so I think we'd just have to set it on the XMLReader that is passed to
the XSLT engine at query time. I'll do that and try to add some namespace-based
queries to the tests.
4. You have mentioned XMLConstant is only used for null values. It is
possible, using some subquerries, the current compilation evaluates
XMLParse() into an XMLConstant before further evaluating it. In these
cases, it is possible to have a valid XML constant that is not null.
Okay, if this is true then I think the only change I need is to remove the
ASSERT in XMLConstant.java that checks for non-null values. Is that right?
Thanks again for the comments,
Army