You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@chemistry.apache.org by Jens Hübel <jh...@opentext.com> on 2010/10/05 09:06:44 UTC
RE: JOINs
Hi Florent,
taking a look at this I have the impression that the basic ideas are still the same so there are no objections from my side in general. I am happy that someone tries to use this and open for any kind of improvement... A couple of more detailed questions and comments following ...
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?
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.
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?
4)
If we shift to the new interface/implementation you also should update the documentation:
http://incubator.apache.org/chemistry/queryintegration.html
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.
Jens
-----Original Message-----
From: Florent Guillaume [mailto:fg@nuxeo.com]
Sent: Mittwoch, 29. September 2010 01:09
To: chemistry-dev@incubator.apache.org
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 <jh...@opentext.com> 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:fg@nuxeo.com]
> Sent: Dienstag, 28. September 2010 19:31
> To: chemistry-dev@incubator.apache.org
> 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 <jh...@opentext.com> 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:fg@nuxeo.com]
>> 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
Re: JOINs
Posted by Florent Guillaume <fg...@nuxeo.com>.
Thanks Jens. With a few small tweaks my code now works with your changes.
Florent
On Fri, Oct 22, 2010 at 3:56 PM, Jens Hübel <jh...@opentext.com> wrote:
> According to the discussion we had I have checked in the necessary changes today:
>
> (1) removed the old walker code
> (2) changed walker grammar so that Florent's PredicateWalker is now used.
> (3) splitted interface PredicateWalter and let it inherit from PredicateWalkerBase
> so that people can implement their own walking logic if they like without changing grammar
> (4) moved code from (1) to test classes so that we have an example for (3)
> (5) changed return value of all boolean methods in interface to Boolean so that you can
> return null
>
> As a side effect classes are better decoupled now (see past discussions). We still need a good example for the PredicateWalker and adjust the Wiki documentation... also exception handling needs to be improved as already mentioned.
>
> I hope that I did not break anything, it was mainly cleanup.
>
> @Florent: To the best of my knowledge I tried to respect all your requirements mentioned below. Perhaps you could check at some point in time if all this still works for you and if not make the necessary changes. I do not expect any bigger issues but you never know...
>
> In case someone has used the old interface and prefers to keep it, this is still possible by moving the older walker from (1), (4) to the calling application.
>
> Jens
>
> -----Original Message-----
> From: Florent Guillaume [mailto:fg@nuxeo.com]
> Sent: Donnerstag, 7. Oktober 2010 16:12
> To: chemistry-dev@incubator.apache.org
> Subject: Re: JOINs
>
> Hi Jens,
>
> On Thu, Oct 7, 2010 at 3:00 PM, Jens Hübel <jh...@opentext.com> wrote:
>>> 2)
>> What about using Boolean instead of boolean? Then an implementation could return null instead of false indicating that the return value is not of interest in this case. Would be less confusing than "false".
>
> That would work yes.
>
>> And one more question:
>>
>> I think we would need to switch the query clause in the walker grammar so that the new implementation is used.
>> CmisQueryWalker.g From:
>>
>> query [QueryObject qo]
>> @init {
>> queryObj = qo;
>> }:
>> ^(SELECT select_list from_clause order_by_clause? where_clause)
>> {
>> queryObj.resolveTypes();
>> queryObj.processWhereClause($where_clause.tree);
>> }
>> ;
>>
>> To:
>>
>> query [QueryObject qo, PredicateWalker pw]
>> @init {
>> queryObj = qo;
>> predicateWalker = pw;
>> }:
>> ^(SELECT select_list from_clause order_by_clause? where_clause)
>> {
>> queryObj.resolveTypes();
>> predicateWalker.walkPredicate($where_clause.tree);
>> }
>> ;
>>
>> I wonder how you have currently solved this for Nuxeo? The only way I found for a little playground project was this ugly construction:
>> org.apache.chemistry.opencmis.server.support.query.CmisQueryWalker.query_return qr = walker.query(queryObj);
>> Tree whereTree = ((Tree) qr.getTree()).getChild(2);
>> Tree whereChild = whereTree == null ? null : whereTree.getChild(0);
>> if (null != whereChild)
>> walkPredicate(whereChild);
>>
>> I assume that you just did not change this to not break existing code yet. (it must not be this construct, but conceptually to
>> have a convenient mechanism to kick-off the walker).
>
> In Nuxeo at the moment I'm doing:
>
> Tree whereNode = query.getWhereTree() == null ? null :
> query.getWhereTree().getChild(0);
> if (whereNode != null) {
> new AnalyzingWalker().walkPredicate(whereNode);
> }
>
> This works because processWhereClause now saves the tree (I added this
> recently) and does nothing if queryProcessor is null.
>
>> Perhaps it's also worth thinking about making this more flexible, e.g. take the walker out of QueryObject (what you already did) and have a base interface PredicateWalkerBase that only has a single method walkPredicate() and then derive custom walker interfaces from this, so that you can define your walker interface as you like it. In this way we even could use your mechanism and my original one. We could then deliver PredicateWalker as default implementation and a user could define another interface if he needs. The CmisQueryWalker.g depends only on this single method. What do you think? This would be my favorite at the moment.
>
> Works for me (as long we allow the walker to be null just in case).
> The only important thing I want to keep is that I want to be able to
> call the walkers by hand several times on the same tree, so I still
> need access to the WhereTree from outside the CmisQueryWalker.g /
> QueryObject. I guess the first walker could save its argument...
>
>> Probably you also have noticed that currently only RuntimeExceptions are raised in case that something goes wrong. This is something that we still need to fix to get more useful error messages out of the parser. It's just not implemented yet, because it took me some time to figure out to do custom error handling with antlr and first attempt did not work.
>
> Yes I want to change this as well, currently some parsing errors go to stderr...
>
> Florent
>
>
>> -----Original Message-----
>> From: Florent Guillaume [mailto:fg@nuxeo.com]
>> Sent: Dienstag, 5. Oktober 2010 11:41
>> To: chemistry-dev@incubator.apache.org
>> Subject: Re: JOINs
>>
>> On Tue, Oct 5, 2010 at 9:06 AM, Jens Hübel <jh...@opentext.com> 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:fg@nuxeo.com]
>>> Sent: Mittwoch, 29. September 2010 01:09
>>> To: chemistry-dev@incubator.apache.org
>>> 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 <jh...@opentext.com> 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:fg@nuxeo.com]
>>>> Sent: Dienstag, 28. September 2010 19:31
>>>> To: chemistry-dev@incubator.apache.org
>>>> 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 <jh...@opentext.com> 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:fg@nuxeo.com]
>>>>> 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
RE: JOINs
Posted by Jens Hübel <jh...@opentext.com>.
According to the discussion we had I have checked in the necessary changes today:
(1) removed the old walker code
(2) changed walker grammar so that Florent's PredicateWalker is now used.
(3) splitted interface PredicateWalter and let it inherit from PredicateWalkerBase
so that people can implement their own walking logic if they like without changing grammar
(4) moved code from (1) to test classes so that we have an example for (3)
(5) changed return value of all boolean methods in interface to Boolean so that you can
return null
As a side effect classes are better decoupled now (see past discussions). We still need a good example for the PredicateWalker and adjust the Wiki documentation... also exception handling needs to be improved as already mentioned.
I hope that I did not break anything, it was mainly cleanup.
@Florent: To the best of my knowledge I tried to respect all your requirements mentioned below. Perhaps you could check at some point in time if all this still works for you and if not make the necessary changes. I do not expect any bigger issues but you never know...
In case someone has used the old interface and prefers to keep it, this is still possible by moving the older walker from (1), (4) to the calling application.
Jens
-----Original Message-----
From: Florent Guillaume [mailto:fg@nuxeo.com]
Sent: Donnerstag, 7. Oktober 2010 16:12
To: chemistry-dev@incubator.apache.org
Subject: Re: JOINs
Hi Jens,
On Thu, Oct 7, 2010 at 3:00 PM, Jens Hübel <jh...@opentext.com> wrote:
>> 2)
> What about using Boolean instead of boolean? Then an implementation could return null instead of false indicating that the return value is not of interest in this case. Would be less confusing than "false".
That would work yes.
> And one more question:
>
> I think we would need to switch the query clause in the walker grammar so that the new implementation is used.
> CmisQueryWalker.g From:
>
> query [QueryObject qo]
> @init {
> queryObj = qo;
> }:
> ^(SELECT select_list from_clause order_by_clause? where_clause)
> {
> queryObj.resolveTypes();
> queryObj.processWhereClause($where_clause.tree);
> }
> ;
>
> To:
>
> query [QueryObject qo, PredicateWalker pw]
> @init {
> queryObj = qo;
> predicateWalker = pw;
> }:
> ^(SELECT select_list from_clause order_by_clause? where_clause)
> {
> queryObj.resolveTypes();
> predicateWalker.walkPredicate($where_clause.tree);
> }
> ;
>
> I wonder how you have currently solved this for Nuxeo? The only way I found for a little playground project was this ugly construction:
> org.apache.chemistry.opencmis.server.support.query.CmisQueryWalker.query_return qr = walker.query(queryObj);
> Tree whereTree = ((Tree) qr.getTree()).getChild(2);
> Tree whereChild = whereTree == null ? null : whereTree.getChild(0);
> if (null != whereChild)
> walkPredicate(whereChild);
>
> I assume that you just did not change this to not break existing code yet. (it must not be this construct, but conceptually to
> have a convenient mechanism to kick-off the walker).
In Nuxeo at the moment I'm doing:
Tree whereNode = query.getWhereTree() == null ? null :
query.getWhereTree().getChild(0);
if (whereNode != null) {
new AnalyzingWalker().walkPredicate(whereNode);
}
This works because processWhereClause now saves the tree (I added this
recently) and does nothing if queryProcessor is null.
> Perhaps it's also worth thinking about making this more flexible, e.g. take the walker out of QueryObject (what you already did) and have a base interface PredicateWalkerBase that only has a single method walkPredicate() and then derive custom walker interfaces from this, so that you can define your walker interface as you like it. In this way we even could use your mechanism and my original one. We could then deliver PredicateWalker as default implementation and a user could define another interface if he needs. The CmisQueryWalker.g depends only on this single method. What do you think? This would be my favorite at the moment.
Works for me (as long we allow the walker to be null just in case).
The only important thing I want to keep is that I want to be able to
call the walkers by hand several times on the same tree, so I still
need access to the WhereTree from outside the CmisQueryWalker.g /
QueryObject. I guess the first walker could save its argument...
> Probably you also have noticed that currently only RuntimeExceptions are raised in case that something goes wrong. This is something that we still need to fix to get more useful error messages out of the parser. It's just not implemented yet, because it took me some time to figure out to do custom error handling with antlr and first attempt did not work.
Yes I want to change this as well, currently some parsing errors go to stderr...
Florent
> -----Original Message-----
> From: Florent Guillaume [mailto:fg@nuxeo.com]
> Sent: Dienstag, 5. Oktober 2010 11:41
> To: chemistry-dev@incubator.apache.org
> Subject: Re: JOINs
>
> On Tue, Oct 5, 2010 at 9:06 AM, Jens Hübel <jh...@opentext.com> 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:fg@nuxeo.com]
>> Sent: Mittwoch, 29. September 2010 01:09
>> To: chemistry-dev@incubator.apache.org
>> 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 <jh...@opentext.com> 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:fg@nuxeo.com]
>>> Sent: Dienstag, 28. September 2010 19:31
>>> To: chemistry-dev@incubator.apache.org
>>> 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 <jh...@opentext.com> 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:fg@nuxeo.com]
>>>> 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
Re: JOINs
Posted by Florent Guillaume <fg...@nuxeo.com>.
Hi Jens,
On Thu, Oct 7, 2010 at 3:00 PM, Jens Hübel <jh...@opentext.com> wrote:
>> 2)
> What about using Boolean instead of boolean? Then an implementation could return null instead of false indicating that the return value is not of interest in this case. Would be less confusing than "false".
That would work yes.
> And one more question:
>
> I think we would need to switch the query clause in the walker grammar so that the new implementation is used.
> CmisQueryWalker.g From:
>
> query [QueryObject qo]
> @init {
> queryObj = qo;
> }:
> ^(SELECT select_list from_clause order_by_clause? where_clause)
> {
> queryObj.resolveTypes();
> queryObj.processWhereClause($where_clause.tree);
> }
> ;
>
> To:
>
> query [QueryObject qo, PredicateWalker pw]
> @init {
> queryObj = qo;
> predicateWalker = pw;
> }:
> ^(SELECT select_list from_clause order_by_clause? where_clause)
> {
> queryObj.resolveTypes();
> predicateWalker.walkPredicate($where_clause.tree);
> }
> ;
>
> I wonder how you have currently solved this for Nuxeo? The only way I found for a little playground project was this ugly construction:
> org.apache.chemistry.opencmis.server.support.query.CmisQueryWalker.query_return qr = walker.query(queryObj);
> Tree whereTree = ((Tree) qr.getTree()).getChild(2);
> Tree whereChild = whereTree == null ? null : whereTree.getChild(0);
> if (null != whereChild)
> walkPredicate(whereChild);
>
> I assume that you just did not change this to not break existing code yet. (it must not be this construct, but conceptually to
> have a convenient mechanism to kick-off the walker).
In Nuxeo at the moment I'm doing:
Tree whereNode = query.getWhereTree() == null ? null :
query.getWhereTree().getChild(0);
if (whereNode != null) {
new AnalyzingWalker().walkPredicate(whereNode);
}
This works because processWhereClause now saves the tree (I added this
recently) and does nothing if queryProcessor is null.
> Perhaps it's also worth thinking about making this more flexible, e.g. take the walker out of QueryObject (what you already did) and have a base interface PredicateWalkerBase that only has a single method walkPredicate() and then derive custom walker interfaces from this, so that you can define your walker interface as you like it. In this way we even could use your mechanism and my original one. We could then deliver PredicateWalker as default implementation and a user could define another interface if he needs. The CmisQueryWalker.g depends only on this single method. What do you think? This would be my favorite at the moment.
Works for me (as long we allow the walker to be null just in case).
The only important thing I want to keep is that I want to be able to
call the walkers by hand several times on the same tree, so I still
need access to the WhereTree from outside the CmisQueryWalker.g /
QueryObject. I guess the first walker could save its argument...
> Probably you also have noticed that currently only RuntimeExceptions are raised in case that something goes wrong. This is something that we still need to fix to get more useful error messages out of the parser. It's just not implemented yet, because it took me some time to figure out to do custom error handling with antlr and first attempt did not work.
Yes I want to change this as well, currently some parsing errors go to stderr...
Florent
> -----Original Message-----
> From: Florent Guillaume [mailto:fg@nuxeo.com]
> Sent: Dienstag, 5. Oktober 2010 11:41
> To: chemistry-dev@incubator.apache.org
> Subject: Re: JOINs
>
> On Tue, Oct 5, 2010 at 9:06 AM, Jens Hübel <jh...@opentext.com> 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:fg@nuxeo.com]
>> Sent: Mittwoch, 29. September 2010 01:09
>> To: chemistry-dev@incubator.apache.org
>> 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 <jh...@opentext.com> 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:fg@nuxeo.com]
>>> Sent: Dienstag, 28. September 2010 19:31
>>> To: chemistry-dev@incubator.apache.org
>>> 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 <jh...@opentext.com> 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:fg@nuxeo.com]
>>>> 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
RE: JOINs
Posted by Jens Hübel <jh...@opentext.com>.
Hi Florent,
I did some more playing in this area....
> 2)
What about using Boolean instead of boolean? Then an implementation could return null instead of false indicating that the return value is not of interest in this case. Would be less confusing than "false".
And one more question:
I think we would need to switch the query clause in the walker grammar so that the new implementation is used.
CmisQueryWalker.g From:
query [QueryObject qo]
@init {
queryObj = qo;
}:
^(SELECT select_list from_clause order_by_clause? where_clause)
{
queryObj.resolveTypes();
queryObj.processWhereClause($where_clause.tree);
}
;
To:
query [QueryObject qo, PredicateWalker pw]
@init {
queryObj = qo;
predicateWalker = pw;
}:
^(SELECT select_list from_clause order_by_clause? where_clause)
{
queryObj.resolveTypes();
predicateWalker.walkPredicate($where_clause.tree);
}
;
I wonder how you have currently solved this for Nuxeo? The only way I found for a little playground project was this ugly construction:
org.apache.chemistry.opencmis.server.support.query.CmisQueryWalker.query_return qr = walker.query(queryObj);
Tree whereTree = ((Tree) qr.getTree()).getChild(2);
Tree whereChild = whereTree == null ? null : whereTree.getChild(0);
if (null != whereChild)
walkPredicate(whereChild);
I assume that you just did not change this to not break existing code yet. (it must not be this construct, but conceptually to
have a convenient mechanism to kick-off the walker). Perhaps it's also worth thinking about making this more flexible, e.g. take the walker out of QueryObject (what you already did) and have a base interface PredicateWalkerBase that only has a single method walkPredicate() and then derive custom walker interfaces from this, so that you can define your walker interface as you like it. In this way we even could use your mechanism and my original one. We could then deliver PredicateWalker as default implementation and a user could define another interface if he needs. The CmisQueryWalker.g depends only on this single method. What do you think? This would be my favorite at the moment.
Probably you also have noticed that currently only RuntimeExceptions are raised in case that something goes wrong. This is something that we still need to fix to get more useful error messages out of the parser. It's just not implemented yet, because it took me some time to figure out to do custom error handling with antlr and first attempt did not work.
Jens
-----Original Message-----
From: Florent Guillaume [mailto:fg@nuxeo.com]
Sent: Dienstag, 5. Oktober 2010 11:41
To: chemistry-dev@incubator.apache.org
Subject: Re: JOINs
On Tue, Oct 5, 2010 at 9:06 AM, Jens Hübel <jh...@opentext.com> 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:fg@nuxeo.com]
> Sent: Mittwoch, 29. September 2010 01:09
> To: chemistry-dev@incubator.apache.org
> 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 <jh...@opentext.com> 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:fg@nuxeo.com]
>> Sent: Dienstag, 28. September 2010 19:31
>> To: chemistry-dev@incubator.apache.org
>> 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 <jh...@opentext.com> 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:fg@nuxeo.com]
>>> 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
Re: JOINs
Posted by Florent Guillaume <fg...@nuxeo.com>.
On Tue, Oct 5, 2010 at 9:06 AM, Jens Hübel <jh...@opentext.com> 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:fg@nuxeo.com]
> Sent: Mittwoch, 29. September 2010 01:09
> To: chemistry-dev@incubator.apache.org
> 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 <jh...@opentext.com> 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:fg@nuxeo.com]
>> Sent: Dienstag, 28. September 2010 19:31
>> To: chemistry-dev@incubator.apache.org
>> 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 <jh...@opentext.com> 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:fg@nuxeo.com]
>>> 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