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

Add SearchRequest deep cocpy functionality #1338

Closed
wants to merge 1 commit into from

Conversation

Da-1kun
Copy link

@Da-1kun Da-1kun commented Oct 7, 2021

Signed-off-by: Daichi Nuki [email protected]

Description

Add SearchRequest deep cocpy functionality

Issues Resolved

[List any issues this PR will resolve]
Deep copy for QueryBuilder/SearchRequest #869

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@opensearch-ci-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Wrapper Validation success 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Wrapper Validation success 67b8f8e

1 similar comment
@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Wrapper Validation success 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Wrapper Validation success 67b8f8e

1 similar comment
@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Wrapper Validation success 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success 67b8f8e

3 similar comments
@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Wrapper Validation success 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Wrapper Validation success 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Wrapper Validation success 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 67b8f8e

1 similar comment
@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Wrapper Validation success 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Wrapper Validation success 67b8f8e

1 similar comment
@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Wrapper Validation success 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success 67b8f8e

1 similar comment
@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 67b8f8e

1 similar comment
@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Wrapper Validation success 67b8f8e

1 similar comment
@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Wrapper Validation success 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success 67b8f8e

Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should add a copy constructor that performs a deep copy rather than adding a new deepCopy method, that feels more idiomatic, doesn't it?

@Bukhtawar
Copy link
Collaborator

Bukhtawar commented Oct 9, 2021

Thanks @Da-1kun does

public void writeTo(StreamOutput out) throws IOException {
and deserializing the same back doesn't work as a deep copy? Is that too costly?

Alternatively you could do a SearchRequest#clone and use corresponding setters for indices and types to set a clone of those types?

I noted the copy ctor is already taken to do a shallow copy. That might need to be overloaded

@tlfeng tlfeng added enhancement Enhancement or improvement to existing feature or request Indexing & Search v2.0.0 Version 2.0.0 labels Oct 13, 2021
@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Wrapper Validation success 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 67b8f8e

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success 67b8f8e

@Da-1kun Da-1kun closed this Nov 17, 2021
@dblock
Copy link
Member

dblock commented Nov 19, 2021

@Da-1kun Did you not want to move forward with this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement or improvement to existing feature or request Indexing & Search v2.0.0 Version 2.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants