Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
kdh6429 opened this issue Jul 19, 2023 · 6 comments
Open

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

kdh6429 opened this issue Jul 19, 2023 · 6 comments

Comments

@kdh6429
Copy link

kdh6429 commented Jul 19, 2023

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 kdh6429 changed the title [PROPOSAL] support rebuildable or deep copy on SearchRequest.Builder [PROPOSAL] support rebuild or deep copy on SearchRequest.Builder Jul 19, 2023
@andrross
Copy link
Member

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
Copy link
Author

kdh6429 commented Jul 20, 2023

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

@wbeckler wbeckler removed the untriaged label Aug 3, 2023
@assafcoh
Copy link

assafcoh commented Oct 25, 2023

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
Copy link
Member

dblock commented Oct 25, 2023

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
Copy link

@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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants