Skip to content

Conversation

@slobodanadamovic
Copy link
Contributor

This change avoids unnecessary substring allocations and recursion calls when more than two consecutive wildcards (*) are detected. Instead skipping and calling a method recursively, we now try to skip all consecutive * chars at once.

@slobodanadamovic slobodanadamovic self-assigned this Aug 3, 2023
@elasticsearchmachine elasticsearchmachine added v8.10.0 needs:triage Requires assignment of a team area label labels Aug 3, 2023
@slobodanadamovic slobodanadamovic added :Core/Infra/Core Core issues without another label Team:Core/Infra Meta label for core/infra team labels Aug 3, 2023
@elasticsearchmachine elasticsearchmachine removed the needs:triage Requires assignment of a team area label label Aug 3, 2023
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@elasticsearchmachine
Copy link
Collaborator

Hi @slobodanadamovic, I've created a changelog YAML for you.

Copy link
Contributor

@ldematte ldematte left a comment

Choose a reason for hiding this comment

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

Some minor comments on testing more edge cases, but looks good.

assertFalse(Regex.simpleMatch("fff******ddd", "fffabcdd"));
}

public void testArbitraryWildcardMatch() {
Copy link
Contributor

@ldematte ldematte Aug 3, 2023

Choose a reason for hiding this comment

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

Nit: to cover all branches, consider adding a use case that explicitly use multiple wildcards at the end of the pattern (maybe to the test before this one), and a use case where you have multiple wildcards in multiple locations (like dd**dd**dd).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion. I've initially randomized the test which should cover dd****, ****dd and dd****dd cases but not dd**dd**dd. I've changed the test to be more explicit in testing all cases in a single run instead of depending on randomization.

@slobodanadamovic slobodanadamovic added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Aug 4, 2023
@slobodanadamovic
Copy link
Contributor Author

@elasticmachine update branch

@slobodanadamovic
Copy link
Contributor Author

Failure seems unrelated and probably temporary:

Failed to fetch https://esm.ubuntu.com/ubuntu/dists/trusty-infra-updates/main/binary-amd64/Packages HttpError503 

@elasticmachine run elasticsearch-ci/part-1

@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.9
slobodanadamovic added a commit to slobodanadamovic/elasticsearch that referenced this pull request Aug 4, 2023
This change avoids unnecessary substring allocations and recursion calls when more than two consecutive wildcards (`*`) are detected. Instead skipping and calling a method recursively, we now try to skip all consecutive `*` chars at once.
elasticsearchmachine pushed a commit that referenced this pull request Aug 4, 2023
This change avoids unnecessary substring allocations and recursion calls when more than two consecutive wildcards (`*`) are detected. Instead skipping and calling a method recursively, we now try to skip all consecutive `*` chars at once.
@slobodanadamovic
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
7.17

Questions ?

Please refer to the Backport tool documentation

slobodanadamovic added a commit to slobodanadamovic/elasticsearch that referenced this pull request Aug 8, 2023
This change avoids unnecessary substring allocations and recursion calls when more than two consecutive wildcards (`*`) are detected. Instead skipping and calling a method recursively, we now try to skip all consecutive `*` chars at once. (cherry picked from commit 868df2a)
slobodanadamovic added a commit that referenced this pull request Aug 8, 2023
…98277) This change avoids unnecessary substring allocations and recursion calls when more than two consecutive wildcards (`*`) are detected. Instead skipping and calling a method recursively, we now try to skip all consecutive `*` chars at once.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Core/Infra/Core Core issues without another label >enhancement Team:Core/Infra Meta label for core/infra team v8.9.1 v8.10.0

4 participants