Skip to content

ESQL: Implement LOOKUP, an "inline" enrich#107987

Merged
elasticsearchmachine merged 105 commits intoelastic:mainfrom
nik9000:lookup_real___
Jun 7, 2024
Merged

ESQL: Implement LOOKUP, an "inline" enrich#107987
elasticsearchmachine merged 105 commits intoelastic:mainfrom
nik9000:lookup_real___

Conversation

@nik9000
Copy link
Copy Markdown
Member

@nik9000 nik9000 commented Apr 28, 2024

This adds support for LOOKUP, a command that implements a sort of
inline ENRICH, using data that is passed in the request:

$ curl -uelastic:password -HContent-Type:application/json -XPOST \
    'localhost:9200/_query?error_trace&pretty&format=txt' \
-d'{
    "query": "ROW a=1::LONG | LOOKUP t ON a",
    "tables": {
        "t": {
            "a:long":     [    1,     4,     2],
            "v1:integer": [   10,    11,    12],
            "v2:keyword": ["cat", "dog", "wow"]
        }
    },
    "version": "2024.04.01"
}'
      v1       |      v2       |       a       
---------------+---------------+---------------
10             |cat            |1              

This required these PRs:

Closes #107306

@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

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

@nik9000
Copy link
Copy Markdown
Member Author

nik9000 commented Apr 28, 2024

I've opened this as draft because I've not added any real tests. And because stuff like:

curl -uelastic:password -HContent-Type:application/json -XPOST 'localhost:9200/_query?error_trace&pretty' -d'{
    "query": "ROW a=1::LONG | LOOKUP t1 ON a | LOOKUP t2 ON v1",
    "tables": {
        "t1": {
            "a:long":     [1, 4, 2],
            "v1:long":    [5, 8, 9]
        },
        "t2": {
            "v1:long":    [    5,     8,     9],
            "v2:keyword": ["cat", "dog", "wow"]
        }
    },
    "version": "2024.04.01"
}'

fails with array index out of bounds exceptions that don't make sense to me yet.

@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Hi @nik9000, I've updated the changelog YAML for you.

@nik9000
Copy link
Copy Markdown
Member Author

nik9000 commented Apr 28, 2024

So I ran a little test:

curl -uelastic:password -XDELETE localhost:9200/test
for a in {0..99}; do
   rm -f /tmp/evil
   for b in {0..999}; do
      echo '{"index": {}}' >> /tmp/evil
      echo '{"a": 1}' >> /tmp/evil
      echo '{"index": {}}' >> /tmp/evil
      echo '{"a": 4}' >> /tmp/evil
      echo '{"index": {}}' >> /tmp/evil
      echo '{"a": 2}' >> /tmp/evil
   done
   echo >> /tmp/evil
   echo -n "$a:  "
   curl -s -HContent-Type:application/json -uelastic:password -XPOST localhost:9200/test/_bulk?pretty --data-binary @/tmp/evil | grep \"errors\"
done

curl -HContent-Type:application/json -uelastic:password -XPOST localhost:9200/test/_forcemerge?max_num_segments=1
curl -HContent-Type:application/json -uelastic:password -XPOST localhost:9200/test/_refresh

Then I ran these two a bunch:

curl -uelastic:password -HContent-Type:application/json -XPOST \
    'localhost:9200/_query?error_trace&pretty' \
-d'{
    "query": "FROM test | EVAL v1=a+1 | STATS COUNT(v1)",
    "profile": true,
    "version": "2024.04.01"
}'
curl -uelastic:password -HContent-Type:application/json -XPOST \
    'localhost:9200/_query?error_trace&pretty' \
-d'{
    "query": "FROM test | LOOKUP t ON a | STATS COUNT(v1)",
    "profile": true,
    "tables": {
        "t": {
            "a:long":     [    1,     4,     2],
            "v1:integer": [   10,    11,    12]
        }
    },
    "version": "2024.04.01"
}'

That's 300,000 documents - not many, but small enough to test fast on my laptop. From the profile EVAL v1=a + 1 took 1ms or about 3 nanoseconds per value. The actual addition is likely less than .3ns per operation but I guess the block accounting gets us slower. LOOKUP t ON a took 10ms or about 33ns per value. That's not scientific, but it's believable. About 28ns of those 33ns is hashing - which isn't a huge surprise - we know we're using our slowest hash implementation. We have faster hash implementations. We just have to flip a few things around to plug them in properly.

@nik9000
Copy link
Copy Markdown
Member Author

nik9000 commented Apr 28, 2024

My last push seems to have fixed double-layered lookup:

$ curl -uelastic:password -HContent-Type:application/json -XPOST 'localhost:9200/_query?error_trace&pretty' -d'{
    "query": "ROW a=1::LONG | LOOKUP t1 ON a | LOOKUP t2 ON v1",
    "tables": {
        "t1": {
            "a:long":     [1, 4, 2],
            "v1:long":    [5, 8, 9]
        },
        "t2": {
            "v1:long":    [    5,     8,     9],
            "v2:keyword": ["cat", "dog", "wow"]
        }
    },
    "version": "2024.04.01"
}'
{
  "columns" : [
    {
      "name" : "a",
      "type" : "long"
    },
    {
      "name" : "v1",
      "type" : "long"
    },
    {
      "name" : "v2",
      "type" : "keyword"
    }
  ],
  "values" : [
    [
      1,
      5,
      "cat"
    ]
  ]
}

@nik9000
Copy link
Copy Markdown
Member Author

nik9000 commented Apr 29, 2024

This needs a bunch more tests and docs.

This is a feature that'll likely stay in experimental for a few versions regardless of how difficult it is for us to settle the external facing stuff.


intIntByKeywordKeyword
required_capability: lookup
ROW aa="foo", ab="zoo"
Copy link
Copy Markdown
Member

@costin costin Jun 5, 2024

Choose a reason for hiding this comment

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

Please add a test where aa and ab (as lookup keys) have the same value - `row aa = "foo", ab="foo" | lookup big on aa, ab".
Followed by variations:

  • lookup on field with empty value
  • lookup on function over field:
    ~ right now it can throw a validation error
    ~ moving forward we should be able to support it (by extracting it into a synthetic eval)
    can be a follow-up.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

"lookup on function over field:" - you mean like LOOKUP foo ON CONCAT(a, b)?

Copy link
Copy Markdown
Member

@costin costin 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 Nik for making this happen and incorporating the various feedback received.

return new Lookup(source, lookup.child(), tableNameExpression, lookup.matchFields(), localRelation);
}

private LocalRelation tableMapAsRelation(Source source, Map<String, Column> mapTable) {
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.

Sorry, just saw this message now: this resolution of tables can be done in the PreAnalyze phase, similar to how we do it for enrich or the rest of the indices.

Added a task for it in #109353

Comment on lines +364 to +365
EsField field = new EsField(name, column.type(), Map.of(), false, false);
attributes.add(new FieldAttribute(source, null, name, field));
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.

The source of the attribute should be decoupled from the Attribute itself. MetadataAttribute itself is stretching the concept since it's just a special FieldAttribute (who's name starts with _).

}

public boolean duplicatesResolved() {
return left().outputSet().intersect(right().outputSet()).isEmpty();
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.

Then let's remove it.

case 2 -> RIGHT;
case 4 -> FULL;
case 5 -> CROSS;
default -> throw new IllegalArgumentException("unsupported join [" + id + "]");
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.

We have EsqlIllegalArgumentException to differentiate between exceptions thrown by ESQL code vs other packages/JDK.

Comment on lines +124 to +134
if (left instanceof FragmentExec) {
if (right instanceof FragmentExec) {
throw new EsqlIllegalArgumentException("can't plan binary [" + p.nodeName() + "]");
}
// in case of a fragment, push to it any current streaming operator
return new FragmentExec(p);
}
if (right instanceof FragmentExec) {
// in case of a fragment, push to it any current streaming operator
return new FragmentExec(p);
}
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.

we won't have general binary plans, because we cannot map them.

We don't have yet binary plans, but this PR is the first step in the right direction. see #99841

@costin
Copy link
Copy Markdown
Member

costin commented Jun 5, 2024

cc @astefan and @bpintea for awareness

@nik9000 nik9000 added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Jun 5, 2024
@nik9000
Copy link
Copy Markdown
Member Author

nik9000 commented Jun 5, 2024

OK! I've pushed the last update from a comment. I think we've extracted all of the unfinished comments into our lovely followup meta issue (#109353). I'm marking this auto-merge and beginning to land it. Thanks so much friends. 186 comments, 96 commits. Over a little more than a month. And 16 pre-PRs that had to land before that. And followups. This is a big one!

@nik9000
Copy link
Copy Markdown
Member Author

nik9000 commented Jun 7, 2024

Oh darn. I thought the bwc failures were because of a release, but that doesn't make sense - we didn't have a branch cut. they were real. I fixed them and pushed.

@elasticsearchmachine elasticsearchmachine merged commit 7916e6a into elastic:main Jun 7, 2024
@nik9000 nik9000 deleted the lookup_real___ branch June 7, 2024 01:39
craigtaverner pushed a commit to craigtaverner/elasticsearch that referenced this pull request Jun 11, 2024
This adds support for `LOOKUP`, a command that implements a sort of
inline `ENRICH`, using data that is passed in the request:

```
$ curl -uelastic:password -HContent-Type:application/json -XPOST \
    'localhost:9200/_query?error_trace&pretty&format=txt' \
-d'{
    "query": "ROW a=1::LONG | LOOKUP t ON a",
    "tables": {
        "t": {
            "a:long":     [    1,     4,     2],
            "v1:integer": [   10,    11,    12],
            "v2:keyword": ["cat", "dog", "wow"]
        }
    },
    "version": "2024.04.01"
}'
      v1       |      v2       |       a
---------------+---------------+---------------
10             |cat            |1
```

This required these PRs: * elastic#107624 * elastic#107634 * elastic#107701 * elastic#107762 *

Closes elastic#107306
craigtaverner added a commit to craigtaverner/elasticsearch that referenced this pull request Jun 11, 2024
The second prototype replaced MultiTypeField.Unresolved with MultiTypeField, but this clashed with existing behaviour around mapping unused MultiTypeFields to `unsupported` and `null`, so this new attempt simply adds new fields, resulting in more than one field with the same name.
We still need to store this new field in EsRelation, so that physical planner can insert it into FieldExtractExec, so this is quite similar to the second protototype.

The following query works in this third prototype:

```
multiIndexIpString
FROM sample_data* METADATA _index
| EVAL client_ip = TO_IP(client_ip)
| KEEP _index, @timestamp, client_ip, event_duration, message
| SORT _index ASC, @timestamp DESC
```

As with the previous prototyep, we no longer need an aggregation to force the conversion function onto the data node, as the 'real' conversion is now done at field extraction time using the converter function previously saved in the EsRelation and replanned into the EsQueryExec.

Support row-stride-reader for LoadFromMany

Add missing ESQL version after rebase on main

Fixed missing block release

Simplify UnresolvedUnionTypes

Support other commands, notably WHERE

Update docs/changelog/107545.yaml

Fix changelog

Removed unused code

Slight code reduction in analyser of union types

Removed unused interface method

Fix bug in copying blocks (array overrun)

Convert MultiTypeEsField.UnresolvedField back to InvalidMappedField

This is to ensure older behaviour still works.

Simplify InvalidMappedField support

Rather than complex code to recreate InvalidMappedField from MultiTypeEsField.UnresolvedField, we rely on the fact that this is the parent class anyway, so we can resolve this during plan serialization/deserialization anyway. Much simpler

Simplify InvalidMappedField support further

Combining InvalidMappedField and MultiTypeEsField.UnresolvedField into one class simplifies plan serialization even further.

InvalidMappedField is used slightly differently in QL

We need to separate the aggregatable used in the original really-invalid mapped field from the aggregatable used if the field can indeed be used as a union-type in ES|QL.

Updated version limitation after 8.14 branch

Try debug CI failures in multi-node clusters

Support type conversion in rowstride reader on single leaf

Disable union_types from CsvTests

Keep track of per-shard converters for LoadFromMany

Simplify block loader convert function

Code cleanup

Added unit test for ValuesSourceReaderOperator including field type conversions at block loading

Added test for @timestamp and fixed related bug

It turns out that most, but not all, DataType values have the same esType as typeName, and @timestamp is one that does not, using `date` for esType and `datetime` for typename. Our EsqlIndexResolver was recording multi-type fields with `esType`, while later the actual type conversion was using an evaluator that relied on DataTypes.typeFromName(typeName).
So we fixed the EsqlIndexResolver to rather use typeName.

Added more tests, with three indices combined and two type conversions

Disable lucene-pushdown on union-type fields

Since the union-type rewriter replaced conversion functions with new FieldAttributes, these were passing the check for being possible to push-down, which was incorrect. Now we prevent that.

Set union-type aggregatable flag to false always

This simplifies the push-down check.

Fixed tests after rebase on main

Add unit tests for union-types (same field, different type)

Remove generic warnings

Test code cleanup and clarifying comments

Remove -IT_tests_only in favor of CsvTests assumeFalse

Improved comment

Code review updates

Code review updates

Remove changes to ql/EsRelation

And it turned out the latest version of union type no longer needed these changes anyway, and was using the new EsRelation in the ESQL module without these changes.

Port InvalidMappedField to ESQL

Note, this extends the QL version of InvalidMappedField, so is not a complete port. This is necessary because of the intertwining of QL IndexResolver and EsqlIndexResolver. Once those classes are disentangled, we can completely break InvalidMappedField from QL and make it a forbidden type.

Fix capabilities line after rebase on main

Revert QL FieldAttribute and extend with ESQL FieldAttribute

So as to remove any edits to QL code, we extend FieldAttribute in the ESQL code with the changes required, since is simply to include the `field` in the hascode and equals methods.

Revert "Revert QL FieldAttribute and extend with ESQL FieldAttribute"

This reverts commit 168c6c75436e26b83e083cd3de8e18062e116bc9.

Switch UNION_TYPES from EsqlFeatures to EsqlCapabilities

Make hashcode and equals aligned

And removed unused method from earlier union-types work where we kept the NodeId during re-writing (which we no longer do).

Replace required_feature with required_capability after rebase

Switch union_types capability back to feature, because capabilities do not work in mixed clusters

Revert "Switch union_types capability back to feature, because capabilities do not work in mixed clusters"

This reverts commit 56d58bedf756dbad703c07bf4cdb991d4341c1ae.

Added test for multiple columns from same fields

Both IP and Date are tested

Fix bug with incorrectly resolving invalid types

And added more tests

Fixed bug with multiple fields of same name

This fix simply removes the original field already at the EsRelation level, which covers all test cases but has the side effect of having the final field no-longer be unsupported/null when the alias does not overwrite the field with the same name.
This is not exactly the correct semantic intent.
The original field name should be unsupported/null unless the user explicitly overwrote the name with `field=TO_TYPE(field)`, which effectively deletes the old field anyway.

Fixed bug with multiple conversions of the same field

This also fixes the issue with the previous fix that incorrectly reported the converted type for the original field.

More tests with multiple fields and KEEP/DROP combinations

Replace skip with capabilities in YML tests

Fixed missing ql->esql import change afer merging main

Merged two InvalidMappedField classes

After the QL code was ported to esql.core, we can now make the edits directly in InvalidMappedField instead of having one extend the other.

Move FieldAttribute edits from QL to ESQL

ESQL: Prepare analyzer for LOOKUP (elastic#109045)

This extracts two fairly uncontroversial changes that were in the main
LOOKUP PR into a smaller change that's easier to review.

ESQL: Move serialization for EsField (elastic#109222)

This moves the serialization logic for `EsField` into the `EsField`
subclasses to better align with the way rest of Elasticsearch works. It
also switches them from ESQL's home grown `writeNamed` thing to
`NamedWriteable`. These are wire compatible with one another.

ESQL: Move serialization of `Attribute` (elastic#109267)

This moves the serialization of `Attribute` classes used in ESQL into
the classes themselves to better line up with the rest of Elasticsearch.

ES|QL: add MV_APPEND function (elastic#107001)

Adding `MV_APPEND(value1, value2)` function, that appends two values
creating a single multi-value. If one or both the inputs are
multi-values, the result is the concatenation of all the values, eg.

```
MV_APPEND([a, b], [c, d]) -> [a, b, c, d]
```

~I think for this specific case it makes sense to consider `null` values
as empty arrays, so that~ ~MV_APPEND(value, null) -> value~ ~It is
pretty uncommon for ESQL (all the other functions, apart from
`COALESCE`, short-circuit to `null` when one of the values is null), so
let's discuss this behavior.~

[EDIT] considering the feedback from Andrei, I changed this logic and
made it consistent with the other functions: now if one of the
parameters is null, the function returns null

[ES|QL] Convert string to datetime when the other size of an arithmetic operator is date_period or time_duration (elastic#108455)

* convert string to datetime when the other side of binary operator is temporal amount

ESQL: Move `NamedExpression` serialization (elastic#109380)

This moves the serialization for the remaining `NamedExpression`
subclass into the class itself, and switches all direct serialization of
`NamedExpression`s to `readNamedWriteable` and friends. All other
`NamedExpression` subclasses extend from `Attribute` who's serialization
was moved ealier. They are already registered under the "category class"
for `Attribute`. This also registers them as `NamedExpression`s.

ESQL: Implement LOOKUP, an "inline" enrich (elastic#107987)

This adds support for `LOOKUP`, a command that implements a sort of
inline `ENRICH`, using data that is passed in the request:

```
$ curl -uelastic:password -HContent-Type:application/json -XPOST \
    'localhost:9200/_query?error_trace&pretty&format=txt' \
-d'{
    "query": "ROW a=1::LONG | LOOKUP t ON a",
    "tables": {
        "t": {
            "a:long":     [    1,     4,     2],
            "v1:integer": [   10,    11,    12],
            "v2:keyword": ["cat", "dog", "wow"]
        }
    },
    "version": "2024.04.01"
}'
      v1       |      v2       |       a
---------------+---------------+---------------
10             |cat            |1
```

This required these PRs: * elastic#107624 * elastic#107634 * elastic#107701 * elastic#107762 *

Closes elastic#107306

parent 32ac5ba755dd5c24364a210f1097ae093fdcbd75
author Craig Taverner <craig@amanzi.com> 1717779549 +0200
committer Craig Taverner <craig@amanzi.com> 1718115775 +0200

Fixed compile error after merging in main

Fixed strange merge issues from main

Remove version from ES|QL test queries after merging main

Fixed union-types on nested fields

Switch to Luigi's solution, and expand nested tests

Cleanup after rebase
craigtaverner added a commit that referenced this pull request Jun 19, 2024
* Union Types Support

The second prototype replaced MultiTypeField.Unresolved with MultiTypeField, but this clashed with existing behaviour around mapping unused MultiTypeFields to `unsupported` and `null`, so this new attempt simply adds new fields, resulting in more than one field with the same name.
We still need to store this new field in EsRelation, so that physical planner can insert it into FieldExtractExec, so this is quite similar to the second protototype.

The following query works in this third prototype:

```
multiIndexIpString
FROM sample_data* METADATA _index
| EVAL client_ip = TO_IP(client_ip)
| KEEP _index, @timestamp, client_ip, event_duration, message
| SORT _index ASC, @timestamp DESC
```

As with the previous prototyep, we no longer need an aggregation to force the conversion function onto the data node, as the 'real' conversion is now done at field extraction time using the converter function previously saved in the EsRelation and replanned into the EsQueryExec.

Support row-stride-reader for LoadFromMany

Add missing ESQL version after rebase on main

Fixed missing block release

Simplify UnresolvedUnionTypes

Support other commands, notably WHERE

Update docs/changelog/107545.yaml

Fix changelog

Removed unused code

Slight code reduction in analyser of union types

Removed unused interface method

Fix bug in copying blocks (array overrun)

Convert MultiTypeEsField.UnresolvedField back to InvalidMappedField

This is to ensure older behaviour still works.

Simplify InvalidMappedField support

Rather than complex code to recreate InvalidMappedField from MultiTypeEsField.UnresolvedField, we rely on the fact that this is the parent class anyway, so we can resolve this during plan serialization/deserialization anyway. Much simpler

Simplify InvalidMappedField support further

Combining InvalidMappedField and MultiTypeEsField.UnresolvedField into one class simplifies plan serialization even further.

InvalidMappedField is used slightly differently in QL

We need to separate the aggregatable used in the original really-invalid mapped field from the aggregatable used if the field can indeed be used as a union-type in ES|QL.

Updated version limitation after 8.14 branch

Try debug CI failures in multi-node clusters

Support type conversion in rowstride reader on single leaf

Disable union_types from CsvTests

Keep track of per-shard converters for LoadFromMany

Simplify block loader convert function

Code cleanup

Added unit test for ValuesSourceReaderOperator including field type conversions at block loading

Added test for @timestamp and fixed related bug

It turns out that most, but not all, DataType values have the same esType as typeName, and @timestamp is one that does not, using `date` for esType and `datetime` for typename. Our EsqlIndexResolver was recording multi-type fields with `esType`, while later the actual type conversion was using an evaluator that relied on DataTypes.typeFromName(typeName).
So we fixed the EsqlIndexResolver to rather use typeName.

Added more tests, with three indices combined and two type conversions

Disable lucene-pushdown on union-type fields

Since the union-type rewriter replaced conversion functions with new FieldAttributes, these were passing the check for being possible to push-down, which was incorrect. Now we prevent that.

Set union-type aggregatable flag to false always

This simplifies the push-down check.

Fixed tests after rebase on main

Add unit tests for union-types (same field, different type)

Remove generic warnings

Test code cleanup and clarifying comments

Remove -IT_tests_only in favor of CsvTests assumeFalse

Improved comment

Code review updates

Code review updates

Remove changes to ql/EsRelation

And it turned out the latest version of union type no longer needed these changes anyway, and was using the new EsRelation in the ESQL module without these changes.

Port InvalidMappedField to ESQL

Note, this extends the QL version of InvalidMappedField, so is not a complete port. This is necessary because of the intertwining of QL IndexResolver and EsqlIndexResolver. Once those classes are disentangled, we can completely break InvalidMappedField from QL and make it a forbidden type.

Fix capabilities line after rebase on main

Revert QL FieldAttribute and extend with ESQL FieldAttribute

So as to remove any edits to QL code, we extend FieldAttribute in the ESQL code with the changes required, since is simply to include the `field` in the hascode and equals methods.

Revert "Revert QL FieldAttribute and extend with ESQL FieldAttribute"

This reverts commit 168c6c75436e26b83e083cd3de8e18062e116bc9.

Switch UNION_TYPES from EsqlFeatures to EsqlCapabilities

Make hashcode and equals aligned

And removed unused method from earlier union-types work where we kept the NodeId during re-writing (which we no longer do).

Replace required_feature with required_capability after rebase

Switch union_types capability back to feature, because capabilities do not work in mixed clusters

Revert "Switch union_types capability back to feature, because capabilities do not work in mixed clusters"

This reverts commit 56d58bedf756dbad703c07bf4cdb991d4341c1ae.

Added test for multiple columns from same fields

Both IP and Date are tested

Fix bug with incorrectly resolving invalid types

And added more tests

Fixed bug with multiple fields of same name

This fix simply removes the original field already at the EsRelation level, which covers all test cases but has the side effect of having the final field no-longer be unsupported/null when the alias does not overwrite the field with the same name.
This is not exactly the correct semantic intent.
The original field name should be unsupported/null unless the user explicitly overwrote the name with `field=TO_TYPE(field)`, which effectively deletes the old field anyway.

Fixed bug with multiple conversions of the same field

This also fixes the issue with the previous fix that incorrectly reported the converted type for the original field.

More tests with multiple fields and KEEP/DROP combinations

Replace skip with capabilities in YML tests

Fixed missing ql->esql import change afer merging main

Merged two InvalidMappedField classes

After the QL code was ported to esql.core, we can now make the edits directly in InvalidMappedField instead of having one extend the other.

Move FieldAttribute edits from QL to ESQL

ESQL: Prepare analyzer for LOOKUP (#109045)

This extracts two fairly uncontroversial changes that were in the main
LOOKUP PR into a smaller change that's easier to review.

ESQL: Move serialization for EsField (#109222)

This moves the serialization logic for `EsField` into the `EsField`
subclasses to better align with the way rest of Elasticsearch works. It
also switches them from ESQL's home grown `writeNamed` thing to
`NamedWriteable`. These are wire compatible with one another.

ESQL: Move serialization of `Attribute` (#109267)

This moves the serialization of `Attribute` classes used in ESQL into
the classes themselves to better line up with the rest of Elasticsearch.

ES|QL: add MV_APPEND function (#107001)

Adding `MV_APPEND(value1, value2)` function, that appends two values
creating a single multi-value. If one or both the inputs are
multi-values, the result is the concatenation of all the values, eg.

```
MV_APPEND([a, b], [c, d]) -> [a, b, c, d]
```

~I think for this specific case it makes sense to consider `null` values
as empty arrays, so that~ ~MV_APPEND(value, null) -> value~ ~It is
pretty uncommon for ESQL (all the other functions, apart from
`COALESCE`, short-circuit to `null` when one of the values is null), so
let's discuss this behavior.~

[EDIT] considering the feedback from Andrei, I changed this logic and
made it consistent with the other functions: now if one of the
parameters is null, the function returns null

[ES|QL] Convert string to datetime when the other size of an arithmetic operator is date_period or time_duration (#108455)

* convert string to datetime when the other side of binary operator is temporal amount

ESQL: Move `NamedExpression` serialization (#109380)

This moves the serialization for the remaining `NamedExpression`
subclass into the class itself, and switches all direct serialization of
`NamedExpression`s to `readNamedWriteable` and friends. All other
`NamedExpression` subclasses extend from `Attribute` who's serialization
was moved ealier. They are already registered under the "category class"
for `Attribute`. This also registers them as `NamedExpression`s.

ESQL: Implement LOOKUP, an "inline" enrich (#107987)

This adds support for `LOOKUP`, a command that implements a sort of
inline `ENRICH`, using data that is passed in the request:

```
$ curl -uelastic:password -HContent-Type:application/json -XPOST \
    'localhost:9200/_query?error_trace&pretty&format=txt' \
-d'{
    "query": "ROW a=1::LONG | LOOKUP t ON a",
    "tables": {
        "t": {
            "a:long":     [    1,     4,     2],
            "v1:integer": [   10,    11,    12],
            "v2:keyword": ["cat", "dog", "wow"]
        }
    },
    "version": "2024.04.01"
}'
      v1       |      v2       |       a
---------------+---------------+---------------
10             |cat            |1
```

This required these PRs: * #107624 * #107634 * #107701 * #107762 *

Closes #107306

parent 32ac5ba755dd5c24364a210f1097ae093fdcbd75
author Craig Taverner <craig@amanzi.com> 1717779549 +0200
committer Craig Taverner <craig@amanzi.com> 1718115775 +0200

Fixed compile error after merging in main

Fixed strange merge issues from main

Remove version from ES|QL test queries after merging main

Fixed union-types on nested fields

Switch to Luigi's solution, and expand nested tests

Cleanup after rebase

* Added more tests from code review

Note that one test, `multiIndexIpStringStatsInline` is muted due to failing with the error:

    UnresolvedException: Invalid call to dataType on an unresolved object ?client_ip

* Make CsvTests consistent with integration tests for capabilities

The integration tests do not fail the tests if the capability does not even exist on cluster nodes, instead the tests are ignored. The same behaviour should happen with CsvTests for consistency.

* Return assumeThat to assertThat, but change order

This way we don't have to add more features to the test framework in this PR, but we would probably want a mute feature (like a `skip` line).

* Move serialization of MultiTypeEsField to NamedWritable approach

Since the sub-fields are AbstractConvertFunction expressions, and Expression is not yet fully supported as a category class for NamedWritable, we need a few slight tweaks to this, notably registering this explicitly in the EsqlPlugin, as well as calling PlanStreamInput.readExpression() instead of StreamInput.readNamedWritable(Expression.class). These can be removed later once Expression is fully supported as a category class.

* Remove attempt to mute two failed tests

We used required_capability to mute the tests, but this caused issues with CsvTests which also uses this as a spelling mistake checker for typing the capability name wrong, so we tried to use muted-tests.yml, but that only mutes tests in specific run configurations (ie. we need to mute each and every IT class separately).

So now we just remove the tests entirely. We left a comment in the muted-tests.yml file for future reference about how to mute csv-spec tests.

* Fix rather massive issue with performance of testConcurrentSerialization

Recreating the config on every test was very expensive.

* Code review by Nik

---------

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@astefan astefan mentioned this pull request Aug 20, 2024
3 tasks
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!) >enhancement ES|QL-ui Impacts ES|QL UI Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.15.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ESQL: Support provided table for enrich

5 participants