Deep copy for QueryBuilder/SearchRequest

[Hronom]

Is your feature request related to a problem? Please describe.
We use multi query and during building multi query using Java API we need have a possibility to get deep copy of QueryBuilder/SearchRequest to re-use common parts that we create for each query.

Describe the solution you'd like
Have something like Solr have SolrQuery.getCopy() https://github.com/apache/solr/blob/main/solr/solrj/src/java/org/apache/solr/client/solrj/SolrQuery.java#L1178

[Bukhtawar]

@Hronom Thanks for opening this issue. To better understand your use case, have you looked at below ctor for SearchRequest

OpenSearch/server/src/main/java/org/opensearch/action/search/SearchRequest.java

Lines 123 to 130 in 6252370

	/**
	* Constructs a new search request from the provided search request
	*/
	public SearchRequest(SearchRequest searchRequest) {
	this(searchRequest, searchRequest.indices, searchRequest.localClusterAlias,
	searchRequest.absoluteStartMillis, searchRequest.finalReduce);
	}

[Hronom]

Hello @Bukhtawar yep checked it, but if you go inside you will see that it not create deep copy, it creates soft copy:

    private SearchRequest(SearchRequest searchRequest, String[] indices, String localClusterAlias, long absoluteStartMillis,
                          boolean finalReduce) {
        this.allowPartialSearchResults = searchRequest.allowPartialSearchResults;
        this.batchedReduceSize = searchRequest.batchedReduceSize;
        this.ccsMinimizeRoundtrips = searchRequest.ccsMinimizeRoundtrips;
        this.indices = indices;
        this.indicesOptions = searchRequest.indicesOptions;
        this.maxConcurrentShardRequests = searchRequest.maxConcurrentShardRequests;
        this.preference = searchRequest.preference;
        this.preFilterShardSize = searchRequest.preFilterShardSize;
        this.requestCache = searchRequest.requestCache;
        this.routing = searchRequest.routing;
        this.scroll = searchRequest.scroll;
        this.searchType = searchRequest.searchType;
        this.source = searchRequest.source;
        this.types = searchRequest.types;
        this.localClusterAlias = localClusterAlias;
        this.absoluteStartMillis = absoluteStartMillis;
        this.finalReduce = finalReduce;
    }

[Da-1kun]

May I work on it?

[dblock]

May I work on it?

No need to ask for permission, make PRs.

[Da-1kun]

@dblock @Bukhtawar
Made PR!

[Vishalks]

I am working on this

[Vishalks]

@Hronom Is this still an ask to get a deep copy of SearchRequest? If yes, I can start working on it and create a PR.

[Vishalks]

Hi @Hronom, following up on this again to check if this is still an ask. If not, I will be closing it.

[Hronom]

@Vishalks still an ask, sometime you have common parts and common code that fill this parts in search request and later on you need to copy what was already input there to use in multi search request, so this is where deep copy needed.

Given complexity of how DTO objects organized it's nice to have such feature.

[Vishalks]

Thanks for the reply @Hronom. I will start working on this soon.

[techrajdeep]

I am looking for an issue for my first contribution . Can you help me here. Can I take on this .

[Vishalks]

@techrajdeep Sure. Feel free to take this up and put up a PR for this change. Assigning it to you.

[Vishalks]

@techrajdeep Did you get a chance to start working on this? Since this is a long pending ask, we would like to get this done soon :)

[Vishalks]

@techrajdeep Following up on this one more time. Feel free to put in any queries if you have.

[austintlee]

@Hronom for the use case you are describing, the copy ctor mentioned above could work. The only thing that makes it a shallow copy is that it's not making a true copy of the 'indices' array, but do you really need a copy of it?

[Hronom]

@austintlee yes we need it. To be more specific, imagine situation:
You have:

1. Common filters
2. Three specific "use case" filters

Now, you wanna do a multi-search query, so you start create query using java builders.
As first step - you put common filters.
As second step - you clone what you already build and add addition three specific "use case" filters to each cloned query object. And then - you put all three queries in the multi-search query.

Shallow copy not helps here, as they point to the same array, as same if you want to adjust objects inside it.

Solr, competitor of OpenSearch and Elasticsearch, has this functionality:

Have something like Solr have SolrQuery.getCopy() > https://github.com/apache/solr/blob/main/solr/solrj/src/java/org/apache/solr/client/solrj/SolrQuery.java#L1178
it works as expected and have a deep copy.

However, when you try to migrate from Solr to OpenSearch or Elasticsearch you don't have such possibility. Given the complexity of how serialization/deserialization organized in OpenSearch and Elasticsearch in client library it brings a major blocker as it hard to add.

Serialization/deserialization in OpenSearch and Elasticsearch java client not based on popular library like Jackson with simple DTO objects. For some reason Java Client has complex structure. I think it was historically done like that.

It brings also complexity for extensibility, when you add plugin that provides custom response/request possibilities. Great example is LTR plugin, I believe OpenSearch and Elasticsearch should revisit this and create modern simple and extensible java client, maybe it will require to create new HTTP, REST API that is friendly to JSON format as same as friendly to serialziation/deserialization libraries, as same as based on simple POJO DTO.
But this is other story...

[austintlee]

In that case, the change you want should be in the client side:

https://github.com/opensearch-project/opensearch-java/blob/main/java-client/src/main/java/org/opensearch/client/opensearch/core/SearchRequest.java

That will match your Solr example.

And to get a deep copy, you can serialize a SearchRequest and deserialize it into a new object (or you can put that routine into a utility method).

I hear you on the ser/de argument. But if you want something quick, I suggest you go with the above approach and maybe raise a separate META feature request or an RFC to propose the client overhaul that you are talking about.

[Hronom]

@austintlee you got it right:

In that case, the change you want should be in the client side:

https://github.com/opensearch-project/opensearch-java/blob/main/java-client/src/main/java/org/opensearch/client/opensearch/core/SearchRequest.java

That will match your Solr example.
this was initial request to add method on client side to QueryBuilder/SearchRequest like in Solr to do deep copy.

But unfortunately, you also need to make sure that all other classes who involved in query building has same support for deep copy. So when you call this method on QueryBuilder/SearchRequest it will go trough child objects and do copy for them accordingly.

And to get a deep copy, you can serialize a SearchRequest and deserialize it into a new object (or you can put that routine into a utility method).

That was first attempt to fix this, but... as I mention, the way how code organized for java client - it's not friendly for serializers/deserializers (for example I remember there was also recursion), so this approach fail.
However I was trying this around 1-2 years ago, maybe there situation has changed or I did something wrong.

I hear you on the ser/de argument. But if you want something quick, I suggest you go with the above approach and maybe raise a separate META feature request or an RFC to propose the client overhaul that you are talking about.

Unfortunately, I don't have time for this now. Not sure even about META feature request too...
Maybe there architects from AWS side can help to drive RFC for this project on volunteer basics. I found this request pretty blocking when you build complex systems.
Of-course there other workarounds, but they not shining=)

[channel-dante]

It is very inconvenient when you want to query only by changing the from value. Are you still not considering adding this?

[Yury-Fridlyand]

As a workaround I can propose to serialize and deserialize a SearchRequest object. This procedure creates a new deep copy of original object.
Please, refer to example discussed here: #7829 (comment). Hope this helps.

[ansjcy]

Hi folks, I sent out a PR to address this by serializing and deserializing a SearchRequest as mentioned here. Alternatively, instead of using serialization operations, we can use corresponding setters for indices and types to set a clone of those types, however it might introduce burdens to maintain that function while we keep adding non-primitive fields in SearchRequest. Also fields like SearchSourceBuilder doesn't have good deep copy / ctor support. Thus I think doing the serialization and deserialization would be a better solution here.

Also, we can implement these logic in a ctor, however the current ctor is already taken to do shallow copying. Not sure what performance impact it would introduce to change the ctor to do deep copy instead. Let me know what you would suggest!

[ansjcy]

Here are some cases we actually need a deep copy of a SearchRequest, but we handled them in not-so-elegant ways because lacking of this method:

• https://github.com/opensearch-project/search-processor/blob/c31475e5465fdce80c7c9b86519a4d8d0141c06b/amazon-kendra-intelligent-ranking/src/main/java/org/opensearch/search/relevance/actionfilter/SearchActionFilter.java#L91
• https://github.com/opensearch-project/alerting/blob/719db46c7312edc265952c3ffe9c5233eb9dfe1f/alerting/src/main/kotlin/org/opensearch/alerting/InputService.kt#L199