On Wed, Aug 26, 2015 at 09:58:39PM +0100, Sanne Grinovero wrote: > Assuming you build a Lucene Query the following way: > > queryBuilder.keyword().onField( "age" ).matching( 5 ).createQuery(); > > What is your expectation, if the "age" field is being indexed as a NumericField? I would say that this throws an exception. At least atm. Numeric encoded fields needs to be targeted by a range query. One could imagine to transparently create a range query in this case. I guess by the metadata we could tell that we have a numeric field. However, that's a a lot of magic then, in particular since we always that that a keyword query maps to a TermQuery.

--Hardy _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev

For the record, Gustavo replied forgetting to CC the list stating that he would expect it to produce a valid NumericRangeQuery, and that seems to be the expectation of Adrian too, gauging at some code he wrote. That surprised me. I'm personally feeling more comfortable with Hardy's answer, as I'm looking at "keyword()" as something which explicitly demands for a TermQuery. However the current DSL implementation will have this produce a valid NumericRangeQuery - except in some corner cases I'm working on - so I don't think we can change this in a minor release, not least as the HQL parser and downstream projects rely on this. There is an open issue caused by the fact that the current implementation isn't consistent as it fails to recognize the field as Numeric when it's wrapped by one of the other FieldBridge decorators (a simple instanceof isn't safe for such purposes). So.. questions: 1# Deprecate? I'll have to fix the inconsistency for now and make it produce a working query in all cases... but do we want to deprecate this and have it produce a WARN to state that you really shouldn't use a keyword() on numeric fields? 2# Alternatives? If we decide that keyword() should not be abused for this and will be deprecated, should we give an alternative method like "exactEqualsQuery()" to provide a flexible alternative (working with either keyword, number, null tokens). Thanks, Sanne On 27 August 2015 at 10:36, Gunnar Morling <gunnar(a)hibernate.org&gt; wrote: > 2015-08-27 11:27 GMT+02:00 Hardy Ferentschik <hardy(a)hibernate.org&gt;: >> On Wed, Aug 26, 2015 at 09:58:39PM +0100, Sanne Grinovero wrote: >>> Assuming you build a Lucene Query the following way: >>> >>> queryBuilder.keyword().onField( "age" ).matching( 5 ).createQuery(); >>> >>> What is your expectation, if the "age" field is being indexed as a NumericField? >> >> I would say that this throws an exception. At least atm. Numeric encoded >> fields needs to be targeted by a range query. One could imagine to transparently >> create a range query in this case. I guess by the metadata we could tell that we have >> a numeric field. However, that's a a lot of magic then, in particular since we >> always that that a keyword query maps to a TermQuery. > > Hum, but that's already happening, no? > > See DSLTest.testUseOfFieldBridge() where we do exactly that: > > monthQb.keyword().onField( "monthValue" ).matching( 2 ).createQuery(); > > Internally it's creating a NumericRangeQuery with start/end set to the > same value. > > If the field uses a null encoding bridge, it'd be a String field, > though, if I am not mistaken (unless we use some numeric null encoding > as discussed recently), so a simple TermQuery query would be the right > thing. > > >> >> --Hardy >> >> >> _______________________________________________ >> hibernate-dev mailing list >> hibernate-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/hibernate-dev _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev

On 27 August 2015 at 11:07, Gunnar Morling <gunnar(a)hibernate.org&gt; wrote: > Is there any strong reason for not supporting this on numeric fields? > Assuming the remaining corner cases can be fixed (null handling > mainly?), my vote goes for keeping the current behaviour. Right we can't just change behaviour currently, especially as the fix I'm working on needs backporting so that would break stuff in a micro release, but *if* we agree this shouldn't have been the intent, we can start the education and deprecation procedures at least by logging warnings, rather than say kill it in 6.0. Regarding "strong reason", my doubt is just that the method seems wrong for the purpose: keyWORD(), and it seems to imply we override an explicit user choice.

For example, I intentionally asked it extremely neutrally and you all gave different opinions ;-) > I suppose many people don't think in the technical terms of > "TermQuery" or "NumericRangeQuery" but just in terms of the data they > want to look for. Different methods to be used based on the field type > will make things harder for them. Also consider potential future > alternative backends of the query DSL (ES) where string/numeric/date > match queries can be handled uniformly. Sure people don't have to grasp all implementation details, in fact the API signature returns "Query" and it's probably not visible to users which kind it is. But I wonder that - in case people just want something which works for equality - if we shouldn't make a better named method for that. Also, I hate it when frameworks "know better" and for some automagic reason it overrides an explicit user request. It seems to me the user is explicitly demanding to build a keyword-based query, and we override that because of some metadata we happen to have in this case. This whole logic might even work in this case but it's certainly inconsistent with other DSL usage, as often we don't have the full picture of metadata and just rely on the user to build the right query type and set the correct options. So that wraps why I'm concerned and surprised by this; I agree that we should have an easy equality query builder too - just torn about it being named like this as it's certainly violating some "least surprise" principle. But if the majority thinks it was well understood (except obviously the low-level thinkers like myself and Hardy) we can keep the status quo. Sanne > > > 2015-08-27 11:53 GMT+02:00 Sanne Grinovero <sanne(a)hibernate.org&gt;: >> For the record, Gustavo replied forgetting to CC the list stating that >> he would expect it to produce a valid NumericRangeQuery, and that >> seems to be the expectation of Adrian too, gauging at some code he >> wrote. >> >> That surprised me. I'm personally feeling more comfortable with >> Hardy's answer, as I'm looking at "keyword()" as something which >> explicitly demands for a TermQuery. >> >> However the current DSL implementation will have this produce a valid >> NumericRangeQuery - except in some corner cases I'm working on - so I >> don't think we can change this in a minor release, not least as the >> HQL parser and downstream projects rely on this. >> >> There is an open issue caused by the fact that the current >> implementation isn't consistent as it fails to recognize the field as >> Numeric when it's wrapped by one of the other FieldBridge decorators >> (a simple instanceof isn't safe for such purposes). >> >> So.. questions: >> >> 1# Deprecate? >> I'll have to fix the inconsistency for now and make it produce a >> working query in all cases... but do we want to deprecate this and >> have it produce a WARN to state that you really shouldn't use a >> keyword() on numeric fields? >> >> 2# Alternatives? >> If we decide that keyword() should not be abused for this and will be >> deprecated, should we give an alternative method like >> "exactEqualsQuery()" to provide a flexible alternative (working with >> either keyword, number, null tokens). >> >> Thanks, >> Sanne >> >> >> >> On 27 August 2015 at 10:36, Gunnar Morling <gunnar(a)hibernate.org&gt; wrote: >>> 2015-08-27 11:27 GMT+02:00 Hardy Ferentschik <hardy(a)hibernate.org&gt;: >>>> On Wed, Aug 26, 2015 at 09:58:39PM +0100, Sanne Grinovero wrote: >>>>> Assuming you build a Lucene Query the following way: >>>>> >>>>> queryBuilder.keyword().onField( "age" ).matching( 5 ).createQuery(); >>>>> >>>>> What is your expectation, if the "age" field is being indexed as a NumericField? >>>> >>>> I would say that this throws an exception. At least atm. Numeric encoded >>>> fields needs to be targeted by a range query. One could imagine to transparently >>>> create a range query in this case. I guess by the metadata we could tell that we have >>>> a numeric field. However, that's a a lot of magic then, in particular since we >>>> always that that a keyword query maps to a TermQuery. >>> >>> Hum, but that's already happening, no? >>> >>> See DSLTest.testUseOfFieldBridge() where we do exactly that: >>> >>> monthQb.keyword().onField( "monthValue" ).matching( 2 ).createQuery(); >>> >>> Internally it's creating a NumericRangeQuery with start/end set to the >>> same value. >>> >>> If the field uses a null encoding bridge, it'd be a String field, >>> though, if I am not mistaken (unless we use some numeric null encoding >>> as discussed recently), so a simple TermQuery query would be the right >>> thing. >>> >>> >>>> >>>> --Hardy >>>> >>>> >>>> _______________________________________________ >>>> hibernate-dev mailing list >>>> hibernate-dev(a)lists.jboss.org >>>> https://lists.jboss.org/mailman/listinfo/hibernate-dev >> _______________________________________________ >> hibernate-dev mailing list >> hibernate-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/hibernate-dev _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev

Regarding "strong reason", my doubt is just that the method seems wrong for the purpose: keyWORD(), and it seems to imply we override an explicit user choice.

But I wonder that - in case people just want something which works for equality - if we shouldn't make a better named method for that.

Also, I hate it when frameworks "know better" and for some automagic reason it overrides an explicit user request. It seems to me the user is explicitly demanding to build a keyword-based query, and we override that because of some metadata we happen to have in this case. This whole logic might even work in this case but it's certainly inconsistent with other DSL usage, as often we don't have the full picture of metadata and just rely on the user to build the right query type and set the correct options.

On 27 Aug 2015, at 11:53, Sanne Grinovero <sanne(a)hibernate.org&gt; wrote: For the record, Gustavo replied forgetting to CC the list stating that he would expect it to produce a valid NumericRangeQuery, and that seems to be the expectation of Adrian too, gauging at some code he wrote. That surprised me. I'm personally feeling more comfortable with Hardy's answer, as I'm looking at "keyword()" as something which explicitly demands for a TermQuery. However the current DSL implementation will have this produce a valid NumericRangeQuery - except in some corner cases I'm working on - so I don't think we can change this in a minor release, not least as the HQL parser and downstream projects rely on this. There is an open issue caused by the fact that the current implementation isn't consistent as it fails to recognize the field as Numeric when it's wrapped by one of the other FieldBridge decorators (a simple instanceof isn't safe for such purposes). So.. questions: 1# Deprecate? I'll have to fix the inconsistency for now and make it produce a working query in all cases... but do we want to deprecate this and have it produce a WARN to state that you really shouldn't use a keyword() on numeric fields? 2# Alternatives? If we decide that keyword() should not be abused for this and will be deprecated, should we give an alternative method like "exactEqualsQuery()" to provide a flexible alternative (working with either keyword, number, null tokens). Thanks, Sanne On 27 August 2015 at 10:36, Gunnar Morling <gunnar(a)hibernate.org&gt; wrote: > 2015-08-27 11:27 GMT+02:00 Hardy Ferentschik <hardy(a)hibernate.org&gt;: >> On Wed, Aug 26, 2015 at 09:58:39PM +0100, Sanne Grinovero wrote: >>> Assuming you build a Lucene Query the following way: >>> >>> queryBuilder.keyword().onField( "age" ).matching( 5 ).createQuery(); >>> >>> What is your expectation, if the "age" field is being indexed as a NumericField? >> >> I would say that this throws an exception. At least atm. Numeric encoded >> fields needs to be targeted by a range query. One could imagine to transparently >> create a range query in this case. I guess by the metadata we could tell that we have >> a numeric field. However, that's a a lot of magic then, in particular since we >> always that that a keyword query maps to a TermQuery. > > Hum, but that's already happening, no? > > See DSLTest.testUseOfFieldBridge() where we do exactly that: > > monthQb.keyword().onField( "monthValue" ).matching( 2 ).createQuery(); > > Internally it's creating a NumericRangeQuery with start/end set to the > same value. > > If the field uses a null encoding bridge, it'd be a String field, > though, if I am not mistaken (unless we use some numeric null encoding > as discussed recently), so a simple TermQuery query would be the right > thing. > > >> >> --Hardy >> >> >> _______________________________________________ >> hibernate-dev mailing list >> hibernate-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/hibernate-dev _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev