Skip to content

Fix ES|QL locate with supplementary (4-byte) character#107172

Merged
ChrisHegarty merged 13 commits intoelastic:mainfrom
ChrisHegarty:esql_supp_char
Apr 8, 2024
Merged

Fix ES|QL locate with supplementary (4-byte) character#107172
ChrisHegarty merged 13 commits intoelastic:mainfrom
ChrisHegarty:esql_supp_char

Conversation

@ChrisHegarty
Copy link
Copy Markdown
Contributor

This commit fixes the ES|QL locate with supplementary (4-byte) character.

@ChrisHegarty ChrisHegarty added >refactoring Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) :Analytics/ES|QL AKA ESQL labels Apr 5, 2024
@ChrisHegarty ChrisHegarty requested review from nik9000 and tteofili April 5, 2024 21:08
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-analytical-engine (Team:Analytics)

assertThat(process("界世", "界世", 0), equalTo(1));
}

public void testSupplementaryCharacter() {
Copy link
Copy Markdown
Contributor Author

@ChrisHegarty ChrisHegarty Apr 5, 2024

Choose a reason for hiding this comment

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

I put all these tests in one place, because supplementary chars hurt ones head - it's easier to reason about them when they're all together.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

After this is in I can see about breaking these in the the parameterize stuff.

@ChrisHegarty
Copy link
Copy Markdown
Contributor Author

@elasticmachine update branch

@ChrisHegarty
Copy link
Copy Markdown
Contributor Author

@elasticmachine update branch

@ChrisHegarty
Copy link
Copy Markdown
Contributor Author

@elasticmachine update branch

@ChrisHegarty
Copy link
Copy Markdown
Contributor Author

@elasticmachine update branch

Copy link
Copy Markdown
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.

thanks @ChrisHegarty for this, LGTM

assertThat(process("界世", "界世", 0), equalTo(1));
}

public void testSupplementaryCharacter() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

After this is in I can see about breaking these in the the parameterize stuff.

@ChrisHegarty ChrisHegarty merged commit 9edd67f into elastic:main Apr 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/ES|QL AKA ESQL >refactoring Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.14.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants