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

[BUG] When Query Types That Extend QueryBase Are Converted To Builders Inherited Data Is Lost #1172

Closed
PiSoup opened this issue Aug 29, 2024 · 3 comments · Fixed by #1181
Closed
Labels
bug Something isn't working

Comments

@PiSoup
Copy link
Contributor

PiSoup commented Aug 29, 2024

What is the bug?

Calling .toBuilder() on a query such as MatchQuery will lose the queryName and boost fields that are inherited from QueryBase. Odds are this is true for many other query types though I've not audited them all.

How can one reproduce the bug?

An example modified version of MatchQueryTest which sets queryName will fail.

    @Test
    public void toBuilder() {
        MatchQuery origin = new MatchQuery.Builder().field("field").query(FieldValue.of("1")).queryName("name").build();
        MatchQuery copied = origin.toBuilder().build();

        assertEquals(toJson(copied), toJson(origin));
    }
@PiSoup PiSoup added bug Something isn't working untriaged labels Aug 29, 2024
@Xtansia Xtansia removed the untriaged label Aug 29, 2024
@Xtansia
Copy link
Collaborator

Xtansia commented Aug 29, 2024

That's a good catch @PiSoup! Thanks for reporting this. Would you be interested in helping to fix this?

@PiSoup
Copy link
Contributor Author

PiSoup commented Aug 30, 2024

I'd be delighted. I'll take a look next week.

PiSoup added a commit to PiSoup/opensearch-java that referenced this issue Sep 3, 2024
Calling `toBuilder` on classes which implement `QueryBase` will now preserve `boost` and `queryName`.

Signed-off-by: Chris Danisch <[email protected]>
@PiSoup
Copy link
Contributor Author

PiSoup commented Sep 3, 2024

Opened the PR #1181. I only updated the match test since the change is identical for all the query types.

PiSoup added a commit to PiSoup/opensearch-java that referenced this issue Sep 3, 2024
Calling `toBuilder` on classes which implement `QueryBase` will now preserve `boost` and `queryName`.

Signed-off-by: Chris Danisch <[email protected]>
PiSoup added a commit to PiSoup/opensearch-java that referenced this issue Sep 4, 2024
Calling `toBuilder` on classes which implement `QueryBase` will now preserve `boost` and `queryName`.

Signed-off-by: Chris Danisch <[email protected]>
@Xtansia Xtansia closed this as completed in 020e0b0 Sep 4, 2024
Xtansia pushed a commit to Xtansia/opensearch-java that referenced this issue Sep 4, 2024
…ch-project#1181)

* Fixes opensearch-project#1172

Calling `toBuilder` on classes which implement `QueryBase` will now preserve `boost` and `queryName`.

Signed-off-by: Chris Danisch <[email protected]>

* Updated changelog

Signed-off-by: Chris Danisch <[email protected]>

* Added toBuilder to QueryBase instead.

Signed-off-by: Chris Danisch <[email protected]>

---------

Signed-off-by: Chris Danisch <[email protected]>
(cherry picked from commit 020e0b0)
Xtansia pushed a commit to Xtansia/opensearch-java that referenced this issue Sep 4, 2024
…ch-project#1181)

* Fixes opensearch-project#1172

Calling `toBuilder` on classes which implement `QueryBase` will now preserve `boost` and `queryName`.

Signed-off-by: Chris Danisch <[email protected]>

* Updated changelog

Signed-off-by: Chris Danisch <[email protected]>

* Added toBuilder to QueryBase instead.

Signed-off-by: Chris Danisch <[email protected]>

---------

Signed-off-by: Chris Danisch <[email protected]>
(cherry picked from commit 020e0b0)
Signed-off-by: Thomas Farr <[email protected]>
dblock pushed a commit that referenced this issue Sep 5, 2024
…1184)

* Fixes #1172

Calling `toBuilder` on classes which implement `QueryBase` will now preserve `boost` and `queryName`.



* Updated changelog



* Added toBuilder to QueryBase instead.



---------


(cherry picked from commit 020e0b0)

Signed-off-by: Chris Danisch <[email protected]>
Signed-off-by: Thomas Farr <[email protected]>
Co-authored-by: Chris Danisch <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants