lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chris Hostetter <>
Subject Re: XML based Query Parser
Date Mon, 27 Feb 2006 01:28:31 GMT

: Further to our discussions some time ago I've had some time to put
: together an XML-based query parser with support for many "advanced"
: query types not supported in the current Query parser.
: More details and code here:

So I *finally* got a chance to look at this in depth.  In general, i think
it looks great, but I have a few questions/comments...

1) The factory classes should have "removeBuilder" methods so people
   subclassing parsers can flat out remove support for a particular
   tag, not just replace it.

2) This DOM version definitely seems easier to follow/use then the SAX
   version (allthough thta could just be because i'm more familiar
   with DOM then SAX) .. but it still seems like an intermediate
   representation that wasn't org.w3c.dom.Element would be handy -- if
   for no other reason then so the methods in DOMUtils could be put
   directly in the "Element" class.

3) I'm still confused about how state information could/would be
   passed up or down the tree by builders.  you mentioned something
   before about using ThreadLocal (which i'm not very familiar with)
   but i don't see any examples of that here.

4) The various getQuery (and getFilter and getSpanQuery) methods
   should use e.getTagName when throwing ParserExceptions about not
   finding what they're looking for,  That way, if i write a
   parser that has...
      queryFactory.addBuilder("tf",new TermQueryObjectBuilder());
   ... the ParserExceptions thrown by TermQueryObjectBuilder.getQuery
   when i forget to include a value="foo" can refrence the tag name
   i'm using (tf), and not the hardcoded constant "TermQuery"

5) it seems like a lot of redundency could be removed between the
   various builders by refactoring some more utility functions --
   particularly in the error cases  (see attached patch)


View raw message