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

Disable TO_UPPER(null) BWC tests prior to 8.17 #119213

Merged

Conversation

bpintea
Copy link
Contributor

@bpintea bpintea commented Dec 23, 2024

TO_UPPER/TO_LOWER resolution incorrectly returned child's type (that could also be null, type NULL), instead of KEYWORD/TEXT. So a test like TO_UPPER(null) == "..." fails on type mismatch.
This was fixed collaterally by #114334 (8.17.0)

Also, correct some of the tests skipping (that had however no impact, due to testing range).

TO_UPPER/TO_LOWER incorrectly returned child's type, instead of KEYWORD.
So a test like `TO_UPPER(null) == "..."` failed on type mismatch.
@bpintea bpintea added >test Issues or PRs that are addressing/adding tests auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Analytics/ES|QL AKA ESQL v9.0.0 v8.18.0 labels Dec 23, 2024
@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Dec 23, 2024
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine elasticsearchmachine merged commit edb3818 into elastic:main Dec 23, 2024
16 checks passed
@bpintea bpintea deleted the test/disable_to_case_in_some_bwc branch December 23, 2024 14:16
bpintea added a commit to bpintea/elasticsearch that referenced this pull request Dec 23, 2024
TO_UPPER/TO_LOWER resolution incorrectly returned child's type (that
could also be `null`, type `NULL`), instead of KEYWORD/TEXT. So a test
like `TO_UPPER(null) == "..."` fails on type mismatch. This was fixed
collaterally by elastic#114334 (8.17.0)

Also, correct some of the tests skipping (that had however no impact,
due to testing range).

(cherry picked from commit edb3818)
elasticsearchmachine pushed a commit that referenced this pull request Dec 23, 2024
* ESQL: Rewrite TO_UPPER/TO_LOWER comparisons (#118870)

This adds an optimization rule to rewrite TO_UPPER/TO_LOWER comparisons
against a string into an InsensitiveEquals comparison. The rewrite can
also result right away into a TRUE/FALSE, in case the string doesn't
match the caseness of the function.

This also allows later pushing down the predicate to lucene as a
case-insensitive term-query.

Fixes #118304.

* Disable `TO_UPPER(null)`-tests prior to 8.17 (#119213)

TO_UPPER/TO_LOWER resolution incorrectly returned child's type (that
could also be `null`, type `NULL`), instead of KEYWORD/TEXT. So a test
like `TO_UPPER(null) == "..."` fails on type mismatch. This was fixed
collaterally by #114334 (8.17.0)

Also, correct some of the tests skipping (that had however no impact,
due to testing range).

(cherry picked from commit edb3818)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/ES|QL AKA ESQL auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) >test Issues or PRs that are addressing/adding tests v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants