Re: [DISCUSS] Change Query API to make queries immutable in 6.0

2015-04-03 Thread Robert Muir
On Thu, Apr 2, 2015 at 6:23 PM, Adrien Grand jpou...@gmail.com wrote: On Fri, Apr 3, 2015 at 12:10 AM, Reitzel, Charles charles.reit...@tiaa-cref.org wrote: Unfortunately, since boost is used in hashCode() and equals() calculations, changing the boost will still make the queries trappy. You

Re: [DISCUSS] Change Query API to make queries immutable in 6.0

2015-04-02 Thread Adrien Grand
I did not close this door, I agree this is something that should be considered and tried to list the pros/cons that I could think about. However I would like it to be dealt with in a different issue as it will already be a big change to change those 4 queries. Would would be ok to first make

Re: [DISCUSS] Change Query API to make queries immutable in 6.0

2015-04-02 Thread Michael McCandless
On Thu, Apr 2, 2015 at 3:40 AM, Adrien Grand jpou...@gmail.com wrote: Would would be ok to first make queries immutable up to the boost and then discuss if/how/when we should go fully immutable with a new API to change boosts? +1 ... progress not perfection. Mike McCandless

RE: [DISCUSS] Change Query API to make queries immutable in 6.0

2015-04-02 Thread Reitzel, Charles
this bug. Once that is done, then it might make sense to perform the exhaustive updates to prevent a relapse in the future. -Original Message- From: Robert Muir [mailto:rcm...@gmail.com] Sent: Thursday, April 02, 2015 9:46 AM To: dev@lucene.apache.org Subject: Re: [DISCUSS] Change Query API

Re: [DISCUSS] Change Query API to make queries immutable in 6.0

2015-04-02 Thread Adrien Grand
On Fri, Apr 3, 2015 at 12:10 AM, Reitzel, Charles charles.reit...@tiaa-cref.org wrote: Unfortunately, since boost is used in hashCode() and equals() calculations, changing the boost will still make the queries trappy. You will do all that work to make everything-but-boost immutable and

Re: [DISCUSS] Change Query API to make queries immutable in 6.0

2015-04-02 Thread Robert Muir
Boosts might not make sense to become immutable, it might make the code too complex. Who is to say until the other stuff is fixed first. The downsides might outweight the upsides. So yeah, if you want to say if anyone disagrees with what the future might look like i'm gonna -1 your progress, then

Re: [DISCUSS] Change Query API to make queries immutable in 6.0

2015-04-02 Thread david.w.smi...@gmail.com
On Thu, Apr 2, 2015 at 3:40 AM, Adrien Grand jpou...@gmail.com wrote: first make queries immutable up to the boost and then discuss if/how/when we should go fully immutable with a new API to change boosts? The “if” part concerns me; I don’t mind it being a separate issue to make the changes

Re: [DISCUSS] Change Query API to make queries immutable in 6.0

2015-04-02 Thread Yonik Seeley
If we were designing things from scratch again, would boost really be on Query, or would it be on BooleanClause? Just throwing that out there... although it may make it easier to implement immutable queries (and perhaps make more sense too), it may also be too big of a change to be worth while.

Re: [DISCUSS] Change Query API to make queries immutable in 6.0

2015-04-02 Thread david.w.smi...@gmail.com
On Thu, Apr 2, 2015 at 9:45 AM, Robert Muir rcm...@gmail.com wrote: They are also only relevant when scores are needed: so we can prevent nasty filter caching bugs as a step, by making everything else immutable. That’s a good point. +1 to your progress Adrien! ~ David Smiley Freelance

Re: [DISCUSS] Change Query API to make queries immutable in 6.0

2015-04-01 Thread david.w.smi...@gmail.com
I’m +1 to going all the way (fully immutable) but the proposal stops short by skipping the boost. I agree with Terry’s comments — what a shame to make Queries “more immutable” but not really quite immutable. It kinda misses the point? Otherwise why bother? If this is about progress not

[DISCUSS] Change Query API to make queries immutable in 6.0

2015-03-31 Thread Adrien Grand
Recent changes that added automatic filter caching to IndexSearcher uncovered some traps with our queries when it comes to using them as cache keys. The problem comes from the fact that some of our main queries are mutable, and modifying them while they are used as cache keys makes the entry that

Re: [DISCUSS] Change Query API to make queries immutable in 6.0

2015-03-31 Thread Robert Muir
Same with BooleanQuery. the go-to ctor should just take 'clauses' On Tue, Mar 31, 2015 at 5:18 AM, Michael McCandless luc...@mikemccandless.com wrote: +1 For PhraseQuery we could also have a common-case ctor that just takes the terms (and assumes sequential positions)? Mike McCandless

Re: [DISCUSS] Change Query API to make queries immutable in 6.0

2015-03-31 Thread Michael McCandless
+1 For PhraseQuery we could also have a common-case ctor that just takes the terms (and assumes sequential positions)? Mike McCandless http://blog.mikemccandless.com On Tue, Mar 31, 2015 at 5:10 AM, Adrien Grand jpou...@gmail.com wrote: Recent changes that added automatic filter caching to

Re: [DISCUSS] Change Query API to make queries immutable in 6.0

2015-03-31 Thread Adrien Grand
Hi Charles, On Tue, Mar 31, 2015 at 4:12 PM, Reitzel, Charles charles.reit...@tiaa-cref.org wrote: Am I missing something? Across the project, I’m seeing over 1,000 references to BooleanQuery.add(). Already, this seems like a pretty major refactoring. And I haven’t checked the other

Re: [DISCUSS] Change Query API to make queries immutable in 6.0

2015-03-31 Thread Adrien Grand
On Tue, Mar 31, 2015 at 4:32 PM, Terry Smith sheb...@gmail.com wrote: Thanks for the explanation. It seems a pity to make queries just nearly immutable. Do you have any interest in adding a boost parameter to clone() so they really could be immutable? We could have a single method, but if we

RE: [DISCUSS] Change Query API to make queries immutable in 6.0

2015-03-31 Thread Reitzel, Charles
. From: Terry Smith [mailto:sheb...@gmail.com] Sent: Tuesday, March 31, 2015 9:38 AM To: dev@lucene.apache.org Subject: Re: [DISCUSS] Change Query API to make queries immutable

Re: [DISCUSS] Change Query API to make queries immutable in 6.0

2015-03-31 Thread Adrien Grand
Hi Terry, Indeed this is for query rewriting. For instance if you have a boolean query with a boost of 5 that wraps a single MUST clause with a term query, then we rewrite to this to the inner term query and update its boost using clone() and setBoost() in order to not modify in-place a

Re: [DISCUSS] Change Query API to make queries immutable in 6.0

2015-03-31 Thread Terry Smith
Adrien, Thanks for the explanation. It seems a pity to make queries just nearly immutable. Do you have any interest in adding a boost parameter to clone() so they really could be immutable? --Terry On Tue, Mar 31, 2015 at 9:44 AM, Adrien Grand jpou...@gmail.com wrote: Hi Terry, Indeed this

Re: [DISCUSS] Change Query API to make queries immutable in 6.0

2015-03-31 Thread Mark Miller
+1. Makes a lot of sense in general I think. Keeps them nicely thread safe as well, and as they are commonly used for keys in caches and such, that's a nice intrinsic property. - Mark On Tue, Mar 31, 2015 at 7:24 AM Robert Muir rcm...@gmail.com wrote: Same with BooleanQuery. the go-to ctor

Re: [DISCUSS] Change Query API to make queries immutable in 6.0

2015-03-31 Thread Terry Smith
Adrien, I missed the reason that boost is going to stay mutable. Is this to support query rewriting? --Terry On Tue, Mar 31, 2015 at 7:21 AM, Robert Muir rcm...@gmail.com wrote: Same with BooleanQuery. the go-to ctor should just take 'clauses' On Tue, Mar 31, 2015 at 5:18 AM, Michael