opensearch-java icon indicating copy to clipboard operation
opensearch-java copied to clipboard

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

Open kdh6429 opened this issue 2 years ago • 6 comments

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.

kdh6429 avatar Jul 19 '23 00:07 kdh6429

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?

andrross avatar Jul 19 '23 16:07 andrross

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

kdh6429 avatar Jul 20 '23 01:07 kdh6429

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?

assafcoh avatar Oct 25 '23 11:10 assafcoh

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?

dblock avatar Oct 25 '23 18:10 dblock

@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.

assafcoh avatar Oct 29 '23 07:10 assafcoh

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

tloubrieu-jpl avatar Apr 26 '24 17:04 tloubrieu-jpl