ESQL: add =~ operator (case insensitive equality) by luigidellaquila · Pull Request #103656 · elastic/elasticsearch

luigidellaquila · 2023-12-21T15:40:40Z

Add an operator to perform case insensitive equality comparison of strings.
The operator symbol is =~, eg.

row a = "FooBar" | where a =~ "foobar"

~~The operator acts as a normal == for all non-string types~~
The operator is defined only for string values

For multi-values the behavior is consistent with ==, ie. multi_value =~ "string" returns null

Related to #103599

elasticsearchmachine · 2023-12-27T11:47:10Z

Pinging @elastic/es-ql (Team:QL)

elasticsearchmachine · 2023-12-27T11:47:10Z

Pinging @elastic/elasticsearch-esql (:Query Languages/ES|QL)

costin

No need to touch the QL side - see my comments.
Also the operator need to work on multi-value fields - please add unwrapping and tests.

costin · 2023-12-28T05:29:12Z

x-pack/plugin/esql/qa/testFixtures/src/main/resources/ignore-case.csv-spec

+
+evalEquals#[skip:-8.12.99, reason:case insensitive operators implemented in v 8.13]
+from employees | where emp_no == 10001 
+| eval a = first_name =~ "georgi", b = first_name == "georgi", c = first_name =~ "GEORGI", d = first_name =~ "Geor" 


try "GeoRgI" or something similar just in cae.

costin · 2023-12-28T05:31:19Z

x-pack/plugin/esql/src/main/antlr/EsqlBaseLexer.g4

 TRUE : 'true';

 EQ  : '==';
+EQ_IGNORE_CASE  : '=~';


I propose we use SEQ for "string equality" (just like in EQL) or IEQ for "Insensitive Equality".
Same for the backing class.

costin · 2023-12-28T05:35:40Z

...va/org/elasticsearch/xpack/ql/expression/predicate/operator/comparison/EqualsIgnoreCase.java

+
+import static org.elasticsearch.xpack.ql.expression.TypeResolutions.ParamOrdinal.DEFAULT;
+
+public class EqualsIgnoreCase extends BinaryComparison {


StringEquals or InsensitiveEquals

costin · 2023-12-28T05:35:56Z

...in/java/org/elasticsearch/xpack/ql/expression/predicate/operator/comparison/Comparisons.java

        return i == null ? null : i.intValue() == 0;
    }

+    public static Boolean eqIgnoreCase(Object l, Object r) {


Who's using this method?
If needed see StringComparisons inside EQL

Removed (together with all the other changes in QL)

costin · 2023-12-28T05:36:22Z

...va/org/elasticsearch/xpack/ql/expression/predicate/operator/comparison/EqualsIgnoreCase.java

+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+package org.elasticsearch.xpack.ql.expression.predicate.operator.comparison;


This should be in the ESQL package.

costin · 2023-12-28T05:39:10Z

x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/planner/ExpressionTranslators.java

                return new RangeQuery(source, name, null, false, value, true, format, zoneId);
            }
-            if (bc instanceof Equals || bc instanceof NullEquals || bc instanceof NotEquals) {
+            if (bc instanceof Equals || bc instanceof NullEquals || bc instanceof NotEquals || bc instanceof EqualsIgnoreCase) {


No need to touch QL - see InsensitiveBinaryComparisons inside EQL on how to extend the method (add a dedicated TranslatorHandler).

luigidellaquila · 2023-12-28T16:56:57Z

Thanks for the feedback @costin, I pushed some changes and moved all the logic in ESQL module (apart from one small fix in QL).

Also the operator need to work on multi-value fields

I think this aspect needs some clarifications, let's move the discussion off-line

elasticsearchmachine · 2024-01-02T19:46:43Z

Pinging @elastic/es-analytics-geo (Team:Analytics)

astefan

What's the expected behavior when doing something like first_name =~ last_name?
The original issue this PR covers says

Introduce =~ operator for performing case insensitive equality between a string field and a string literal
The field can be both single and multi value. The literal has to be single value.

Should I see an error, or warning or did the requirements change in the meantime and the original issue and/or the PR description needs an update?

On a similar note, if I do where last_name =~ 123 I get no results and no relevant warning. What is the expected behavior in this case?

Another similar behavior is with where last_name =~ languages where I get a CCE:

class org.elasticsearch.compute.data.IntArrayBlock cannot be cast to class org.elasticsearch.compute.data.BytesRefBlock

but if I do where last_name == languages I am getting a more meaningful error message:

"reason": "Found 1 problem\nline 1:24: first argument of [last_name == languages] is [keyword] so second argument must also be [keyword] but was [integer]",
                "stack_trace": "org.elasticsearch.xpack.esql.analysis.VerificationException: Found 1 problem\nline 1:24: first argument of [last_name == languages] is [keyword] so second argument must also be [keyword] but was [integer]

astefan · 2024-01-04T14:25:12Z

This test also fails row last_name = "lortz" | where "lOrTz" =~ last_name:

org.elasticsearch.xpack.ql.QlIllegalArgumentException: Should not fold expression\r\n\tat org.elasticsearch.xpack.ql.expression.Expression.fold(Expression.java:82)\r\n\tat org.elasticsearch.xpack.ql.expression.Literal.of(Literal.java:110)\r\n\tat org.elasticsearch.xpack.ql.optimizer.OptimizerRules$ConstantFolding.rule(OptimizerRules.java:101)

whereas row last_name = "lortz" | where "lortz" == last_name doesn't.

luigidellaquila · 2024-01-04T15:29:52Z

What's the expected behavior when doing something like first_name =~ last_name?

I don't have a strong opinion on this.
I supported it in the implementation just because also == supports it, but there is an optimization in case the right-hand side is foldable.
On the other hand, LIKE and RLIKE accept only a literal, so =~ could be the same.

For sure having a string literal there will reduce validation risks, solving possible problems at parsing time.

On a similar note, if I do where last_name =~ 123 I get no results and no relevant warning

I think it's a bug (lack of validation), it should throw an exception as == does.

Another similar behavior is with where last_name =~ languages where I get a CCE:

same as above. I'll check and fix it.

This test also fails row last_name = "lortz" | where "lOrTz" =~ last_name:

this is interesting, I'll investigate

Thanks!

astefan · 2024-01-04T15:35:51Z

What's the expected behavior when doing something like first_name =~ last_name?

I don't have a strong opinion on this. I supported it in the implementation just because also == supports it, but there is an optimization in case the right-hand side is foldable. On the other hand, LIKE and RLIKE accept only a literal, so =~ could be the same.

If this is supported on purpose, then there should be tests covering it.
But, at the same time, the original issue mentions a literal. My questions spawned from this confusion between the issue description and the implementation itself which should be clarified.

luigidellaquila · 2024-01-05T17:00:12Z

Thanks for you feedback @astefan
I pushed two fixes and more tests to cover validation and field-to-field comparison.

luigidellaquila · 2024-01-17T14:06:48Z

@elasticmachine update branch

… esql/equals_tilde

elasticsearchmachine · 2024-01-18T09:00:13Z

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

costin

Thanks Luigi - this looks pretty complete.
I've added a comment regarding testing and whether field attributes are allowed or not both sides.

costin · 2024-01-19T03:08:15Z

x-pack/plugin/esql/qa/testFixtures/src/main/resources/ignore-case.csv-spec

It's not clear from the tests if the right hand side allows only literals (the original requirement), folded expressions or generic expressions (which you added).
I think it's the first variant but I don't see any tests validating this.
So please add more test to either validate that literals/folded expressions are required (and fields are not allowed) or vice-versa - queries that have fields on both sides and more over expressions:
where concat(field, "constant") =~ concat(field, concat("con", "stant)) etc...

Thanks @costin
Yes, the implementation supports any kind of expression, both on the left and on the right.
I added a few more tests for this.

costin

Please add some tests between multi-values and single values.

And until we figure out #103599 fully when it comes to mv and wildcard behavior, please disable/prevent usage of this operator between fields in the Verifier.

costin · 2024-01-24T18:37:46Z

x-pack/plugin/esql/qa/testFixtures/src/main/resources/ignore-case.csv-spec

+
+
+expressionsLeftRight#[skip:-8.12.99, reason:case insensitive operators implemented in v 8.13]
+from employees | where substring(first_name, 1, 2) =~ substring(last_name, -2) | keep emp_no, first_name, last_name | sort emp_no;


I've thought about this some more and we need to conclude how to address the missing requirements (multi-value and wildcard matching) from
#103599 before enabling this functionality.
If =~ is asymmetric like EQL : than comparing two fields can't happen. in ESQL we might do thing differently but until we reach a conclusion, I prefer we don't ship these features to begin with; it's better to enable them later than have them disabled.

In other words, please keep the comparison only between a literal/folded expression and a field but not between two fields.

in ESQL we might do thing differently but until we reach a conclusion, I prefer we don't ship these features to begin with

👍 I think it's a wise decision, I'll disable field vs field comparison at validation time (I'll allow only foldable expressions on the right) until we have a final decision on all the aspects.

luigidellaquila · 2024-01-25T09:09:05Z

@elasticmachine update branch

costin

Thanks for addressing the feedback - let's get this in!

## Summary Keep in sync with new ES|QL builtin function addition: elastic/elasticsearch#103656 Do not merge until the related ER PR is still in review. ### Checklist - [x] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios

luigidellaquila added 2 commits December 21, 2023 12:46

Introduce =~ (case insensitive equals) operator

87f2ec8

Lucene pushdown

4c1e89c

luigidellaquila added >feature :Analytics/ES|QL AKA ESQL labels Dec 21, 2023

Fix pushdown for non-strings

67ac517

luigidellaquila requested a review from costin December 27, 2023 11:46

luigidellaquila marked this pull request as ready for review December 27, 2023 11:46

elasticsearchmachine added the Team:QL (Deprecated) Meta label for query languages team label Dec 27, 2023

luigidellaquila added the v8.13.0 label Dec 27, 2023

costin requested a review from astefan December 28, 2023 05:28

costin requested changes Dec 28, 2023

View reviewed changes

luigidellaquila added 2 commits December 28, 2023 11:59

Implement constant evaluator using Automaton

b268716

Implement review suggestions

c54f444

wchaparro added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Jan 2, 2024

elasticsearchmachine removed the Team:QL (Deprecated) Meta label for query languages team label Jan 2, 2024

luigidellaquila mentioned this pull request Jan 3, 2024

ESQL: Add case-insensitive behavior #103599

Open

9 tasks

astefan reviewed Jan 4, 2024

View reviewed changes

luigidellaquila added 2 commits January 5, 2024 16:58

Fix folding and verification

e002329

Merge branch 'main' into esql/equals_tilde

1f16348

luigidellaquila requested review from a team as code owners January 5, 2024 16:57

luigidellaquila added 4 commits January 16, 2024 16:21

Merge branch 'main' into esql/equals_tilde

929b34f

Simplify validation

5bca81c

Reduce the scope to exact match (no wildcards)

1999e3e

More tests

aac2340

Merge branch 'main' into esql/equals_tilde

65941c9

luigidellaquila requested a review from costin January 18, 2024 08:21

luigidellaquila added 2 commits January 18, 2024 09:59

Merge branch 'main' into esql/equals_tilde

4051636

Merge remote-tracking branch 'luigidellaquila/esql/equals_tilde' into…

84e58d7

… esql/equals_tilde

costin reviewed Jan 19, 2024

View reviewed changes

More tests

c13ab85

luigidellaquila requested a review from costin January 24, 2024 08:27

costin requested changes Jan 24, 2024

View reviewed changes

Implement review suggestions

e8023eb

Merge branch 'main' into esql/equals_tilde

453a77f

luigidellaquila requested a review from costin January 25, 2024 10:23

costin approved these changes Jan 25, 2024

View reviewed changes

luigidellaquila merged commit 79b7dbb into elastic:main Jan 26, 2024

luigidellaquila mentioned this pull request Jan 29, 2024

ESQL: docs for =~ operator #104844

Open

leemthompo mentioned this pull request Feb 1, 2024

[DOCS][ESQL] Document case insensitive equals operator #105031

Closed

costin mentioned this pull request Feb 19, 2024

ESQL: Temporary disable =~ case-insensitive operator #105603

Closed


		import static org.elasticsearch.xpack.ql.expression.TypeResolutions.ParamOrdinal.DEFAULT;

		public class EqualsIgnoreCase extends BinaryComparison {



		expressionsLeftRight#[skip:-8.12.99, reason:case insensitive operators implemented in v 8.13]
		from employees \| where substring(first_name, 1, 2) =~ substring(last_name, -2) \| keep emp_no, first_name, last_name \| sort emp_no;

Conversation

luigidellaquila commented Dec 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticsearchmachine commented Dec 27, 2023

Uh oh!

elasticsearchmachine commented Dec 27, 2023

Uh oh!

costin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luigidellaquila commented Dec 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticsearchmachine commented Jan 2, 2024

Uh oh!

astefan left a comment

Choose a reason for hiding this comment

Uh oh!

astefan commented Jan 4, 2024

Uh oh!

luigidellaquila commented Jan 4, 2024

Uh oh!

astefan commented Jan 4, 2024

Uh oh!

luigidellaquila commented Jan 5, 2024

Uh oh!

luigidellaquila commented Jan 17, 2024

Uh oh!

elasticsearchmachine commented Jan 18, 2024

Uh oh!

costin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

costin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luigidellaquila commented Jan 25, 2024

Uh oh!

costin left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

luigidellaquila commented Dec 21, 2023 •

edited

Loading

luigidellaquila commented Dec 28, 2023 •

edited

Loading