ES|QL: Replace RRF with FUSE #130693
ES|QL: Replace RRF with FUSE #130693
Conversation
ioanatia
added
>non-issue
:Search Relevance/Search
| @@ -263,25 +262,10 @@ sampleCommand | |||
| : SAMPLE probability=constant | |||
| ; | |||
|
|
|||
| // | |||
There was a problem hiding this comment.
moved fork, completion and change_point before the // In development section
| ); | ||
|
|
||
| return new OrderBy(source, dedup, order); | ||
| return new Dedup(source, new RrfScoreEval(source, input, scoreAttr, forkAttr), aggregates, groupings); |
There was a problem hiding this comment.
this will be changed in a follow up PR where we continue to work on FUSE - we called this Dedup initially because we thought that it would be its own command - now that command is actually FUSE.
There was a problem hiding this comment.
Shouldn't we rename Dedup to FusePlan or similar so it's easier to follow?
There was a problem hiding this comment.
yep done in a follow up - I have a lot more changes on the planning side - did not want to add them here where we just remove RRF
| | rrf | ||
| | rrf | ||
| """)); | ||
| assertThat(e.getMessage(), containsString("RRF can only be used after FORK, but found RRF")); |
There was a problem hiding this comment.
FUSE does not have the restriction of only following fork - so we removed that validation and tests
elasticsearchmachine
added
Team:Search Relevance
|
Pinging @elastic/es-search-relevance (Team:Search Relevance) |
| import static org.elasticsearch.xpack.esql.common.Failure.fail; | ||
|
|
||
| public class RrfScoreEval extends UnaryPlan implements PostAnalysisVerificationAware, LicenseAware { | ||
| public class RrfScoreEval extends UnaryPlan implements LicenseAware { |
There was a problem hiding this comment.
Are we renaming this class as well?
There was a problem hiding this comment.
I am not 100 % sure yet which is why I haven't touched it yet - we could have a FuseScoreEval to do both RRF and linear, or have two classes RrfScoreEval and LinearScoreEval.
...c/internalClusterTest/java/org/elasticsearch/xpack/esql/action/FuseWithInvalidLicenseIT.java
Show resolved
Hide resolved
| ); | ||
|
|
||
| return new OrderBy(source, dedup, order); | ||
| return new Dedup(source, new RrfScoreEval(source, input, scoreAttr, forkAttr), aggregates, groupings); |
There was a problem hiding this comment.
Shouldn't we rename Dedup to FusePlan or similar so it's easier to follow?
Related #130434
meta issue: #123389
Will merge only after Kibana merges a change to change the autocomplete we have for RRF to be the autocomplete functionality for FUSE (elastic/kibana#226743)