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

Adds startsWith and nstartsWith to Elastic Search #5020

Merged
merged 5 commits into from
May 8, 2022

Conversation

frzmohammadali
Copy link

Summary of the changes

Two filter operation handlers has been added for string operations with ElasticSearch:

  • startsWith
  • nstartsWith

Elastic search query to be used for these operations is the PrefixQuery under term-level type of queries.

Tests are available in StringPrefixTests.cs file.

Feedbacks are more than welcome ;)

Copy link
Member

@PascalSenn PascalSenn left a comment

Choose a reason for hiding this comment

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

@frzmohammadali Awesome work! This PR looks already really good :)

A few things i noticed:

  • We have a pretty strict 100 characters line length. Can you break the queries when possible?
  • Could it be that there are some query snapshots missing or is there something wrong with the test helper?

@frzmohammadali
Copy link
Author

@PascalSenn Thanks a lot for your feedback. I'll take care of the points you mentioned and submit an update soon (this evening hopefully) :)

Copy link
Member

@PascalSenn PascalSenn left a comment

Choose a reason for hiding this comment

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

Very well done ali!

@PascalSenn PascalSenn changed the title Mof/elasticsearch startsWith and nstartsWith string filter operations Adds startsWith and nstartsWith to Elastic Search May 8, 2022
@PascalSenn PascalSenn merged commit 910a84b into feat/elastic-search May 8, 2022
@PascalSenn PascalSenn deleted the mof/elasticsearch-string-ops branch May 8, 2022 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants