[BUG] Collapse not preserved when chaining a Search instance

Mdelaf commented 3 months ago

What is the bug?

Chain operations over a Search object do not preserve the _collapse attribute.

How can one reproduce the bug?

>>> from pprint import pp
>>> from opensearchpy import Search
>>> 
>>> s = Search(index="index_name")
>>> s = s.filter("term", color="red")
>>> pp(s.to_dict())
{'query': {'bool': {'filter': [{'term': {'color': 'red'}}]}}}
>>> s = s.collapse(field="category")
>>> pp(s.to_dict())
{'query': {'bool': {'filter': [{'term': {'color': 'red'}}]}},
 'collapse': {'field': 'category'}}
>>> s = s.filter("term", brand="something")
>>> pp(s.to_dict())
{'query': {'bool': {'filter': [{'term': {'color': 'red'}},
                               {'term': {'brand': 'something'}}]}}}

What is the expected behavior?

The _collapse property should be preserved when chaining additional methods after to the Search object.

What is your host/environment?

Python 3.10.12 (main, Nov 20 2023, 15:14:05) [GCC 11.4.0] on linux OpenSearch 2.6.0

Do you have any screenshots?

Not needed.

Do you have any additional context?

I think this bug was introduced during the implementation of collapse, because the _collapse property is not copied in the _clone method.

saimedhi commented 3 months ago

Hello @Mdelaf, thank you for reporting this issue; would you be interested in contributing a fix, please?

imvtsl commented 3 months ago

@Mdelaf In case you don't want to work on the fix, I would be happy to pitch in and contribute to the fix. Please let us know if you are working on the fix.

imvtsl commented 2 months ago

Hello @saimedhi @Mdelaf hasn't replied. I already waited a week now for @Mdelaf. Do you mind if I go ahead and raise PR for the fix?

saimedhi commented 2 months ago

Hello @saimedhi @Mdelaf hasn't replied. I already waited a week now for @Mdelaf. Do you mind if I go ahead and raise PR for the fix?

@imvtsl Please feel free to raise PR with fix. Thank you.

imvtsl commented 2 months ago

Thanks @saimedhi . I will raise a PR with fix.

imvtsl commented 2 months ago

I raised this PR for the fix. @Mdelaf was right in his analysis that collapse property is not copied in the clone method.

saimedhi commented 2 months ago

Resolved by PR #771

CorneeSean commented 6 days ago

Hello, Not sure whether we should reopen this, but we switched to AsyncOpensearch and AsyncSearch helper. It lacks support for collapse althogether: https://github.com/opensearch-project/opensearch-py/blob/main/opensearchpy/_async/helpers/search.py

dblock commented 5 days ago

Yes, https://github.com/opensearch-project/opensearch-py/pull/771/files should have had async support too :( Care to contribute?

opensearch-project / opensearch-py