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.

Sign up for GitHub

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

Jump to bottom

[Refactor] Replace recursive Glob.globMatch with iterative approach #12065

Closed
2 tasks
cwperks opened this issue Jan 29, 2024 · 5 comments · Fixed by #13104
Closed
2 tasks

[Refactor] Replace recursive Glob.globMatch with iterative approach #12065

cwperks opened this issue Jan 29, 2024 · 5 comments · Fixed by #13104
Assignees
@niyatiagg
Labels
bug Something isn't working good first issue Good for newcomers Search Search query, autocomplete ...etc

Comments

@cwperks
Copy link
Member

cwperks commented Jan 29, 2024 •
edited
Loading

Describe the bug

This issue is related to a recent pull request: #11060

Similar to core's Regex class, there is another internal class called Glob which also uses recursion to evaluate a glob match. This method can also be updated to use an iterative approach.

Additional Context

In #11060, there was a recursive method for evaluating core's own Regex class. The method simpleMatchWithNormalizedString was refactored from a recursive approach to an iterative approach for more predictable space and runtime complexity.

  • Refactor Glob.globMatch to use iterative method
  • Update FilterPathTests to cover more scenarios

Related component

Search

To Reproduce

Should maintain same behavior as now

Expected behavior

Should maintain same behavior as now

Additional Details

No response
@cwperks cwperks added bug Something isn't working untriaged labels Jan 29, 2024
@github-actions github-actions bot added the Search Search query, autocomplete ...etc label Jan 29, 2024
@peternied peternied added good first issue Good for newcomers and removed untriaged labels Jan 31, 2024
@peternied
Copy link
Member

[Triage - attendees 1 2 3 4 5 6 7 8]
@cwperks thanks for filing, we'd gladly review a pull request for this.

@reta reta mentioned this issue Jan 31, 2024
Closed
@robinf95
Copy link
Contributor

robinf95 commented Feb 1, 2024

I'm currently working on it.

robinf95 added a commit to robinf95/OpenSearch that referenced this issue Feb 7, 2024
@robinf95
Implement unit tests for Glob.globMatch (opensearch-project#12065)
758a388 
Signed-off-by: Robin Friedmann <[email protected]>
robinf95 added a commit to robinf95/OpenSearch that referenced this issue Feb 7, 2024
@robinf95
Refactored Glob.globMatch by using iterative approach; added unit tes…
fa59c6c 
…ts (opensearch-project#12065)

Signed-off-by: Robin Friedmann <[email protected]>
This was referenced Feb 7, 2024
Closed
Closed
robinf95 added a commit to robinf95/OpenSearch that referenced this issue Feb 9, 2024
@robinf95
Fix failing test(opensearch-project#12065)
79117b6 
Signed-off-by: Robin Friedmann <[email protected]>

Refactored Glob.globMatch by using iterative approach; added unit tests (opensearch-project#12065)
Signed-off-by: Robin Friedmann <[email protected]>
robinf95 added a commit to robinf95/OpenSearch that referenced this issue Feb 9, 2024
@robinf95
Adjust Glob.globMatch algorithm; Add tests (opensearch-project#12065)
5b6ee3f 
Signed-off-by: Robin Friedmann <[email protected]>
@msfroh
Copy link
Collaborator

msfroh commented Apr 3, 2024

@jainankitk will find a new owner to get this PR across the line

@cwperks
Copy link
Member Author

cwperks commented Apr 3, 2024

@jainankitk I would volunteer to take on this issue. I fixed a very similar issue here.

@niyatiagg
Copy link
Contributor

niyatiagg commented Apr 5, 2024 •
edited
Loading

Can I pick this up?

@jainankitk jainankitk assigned niyatiagg and unassigned jainankitk Apr 5, 2024
@niyatiagg niyatiagg mentioned this issue Apr 5, 2024
Merged
7 tasks
@github-project-automation github-project-automation bot moved this from 🆕 New to ✅ Done in Search Project Board Apr 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@niyatiagg niyatiagg

Labels
bug Something isn't working good first issue Good for newcomers Search Search query, autocomplete ...etc
Projects
Search Project Board
Archived in project
Milestone
No milestone
6 participants
@msfroh @peternied @jainankitk @cwperks @robinf95 @niyatiagg