You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cayenne.apache.org by Andrus Adamchik <an...@objectstyle.org> on 2014/10/05 17:10:09 UTC

Chainable SelectQuery

In preparation to 3.2.M2, I am working on polishing our new query APIs. Good examples of the new API are a new SelectById query [1] and of course previously available SQLSelect. Now started working on SelectQuery, which is a more subtle matter as it affects every single Cayenne user. So instead of committing it right away, I created a pull request #16 [2] and now would like to hear comments before this goes to the main repo.

This changes SelectQuery and tweaks ExpressionFactory and Expression. You get the most milage out of it if you statically import ExpressionFactory. A good example is the Main file from our tutorials [3]:

// static imports
import static org.apache.cayenne.exp.ExpressionFactory.exp;
import static org.apache.cayenne.exp.ExpressionFactory.or;
...

// a single chain from query to object list
List<Painting> paintings2 = SelectQuery.query(Painting.class, qualifier2).select(context);
...

// static use of "exp" (former "Expression.fromString")
// immediate parameter binding
Expression qualifier3 = exp("artist.dateOfBirth < $date", "date", c.getTime());

// static use of 'or' seems cleaner than chaining expressions with 'exp.orExp(..)'
List<Painting> paintings4 = SelectQuery.query(Painting.class, or(qualifier2, qualifier3)).select(context);

Comments?

Andrus

[1] https://github.com/apache/cayenne/blob/master/cayenne-server/src/main/java/org/apache/cayenne/query/SelectById.java
[2] https://github.com/apache/cayenne/pull/16
[3] https://github.com/andrus/cayenne/blob/9dca05242d393aba5892b6ce33e36fc7e2078bca/tutorials/tutorial/src/main/java/org/apache/cayenne/tutorial/Main.java

Re: Chainable SelectQuery

Posted by Aristedes Maniatis <ar...@maniatis.org>.
On 17/11/2014 5:30pm, Andrus Adamchik wrote:
> What is everyone's thought on "select" -> "find" change?

I think "find" is slightly better than "select". But only slightly. Far more important is consistency and if we have a dozen other places where we still use 'select' terminology than we should keep that. SelectQuery class will probably still be called SelectQuery and not FindQuery.

Ari

-- 
-------------------------->
Aristedes Maniatis
GPG fingerprint CBFB 84B4 738D 4E87 5E5C  5EFA EF6A 7D2E 3E49 102A

Re: Chainable SelectQuery

Posted by Andrus Adamchik <an...@objectstyle.org>.
I am personally of the same opinion re: milestone API evolution.

So we have "exp" -> "where" change, which I'll make. What is everyone's thought on "select" -> "find" change?

Andrus


> On Nov 17, 2014, at 3:00 AM, Aristedes Maniatis <ar...@maniatis.org> wrote:
> 
> On 17/11/2014 1:13am, Andrus Adamchik wrote:
>> It depends. Most of the new API is very new and can be changed with no consequences. Some was already present in 3.2M1 a year ago (ObjectContext.select(..), Property). Still can change, but early adopters will be affected.
> 
> I'm a big +1 on breaking things for early adopters where the result is better naming and a better API. I think the method names chosen are very important to the usability of a library for a newcomer.
> 
> I also think we should not leave too much deprecated mess around from milestone releases. A one off change when a user goes from M1 to M2 is really not that painful. Obviously we need to be more conservative about breaking things from 3.1 release, so I'm only talking about the milestone releases here.
> 
> "where" is much clearer than "exp" and we've already discussed that there is a bit of confusion between 'qualifier', 'expression' and 'filter'.
> 
> 
> Ari
> 
> 
> -- 
> -------------------------->
> Aristedes Maniatis
> GPG fingerprint CBFB 84B4 738D 4E87 5E5C  5EFA EF6A 7D2E 3E49 102A
> 


Re: Chainable SelectQuery

Posted by Aristedes Maniatis <ar...@maniatis.org>.
On 17/11/2014 1:13am, Andrus Adamchik wrote:
> It depends. Most of the new API is very new and can be changed with no consequences. Some was already present in 3.2M1 a year ago (ObjectContext.select(..), Property). Still can change, but early adopters will be affected.

I'm a big +1 on breaking things for early adopters where the result is better naming and a better API. I think the method names chosen are very important to the usability of a library for a newcomer.

I also think we should not leave too much deprecated mess around from milestone releases. A one off change when a user goes from M1 to M2 is really not that painful. Obviously we need to be more conservative about breaking things from 3.1 release, so I'm only talking about the milestone releases here.

"where" is much clearer than "exp" and we've already discussed that there is a bit of confusion between 'qualifier', 'expression' and 'filter'.


Ari


-- 
-------------------------->
Aristedes Maniatis
GPG fingerprint CBFB 84B4 738D 4E87 5E5C  5EFA EF6A 7D2E 3E49 102A

Re: Chainable SelectQuery

Posted by Andrus Adamchik <an...@objectstyle.org>.
It is kind of hard for us to detect and prevent all possible conflicts in the *downstream* code like this. Hope this is not something that completely prevents the use of Cayenne with Scala though.

Andrus


> On Nov 17, 2014, at 7:59 PM, David Feshbach <dj...@gmail.com> wrote:
> 
> One possible reason to avoid "eq" and "ne" is that they are already methods on Scala's AnyRef. When using Cayenne in Scala, I get errors like this in my IDE:
>> org.apache.cayenne.exp.Property[String] and String are unrelated: they will most likely never compare equal
> 
> On Nov 17, 2014, at 9:09 AM, John Huss <jo...@gmail.com> wrote:
> 
>> I definitely do not want "is", especially since the the negated operation
>> has to be consistent - "eq" and "ne" works.
>> 
>> I like "where".
>> 
>> I would stick with "select" since we have SQLSelect and ObjectSelect.
>> Unless we have good alternatives names for those too.
>> 
>> 
>> On Mon, Nov 17, 2014 at 8:42 AM, Andrus Adamchik <an...@objectstyle.org>
>> wrote:
>> 
>>> Cayenne Property class is using 'eq' for the same reason.
>>> 
>>>> On Nov 17, 2014, at 5:07 PM, Mike Kienenberger <mk...@gmail.com>
>>> wrote:
>>>> 
>>>> Chainable fest testing assertions use "isEqualTo" to avoid confusion
>>>> with "equals"
>>>> 
>>>> 
>>>> On Mon, Nov 17, 2014 at 9:01 AM, Michael Gentry <mg...@masslight.net>
>>> wrote:
>>>>> On Sun, Nov 16, 2014 at 9:13 AM, Andrus Adamchik <
>>> andrus@objectstyle.org> wrote:
>>>>>>> "eq" with "is",
>>>>>> 
>>>>>> I actually prefer "eq[uals]". Considering all other operations in the
>>> Property class, "is" creates a bit of asymmetry IMO.
>>>>> 
>>>>> The problem with "equals" is it has other connotations in Java,
>>>>> otherwise I'd like it just fine.  Of course, you could argue that "is"
>>>>> also has other Java connotations (JavaBeans is* method naming
>>>>> convention).
>>>>> 
>>>>> mrg
>>>> 
>>> 
>>> 
> 
> 


Re: Chainable SelectQuery

Posted by David Feshbach <dj...@gmail.com>.
One possible reason to avoid "eq" and "ne" is that they are already methods on Scala's AnyRef. When using Cayenne in Scala, I get errors like this in my IDE:
> org.apache.cayenne.exp.Property[String] and String are unrelated: they will most likely never compare equal

On Nov 17, 2014, at 9:09 AM, John Huss <jo...@gmail.com> wrote:

> I definitely do not want "is", especially since the the negated operation
> has to be consistent - "eq" and "ne" works.
> 
> I like "where".
> 
> I would stick with "select" since we have SQLSelect and ObjectSelect.
> Unless we have good alternatives names for those too.
> 
> 
> On Mon, Nov 17, 2014 at 8:42 AM, Andrus Adamchik <an...@objectstyle.org>
> wrote:
> 
>> Cayenne Property class is using 'eq' for the same reason.
>> 
>>> On Nov 17, 2014, at 5:07 PM, Mike Kienenberger <mk...@gmail.com>
>> wrote:
>>> 
>>> Chainable fest testing assertions use "isEqualTo" to avoid confusion
>>> with "equals"
>>> 
>>> 
>>> On Mon, Nov 17, 2014 at 9:01 AM, Michael Gentry <mg...@masslight.net>
>> wrote:
>>>> On Sun, Nov 16, 2014 at 9:13 AM, Andrus Adamchik <
>> andrus@objectstyle.org> wrote:
>>>>>> "eq" with "is",
>>>>> 
>>>>> I actually prefer "eq[uals]". Considering all other operations in the
>> Property class, "is" creates a bit of asymmetry IMO.
>>>> 
>>>> The problem with "equals" is it has other connotations in Java,
>>>> otherwise I'd like it just fine.  Of course, you could argue that "is"
>>>> also has other Java connotations (JavaBeans is* method naming
>>>> convention).
>>>> 
>>>> mrg
>>> 
>> 
>> 


Re: Chainable SelectQuery

Posted by John Huss <jo...@gmail.com>.
I definitely do not want "is", especially since the the negated operation
has to be consistent - "eq" and "ne" works.

I like "where".

I would stick with "select" since we have SQLSelect and ObjectSelect.
Unless we have good alternatives names for those too.


On Mon, Nov 17, 2014 at 8:42 AM, Andrus Adamchik <an...@objectstyle.org>
wrote:

> Cayenne Property class is using 'eq' for the same reason.
>
> > On Nov 17, 2014, at 5:07 PM, Mike Kienenberger <mk...@gmail.com>
> wrote:
> >
> > Chainable fest testing assertions use "isEqualTo" to avoid confusion
> > with "equals"
> >
> >
> > On Mon, Nov 17, 2014 at 9:01 AM, Michael Gentry <mg...@masslight.net>
> wrote:
> >> On Sun, Nov 16, 2014 at 9:13 AM, Andrus Adamchik <
> andrus@objectstyle.org> wrote:
> >>>> "eq" with "is",
> >>>
> >>> I actually prefer "eq[uals]". Considering all other operations in the
> Property class, "is" creates a bit of asymmetry IMO.
> >>
> >> The problem with "equals" is it has other connotations in Java,
> >> otherwise I'd like it just fine.  Of course, you could argue that "is"
> >> also has other Java connotations (JavaBeans is* method naming
> >> convention).
> >>
> >> mrg
> >
>
>

Re: Chainable SelectQuery

Posted by Andrus Adamchik <an...@objectstyle.org>.
Cayenne Property class is using 'eq' for the same reason. 

> On Nov 17, 2014, at 5:07 PM, Mike Kienenberger <mk...@gmail.com> wrote:
> 
> Chainable fest testing assertions use "isEqualTo" to avoid confusion
> with "equals"
> 
> 
> On Mon, Nov 17, 2014 at 9:01 AM, Michael Gentry <mg...@masslight.net> wrote:
>> On Sun, Nov 16, 2014 at 9:13 AM, Andrus Adamchik <an...@objectstyle.org> wrote:
>>>> "eq" with "is",
>>> 
>>> I actually prefer "eq[uals]". Considering all other operations in the Property class, "is" creates a bit of asymmetry IMO.
>> 
>> The problem with "equals" is it has other connotations in Java,
>> otherwise I'd like it just fine.  Of course, you could argue that "is"
>> also has other Java connotations (JavaBeans is* method naming
>> convention).
>> 
>> mrg
> 


Re: Chainable SelectQuery

Posted by Mike Kienenberger <mk...@gmail.com>.
Chainable fest testing assertions use "isEqualTo" to avoid confusion
with "equals"


On Mon, Nov 17, 2014 at 9:01 AM, Michael Gentry <mg...@masslight.net> wrote:
> On Sun, Nov 16, 2014 at 9:13 AM, Andrus Adamchik <an...@objectstyle.org> wrote:
>>> "eq" with "is",
>>
>> I actually prefer "eq[uals]". Considering all other operations in the Property class, "is" creates a bit of asymmetry IMO.
>
> The problem with "equals" is it has other connotations in Java,
> otherwise I'd like it just fine.  Of course, you could argue that "is"
> also has other Java connotations (JavaBeans is* method naming
> convention).
>
> mrg

Re: Chainable SelectQuery

Posted by Michael Gentry <mg...@masslight.net>.
On Sun, Nov 16, 2014 at 9:13 AM, Andrus Adamchik <an...@objectstyle.org> wrote:
>> "eq" with "is",
>
> I actually prefer "eq[uals]". Considering all other operations in the Property class, "is" creates a bit of asymmetry IMO.

The problem with "equals" is it has other connotations in Java,
otherwise I'd like it just fine.  Of course, you could argue that "is"
also has other Java connotations (JavaBeans is* method naming
convention).

mrg

Re: Chainable SelectQuery

Posted by Andrus Adamchik <an...@objectstyle.org>.
> On Nov 16, 2014, at 4:49 PM, Michael Gentry <mg...@masslight.net> wrote:
> Perhaps I've missed the boat on this one

It depends. Most of the new API is very new and can be changed with no consequences. Some was already present in 3.2M1 a year ago (ObjectContext.select(..), Property). Still can change, but early adopters will be affected.

> Replace "exp" with "where",

I am ok with "where". We have complimentary methods in ObjectSelect: "and" and "or". Where..and...or mirrors SQL, which is probably a good thing.

> "eq" with "is",

I actually prefer "eq[uals]". Considering all other operations in the Property class, "is" creates a bit of asymmetry IMO.

> "select" with "fetch" (since it is actually fetching your records), etc.  

I am open to it, though we'll need to review the API across the board. E.g. "select" / "selectOne" is already used in ObjectContext and originated from the new "Select" interface. I think "find*" is often used in other frameworks/languages, and sounds better than "select". We may refactor to use find/findFirst/findAll.

> PS. Yesterday afternoon I was going through some of the JIRA issues
> and saw some interesting screenshots for modeler changes where
> attributes and relationships were being merged.  Quite interested in
> checking that out, too.

Yep. That's committed to master.

Andrus

Re: Chainable SelectQuery

Posted by Michael Gentry <mg...@masslight.net>.
Hi Andrus,

Perhaps I've missed the boat on this one (been too busy at work, etc),
but I think it might be more readable to not abbreviate names and
perhaps make it read more like a sentence.  Given your example,
perhaps something like:

List<Artist> artists =
  ObjectSelect.query(Artist.class)
  .where(Artist.ARTIST_NAME.is("me"))
  .orderBy(Artist.DATE_OF_BIRTH.ascending(), Artist.ARTIST_NAME.descending())
  .prefetch(Artist.PAINTING_ARRAY.joint())
  .localCache("cg2", "cg1").fetch(context);

Replace "exp" with "where", "eq" with "is", "select" with "fetch"
(since it is actually fetching your records), etc.  (Choosing good
names is quite difficult -- I'm just tossing some ideas out.)

So: Create a query for Artist where name is "me", order by date of
birth (ascending), name (descending), prefetching paintings and use
local cache.  OK, fetch them into this context.

I really need to update and try the new code.

Thanks,

mrg

PS. Yesterday afternoon I was going through some of the JIRA issues
and saw some interesting screenshots for modeler changes where
attributes and relationships were being merged.  Quite interested in
checking that out, too.


On Sat, Nov 15, 2014 at 5:39 AM, Andrus Adamchik <an...@objectstyle.org> wrote:
>
>> On Oct 11, 2014, at 4:06 AM, Andrus Adamchik <an...@objectstyle.org> wrote:
>>
>> 1. Instead of drastically redoing good old SelectQuery, we'll create a separate class called ObjectSelect featuring fluent API. No deprecations in SelectQuery.
>
>> [..]
>
>> Also cases when you need to reset/replace previous settings. So both styles have their use.
>>
>> 2. Instead of "qualifier" the new query might just use "exp" (in addition to "and" and "or").
>>
>> 3. ExpressionFactory.exp(..) and ObjectSelect.exp(..) should allow for both named parameter binding as well as vararg positional binding.
>>
>> 4. Still need to come up with fluent API for orderings and prefetches. Separation from SelectQuery should simplify this task.
>>
>> 5. The last method in a chain doesn't have to be limited to "select" and "selectOne". It can be query-specific, depending on what operations a given query supports. E.g. we can have queries with "count(context)", etc. This is something we should explore.
>>
>> 6. Corollary to #5, we should explore iterator methods in query chains (need to make sure that we don't reinvent the wheels invented in Java 8).
>>
>> 7. Non-selecting queries should use such API too. E.g. in addition to SQLSelect we might have SQLOperation with chain terminating with "int update(context)" or "int[] batchUpdate(context)" or "QueryResponse exec(context)". Same goes for ProcedureQuery.
>
> The new query is checked in. It supports all the options of SelectQuery (including more exotic ones like generic query of fetching by DbEntity name), and is fully fluent. So you can do this:
>
>   List<Artist> artists =
>         ObjectSelect.query(Artist.class)
>         .exp(Artist.ARTIST_NAME.eq("me"))
>         .orderBy(Artist.DATE_OF_BIRTH.asc(), Artist.ARTIST_NAME.desc())
>         .prefetch(Artist.PAINTING_ARRAY.joint())
>         .localCache("cg2", "cg1").select(context);
>
> or this (kinda shows why chains are cool) :
>
>   for(Artist a  : ObjectSelect.query(Artist.class).select(context)) {
>         ...
>   }
>
> or this:
>
>   Artist a = ObjectSelect.query(Artist.class, Artist.ARTIST_NAME.eq("me")).selectOne(context);
>
> Also Expression, SQLSelect and SQLTemplate now fully support positional parameters. Items 5,6,7 still need to be explored.
>
> Enjoy!
>
> Andrus
>

Re: Chainable SelectQuery

Posted by Andrus Adamchik <an...@objectstyle.org>.
> On Nov 29, 2014, at 4:09 AM, Aristedes Maniatis <ar...@maniatis.org> wrote:
> 
> On 29/11/2014 1:01am, Andrus Adamchik wrote:
>> Just added another flavor of select method - 'selectFirst'. Unlike 'selectOne' intended for queries that expect at most one row back, 'selectFirst' is a quick way to get the top-of-the-list object, even if the query matched many objects. I.e. it is equivalent to "limit(1).selectOne(context)".
> 
> * Does selectOne() throw an exception if it gets back more than one row? Is that the main difference between the functions for a user?

Correct.

> * For performance reasons, should selectOne put LIMIT 2 into the SQL since it will only discard additional objects anyway?

We may. Though since it throws in such cases, it would only be relevant if there's a programmer error (query syntax implies > 1 row) or ref integrity problem in DB (no unique constraint where it is expected), so this has never been a problem in practice.

Andrus

Re: Chainable SelectQuery

Posted by Aristedes Maniatis <ar...@maniatis.org>.
On 29/11/2014 1:01am, Andrus Adamchik wrote:
> Just added another flavor of select method - 'selectFirst'. Unlike 'selectOne' intended for queries that expect at most one row back, 'selectFirst' is a quick way to get the top-of-the-list object, even if the query matched many objects. I.e. it is equivalent to "limit(1).selectOne(context)".

* Does selectOne() throw an exception if it gets back more than one row? Is that the main difference between the functions for a user?
* For performance reasons, should selectOne put LIMIT 2 into the SQL since it will only discard additional objects anyway?


Ari



-- 
-------------------------->
Aristedes Maniatis
GPG fingerprint CBFB 84B4 738D 4E87 5E5C  5EFA EF6A 7D2E 3E49 102A

Re: Chainable SelectQuery

Posted by Andrus Adamchik <an...@objectstyle.org>.
Just added another flavor of select method - 'selectFirst'. Unlike 'selectOne' intended for queries that expect at most one row back, 'selectFirst' is a quick way to get the top-of-the-list object, even if the query matched many objects. I.e. it is equivalent to "limit(1).selectOne(context)".

Andrus

Re: Chainable SelectQuery

Posted by Andrus Adamchik <an...@objectstyle.org>.
> On Oct 11, 2014, at 4:06 AM, Andrus Adamchik <an...@objectstyle.org> wrote:
> 
> 1. Instead of drastically redoing good old SelectQuery, we'll create a separate class called ObjectSelect featuring fluent API. No deprecations in SelectQuery.

> [..]

> Also cases when you need to reset/replace previous settings. So both styles have their use. 
> 
> 2. Instead of "qualifier" the new query might just use "exp" (in addition to "and" and "or"). 
> 
> 3. ExpressionFactory.exp(..) and ObjectSelect.exp(..) should allow for both named parameter binding as well as vararg positional binding.
> 
> 4. Still need to come up with fluent API for orderings and prefetches. Separation from SelectQuery should simplify this task.
> 
> 5. The last method in a chain doesn't have to be limited to "select" and "selectOne". It can be query-specific, depending on what operations a given query supports. E.g. we can have queries with "count(context)", etc. This is something we should explore.
> 
> 6. Corollary to #5, we should explore iterator methods in query chains (need to make sure that we don't reinvent the wheels invented in Java 8).
> 
> 7. Non-selecting queries should use such API too. E.g. in addition to SQLSelect we might have SQLOperation with chain terminating with "int update(context)" or "int[] batchUpdate(context)" or "QueryResponse exec(context)". Same goes for ProcedureQuery.

The new query is checked in. It supports all the options of SelectQuery (including more exotic ones like generic query of fetching by DbEntity name), and is fully fluent. So you can do this: 

  List<Artist> artists = 
	ObjectSelect.query(Artist.class)
	.exp(Artist.ARTIST_NAME.eq("me"))
	.orderBy(Artist.DATE_OF_BIRTH.asc(), Artist.ARTIST_NAME.desc())
	.prefetch(Artist.PAINTING_ARRAY.joint())
	.localCache("cg2", "cg1").select(context);

or this (kinda shows why chains are cool) :

  for(Artist a  : ObjectSelect.query(Artist.class).select(context)) {
  	...
  }

or this:

  Artist a = ObjectSelect.query(Artist.class, Artist.ARTIST_NAME.eq("me")).selectOne(context);

Also Expression, SQLSelect and SQLTemplate now fully support positional parameters. Items 5,6,7 still need to be explored. 

Enjoy!

Andrus


Re: Chainable SelectQuery

Posted by Andrus Adamchik <an...@objectstyle.org>.
The first step in this direction is committed under CAY-1960. Now you can do this (with or without a static import) :

  import static org.apache.cayenne.exp.ExpressionFactory.*;

  and(E1.P1.eq("a"), E1.P2.ge(5), E1.P3.le(77));
  or(E1.P1.eq("a"), E1.P2.ge(5));

  // new positional binding
  exp("a = $a and b = $b", "param1", "param2");

  // equivalent name binding
  exp("a = $a and b = $b").params(new HashMap() {{
    put("a", "param1");  
    put("b", "param2"); 
  }});

Creation of expressions from Strings and binding parameters to them is now made much easier.

Andrus

On Oct 11, 2014, at 4:06 AM, Andrus Adamchik <an...@objectstyle.org> wrote:

> I think this was a very productive discussion. Based on it and some more thinking, here is my takeaway:
> 
> 1. Instead of drastically redoing good old SelectQuery, we'll create a separate class called ObjectSelect featuring fluent API. No deprecations in SelectQuery. In fact the existing API still appears to be very useful in dynamic query configuration scenarios. I encounter them rather often in framework'y code where the decisions need to be made based on preconfigured variables:
> 
> 1a. if(cachePolicy == LOCAL_CACHE) {
>       query.useLocalCache();
>    }
> 
> vs.
> 
>    query.setCachePolicy(cachePolicy);
> 
> 1b. SelectQuery<?> q;
>    if(fetchDataRows) {
>       q = SelectQuery.dataRowsQuery(type);
>    }
>    else {
>       q = SelectQuery.query(type);
>    }
> 
> vs. 
> 
>   SelectQuery q = new SelectQuery(type);
>   q.setFetchingDataRows(fetchDataRows);
> 
> Also cases when you need to reset/replace previous settings. So both styles have their use. 
> 
> 2. Instead of "qualifier" the new query might just use "exp" (in addition to "and" and "or"). 
> 
> 3. ExpressionFactory.exp(..) and ObjectSelect.exp(..) should allow for both named parameter binding as well as vararg positional binding.
> 
> 4. Still need to come up with fluent API for orderings and prefetches. Separation from SelectQuery should simplify this task.
> 
> 5. The last method in a chain doesn't have to be limited to "select" and "selectOne". It can be query-specific, depending on what operations a given query supports. E.g. we can have queries with "count(context)", etc. This is something we should explore.
> 
> 6. Corollary to #5, we should explore iterator methods in query chains (need to make sure that we don't reinvent the wheels invented in Java 8).
> 
> 7. Non-selecting queries should use such API too. E.g. in addition to SQLSelect we might have SQLOperation with chain terminating with "int update(context)" or "int[] batchUpdate(context)" or "QueryResponse exec(context)". Same goes for ProcedureQuery.
> 
> Andrus
> 
> On Oct 5, 2014, at 11:10 AM, Andrus Adamchik <an...@objectstyle.org> wrote:
> 
>> In preparation to 3.2.M2, I am working on polishing our new query APIs. Good examples of the new API are a new SelectById query [1] and of course previously available SQLSelect. Now started working on SelectQuery, which is a more subtle matter as it affects every single Cayenne user. So instead of committing it right away, I created a pull request #16 [2] and now would like to hear comments before this goes to the main repo.
>> 
>> This changes SelectQuery and tweaks ExpressionFactory and Expression. You get the most milage out of it if you statically import ExpressionFactory. A good example is the Main file from our tutorials [3]:
>> 
>> // static imports
>> import static org.apache.cayenne.exp.ExpressionFactory.exp;
>> import static org.apache.cayenne.exp.ExpressionFactory.or;
>> ...
>> 
>> // a single chain from query to object list
>> List<Painting> paintings2 = SelectQuery.query(Painting.class, qualifier2).select(context);
>> ...
>> 
>> // static use of "exp" (former "Expression.fromString")
>> // immediate parameter binding
>> Expression qualifier3 = exp("artist.dateOfBirth < $date", "date", c.getTime());
>> 
>> // static use of 'or' seems cleaner than chaining expressions with 'exp.orExp(..)'
>> List<Painting> paintings4 = SelectQuery.query(Painting.class, or(qualifier2, qualifier3)).select(context);
>> 
>> Comments?
>> 
>> Andrus
>> 
>> [1] https://github.com/apache/cayenne/blob/master/cayenne-server/src/main/java/org/apache/cayenne/query/SelectById.java
>> [2] https://github.com/apache/cayenne/pull/16
>> [3] https://github.com/andrus/cayenne/blob/9dca05242d393aba5892b6ce33e36fc7e2078bca/tutorials/tutorial/src/main/java/org/apache/cayenne/tutorial/Main.java
> 
> 


Re: Chainable SelectQuery

Posted by Andrus Adamchik <an...@objectstyle.org>.
On Oct 11, 2014, at 9:02 AM, Michael Gentry <mg...@masslight.net> wrote:

> On Fri, Oct 10, 2014 at 9:06 PM, Andrus Adamchik <an...@objectstyle.org> wrote:
>> 6. Corollary to #5, we should explore iterator methods in query chains (need to make sure that we don't reinvent the wheels invented in Java 8).
> 
> Hi Andrus,
> 
> I couldn't tell from your #6 if you were saying we should be requiring
> Java 8's lambdas or try to do something similar in a lower version of
> Java.  

More like checking that the new fluent API doesn't look unnatural under Java 8 that itself has "stream" API.

> Given that our current minimum Java requirement is 1.5, that
> would be quite a leap, at least from our user's perspective.  I know
> from my own uses that I'm still on Java 1.6 (although trying to
> upgrade to 1.7) at work and Java 1.7 at home.

3.1 minimum requirement is 1.5. 3.2 actually requires 1.6 [1], and I was going to revive our 1.7 switchover discussion. But not suggesting to switch to Java 8 at this time. We may introduce a Java 8 module soon that will feature ExtendedTypes for the new datetime package [2]. But that'll be optional, kind of like our earlier support of enums, while the base Cayenne was on Java 1.4.

Andrus


[1] http://markmail.org/message/636xe57yupcw45ya
[2] http://docs.oracle.com/javase/8/docs/technotes/guides/datetime/index.html

Re: Chainable SelectQuery

Posted by Michael Gentry <mg...@masslight.net>.
On Fri, Oct 10, 2014 at 9:06 PM, Andrus Adamchik <an...@objectstyle.org> wrote:
> 6. Corollary to #5, we should explore iterator methods in query chains (need to make sure that we don't reinvent the wheels invented in Java 8).

Hi Andrus,

I couldn't tell from your #6 if you were saying we should be requiring
Java 8's lambdas or try to do something similar in a lower version of
Java.  Given that our current minimum Java requirement is 1.5, that
would be quite a leap, at least from our user's perspective.  I know
from my own uses that I'm still on Java 1.6 (although trying to
upgrade to 1.7) at work and Java 1.7 at home.

mrg

Re: Chainable SelectQuery

Posted by Malcolm Edgar <ma...@gmail.com>.
I would strongly favor not impacting existing classes, but introduce a new
fluent API or builder patterns.

regards Malcolm

On Sat, Oct 11, 2014 at 12:06 PM, Andrus Adamchik <an...@objectstyle.org>
wrote:

> I think this was a very productive discussion. Based on it and some more
> thinking, here is my takeaway:
>
> 1. Instead of drastically redoing good old SelectQuery, we'll create a
> separate class called ObjectSelect featuring fluent API. No deprecations in
> SelectQuery. In fact the existing API still appears to be very useful in
> dynamic query configuration scenarios. I encounter them rather often in
> framework'y code where the decisions need to be made based on preconfigured
> variables:
>
> 1a. if(cachePolicy == LOCAL_CACHE) {
>        query.useLocalCache();
>     }
>
> vs.
>
>     query.setCachePolicy(cachePolicy);
>
> 1b. SelectQuery<?> q;
>     if(fetchDataRows) {
>        q = SelectQuery.dataRowsQuery(type);
>     }
>     else {
>        q = SelectQuery.query(type);
>     }
>
> vs.
>
>    SelectQuery q = new SelectQuery(type);
>    q.setFetchingDataRows(fetchDataRows);
>
> Also cases when you need to reset/replace previous settings. So both
> styles have their use.
>
> 2. Instead of "qualifier" the new query might just use "exp" (in addition
> to "and" and "or").
>
> 3. ExpressionFactory.exp(..) and ObjectSelect.exp(..) should allow for
> both named parameter binding as well as vararg positional binding.
>
> 4. Still need to come up with fluent API for orderings and prefetches.
> Separation from SelectQuery should simplify this task.
>
> 5. The last method in a chain doesn't have to be limited to "select" and
> "selectOne". It can be query-specific, depending on what operations a given
> query supports. E.g. we can have queries with "count(context)", etc. This
> is something we should explore.
>
> 6. Corollary to #5, we should explore iterator methods in query chains
> (need to make sure that we don't reinvent the wheels invented in Java 8).
>
> 7. Non-selecting queries should use such API too. E.g. in addition to
> SQLSelect we might have SQLOperation with chain terminating with "int
> update(context)" or "int[] batchUpdate(context)" or "QueryResponse
> exec(context)". Same goes for ProcedureQuery.
>
> Andrus
>
> On Oct 5, 2014, at 11:10 AM, Andrus Adamchik <an...@objectstyle.org>
> wrote:
>
> > In preparation to 3.2.M2, I am working on polishing our new query APIs.
> Good examples of the new API are a new SelectById query [1] and of course
> previously available SQLSelect. Now started working on SelectQuery, which
> is a more subtle matter as it affects every single Cayenne user. So instead
> of committing it right away, I created a pull request #16 [2] and now would
> like to hear comments before this goes to the main repo.
> >
> > This changes SelectQuery and tweaks ExpressionFactory and Expression.
> You get the most milage out of it if you statically import
> ExpressionFactory. A good example is the Main file from our tutorials [3]:
> >
> > // static imports
> > import static org.apache.cayenne.exp.ExpressionFactory.exp;
> > import static org.apache.cayenne.exp.ExpressionFactory.or;
> > ...
> >
> > // a single chain from query to object list
> > List<Painting> paintings2 = SelectQuery.query(Painting.class,
> qualifier2).select(context);
> > ...
> >
> > // static use of "exp" (former "Expression.fromString")
> > // immediate parameter binding
> > Expression qualifier3 = exp("artist.dateOfBirth < $date", "date",
> c.getTime());
> >
> > // static use of 'or' seems cleaner than chaining expressions with
> 'exp.orExp(..)'
> > List<Painting> paintings4 = SelectQuery.query(Painting.class,
> or(qualifier2, qualifier3)).select(context);
> >
> > Comments?
> >
> > Andrus
> >
> > [1]
> https://github.com/apache/cayenne/blob/master/cayenne-server/src/main/java/org/apache/cayenne/query/SelectById.java
> > [2] https://github.com/apache/cayenne/pull/16
> > [3]
> https://github.com/andrus/cayenne/blob/9dca05242d393aba5892b6ce33e36fc7e2078bca/tutorials/tutorial/src/main/java/org/apache/cayenne/tutorial/Main.java
>
>

Re: Chainable SelectQuery

Posted by Andrus Adamchik <an...@objectstyle.org>.
I think this was a very productive discussion. Based on it and some more thinking, here is my takeaway:

1. Instead of drastically redoing good old SelectQuery, we'll create a separate class called ObjectSelect featuring fluent API. No deprecations in SelectQuery. In fact the existing API still appears to be very useful in dynamic query configuration scenarios. I encounter them rather often in framework'y code where the decisions need to be made based on preconfigured variables:

1a. if(cachePolicy == LOCAL_CACHE) {
       query.useLocalCache();
    }

vs.

    query.setCachePolicy(cachePolicy);

1b. SelectQuery<?> q;
    if(fetchDataRows) {
       q = SelectQuery.dataRowsQuery(type);
    }
    else {
       q = SelectQuery.query(type);
    }

vs. 

   SelectQuery q = new SelectQuery(type);
   q.setFetchingDataRows(fetchDataRows);

Also cases when you need to reset/replace previous settings. So both styles have their use. 

2. Instead of "qualifier" the new query might just use "exp" (in addition to "and" and "or"). 

3. ExpressionFactory.exp(..) and ObjectSelect.exp(..) should allow for both named parameter binding as well as vararg positional binding.

4. Still need to come up with fluent API for orderings and prefetches. Separation from SelectQuery should simplify this task.

5. The last method in a chain doesn't have to be limited to "select" and "selectOne". It can be query-specific, depending on what operations a given query supports. E.g. we can have queries with "count(context)", etc. This is something we should explore.

6. Corollary to #5, we should explore iterator methods in query chains (need to make sure that we don't reinvent the wheels invented in Java 8).

7. Non-selecting queries should use such API too. E.g. in addition to SQLSelect we might have SQLOperation with chain terminating with "int update(context)" or "int[] batchUpdate(context)" or "QueryResponse exec(context)". Same goes for ProcedureQuery.

Andrus

On Oct 5, 2014, at 11:10 AM, Andrus Adamchik <an...@objectstyle.org> wrote:

> In preparation to 3.2.M2, I am working on polishing our new query APIs. Good examples of the new API are a new SelectById query [1] and of course previously available SQLSelect. Now started working on SelectQuery, which is a more subtle matter as it affects every single Cayenne user. So instead of committing it right away, I created a pull request #16 [2] and now would like to hear comments before this goes to the main repo.
> 
> This changes SelectQuery and tweaks ExpressionFactory and Expression. You get the most milage out of it if you statically import ExpressionFactory. A good example is the Main file from our tutorials [3]:
> 
> // static imports
> import static org.apache.cayenne.exp.ExpressionFactory.exp;
> import static org.apache.cayenne.exp.ExpressionFactory.or;
> ...
> 
> // a single chain from query to object list
> List<Painting> paintings2 = SelectQuery.query(Painting.class, qualifier2).select(context);
> ...
> 
> // static use of "exp" (former "Expression.fromString")
> // immediate parameter binding
> Expression qualifier3 = exp("artist.dateOfBirth < $date", "date", c.getTime());
> 
> // static use of 'or' seems cleaner than chaining expressions with 'exp.orExp(..)'
> List<Painting> paintings4 = SelectQuery.query(Painting.class, or(qualifier2, qualifier3)).select(context);
> 
> Comments?
> 
> Andrus
> 
> [1] https://github.com/apache/cayenne/blob/master/cayenne-server/src/main/java/org/apache/cayenne/query/SelectById.java
> [2] https://github.com/apache/cayenne/pull/16
> [3] https://github.com/andrus/cayenne/blob/9dca05242d393aba5892b6ce33e36fc7e2078bca/tutorials/tutorial/src/main/java/org/apache/cayenne/tutorial/Main.java


Re: Chainable SelectQuery

Posted by Andrus Adamchik <an...@objectstyle.org>.
On Oct 7, 2014, at 7:41 PM, Aristedes Maniatis <ar...@maniatis.org> wrote:

>>> 
>>> Can't we parse without any ambiguity:
>>> 
>>>   exp("artist.dateOfBirth < $date", c.getTime());
>> 
>> In case of a single parameter you are right. Good point! 
> 
> In the case of multiple parameters could we just as easily pass parameters in order for a cleaner syntax?
> 
>    exp("artist.dateOfBirth < $date and artist.name = $name", c.getTime(), "adam");
> 
> There are lots of APIs that follow this approach right back to the C printf function.

Hmm, we need to somehow avoid confusing named parameters with positional parameters. EJBQL (JPA-inspired) has separate notation for named and positional (not that it is particularly smart .. to me their positional is really named , just that the name is a number .. am I wrong?). But I guess in this context we can treat named as positional, ignoring the name (so e.g. if the same var name repeats twice, we'd require 2 separate args for the binding). So you are right again :)

Andrus


Re: Chainable SelectQuery

Posted by Andrus Adamchik <an...@objectstyle.org>.
On Oct 7, 2014, at 7:41 PM, Aristedes Maniatis <ar...@maniatis.org> wrote:

>> Yes both are valid. The point of the new one is that deeply nested expressions can not be easily built within a context of query, so you'd build them separately and then pass the final object to the query. So query.or/and(..) is sort of a final step in the qualifier assembly. ExpressionFactory.or/and(..) is to build the clauses you pass in there.
> 
> 
> Fair enough. Then we'll keep both approaches as valid and possible?

Yes.

> I'd suggest the second syntax is the one we'd want to push as the introductory approach in the tutorials however... it is simple and easier to understand without the static imports.

When the expression is at most 2 levels deep, I agree. Since all of our tutorials are like that, we'll use query.and/or.

> Personally I don't see the pattern with static imports as something I'd use, but perhaps other people would.

Between Property API (as John has mentioned) and query.and/or, ExpressionFactory itself becomes somewhat irrelevant to the end user. Once we commit that pull request and I have a chance to switch my big projects to it, I am curious myself of how many places would actually make use ExpressionFactory. "db:" expressions are still a use case. 

Also - will need to change query.and/or into a vararg. Overlooked it on the first path.

Andrus


Re: Chainable SelectQuery

Posted by Aristedes Maniatis <ar...@maniatis.org>.
On 7/10/2014 10:37pm, Andrus Adamchik wrote:

>> Can't we parse without any ambiguity:
>>
>>    exp("artist.dateOfBirth < $date", c.getTime());
> 
> In case of a single parameter you are right. Good point! 

In the case of multiple parameters could we just as easily pass parameters in order for a cleaner syntax?

    exp("artist.dateOfBirth < $date and artist.name = $name", c.getTime(), "adam");

There are lots of APIs that follow this approach right back to the C printf function.


>> // static use of 'or' seems cleaner than chaining expressions with 'exp.orExp(..)'
>>> List<Painting> paintings4 = SelectQuery.query(Painting.class, or(qualifier2, qualifier3)).select(context);
>>
>> Let's see how this plays out with longer queries:
>>
>>    List<Painting> paintings4 = SelectQuery.query(Painting.class, and(or(qualifier2, qualifier3), qualifier4)).select(context);
>>
>> That looks a bit like reverse Polish notation. To my mind, this approach is clearer:
>>
>>    List<Painting> paintings4 = new SelectQuery(Painting.class).or(qualifier1).or(qualifier3).and(qualifier4).select(context);
>>
>>
>> Am I misunderstanding the different approaches to this? It appears both are valid with the patch you are proposing. Also, 
> 
> Yes both are valid. The point of the new one is that deeply nested expressions can not be easily built within a context of query, so you'd build them separately and then pass the final object to the query. So query.or/and(..) is sort of a final step in the qualifier assembly. ExpressionFactory.or/and(..) is to build the clauses you pass in there.


Fair enough. Then we'll keep both approaches as valid and possible? I'd suggest the second syntax is the one we'd want to push as the introductory approach in the tutorials however... it is simple and easier to understand without the static imports.

Personally I don't see the pattern with static imports as something I'd use, but perhaps other people would.


>> I think the use of the words "qualifier" and "expression" are confusing. We use them interchangeably without much reason. Can we pick one and stick to it?
> 
> I thought about it it too. The original reason for 2 terms was that qualifier is always an expression, while expression is not always a qualifier (e.g. path expressions used in orderings). Not sure how to best reconcile it? To add to the confusion I'd even use "filter" instead of qualifier :)

Certainly 'filter' is a better word that is clearer to the users. But perhaps we are too far down the rabbit hole now with too many classes called Expression and too many methods already using that word. How about we keep "Expression" as the thing which you attach to a query and use a different term in our docs for a "path" to attach to orderings.

In your patch, the Javadocs use qualifier and expression completely interchangeably which is confusing. Perhaps the word qualifier should go away completely.


Ari




-- 
-------------------------->
Aristedes Maniatis
GPG fingerprint CBFB 84B4 738D 4E87 5E5C  5EFA EF6A 7D2E 3E49 102A

Re: Chainable SelectQuery

Posted by Andrus Adamchik <an...@objectstyle.org>.
On Oct 6, 2014, at 8:00 AM, Aristedes Maniatis <ar...@maniatis.org> wrote:

> On 6/10/2014 2:10am, Andrus Adamchik wrote:
>> // a single chain from query to object list
>> List<Painting> paintings2 = SelectQuery.query(Painting.class, qualifier2).select(context);
> 
> Since we already have a constructor with the appropriate arguments, why not go for this as the recommended approach:
> 
>    List<Painting> paintings2 = new SelectQuery(Painting.class, qualifier2).select(context);
> 
> I'm not really sure it makes a difference either way, but just throwing it out there. They will both work, but the constructor doesn't have generics yet.

Yeah, generics is the key here. Static factory methods allow us to distinguish between entity and DataRow queries. E.g.:

SelectQuery<DataRow> q = SelectQuery.dataRowQuery(Artist.class);

So I am sorta emphasizing those instead of constructors.

> // static use of "exp" (former "Expression.fromString")
>> // immediate parameter binding
>> Expression qualifier3 = exp("artist.dateOfBirth < $date", "date", c.getTime());
> 
> Can't we parse without any ambiguity:
> 
>    exp("artist.dateOfBirth < $date", c.getTime());

In case of a single parameter you are right. Good point! 



> // static use of 'or' seems cleaner than chaining expressions with 'exp.orExp(..)'
>> List<Painting> paintings4 = SelectQuery.query(Painting.class, or(qualifier2, qualifier3)).select(context);
> 
> Let's see how this plays out with longer queries:
> 
>    List<Painting> paintings4 = SelectQuery.query(Painting.class, and(or(qualifier2, qualifier3), qualifier4)).select(context);
> 
> That looks a bit like reverse Polish notation. To my mind, this approach is clearer:
> 
>    List<Painting> paintings4 = new SelectQuery(Painting.class).or(qualifier1).or(qualifier3).and(qualifier4).select(context);
> 
> 
> Am I misunderstanding the different approaches to this? It appears both are valid with the patch you are proposing. Also, 

Yes both are valid. The point of the new one is that deeply nested expressions can not be easily built within a context of query, so you'd build them separately and then pass the final object to the query. So query.or/and(..) is sort of a final step in the qualifier assembly. ExpressionFactory.or/and(..) is to build the clauses you pass in there.

> I think the use of the words "qualifier" and "expression" are confusing. We use them interchangeably without much reason. Can we pick one and stick to it?

I thought about it it too. The original reason for 2 terms was that qualifier is always an expression, while expression is not always a qualifier (e.g. path expressions used in orderings). Not sure how to best reconcile it? To add to the confusion I'd even use "filter" instead of qualifier :)

Andrus 




Re: Chainable SelectQuery

Posted by Aristedes Maniatis <ar...@maniatis.org>.
On 6/10/2014 2:10am, Andrus Adamchik wrote:
> // a single chain from query to object list
> List<Painting> paintings2 = SelectQuery.query(Painting.class, qualifier2).select(context);

Since we already have a constructor with the appropriate arguments, why not go for this as the recommended approach:

    List<Painting> paintings2 = new SelectQuery(Painting.class, qualifier2).select(context);

I'm not really sure it makes a difference either way, but just throwing it out there. They will both work, but the constructor doesn't have generics yet.


> // static use of "exp" (former "Expression.fromString")
> // immediate parameter binding
> Expression qualifier3 = exp("artist.dateOfBirth < $date", "date", c.getTime());

Can't we parse without any ambiguity:

    exp("artist.dateOfBirth < $date", c.getTime());

 
> // static use of 'or' seems cleaner than chaining expressions with 'exp.orExp(..)'
> List<Painting> paintings4 = SelectQuery.query(Painting.class, or(qualifier2, qualifier3)).select(context);

Let's see how this plays out with longer queries:

    List<Painting> paintings4 = SelectQuery.query(Painting.class, and(or(qualifier2, qualifier3), qualifier4)).select(context);

That looks a bit like reverse Polish notation. To my mind, this approach is clearer:

    List<Painting> paintings4 = new SelectQuery(Painting.class).or(qualifier1).or(qualifier3).and(qualifier4).select(context);


Am I misunderstanding the different approaches to this? It appears both are valid with the patch you are proposing. Also, I think the use of the words "qualifier" and "expression" are confusing. We use them interchangeably without much reason. Can we pick one and stick to it?


Ari



-- 
-------------------------->
Aristedes Maniatis
GPG fingerprint CBFB 84B4 738D 4E87 5E5C  5EFA EF6A 7D2E 3E49 102A

Re: Chainable SelectQuery

Posted by Robert Zeigler <ro...@roxanemy.com>.
On Oct 7, 2014, at 10/79:31 AM , John Huss <jo...@gmail.com> wrote:

> Hmm... I have a lot to say, so I might ramble a bit.
> 
> 1) Static imports
> 
> I'll never been a big user of static imports, largely because the tooling
> (Eclipse) doesn't support them well.  Everytime I've used a static import
> (which is really only in JUnit) I've wanted to import all the members:
> import package.*;  Well, when you organize import Eclipse will replace the
> star with the specific items you've used.  So you end up have to fix the
> import over and over.  On the other side, Eclipse won't automatically add
> the static import or even recommend it based on your code, so it becomes a
> rarely used feature.


As an aside, there is* as option in the preference pane under Java->Code Style->Organize Imports (it’s a tab) called “Number of static imports needed for .*”. If you change that to 1, this behavior will be fixed for you. 

*At least there used to be. I haven’t used Eclipse in a couple of years

Robert

Re: Chainable SelectQuery

Posted by John Huss <jo...@gmail.com>.
I could go along with calling this 4.0

On Thu, Oct 9, 2014 at 5:15 PM, Michael Gentry <mg...@masslight.net>
wrote:

> I think you are announcing Cayenne 4.0 ... :-)
>
>
> On Thu, Oct 9, 2014 at 5:37 PM, Andrus Adamchik <an...@objectstyle.org>
> wrote:
> > Based on this discussion I am leaning towards leaving SelectQuery alone
> (perhaps with changes already there, but no fluent builder methods and no
> deprecation), as the impact on everybody will be very big. And creating an
> alternative fluent ObjectQuery that will feature all the new stuff.
> >
> > Andrus
> >
> >
>

Re: Chainable SelectQuery

Posted by Michael Gentry <mg...@masslight.net>.
I think you are announcing Cayenne 4.0 ... :-)


On Thu, Oct 9, 2014 at 5:37 PM, Andrus Adamchik <an...@objectstyle.org> wrote:
> Based on this discussion I am leaning towards leaving SelectQuery alone (perhaps with changes already there, but no fluent builder methods and no deprecation), as the impact on everybody will be very big. And creating an alternative fluent ObjectQuery that will feature all the new stuff.
>
> Andrus
>
>

Re: Chainable SelectQuery

Posted by Andrus Adamchik <an...@objectstyle.org>.
Based on this discussion I am leaning towards leaving SelectQuery alone (perhaps with changes already there, but no fluent builder methods and no deprecation), as the impact on everybody will be very big. And creating an alternative fluent ObjectQuery that will feature all the new stuff.

Andrus



Re: Chainable SelectQuery

Posted by Mike Kienenberger <mk...@gmail.com>.
I wonder if QueryBuilder is a better approach which would preserve
backwards compatibility.

On Thu, Oct 9, 2014 at 3:08 PM, Michael Gentry <mg...@masslight.net> wrote:
> Yeah, I don't know if changing setXyz to *not* return 'void' would
> break things (like Tapestry).  That would be a huge downer.
>
> On Thu, Oct 9, 2014 at 3:03 PM, Andrus Adamchik <an...@objectstyle.org> wrote:
>> I thought of that. Unfortunately "setXyz" is a 'java bean' setter, so we'll be redefining one of the most established Java conventions.
>>
>> Andrus
>>
>> On Oct 9, 2014, at 2:51 PM, Michael Gentry <mg...@masslight.net> wrote:
>>
>>> I don't know if it will be confusing or not.  At least with the
>>> current method names, I can type query.set[ctrl+space] and let Eclipse
>>> give me a list of set* method names to choose from (same applies for
>>> get*), but that option won't be available with the new chainable API.
>>> Part of me thinks just make setFetchLimit() return 'this' (at least
>>> where it makes sense to preserve existing API):
>>>
>>> query.setFetchLimit(100).setDistinct(true);
>>>
>>> You could continue that in the model objects, too:
>>>
>>> person.setFirstName("John").setLastName("Doe");
>>>
>>> mrg
>>>
>>>
>>> On Thu, Oct 9, 2014 at 1:31 PM, Andrus Adamchik <an...@objectstyle.org> wrote:
>>>>
>>>> On Oct 7, 2014, at 9:30 PM, Michael Gentry <mg...@masslight.net> wrote:
>>>>
>>>>> On Tue, Oct 7, 2014 at 6:44 PM, Aristedes Maniatis <ar...@maniatis.org> wrote:
>>>>>>> That said, if returning this is the direction we're going, then really all
>>>>>>> the currently void methods in SelectQuery should do the same thing - like
>>>>>>> addOrdering for example.
>>>>>>
>>>>>>
>>>>>> Correct, but we also need to gracefully deprecate the old methods and make new fluent ones.
>>>>>
>>>>> I was looking at the changes and was wondering if we really need to
>>>>> deprecate the old methods.  Can't setFetchLimit() live along with
>>>>> limit()?
>>>>
>>>> I suspect having both will be rather confusing. Initially I even wanted to leave SelectQuery alone and implement all the new ideas in a separate query class (ObjectQuery?), but since we've already made a bunch of changes to SelectQuery in the same direction, I ended up with a massive change to the existing class.
>>>>
>>>> Andrus
>>>>
>>>
>>

Re: Chainable SelectQuery

Posted by Michael Gentry <mg...@masslight.net>.
I access my *data* objects all the time in Tapestry templates.  Things
such as firstName and lastName.  The point I was trying to make is if
setFirstName started returning the 'User' instance instead of 'void',
it could possibly break some libraries out there (such as whatever
Tapestry uses to process t:value="person.firstName"), which would be
bad.  Even if we tested Tapestry's accessors, there is no guarantee it
wouldn't break some other framework.

I never access my query objects directly in the template; I was mainly
talking about data objects and do we extend chainability to them.

Thanks,

mrg


On Thu, Oct 9, 2014 at 9:32 PM, Robert Zeigler
<ro...@roxanemy.com> wrote:
> It would only break tapestry of you were accessing properties of your query object from your template or via property expressions in bindings. So if your component had a limit property that was bound to the query's limit, then that might break.
>
> GATAATGCTATTTCTTTAATTTTCGAA
>
>> On Oct 9, 2014, at 2:08 PM, Michael Gentry <mg...@masslight.net> wrote:
>>
>> Yeah, I don't know if changing setXyz to *not* return 'void' would
>> break things (like Tapestry).  That would be a huge downer.
>>
>>> On Thu, Oct 9, 2014 at 3:03 PM, Andrus Adamchik <an...@objectstyle.org> wrote:
>>> I thought of that. Unfortunately "setXyz" is a 'java bean' setter, so we'll be redefining one of the most established Java conventions.
>>>
>>> Andrus
>>>
>>>> On Oct 9, 2014, at 2:51 PM, Michael Gentry <mg...@masslight.net> wrote:
>>>>
>>>> I don't know if it will be confusing or not.  At least with the
>>>> current method names, I can type query.set[ctrl+space] and let Eclipse
>>>> give me a list of set* method names to choose from (same applies for
>>>> get*), but that option won't be available with the new chainable API.
>>>> Part of me thinks just make setFetchLimit() return 'this' (at least
>>>> where it makes sense to preserve existing API):
>>>>
>>>> query.setFetchLimit(100).setDistinct(true);
>>>>
>>>> You could continue that in the model objects, too:
>>>>
>>>> person.setFirstName("John").setLastName("Doe");
>>>>
>>>> mrg
>>>>
>>>>
>>>>> On Thu, Oct 9, 2014 at 1:31 PM, Andrus Adamchik <an...@objectstyle.org> wrote:
>>>>>
>>>>>> On Oct 7, 2014, at 9:30 PM, Michael Gentry <mg...@masslight.net> wrote:
>>>>>>
>>>>>> On Tue, Oct 7, 2014 at 6:44 PM, Aristedes Maniatis <ar...@maniatis.org> wrote:
>>>>>>>> That said, if returning this is the direction we're going, then really all
>>>>>>>> the currently void methods in SelectQuery should do the same thing - like
>>>>>>>> addOrdering for example.
>>>>>>>
>>>>>>>
>>>>>>> Correct, but we also need to gracefully deprecate the old methods and make new fluent ones.
>>>>>>
>>>>>> I was looking at the changes and was wondering if we really need to
>>>>>> deprecate the old methods.  Can't setFetchLimit() live along with
>>>>>> limit()?
>>>>>
>>>>> I suspect having both will be rather confusing. Initially I even wanted to leave SelectQuery alone and implement all the new ideas in a separate query class (ObjectQuery?), but since we've already made a bunch of changes to SelectQuery in the same direction, I ended up with a massive change to the existing class.
>>>>>
>>>>> Andrus
>>>

Re: Chainable SelectQuery

Posted by Robert Zeigler <ro...@roxanemy.com>.
It would only break tapestry of you were accessing properties of your query object from your template or via property expressions in bindings. So if your component had a limit property that was bound to the query's limit, then that might break. 

GATAATGCTATTTCTTTAATTTTCGAA

> On Oct 9, 2014, at 2:08 PM, Michael Gentry <mg...@masslight.net> wrote:
> 
> Yeah, I don't know if changing setXyz to *not* return 'void' would
> break things (like Tapestry).  That would be a huge downer.
> 
>> On Thu, Oct 9, 2014 at 3:03 PM, Andrus Adamchik <an...@objectstyle.org> wrote:
>> I thought of that. Unfortunately "setXyz" is a 'java bean' setter, so we'll be redefining one of the most established Java conventions.
>> 
>> Andrus
>> 
>>> On Oct 9, 2014, at 2:51 PM, Michael Gentry <mg...@masslight.net> wrote:
>>> 
>>> I don't know if it will be confusing or not.  At least with the
>>> current method names, I can type query.set[ctrl+space] and let Eclipse
>>> give me a list of set* method names to choose from (same applies for
>>> get*), but that option won't be available with the new chainable API.
>>> Part of me thinks just make setFetchLimit() return 'this' (at least
>>> where it makes sense to preserve existing API):
>>> 
>>> query.setFetchLimit(100).setDistinct(true);
>>> 
>>> You could continue that in the model objects, too:
>>> 
>>> person.setFirstName("John").setLastName("Doe");
>>> 
>>> mrg
>>> 
>>> 
>>>> On Thu, Oct 9, 2014 at 1:31 PM, Andrus Adamchik <an...@objectstyle.org> wrote:
>>>> 
>>>>> On Oct 7, 2014, at 9:30 PM, Michael Gentry <mg...@masslight.net> wrote:
>>>>> 
>>>>> On Tue, Oct 7, 2014 at 6:44 PM, Aristedes Maniatis <ar...@maniatis.org> wrote:
>>>>>>> That said, if returning this is the direction we're going, then really all
>>>>>>> the currently void methods in SelectQuery should do the same thing - like
>>>>>>> addOrdering for example.
>>>>>> 
>>>>>> 
>>>>>> Correct, but we also need to gracefully deprecate the old methods and make new fluent ones.
>>>>> 
>>>>> I was looking at the changes and was wondering if we really need to
>>>>> deprecate the old methods.  Can't setFetchLimit() live along with
>>>>> limit()?
>>>> 
>>>> I suspect having both will be rather confusing. Initially I even wanted to leave SelectQuery alone and implement all the new ideas in a separate query class (ObjectQuery?), but since we've already made a bunch of changes to SelectQuery in the same direction, I ended up with a massive change to the existing class.
>>>> 
>>>> Andrus
>> 

Re: Chainable SelectQuery

Posted by Andrus Adamchik <an...@objectstyle.org>.
> Yeah, I don't know if changing setXyz to *not* return 'void' would
> break things (like Tapestry).  That would be a huge downer.

Just saw something cool at WOWODC. Instead of changing setXyz to return "this", people would generate a Builder class for each Java class that has fluent methods for every query:

Entity e = Entity.Builder
   .attribute1("a")
   .attribute2("b")
   .rel1(someObject)
   .rel2(someOtherOBject).
   .build(context);

What may not be immediately obvious is that this also forces setting all mandatory relationships ("build" method is only available on an object returned from the last mandatory relationship method). I am not sure I personally have a strong need for this API, but if you initialize your object on creation with lots of attributes, this looks pretty neat actually.

Andrus


> On Oct 9, 2014, at 9:08 PM, Michael Gentry <mg...@masslight.net> wrote:
> 
> Yeah, I don't know if changing setXyz to *not* return 'void' would
> break things (like Tapestry).  That would be a huge downer.
> 
> On Thu, Oct 9, 2014 at 3:03 PM, Andrus Adamchik <an...@objectstyle.org> wrote:
>> I thought of that. Unfortunately "setXyz" is a 'java bean' setter, so we'll be redefining one of the most established Java conventions.
>> 
>> Andrus
>> 
>> On Oct 9, 2014, at 2:51 PM, Michael Gentry <mg...@masslight.net> wrote:
>> 
>>> I don't know if it will be confusing or not.  At least with the
>>> current method names, I can type query.set[ctrl+space] and let Eclipse
>>> give me a list of set* method names to choose from (same applies for
>>> get*), but that option won't be available with the new chainable API.
>>> Part of me thinks just make setFetchLimit() return 'this' (at least
>>> where it makes sense to preserve existing API):
>>> 
>>> query.setFetchLimit(100).setDistinct(true);
>>> 
>>> You could continue that in the model objects, too:
>>> 
>>> person.setFirstName("John").setLastName("Doe");
>>> 
>>> mrg
>>> 
>>> 
>>> On Thu, Oct 9, 2014 at 1:31 PM, Andrus Adamchik <an...@objectstyle.org> wrote:
>>>> 
>>>> On Oct 7, 2014, at 9:30 PM, Michael Gentry <mg...@masslight.net> wrote:
>>>> 
>>>>> On Tue, Oct 7, 2014 at 6:44 PM, Aristedes Maniatis <ar...@maniatis.org> wrote:
>>>>>>> That said, if returning this is the direction we're going, then really all
>>>>>>> the currently void methods in SelectQuery should do the same thing - like
>>>>>>> addOrdering for example.
>>>>>> 
>>>>>> 
>>>>>> Correct, but we also need to gracefully deprecate the old methods and make new fluent ones.
>>>>> 
>>>>> I was looking at the changes and was wondering if we really need to
>>>>> deprecate the old methods.  Can't setFetchLimit() live along with
>>>>> limit()?
>>>> 
>>>> I suspect having both will be rather confusing. Initially I even wanted to leave SelectQuery alone and implement all the new ideas in a separate query class (ObjectQuery?), but since we've already made a bunch of changes to SelectQuery in the same direction, I ended up with a massive change to the existing class.
>>>> 
>>>> Andrus
>>>> 
>>> 
>> 
> 


Re: Chainable SelectQuery

Posted by Michael Gentry <mg...@masslight.net>.
Yeah, I don't know if changing setXyz to *not* return 'void' would
break things (like Tapestry).  That would be a huge downer.

On Thu, Oct 9, 2014 at 3:03 PM, Andrus Adamchik <an...@objectstyle.org> wrote:
> I thought of that. Unfortunately "setXyz" is a 'java bean' setter, so we'll be redefining one of the most established Java conventions.
>
> Andrus
>
> On Oct 9, 2014, at 2:51 PM, Michael Gentry <mg...@masslight.net> wrote:
>
>> I don't know if it will be confusing or not.  At least with the
>> current method names, I can type query.set[ctrl+space] and let Eclipse
>> give me a list of set* method names to choose from (same applies for
>> get*), but that option won't be available with the new chainable API.
>> Part of me thinks just make setFetchLimit() return 'this' (at least
>> where it makes sense to preserve existing API):
>>
>> query.setFetchLimit(100).setDistinct(true);
>>
>> You could continue that in the model objects, too:
>>
>> person.setFirstName("John").setLastName("Doe");
>>
>> mrg
>>
>>
>> On Thu, Oct 9, 2014 at 1:31 PM, Andrus Adamchik <an...@objectstyle.org> wrote:
>>>
>>> On Oct 7, 2014, at 9:30 PM, Michael Gentry <mg...@masslight.net> wrote:
>>>
>>>> On Tue, Oct 7, 2014 at 6:44 PM, Aristedes Maniatis <ar...@maniatis.org> wrote:
>>>>>> That said, if returning this is the direction we're going, then really all
>>>>>> the currently void methods in SelectQuery should do the same thing - like
>>>>>> addOrdering for example.
>>>>>
>>>>>
>>>>> Correct, but we also need to gracefully deprecate the old methods and make new fluent ones.
>>>>
>>>> I was looking at the changes and was wondering if we really need to
>>>> deprecate the old methods.  Can't setFetchLimit() live along with
>>>> limit()?
>>>
>>> I suspect having both will be rather confusing. Initially I even wanted to leave SelectQuery alone and implement all the new ideas in a separate query class (ObjectQuery?), but since we've already made a bunch of changes to SelectQuery in the same direction, I ended up with a massive change to the existing class.
>>>
>>> Andrus
>>>
>>
>

Re: Chainable SelectQuery

Posted by Andrus Adamchik <an...@objectstyle.org>.
I thought of that. Unfortunately "setXyz" is a 'java bean' setter, so we'll be redefining one of the most established Java conventions.

Andrus

On Oct 9, 2014, at 2:51 PM, Michael Gentry <mg...@masslight.net> wrote:

> I don't know if it will be confusing or not.  At least with the
> current method names, I can type query.set[ctrl+space] and let Eclipse
> give me a list of set* method names to choose from (same applies for
> get*), but that option won't be available with the new chainable API.
> Part of me thinks just make setFetchLimit() return 'this' (at least
> where it makes sense to preserve existing API):
> 
> query.setFetchLimit(100).setDistinct(true);
> 
> You could continue that in the model objects, too:
> 
> person.setFirstName("John").setLastName("Doe");
> 
> mrg
> 
> 
> On Thu, Oct 9, 2014 at 1:31 PM, Andrus Adamchik <an...@objectstyle.org> wrote:
>> 
>> On Oct 7, 2014, at 9:30 PM, Michael Gentry <mg...@masslight.net> wrote:
>> 
>>> On Tue, Oct 7, 2014 at 6:44 PM, Aristedes Maniatis <ar...@maniatis.org> wrote:
>>>>> That said, if returning this is the direction we're going, then really all
>>>>> the currently void methods in SelectQuery should do the same thing - like
>>>>> addOrdering for example.
>>>> 
>>>> 
>>>> Correct, but we also need to gracefully deprecate the old methods and make new fluent ones.
>>> 
>>> I was looking at the changes and was wondering if we really need to
>>> deprecate the old methods.  Can't setFetchLimit() live along with
>>> limit()?
>> 
>> I suspect having both will be rather confusing. Initially I even wanted to leave SelectQuery alone and implement all the new ideas in a separate query class (ObjectQuery?), but since we've already made a bunch of changes to SelectQuery in the same direction, I ended up with a massive change to the existing class.
>> 
>> Andrus
>> 
> 


Re: Chainable SelectQuery

Posted by Michael Gentry <mg...@masslight.net>.
I don't know if it will be confusing or not.  At least with the
current method names, I can type query.set[ctrl+space] and let Eclipse
give me a list of set* method names to choose from (same applies for
get*), but that option won't be available with the new chainable API.
Part of me thinks just make setFetchLimit() return 'this' (at least
where it makes sense to preserve existing API):

query.setFetchLimit(100).setDistinct(true);

You could continue that in the model objects, too:

person.setFirstName("John").setLastName("Doe");

mrg


On Thu, Oct 9, 2014 at 1:31 PM, Andrus Adamchik <an...@objectstyle.org> wrote:
>
> On Oct 7, 2014, at 9:30 PM, Michael Gentry <mg...@masslight.net> wrote:
>
>> On Tue, Oct 7, 2014 at 6:44 PM, Aristedes Maniatis <ar...@maniatis.org> wrote:
>>>> That said, if returning this is the direction we're going, then really all
>>>> the currently void methods in SelectQuery should do the same thing - like
>>>> addOrdering for example.
>>>
>>>
>>> Correct, but we also need to gracefully deprecate the old methods and make new fluent ones.
>>
>> I was looking at the changes and was wondering if we really need to
>> deprecate the old methods.  Can't setFetchLimit() live along with
>> limit()?
>
> I suspect having both will be rather confusing. Initially I even wanted to leave SelectQuery alone and implement all the new ideas in a separate query class (ObjectQuery?), but since we've already made a bunch of changes to SelectQuery in the same direction, I ended up with a massive change to the existing class.
>
> Andrus
>

Re: Chainable SelectQuery

Posted by Andrus Adamchik <an...@objectstyle.org>.
On Oct 7, 2014, at 9:30 PM, Michael Gentry <mg...@masslight.net> wrote:

> On Tue, Oct 7, 2014 at 6:44 PM, Aristedes Maniatis <ar...@maniatis.org> wrote:
>>> That said, if returning this is the direction we're going, then really all
>>> the currently void methods in SelectQuery should do the same thing - like
>>> addOrdering for example.
>> 
>> 
>> Correct, but we also need to gracefully deprecate the old methods and make new fluent ones.
> 
> I was looking at the changes and was wondering if we really need to
> deprecate the old methods.  Can't setFetchLimit() live along with
> limit()?

I suspect having both will be rather confusing. Initially I even wanted to leave SelectQuery alone and implement all the new ideas in a separate query class (ObjectQuery?), but since we've already made a bunch of changes to SelectQuery in the same direction, I ended up with a massive change to the existing class.

Andrus


Re: Chainable SelectQuery

Posted by Michael Gentry <mg...@masslight.net>.
On Tue, Oct 7, 2014 at 6:44 PM, Aristedes Maniatis <ar...@maniatis.org> wrote:
>> That said, if returning this is the direction we're going, then really all
>> the currently void methods in SelectQuery should do the same thing - like
>> addOrdering for example.
>
>
> Correct, but we also need to gracefully deprecate the old methods and make new fluent ones.

I was looking at the changes and was wondering if we really need to
deprecate the old methods.  Can't setFetchLimit() live along with
limit()?

Thanks,

mrg

Re: Chainable SelectQuery

Posted by Aristedes Maniatis <ar...@maniatis.org>.
On 8/10/2014 1:31am, John Huss wrote:


> I'm not a fan of methods that return "this" because it is impossible to
> tell (without looking at javadoc) whether a new instance or the same
> instance is returned.

This pattern was named "fluent" by Martin Fowler.

http://martinfowler.com/bliki/FluentInterface.html


> That said, if returning this is the direction we're going, then really all
> the currently void methods in SelectQuery should do the same thing - like
> addOrdering for example.


Correct, but we also need to gracefully deprecate the old methods and make new fluent ones.


> Personally, I prefer to keep the select at the front to make it read like
> SQL.  I'd prefer something like this:
> 
> context.select(SelectQuery.query(Artist.class).where(NAME.eq("Picasso")).orderBy(NAME.asc()).limit(1);

What if you don't want to pass a limit? Remember that the limit runs on the db as part of the query, not on the resulting collection of objects.



> I'd take a bit to examine other ORMs to see what their APIs look like for
> this.


My personal experience is with Rails. And their query syntax is very much fluent style.

Client.where("orders_count > 10").order(:name).limit(10).reverse_order

http://guides.rubyonrails.org/active_record_querying.html

Of course Ruby has advantages as a language to create cleaner APIs over Java. Some things you just cannot easily achieve in Java.



I love this syntax for creating a list view of datarows:

Client.pluck(:id, :name)
# SELECT clients.id, clients.name FROM clients
# => [[1, 'David'], [2, 'Jeremy'], [3, 'Jose']]


Prefetching is done like this:

Client.includes("orders").where(first_name: 'Ryan', orders: { status: 'received' })


And aggregate functions like this:

Client.where(first_name: 'Bob').count



Ari



-- 
-------------------------->
Aristedes Maniatis
GPG fingerprint CBFB 84B4 738D 4E87 5E5C  5EFA EF6A 7D2E 3E49 102A

Re: Chainable SelectQuery

Posted by John Huss <jo...@gmail.com>.
Hmm... I have a lot to say, so I might ramble a bit.

1) Static imports

I'll never been a big user of static imports, largely because the tooling
(Eclipse) doesn't support them well.  Everytime I've used a static import
(which is really only in JUnit) I've wanted to import all the members:
import package.*;  Well, when you organize import Eclipse will replace the
star with the specific items you've used.  So you end up have to fix the
import over and over.  On the other side, Eclipse won't automatically add
the static import or even recommend it based on your code, so it becomes a
rarely used feature.

Another option is to just use a very short class name, which is something
Project Wonder did (also they didn't have the option of changing the
original).  They have a class ERXQ which acts just like ExpressionFactory.
Minus the ERX convention, this becomes a single letter - Q.  I wouldn't
recommend renaming ExpressionFactory to that, but you could have a subclass
of it that could be used just to get a shorter name.

That said, it may all be mute, because I never use any of the static
methods in ExpressionFactory since I build all my expressions using the
generated Property constants in each entity class.

I like the shorter method names.

2) SelectQuery

I'm not a fan of methods that return "this" because it is impossible to
tell (without looking at javadoc) whether a new instance or the same
instance is returned.  In this case even the javadoc doesn't tell you.
Having adopted more of a functional programming mindset in the last few
years, I would be much more interested in an immutable SelectQuery class
that returns copies, or with a builder and an immutable class.  One benefit
to that approach (besides being easier to reason about passing them around)
is that queries can safely be stored as constants and reused.  Of course,
making it immutable wouldn't be backwards compatible so probably a new
class would have to be written in it's place.

That said, if returning this is the direction we're going, then really all
the currently void methods in SelectQuery should do the same thing - like
addOrdering for example.

I'm not sure I like adding the select and selectOne methods on to
SelectQuery because then you have more than one way to do the same thing,
but with little advantage:

context.select(query)   vs
query.select(context)

Personally, I prefer to keep the select at the front to make it read like
SQL.  I'd prefer something like this:

context.select(SelectQuery.query(Artist.class).where(NAME.eq("Picasso")).orderBy(NAME.asc()).limit(1);

or course longer constructor can be used instead, but it is arguably less
readable.

context.select(SelectQuery.query(Artist.class, NAME.eq("Picasso"),
NAME.ascs()).limit(1);

I'd take a bit to examine other ORMs to see what their APIs look like for
this.

All said, I'm glad to see movement happening on this!

John


On Sun, Oct 5, 2014 at 10:10 AM, Andrus Adamchik <an...@objectstyle.org>
wrote:

> In preparation to 3.2.M2, I am working on polishing our new query APIs.
> Good examples of the new API are a new SelectById query [1] and of course
> previously available SQLSelect. Now started working on SelectQuery, which
> is a more subtle matter as it affects every single Cayenne user. So instead
> of committing it right away, I created a pull request #16 [2] and now would
> like to hear comments before this goes to the main repo.
>
> This changes SelectQuery and tweaks ExpressionFactory and Expression. You
> get the most milage out of it if you statically import ExpressionFactory. A
> good example is the Main file from our tutorials [3]:
>
> // static imports
> import static org.apache.cayenne.exp.ExpressionFactory.exp;
> import static org.apache.cayenne.exp.ExpressionFactory.or;
> ...
>
> // a single chain from query to object list
> List<Painting> paintings2 = SelectQuery.query(Painting.class,
> qualifier2).select(context);
> ...
>
> // static use of "exp" (former "Expression.fromString")
> // immediate parameter binding
> Expression qualifier3 = exp("artist.dateOfBirth < $date", "date",
> c.getTime());
>
> // static use of 'or' seems cleaner than chaining expressions with
> 'exp.orExp(..)'
> List<Painting> paintings4 = SelectQuery.query(Painting.class,
> or(qualifier2, qualifier3)).select(context);
>
> Comments?
>
> Andrus
>
> [1]
> https://github.com/apache/cayenne/blob/master/cayenne-server/src/main/java/org/apache/cayenne/query/SelectById.java
> [2] https://github.com/apache/cayenne/pull/16
> [3]
> https://github.com/andrus/cayenne/blob/9dca05242d393aba5892b6ce33e36fc7e2078bca/tutorials/tutorial/src/main/java/org/apache/cayenne/tutorial/Main.java