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

Document new Split and Sort SearchResponseProcessors #7767

Merged

Conversation

dbwiddis
Copy link
Member

@dbwiddis dbwiddis commented Jul 17, 2024

Description

Documents the new Sort and Split SearchResponseProcessors.

Fixes an incorrect field name in the Ingest Split Processor docs.

Issues Resolved

Fixes #7743
Fixes #7744
Fixes #7754

Feature PRs:

Version

2.16.0

Frontend features

N/A

Checklist

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

Copy link

Thank you for submitting your PR. The PR states are In progress (or Draft) -> Tech review -> Doc review -> Editorial review -> Merged.

Before you submit your PR for doc review, make sure the content is technically accurate. If you need help finding a tech reviewer, tag a maintainer.

When you're ready for doc review, tag the assignee of this PR. The doc reviewer may push edits to the PR directly or leave comments and editorial suggestions for you to address (let us know in a comment if you have a preference). The doc reviewer will arrange for an editorial review.

@dbwiddis dbwiddis changed the title Split and sort processors Document new Split and Sort SearchResponseProcessors Jul 17, 2024
@dbwiddis dbwiddis added 3 - Tech review PR: Tech review in progress v2.16.0 labels Jul 17, 2024
@kolchfa-aws kolchfa-aws assigned kolchfa-aws and unassigned hdhalter Jul 17, 2024
@dbwiddis dbwiddis added 4 - Doc review PR: Doc review in progress and removed 3 - Tech review PR: Tech review in progress labels Jul 19, 2024
@dbwiddis
Copy link
Member Author

@kolchfa-aws Ready for doc review!

Signed-off-by: Fanit Kolchina <[email protected]>
Copy link
Collaborator

@kolchfa-aws kolchfa-aws left a comment

Choose a reason for hiding this comment

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

Thank you, @dbwiddis! I edited in place. Moving to editorial review.

@kolchfa-aws kolchfa-aws added 5 - Editorial review PR: Editorial review in progress and removed 4 - Doc review PR: Doc review in progress labels Jul 19, 2024
Copy link
Collaborator

@natebower natebower left a comment

Choose a reason for hiding this comment

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

@dbwiddis @kolchfa-aws Please see my changes and let me know if you have any questions. Thanks!

_ingest-pipelines/processors/split.md Outdated Show resolved Hide resolved
_search-plugins/search-pipelines/sort-processor.md Outdated Show resolved Hide resolved
_search-plugins/search-pipelines/split-processor.md Outdated Show resolved Hide resolved
_search-plugins/search-pipelines/split-processor.md Outdated Show resolved Hide resolved
_search-plugins/search-pipelines/split-processor.md Outdated Show resolved Hide resolved
_search-plugins/search-pipelines/split-processor.md Outdated Show resolved Hide resolved
dbwiddis and others added 6 commits July 22, 2024 08:59
Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>
Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>
Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>
Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>
Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>
Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>
@dbwiddis
Copy link
Member Author

@dbwiddis @kolchfa-aws Please see my changes and let me know if you have any questions. Thanks!

Accepted all the recommendations!

@kolchfa-aws kolchfa-aws merged commit fd629ca into opensearch-project:main Jul 22, 2024
5 checks passed
@dbwiddis
Copy link
Member Author

Note I'm still waiting for the PRs to get merged (hopefully by today) :)

@kolchfa-aws
Copy link
Collaborator

No problem. If they are not merged, we can revert this PR 😄

@dbwiddis
Copy link
Member Author

Well one is now merged, other should be shortly, so we should be gtg!

@hdhalter hdhalter added 3 - Done Issue is done/complete release-notes PR: Include this PR in the automated release notes and removed 5 - Editorial review PR: Editorial review in progress labels Jul 22, 2024
@dbwiddis
Copy link
Member Author

And, the other one is merged. 🎉

@dbwiddis dbwiddis deleted the split-and-sort-processors branch July 22, 2024 19:16
leanneeliatra pushed a commit to leanneeliatra/opensearch-documentation-website-forl that referenced this pull request Jul 24, 2024
…ect#7767)

* Add documentation for Sort SearchRequestProcessor

Signed-off-by: Daniel Widdis <[email protected]>

* Add documentation for Split SearchRequestProcessor

Signed-off-by: Daniel Widdis <[email protected]>

* Doc review

Signed-off-by: Fanit Kolchina <[email protected]>

* Update _ingest-pipelines/processors/split.md

Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>

* Update _search-plugins/search-pipelines/sort-processor.md

Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>

* Update _search-plugins/search-pipelines/split-processor.md

Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>

* Update _search-plugins/search-pipelines/split-processor.md

Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>

* Update _search-plugins/search-pipelines/split-processor.md

Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>

* Update _search-plugins/search-pipelines/split-processor.md

Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>

---------

Signed-off-by: Daniel Widdis <[email protected]>
Signed-off-by: Fanit Kolchina <[email protected]>
Co-authored-by: Fanit Kolchina <[email protected]>
Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: [email protected] <[email protected]>
sandervandegeijn pushed a commit to sandervandegeijn/documentation-website that referenced this pull request Jul 30, 2024
…ect#7767)

* Add documentation for Sort SearchRequestProcessor

Signed-off-by: Daniel Widdis <[email protected]>

* Add documentation for Split SearchRequestProcessor

Signed-off-by: Daniel Widdis <[email protected]>

* Doc review

Signed-off-by: Fanit Kolchina <[email protected]>

* Update _ingest-pipelines/processors/split.md

Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>

* Update _search-plugins/search-pipelines/sort-processor.md

Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>

* Update _search-plugins/search-pipelines/split-processor.md

Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>

* Update _search-plugins/search-pipelines/split-processor.md

Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>

* Update _search-plugins/search-pipelines/split-processor.md

Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>

* Update _search-plugins/search-pipelines/split-processor.md

Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>

---------

Signed-off-by: Daniel Widdis <[email protected]>
Signed-off-by: Fanit Kolchina <[email protected]>
Co-authored-by: Fanit Kolchina <[email protected]>
Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: Sander van de Geijn <[email protected]>
@mingshl
Copy link
Contributor

mingshl commented Sep 3, 2024

@dbwiddis thanks for contributing the sort processor. I saw the functionality is very similar to the sort ingest processor.

But there is a difference between ingest and response: in the search response, there are multiple documents. Can we achieve sorting the documents in the response based on the value in sort_field?

for example: a sample search response:

{
  "took": 0,
  "timed_out": false,
  "_shards": {
    "total": 1,
    "successful": 1,
    "skipped": 0,
    "failed": 0
  },
  "hits": {
    "total": {
      "value": 4,
      "relation": "eq"
    },
    "max_score": 1,
    "hits": [
      {
        "_index": "diary_index",
        "_id": "1",
        "_score": 1,
        "_source": {
          "diary": "how are you",
          "similarity_score": -11.055182
        }
      },
      {
        "_index": "diary_index",
        "_id": "2",
        "_score": 1,
        "_source": {
          "diary": "today is sunny",
          "similarity_score": 8.969885
        }
      },
      {
        "_index": "diary_index",
        "_id": "3",
        "_score": 1,
        "_source": {
          "diary": "today is july fifth",
          "similarity_score": -5.736348
        }
      },
      {
        "_index": "diary_index",
        "_id": "4",
        "_score": 1,
        "_source": {
          "diary": "it is winter",
          "similarity_score": -10.045217
        }
      }
    ]
  }
}

can we sort by the similarity_score field using sort processor?
so the target response is sorting by similarity_score in descending order?

{
  "took": 0,
  "timed_out": false,
  "_shards": {
    "total": 1,
    "successful": 1,
    "skipped": 0,
    "failed": 0
  },
  "hits": {
    "total": {
      "value": 4,
      "relation": "eq"
    },
    "max_score": 1,
    "hits": [

{
        "_index": "diary_index",
        "_id": "2",
        "_score": 1,
        "_source": {
          "diary": "today is sunny",
          "similarity_score": 8.969885
        }
      },
      {
        "_index": "diary_index",
        "_id": "3",
        "_score": 1,
        "_source": {
          "diary": "today is july fifth",
          "similarity_score": -5.736348
        }
      },
      {
        "_index": "diary_index",
        "_id": "4",
        "_score": 1,
        "_source": {
          "diary": "it is winter",
          "similarity_score": -10.045217
        }
      }
      , {
        "_index": "diary_index",
        "_id": "1",
        "_score": 1,
        "_source": {
          "diary": "how are you",
          "similarity_score": -11.055182
        }
      }
    ]
  }
}

@dbwiddis
Copy link
Member Author

dbwiddis commented Sep 3, 2024

@mingshl that's not the spec I was asked to write to, it seems you'd want a "sort hits by field" functionality.

But aren't hits supposed to be sorted by score?

If you need a different functionality, please open a new issue to discuss it. Unfortunately we're within 24 hours of code freeze so there's no way it will get into 2.17.0.

@mingshl
Copy link
Contributor

mingshl commented Sep 3, 2024

@mingshl that's not the spec I was asked to write to, it seems you'd want a "sort hits by field" functionality.

But aren't hits supposed to be sorted by score?

If you need a different functionality, please open a new issue to discuss it. Unfortunately we're within 24 hours of code freeze so there's no way it will get into 2.17.0.

We will need a new RFC to track the rank processor to sort the search hits based on the field in the hit.

@mingshl
Copy link
Contributor

mingshl commented Sep 5, 2024

@mingshl that's not the spec I was asked to write to, it seems you'd want a "sort hits by field" functionality.
But aren't hits supposed to be sorted by score?
If you need a different functionality, please open a new issue to discuss it. Unfortunately we're within 24 hours of code freeze so there's no way it will get into 2.17.0.

We will need a new RFC to track the rank processor to sort the search hits based on the field in the hit.

Raised new RFC opensearch-project/OpenSearch#15631

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Done Issue is done/complete release-notes PR: Include this PR in the automated release notes v2.16.0
Projects
None yet
6 participants