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:

Could be changed to this:

Or this:

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():

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:

Activity

Show:

Fabio Massimo Ercoli March 19, 2019 at 1:41 PM
Edited

Regarding projection I think that the parameter type should be `ProjectionConverter` and not `DslConverter`. Shouldn't it?

Fixed

Details

Assignee

Reporter

Components

Sprint

Fix versions

Priority

Created March 4, 2019 at 10:17 AM
Updated April 4, 2019 at 9:41 AM
Resolved March 26, 2019 at 12:48 PM