ES|QL Reranker command #123074
ES|QL Reranker command #123074
Conversation
afoucret
force-pushed
the
esql-reranker-boostrap
branch
from
9d2471e to
fae22df
Compare
…esql-reranker-boostrap
…esql-reranker-boostrap
…esql-reranker-boostrap
…esql-reranker-boostrap
…esql-reranker-boostrap
…esql-reranker-boostrap
…esql-reranker-boostrap
…renceServicePlugin
| ).<PreAnalysisResult>andThen((l, enrichResolution) -> resolveFieldNames(parsed, enrichResolution, l)); | ||
| ) | ||
| .<PreAnalysisResult>andThen((l, enrichResolution) -> resolveFieldNames(parsed, enrichResolution, l)) | ||
| .<PreAnalysisResult>andThen((l, preAnalysisResult) -> resolveInferences(preAnalysis.inferencePlans, preAnalysisResult, l)); |
There was a problem hiding this comment.
Would be great if we parallelized these resolutions, as they're not interdependent. But not necessarily a current conern.
There was a problem hiding this comment.
Added a tech debt item to the meta-issue for later.
| @@ -81,6 +82,7 @@ public class TransportEsqlQueryAction extends HandledTransportAction<EsqlQueryRe | |||
| private final AsyncTaskManagementService<EsqlQueryRequest, EsqlQueryResponse, EsqlQueryTask> asyncTaskManagementService; | |||
| private final RemoteClusterService remoteClusterService; | |||
| private final UsageService usageService; | |||
| private final InferenceService inferenceService; | |||
There was a problem hiding this comment.
This seems to only be used in c'tor, can it be local?
| this.computeService = new ComputeService( | ||
| searchService, | ||
| transportService, | ||
| exchangeService, | ||
| enrichLookupService, | ||
| lookupFromIndexService, | ||
| inferenceService, |
There was a problem hiding this comment.
Instead of adding a new param here, it might worth looking if the ComputeService c'tor shouldn't maybe take a TransportActionServices as param, as they share a few args.
There was a problem hiding this comment.
I did update the ComputeService constructor as described in your comment.
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/mapper/Mapper.java
Outdated
Show resolved
Hide resolved
| // Ensure the score attribute is present in the output. | ||
| if (rerank.scoreAttribute() instanceof UnresolvedAttribute ua) { | ||
| Attribute resolved = resolveAttribute(ua, childrenOutput); | ||
| if (resolved.resolved() == false || resolved.dataType() != DOUBLE) { |
There was a problem hiding this comment.
add the _score if it is missing so the user doo not have to worry about it
What happens if there's a search function following a RERANK?
Can we add some tests with this too?
he RERANK command overwrite the _score column and sort the result by _score DESC, so having the _score attribute is mandatory.
I guess it might be mandatory for execution, but we could still choose to only keep it projected if the user chooses to. Just like with adding a SORT part of surrogating RERANK, we could choose to finally DROP the _score if not enabled explicitly.
This is somewhat similar to how search functions/operator doe influence the _score, but the user can choose if they want to have it in the output.
| @@ -397,6 +397,47 @@ public static boolean clusterHasInferenceEndpoint(RestClient client) throws IOEx | |||
| return true; | |||
| } | |||
|
|
|||
| public static void createRerankInferenceEndpoint(RestClient client) throws IOException { | |||
There was a problem hiding this comment.
It'd be nice if we could set this service in the main() above too, this way one can have a testing cluster prepared "loaded" with that. If possible, this can be a follow-up.
There was a problem hiding this comment.
Added as a follow-up in the meta-issue.
afoucret
force-pushed
the
esql-reranker-boostrap
branch
from
4a5a19e to
132825d
Compare
…esql-reranker-boostrap
afoucret
removed
auto-backport
…esql-reranker-boostrap
…esql-reranker-boostrap
If the clusters don't support inference test services, skip tests that require inference services. Hence, we should check for rerank tests. Relates #123074
Implementation of the
RERANKcommandCommand syntax
where
query textis a constant string (can use a param instead?queryText)ONclause contains one or several field separated by commaON title, description=overview)ON title, short_description=SUBSTRING(description,0 100))WITHthe name of inference endpoint to be used for reranking. The inference endpoint task type has to bererankDone:
Follow-up: