Skip to content

Conversation

afoucret
Copy link
Contributor

This PR implement text extraction for the KQL parser quoted and unquoted strings.

@afoucret afoucret added >non-issue auto-backport Automatically create backport pull requests when merged :Search Relevance/Search Catch all for Search Relevance v9.0.0 v8.17.0 labels Oct 25, 2024
@elasticsearchmachine elasticsearchmachine added the Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch label Oct 25, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search-relevance (Team:Search Relevance)

Copy link
Contributor

@tteofili tteofili left a comment

Choose a reason for hiding this comment

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

LGTM thanks Aurelien ;)

Copy link
Member

@carlosdelest carlosdelest left a comment

Choose a reason for hiding this comment

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

LGTM! 💯

afoucret and others added 3 commits October 25, 2024 16:18
Co-authored-by: Carlos Delgado <6339205+carlosdelest@users.noreply.github.com>
Copy link
Member

@kderusso kderusso left a comment

Choose a reason for hiding this comment

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

Nice work!

if (preserveWildcards) {
StringBuilder escapedQuery = new StringBuilder(queryText.length());
StringBuilder subpart = new StringBuilder(queryText.length());
for (int i = 0; i < queryText.length(); i++) {
Copy link
Member

Choose a reason for hiding this comment

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

I think you could write this a little more readable, something like:

for (char currentChar : queryText.toCharArray()) {
    if (currentChar == '*') {
        escapedQuery.append(QueryParser.escape(subpart.toString())).append(currentChar);
        subpart.setLength(0); 
    } else {
        subpart.append(currentChar);
    }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Thank you!

StringBuilder subpart = new StringBuilder(queryText.length());
for (int i = 0; i < queryText.length(); i++) {
char currentChar = queryText.charAt(i);
if (currentChar == '*') {
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick - would be nice to use a constant for WILDCARD_CHAR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Thank you!

Copy link
Member

Choose a reason for hiding this comment

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

Nice tests!

afoucret and others added 3 commits October 25, 2024 17:10
@afoucret afoucret merged commit 29d1d9e into elastic:main Oct 25, 2024
16 checks passed
afoucret added a commit to afoucret/elasticsearch that referenced this pull request Oct 25, 2024
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.x

jfreden pushed a commit to jfreden/elasticsearch that referenced this pull request Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged >non-issue :Search Relevance/Search Catch all for Search Relevance Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch v8.17.0 v9.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants