On Tue, Oct 5, 2010 at 9:06 AM, Jens Hübel <[email protected]> wrote: > 1) > You did a lot of reformatting and changes with line-end conventions. Is there > something we could improve in our code formatting templates or other Eclipse > settings?
The only one I can think of is getting rid of invisible spaces (tabs and trailing spaces) by making sure your Java editor strips trailing spaces (it's a Save Action -- note that you shouldn't enable both it and automatic code reformatting on save because the Javadoc formatter re-adds trailing spaces after a bare *). I don't think I did much reformatting besides the whitespace thing. > 2) > Comparing PredicateWalker with QueryConditionProcessor you have introduced a > boolean return value on each method. I don't get what's the meaning of this. > In your implementation it seems that you always return false. Could you > explain a bit what your idea is here? My idea behind this interface was that > many implementations will try to translate CMISQL to SQL. So an > implementation typically will have a member variable for the target SQL query > string and compose this while walking the tree. I did not find a good way to > do this as return values of the individual node visitor methods and so just > left them void. You're right to say that the returned values wouldn't be useful for query translators, but for query evaluators they are. In my Nuxeo implementation (GeneratingWalker in http://hg.nuxeo.org/addons/nuxeo-chemistry/file/5.4/nuxeo-opencmis-impl/src/main/java/org/nuxeo/ecm/core/opencmis/impl/server/CMISQLQueryMaker.java) indeed I don't use the return values, but they are very useful in walkers used for direct evaluation, like the InMemoryQueryProcessor. This could also be useful for a number of other simple implementations of the CMIS connector of simple servers, and the return values aren't a big hindrance otherwise. I could streamline all the return values to Object though, to allow more generic evaluation even of clauses and allow a simple "return null" if you don't want them. The alternative if we want to use a walker with methods returning void in InMemoryQueryProcessor is to have each method put its return value in an instance scratchpad field and get it back from the caller. It works but is ugly -- otoh InMemoryQueryProcessor isn't supposed to be pretty... If you think InMemoryQueryProcessor is the exception then we can used void. > 3) > Recently I have added additional pre- and post- methods in the interface for > AND, OR, NOT. The reason was that you typically have to set parentheses in > your generated SQL call for the contained expressions. So my idea was that if > you have a ... WHERE ... A AND B in the cmis query you have a preAnd method > appending '(' to your generated SQL then process A then have onAnd where you > add " AND " call, then process B and then have a postAnd method where you > append the ')'. I don't see this any longer in your code. What's the new > approach dealing with this? In Nuxeo for instance (which does SQL generation) I do this: @Override public boolean walkAnd(Tree opNode, Tree leftNode, Tree rightNode) { buf.append("("); walkPredicate(leftNode); buf.append(" AND "); walkPredicate(rightNode); buf.append(")"); return false; } I think it's simpler and doesn't add lots of pre- and post- methods. Before deciding to code the PredicateWalker I was trying to use the QueryConditionProcessor and I had to add lots of pre- post- and middle- methods to it to deal with things like IS NULL, LIKE, ANY =, IN ANY, and it really broke down for ANY = which can be generated in very different manners depending on the backend. > 4) > If we shift to the new interface/implementation you also should update the > documentation: > http://incubator.apache.org/chemistry/queryintegration.html Yes I'll do that if we decide on using this. > Unfortunately the InMemory server is not the best example for an > implementation. Usually a repository will translate the query to some form of > database query. The InMemory implementation however is somewhat special in > that implements the query engine on its own (and given that of course very > limited). If we could come up with a good example for translation into an SQL > query or something similar this would be very helpful. Direct SQL (or Lucene) generation is a much bigger endeavor though, as it implies having all the storage changed. I'll be content for now with showing Nuxeo source code (LGPL) as an example :) Florent > -----Original Message----- > From: Florent Guillaume [mailto:[email protected]] > Sent: Mittwoch, 29. September 2010 01:09 > To: [email protected] > Subject: Re: JOINs > > It doesn't change the grammar at all, it's an alternate way of doing > what the AbstractQueryConditionProcessor does to allow dealing with > the WHERE clause. I made InMemoryQueryProcessor use it as it's > cleaner, but I didn't want to remove AbstractQueryConditionProcessor > yet until I know if people depend on it or what people think is best. > > I'll let you take the time to look at this. FYI the base class is > https://svn.apache.org/repos/asf/incubator/chemistry/opencmis/trunk/chemistry-opencmis-server/chemistry-opencmis-server-support/src/main/java/org/apache/chemistry/opencmis/server/support/query/AbstractClauseWalker.java > > Florent > > On Tue, Sep 28, 2010 at 10:47 PM, Jens Hübel <[email protected]> wrote: >> I am currently on travel and don't have a chance to look at this for the >> moment. If this is a replacement for the existing walker we should have a >> discussion on the list which way to proceed (we shouldn't support two in the >> long run). Anyway great to see progress in this area... I will followup once >> I had a chance to take a look at this. >> >> Jens >> >> >> -----Original Message----- >> From: Florent Guillaume [mailto:[email protected]] >> Sent: Dienstag, 28. September 2010 19:31 >> To: [email protected] >> Subject: Re: JOINs >> >> I haven't done the JOIN part yet, but I added a new ClauseWalker >> interface (now used in InMemoryQueryProcessor). It should be simpler >> to use and extend. >> >> Florent >> >> On Mon, Sep 27, 2010 at 1:32 PM, Jens Hübel <[email protected]> wrote: >>> Hi Florent, >>> >>> the support for JOINS is currently weak. I don't think that anybody has >>> used them so far. Feel free to modify them. Probably we also could >>> implement a predefined walker object at some point so that for a simple use >>> case a user does not have to know about the tree structure. >>> >>> Jens >>> >>> >>> -----Original Message----- >>> From: Florent Guillaume [mailto:[email protected]] >>> Sent: Montag, 27. September 2010 13:04 >>> To: List-Chemistry >>> Subject: JOINs >>> >>> Hi, >>> >>> Is anyone using the structures related to JOINs in QueryObject? I >>> think I'll have to modify them a bit to be able to properly implement >>> JOIN-based queries in Nuxeo. >>> >>> Florent >>> >>> -- >>> Florent Guillaume, Director of R&D, Nuxeo >>> Open Source, Java EE based, Enterprise Content Management (ECM) >>> http://www.nuxeo.com http://www.nuxeo.org +33 1 40 33 79 87 >>> >> >> >> >> -- >> Florent Guillaume, Director of R&D, Nuxeo >> Open Source, Java EE based, Enterprise Content Management (ECM) >> http://www.nuxeo.com http://www.nuxeo.org +33 1 40 33 79 87 >> > > > > -- > Florent Guillaume, Director of R&D, Nuxeo > Open Source, Java EE based, Enterprise Content Management (ECM) > http://www.nuxeo.com http://www.nuxeo.org +33 1 40 33 79 87 > -- Florent Guillaume, Director of R&D, Nuxeo Open Source, Java EE based, Enterprise Content Management (ECM) http://www.nuxeo.com http://www.nuxeo.org +33 1 40 33 79 87
