We're updating the issue view to help you get more done. 

Change the syntax of predicates/sorts on raw fields from onRawField(...).matching() to onField(...).matchingRaw()?

Description

Looking back at the code resulting from HSEARCH-3256, it seems we had to implement all the necessary ingredients to make the syntax even nicer: we already had to split the compatibility checks in two, one part for the raw field and another for the DSL converter field.

As a result, the current syntax:

1 2 3 4 .match().onRawField( "field1" ).matching( rawValue ) .range().onRawField( "field1" ).above( rawValue ) .range().onRawField( "field1" ).below( rawValue ) .range().onRawField( "field1" ).from( rawValue1 ).to( rawValue2 )

Could be changed to this:

1 2 3 4 .match().onField( "field1" ).matchingRaw( rawValue ) .range().onField( "field1" ).aboveRaw( rawValue ) .range().onField( "field1" ).belowRaw( rawValue ) .range().onField( "field1" ).fromRaw( rawValue1 ).toRaw( rawValue2 )

Or this:

1 2 3 4 .match().onField( "field1" ).matching( rawValue, DslConverter.DISABLED ) .range().onField( "field1" ).above( rawValue, DslConverter.DISABLED ) .range().onField( "field1" ).below( rawValue, DslConverter.DISABLED ) .range().onField( "field1" ).from( rawValue1, DslConverter.DISABLED ).to( rawValue2, DslConverter.DISABLED )

I tend to prefer the second syntax because it would give us an obvious place where we could explain the semantics of DSL converters in the javadoc, but both syntaxes are acceptable.

I think this would be an improvement, because the user would only have to think about the type of arguments when he actually passes arguments.
The only downside would be that users wouldn't be able to mix targeting raw fields with targeting non-raw fields anymore, but we don't test for it anyway, so that was not an expected use case. Plus, users can still create multiple match predicates (one targeting raw fields, one targeting non-raw fields) and combine them using a boolean junction.

Technically, how would we do this? Simple: we would only check the compatibility of raw fields when onField is called, and we would check compatibility of DSL converters if a non-raw method is called (matching instead of matchingRaw, below instead of belowRaw, ...).

That should be just one step away from the current implementation, and we don't need to add more tests (only to adapt the syntax in current tests), so I don't think it would take much work.

Regarding sorts, we could similarly postpone the checking of DSL converter compatibility to calls of use():

1 2 3 4 5 6 // Instead of: .byRawField( "field1" ).onMissingValue().use( rawValue ) // Do this: .byField( "field1" ).onMissingValue().useRaw( rawValue ) // OR .byField( "field1" ).onMissingValue().use( rawValue, DslConverter.DISABLED )

And additional benefit here is that we would, by default, accept sorts on fields that have a compatible raw type in the targeted indexes but a different DSL converter.

Regarding projections, we could do this:

1 2 3 4 5 6 7 8 9 // Instead of: .rawField( "field1" ) .rawField( "field1", String.class ) // Either keep the same syntax: .rawField( "field1" ) .rawField( "field1", String.class ) // OR add a parameter: .field( "field1", DslConverter.DISABLED ) .field( "field1", String.class, DslConverter.DISABLED )

Environment

None

Status

Assignee

Fabio Massimo Ercoli

Reporter

Yoann Rodière

Labels

None

Suitable for new contributors

None

Feedback Requested

None

Fix versions

Priority

Major