opensearch-project / opensearch-java

Java Client for OpenSearch
Apache License 2.0
119 stars 182 forks source link

[PROPOSAL] support rebuild or deep copy on SearchRequest.Builder #583

Open kdh6429 opened 1 year ago

kdh6429 commented 1 year ago

What/Why

What are you proposing?

Rebuild or deep copy is supported for SearchRequest.Builder. It seems tough but supported in opensearch rest client, but there seems to be no way to support it in opensearch java(refer).

What users have asked for this feature?

What problems are you trying to solve?

I want to run a query multiple times by adding only some values ​​to an already prepared SearchRequest. For example, rerun the query by dynamically changing only the index or from field value.

What is the developer experience going to be?

Create a SearchRequest based on it. And for every query, SearchRequest.Builder is recreated and all field values ​​set are put back in and built.

Are there any security considerations?

Are there any breaking changes to the API

What is the user experience going to be?

Are there breaking changes to the User Experience?

Why should it be built? Any reason not to?

What will it take to execute?

Any remaining open questions?

I wonder why you restricted SearchRequest.Builder to only build once.

andrross commented 1 year ago

I wonder why you restricted SearchRequest.Builder to only build once.

@kdh6429 The SearchRequest.Builder#build method does not make a deep copy from the builder to the built object, so it is not safe to reuse the builder.

This feature request seems reasonable to me. The general pattern I've seen is to add something like a toBuilder() method on a built SearchRequest instance, which will make a deep copy of all the data from the search request into a new mutable builder. This allows for the following pattern:

SearchRequest template = new SearchRequest.Builder()
    .index("my-index")
    // other common parameters
    .build()

SearchRequest request1 = template.toBuilder()
    // specific settings for request 1
    .build();

SearchRequest request2 = template.toBuilder()
    // specific settings for request 2
    .build();

Would that meet your use case?

kdh6429 commented 1 year ago

Yes, it would be very satisfying if this were implemented.

assafcoh commented 11 months ago

We also have a similar use case. We need to dynamically add an additional query filter to an already build SearchRequest. The toBuilder() solution suggested above is perfect!

It would be great if you can also add a toBuilder() method on: BoolQuery class MsearchRequest.class RequestItem.class MultisearchBody

Can you please give us a rough estimation when this enhancement can be expected?

dblock commented 11 months ago

Can you please give us a rough estimation when this enhancement can be expected?

@assafcoh I don't think anyone is working on this, would you like to pitch in?

assafcoh commented 11 months ago

@dblock unfortunately I do not have time to work on this. However I believe this issue should be given higher priority since it is necessary for many projects (I found several issues with similar feature request). Thank you.

tloubrieu-jpl commented 5 months ago

Hello,

I'm interested in this solution and tried the toBuilder() method on the BoolQuery.

But there is a limitation and it does not work for my use case which is:

  1. Web application (restful API, developed with SpringBoot)
  2. I am having a base BoolQuery.Builder setting up base filters on my opensearch documents (e.g. public documents only).
  3. Then, for each user request, user specific filtering criteria are added, e.g. data between date d nd f, or author is John Doe... For doing that step, I copy the base BoolQuery.Builder by using .build.toBuilder() and add the user specific criteria. That works.
  4. But for next user request I cannot call .build().toBuilder() on my initial baseBoolQuery.Builder since build() can only be called once. As a work-around, I currently call the .build() once, then I create 2 new Builders: 1 for my current user request, 1 to re-initialize the base BoolQuery.Builder. But this is not a production acceptable solution because it is not thread-safe: a new user request might arrive before the original base BoolQuery.Builder as been re-initialized. I've stressed test my server and that race condition occurs.

Any better idea on how to handle that case ? I was thinking that the most simple would be to have a copy/clone method directly on the BoolQuery.Builder. Any thoughts on that ?

I could manage in my own code all the attributes of the BoolQuery.Builder, e.g. filter, must, etc... so that I can re-use them whenever I need to, but it sounds silly that I cannot re-use the BoolQuery code more.

Thanks