[FEATURE] Add knn builder and query search support

[adrian-arapiles]

Is your feature request related to a problem?

It's not possible to build a search query using knn. With old java rest high client we can copy and reuse code like that thread cover https://forum.opensearch.org/t/k-nn-query-from-es-high-level-client/3155

What solution would you like?

Get classes for create and use query builders for search.

What alternatives have you considered?

At the moment I will tried to parse a json for raw search request.

Do you have any additional context?

[wbeckler]

This sounds like a great idea. Would someone be willing to propose what a good interface would look like, for others to feedback on?

…

On Wed, Nov 16, 2022 at 5:49 AM Adrián ***@***.***> wrote: Is your feature request related to a problem? It's not possible to build a search query using knn. With old java rest high client we can copy and reuse code like that thread cover https://forum.opensearch.org/t/k-nn-query-from-es-high-level-client/3155 What solution would you like? Get classes for create and use query builders for search. What alternatives have you considered? At the moment I will tried to parse a json for raw search request. Do you have any additional context? — Reply to this email directly, view it on GitHub <#547>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA5PRLQ6B2STHSRA3BLFGUTWITQ57ANCNFSM6AAAAAASCIAT2Y> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[adrian-arapiles]

Hello, I could make a PR if you tech me how to do.
I already fork the repo from main branch, but I face issues with missmatch httpclient version from 4 to 5.
In version 2.4.0 of opensearch-rest-client uses major 4 of httpclient but in main branch already using classes from major 5 of httpclient.
I have ready class and changes for knn search support. The build and tests passed.

Thanks.

[reta]

@adrian-arapiles that's correct, 3.0 has moved to Apache HttpClient 5, whereas versions below stay on Apache HttpClient 4

[reta]

On the general note, the k-NN functionality is provided by the plugin (https://github.com/opensearch-project/k-NN), so we probably should not bake it into core client (since the plugin may not be always available) but make it part of the k-NN plugin, similarly to percolator (https://github.com/opensearch-project/OpenSearch/tree/main/modules/percolator) and others (in fact, it is there already https://github.com/opensearch-project/k-NN/blob/main/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java)

[adrian-arapiles]

@reta

On the general note, the k-NN functionality is provided by the plugin (https://github.com/opensearch-project/k-NN), so we probably should not bake it into core client (since the plugin may not be always available) but make it part of the k-NN plugin, similarly to percolator (https://github.com/opensearch-project/OpenSearch/tree/main/modules/percolator) and others (in fact, it is there already https://github.com/opensearch-project/k-NN/blob/main/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java)

That's true, the knn is a plugin functionality. The code that you share about KnnQueryBuilder is the class that I use until now with old High Rest Client but wont work with new builders java client pattern.
I didn't find any way to import knn libraries in mvn repositories.
By other hand there isn't a way to create a query from json (for example) to workaround this situation.
If you know any idea, please tell me.
Right now the problem is I can't make a knn query with this new client using builder pattern.
In the case that do you want to I share the code for knn builders, tell me I have ready.

Is there any possibility to add support to that for version 2.x until any definitively solution?

Thanks.

[reta]

Thanks @adrian-arapiles

@reta

On the general note, the k-NN functionality is provided by the plugin (https://github.com/opensearch-project/k-NN), so we probably should not bake it into core client (since the plugin may not be always available) but make it part of the k-NN plugin, similarly to percolator (https://github.com/opensearch-project/OpenSearch/tree/main/modules/percolator) and others (in fact, it is there already https://github.com/opensearch-project/k-NN/blob/main/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java)

That's true, the knn is a plugin functionality. The code that you share about KnnQueryBuilder is the class that I use until now with old High Rest Client but wont work with new builders java client pattern.

Could you please elaborate more on that (may be some examples)? Because the original discussion thread has only JSON

I didn't find any way to import knn libraries in mvn repositories.

That has to be fixed by publishing client libraries (for k-NN plugin)

By other hand there isn't a way to create a query from json (for example) to workaround this situation. If you know any idea, please tell me. Right now the problem is I can't make a knn query with this new client using builder pattern. In the case that do you want to I share the code for knn builders, tell me I have ready.

Is my understanding correct that if KnnQueryBuilder (from k-NN plugin) is available, it could be used with new client using builder pattern? Or I missing something

Is there any possibility to add support to that for version 2.x until any definitively solution?

The publishing k-NN plugin client libraries for 2.x should be straightforward. I am not the one making the decision, but adding any temporary API is not a good way to solve this particular problem (in my opinion), once public it could not be easily removed.

Thanks.

[adrian-arapiles]

Thanks @reta

Could you please elaborate more on that (may be some examples)? Because the original discussion thread has only JSON

Yes, sorry. Right now, I am using KnnQueryBuilder copied from knn repo.
So my code looks like that:

...
            // Multimedias is a object with a list of vectors. I'm doing a multisearch with individual search of knn vectors.
            for (Multimedia multimedia : property.multimedias()) {
                KnnQueryBuilder knnQueryBuilder = new KnnQueryBuilder("my-field", multimedia.getPhashVector(), 2);
                BoolQueryBuilder query = QueryBuilders.boolQuery()
                    .must(knnQueryBuilder));

                SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder();
                searchSourceBuilder.query(query);
                searchSourceBuilder.minScore(0.96f);
                searchSourceBuilder.size(1000);
                SearchRequest searchRequest = new SearchRequest("my-index-name");
                searchRequest.source(searchSourceBuilder);
                multiSearchRequest.add(searchRequest);
            }

            MultiSearchResponse searchResults = openSearchClient.multiSearch(multiSearchRequest);
...

So, if I'm not wrong, this code works fine with java high rest client but in new java client that uses different classes, builder pattern, etc it won't work.
Maybe I'm missing something but I didn't find any method to use raw JSON as forum mentioned or how to reuse code from KnnQueryBuilder in new java client.

That has to be fixed by publishing client libraries (for k-NN plugin)

Okay, do you have any issue following that or idea about when this gonna be released? Can I help with that?

Is my understanding correct that if KnnQueryBuilder (from k-NN plugin) is available, it could be used with new client using builder pattern? Or I missing something

I'm not sure about that. I didn't found any way to do that. If you have any clue I gonna investigate and test it.
Anyways I have class KnnQueryBuilder using new builder pattern available to share with you.
b8016c1

The publishing k-NN plugin client libraries for 2.x should be straightforward. I am not the one making the decision, but adding any temporary API is not a good way to solve this particular problem (in my opinion), once public it could not be easily removed.

I agree with that. Maybe could add support to build search from raw json? I think its a good feature and it works as a workaround until knn libraries are released.

Thanks.

[reta]

thanks @adrian-arapiles, I think we could transfer that to https://github.com/opensearch-project/k-NN plugin, the client APIs should be published and should be usable with all clients. @dblock could you help please to transfer this issue? (it seems like I cannot, but I could copy / paste), thanks

[navneet1v]

Adding @vamshin for prioritization.

[jmazanec15]

Seems it would be best to look into adding plugins to opensearch-api-specification: opensearch-project/opensearch-clients#19.

There appears to be an issue already tracking this: opensearch-project/opensearch-api-specification#23.

[SeyedAlirezaFatemi]

Right now, I am using KnnQueryBuilder copied from knn repo.

@adrian-arapiles , How exactly are you doing this? I'm asking since I'm not able to add org.opensearch.plugin:opensearch-knn as a dependent and import from org.opensearch.knn. I'm also using the Java high-level REST client.

[adrian-arapiles]

Right now, I am using KnnQueryBuilder copied from knn repo.

@adrian-arapiles , How exactly are you doing this? I'm asking since I'm not able to add org.opensearch.plugin:opensearch-knn as a dependent and import from org.opensearch.knn. I'm also using the Java high-level REST client.

I just created a KnnQueryBuilder class in my project and copied code from the class. https://github.com/opensearch-project/k-NN/blob/main/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java

Then I use as another QueryBuilder. For example:

float [] value = ...;
int k = 2;
KnnQueryBuilder knnQueryBuilder = new KnnQueryBuilder("field", value, k);
BoolQueryBuilder query = QueryBuilders.boolQuery().must(knnQueryBuilder);

I hope that I explained well. The problems is that you can't get as dependency this class, so I replicated the class in my repo.

[SeyedAlirezaFatemi]

I just created a KnnQueryBuilder class in my project and copied code from the class. https://github.com/opensearch-project/k-NN/blob/main/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java

My problem is that that class has a lot of dependencies on other classes such as org.opensearch.knn.index.KNNMethodContext, org.opensearch.knn.index.mapper.KNNVectorFieldMapper, org.opensearch.knn.index.util.KNNEngine. So do you also copy all those classes to your repo?

[adrian-arapiles]

I just created a KnnQueryBuilder class in my project and copied code from the class. https://github.com/opensearch-project/k-NN/blob/main/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java

My problem is that that class has a lot of dependencies on other classes such as org.opensearch.knn.index.KNNMethodContext, org.opensearch.knn.index.mapper.KNNVectorFieldMapper, org.opensearch.knn.index.util.KNNEngine. So do you also copy all those classes to your repo?

Hi,
Sorry about my late.
I only copied 2 classes, the rest come from opensearch dependencies from maven (in my case).
KnnQueryBuilder.java
KnnQuery.java

[wbeckler]

Just a heads up that the opensearch-java client has an open PR to add knn support:
#524

[navneet1v]

@wbeckler thanks.. @SeyedAlirezaFatemi @adrian-arapiles

[dblock]

Is this the same as #539 and belongs in the opensearch-java project? If so, let's move this issue there and close the above as dup?

[navneet1v]

@dblock the issues seems to be same. @wbeckler can provide more details. But the idea was k-NN query builder was not supported by any opensearch client, and in this issue customer are specifically for k-NN queryBuilder in java client.

[wbeckler]

I am moving this issue back to the opensearch-java client, as it's still a need and that's where the work would go.

[VachaShah]

kNN support was added in opensearch-java 2.6.0.

[Dharin-shah]

Why is this closed, the specific usecase to build a knn query along with other functions is missing? can someone please help here, i tried copying the code, but i guess there are too many depenedencies that needs to be copied too
@SeyedAlirezaFatemi @adrian-arapiles, did the copy help?

My use case is similar

private QueryBuilder getFunctionScoreWithSemanticSearchAndCustomScoreScript(QueryVectorParams queryVectorParams) {
        FullTextQuery currentFullTextQuery = fullTextQuery;
        AbstractQueryBuilder fulltextQuery = currentFullTextQuery.buildQuery().orElseGet(QueryBuilders::matchAllQuery);
        KnnFloatVectorQuery knnQuery = new KnnFloatVectorQuery(queryVectorParams.getModelId(),
            queryVectorParams.getParams(), 500);
        BoolQueryBuilder combinedQuery = QueryBuilders.boolQuery();
        combinedQuery.should(fulltextQuery);

        //combinedQuery.should(knnQuery); //doesnt work
        return FunctionScore.buildWithCustomRankingScript(fulltextQuery, queryVectorParams.getModelId(),
            queryVectorParams.getQueryVector(), queryVectorParams.getParams());
   }

[dblock]

@Dharin-shah Looks like this wasn't complete support. Open a new issue with details of all the things you know are missing?

[Dharin-shah]

import java.io.IOException;
import org.apache.lucene.search.KnnFloatVectorQuery;
import org.apache.lucene.search.Query;
import org.opensearch.common.io.stream.StreamInput;
import org.opensearch.common.io.stream.StreamOutput;
import org.opensearch.common.xcontent.XContentBuilder;
import org.opensearch.index.query.AbstractQueryBuilder;
import org.opensearch.index.query.QueryShardContext;

public class CustomKnnQueryBuilder extends AbstractQueryBuilder<CustomKnnQueryBuilder> {
    private final String field;
    private final float[] vector;
    private final int kval;

    public CustomKnnQueryBuilder(String field, float[] vector, int k) {
        this.field = field;
        this.vector = vector;
        this.kval = k;
    }

    @Override
    protected void doWriteTo(StreamOutput out) throws IOException {
        out.writeString(field);
        out.writeFloatArray(vector);
        out.writeInt(kval);
    }

    public CustomKnnQueryBuilder(StreamInput in) throws IOException {
        field = in.readString();
        vector = in.readFloatArray();
        kval = in.readInt();
    }

    @Override
    protected void doXContent(XContentBuilder builder, Params params) throws IOException {
        builder.startObject("knn");
        builder.startObject(field);
        builder.startArray("vector");
        for (float v : vector) {
            builder.value(v);
        }
        builder.endArray(); 
        builder.field("k", kval);
        builder.endObject();
        builder.endObject();
    }

    @Override
    protected Query doToQuery(QueryShardContext context) {
        return new KnnFloatVectorQuery(field, vector, kval);
    }


    @Override
    protected boolean doEquals(CustomKnnQueryBuilder other) {
        return field.equals(other.field) && kval == other.kval && java.util.Arrays.equals(vector, other.vector);
    }

    @Override
    protected int doHashCode() {
        return java.util.Objects.hash(field, java.util.Arrays.hashCode(vector), kval);
    }

    @Override
    public String getWriteableName() {
        return "custom_knn_query";
    }
}

i have this custom builder that requires no dependencies, but this is just for my use case, where i need to build the query along with other parts of the query