ES|QL: add MV_APPEND function #107001

luigidellaquila · 2024-04-02T13:37:23Z

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

github-actions · 2024-04-02T13:37:36Z

Documentation preview:

✨ Changed pages

elasticsearchmachine · 2024-04-02T13:37:46Z

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

luigidellaquila · 2024-04-02T13:38:09Z

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

@@ -227,5 +229,5 @@ countFunctions#[skip:-8.13.99]
 meta functions |  stats  a = count(*), b = count(*), c = count(*) |  mv_expand c;

 a:long | b:long | c:long
-99     | 99     | 99
+100    | 100    | 100


luigidellaquila · 2024-04-02T13:46:09Z

@elasticmachine update branch

nik9000 · 2024-04-02T14:40:38Z

I think for this specific case it makes sense to consider null values as empty arrays, so that

That's fine - our null is basically an empty list. I'd do it.

nik9000 · 2024-04-02T14:41:18Z

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.

There's a small number that don't - maybe 5 out of the 100.

fang-xing-esql · 2024-04-02T14:59:13Z

...c/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/multivalue/MvAppend.java

+    public MvAppend(
+        Source source,
+        @Param(
+            name = "v1",


In a discussion in a prior PR(#106454 (review)), we'd like to give consistent names to function arguments. If an argument takes multiple input data types, for example both string and numeric, we call it field, we can name them field1 and field2 here.

fang-xing-esql · 2024-04-02T15:10:05Z

...c/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/multivalue/MvAppend.java

+        return Objects.equals(other.field1, field1) && Objects.equals(other.field2, field2);
+    }
+
+    static int calculateOffset(int oldOffset, int fieldValueCount) {


Seems like this method can be removed.

…esql/mv_append_2

fang-xing-esql

LGTM.

bpintea

Left some small remarks otherwise LGTM

bpintea · 2024-05-22T10:56:19Z

...c/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/multivalue/MvAppend.java

+    @Evaluator(extraName = "Int")
+    static void process(IntBlock.Builder builder, int position, IntBlock field1, IntBlock field2) {
+        int count1 = field1.getValueCount(position);
+        int first1 = field1.getFirstValueIndex(position);


Nit: initialising the "firsts" could be moved with the branch using them, but can stay grouped like this too.

bpintea · 2024-05-22T11:05:48Z

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java

+        int second = first;
+        while (second == first) {
+            second = randomInt(fields.length - 1);
+        }


Optional:

Supplier<Int> supplier = () -> randomInt(fields.length - 1); int first = supplier.get(); int second = randomValueOtherThan(first, supplier);

bpintea · 2024-05-22T11:06:49Z

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java

@@ -1876,6 +1876,38 @@ public void testInOnText() {
            """, "mapping-multi-field-variation.json", "text");
    }

+    public void testMvAppendValidation() {
+        String[][] fields = {


+long, ip? Maybe also null?

bpintea · 2024-05-22T11:26:42Z

...c/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/multivalue/MvAppend.java

+            return resolution;
+        }
+        dataType = field1.dataType();
+        return isType(field2, t -> t == dataType, sourceText(), SECOND, dataType.typeName());


Suggested change

return isType(field2, t -> t == dataType, sourceText(), SECOND, dataType.typeName());

return isType(field2, t -> t == NULL || t == dataType, sourceText(), SECOND, dataType.typeName());

Otherwise MV_APPEND(some_type, null) will work, but MV_APPEND(null, some_type) won't. Might be worth adding some tests with this too.

Checking isType(), it already checks for NULL from what I see: predicate.test(e.dataType()) || e.dataType() == NULL.

Anyway, if we consider nulls compatible with any type (Which I deduce from the isType() implementation?), and if the idea is that both fields are interchargeable, we should reassign dataType to field2 here if field1 is NULL (?)

Edit: After reconsidering the code: For a field to be of type "NULL", it means either a constant or a function returning null must have been used as the field right? If those are the cases, maybe it should be the opposite, if any field is null, set dataType to NULL. That way, we would shortcitcuit at plan time if there's a null chain (Correct me if I'm wrong somewhere there)

Checking isType(), it already checks for NULL from what I see: predicate.test(e.dataType()) || e.dataType() == NULL.

In this case the first type, "t" / dataType, needs to be checked against NULL, rather than field2's type (which is e in isType()).

if any field is null, set dataType to NULL.

That's correct (and the code does it right also for the MV_APPEND(null, some_type), case).
But in one case the null result has first argument's type, while in the other null has the NULL type.

The NULL type is a pathological situation, that can only originate from a ROW a = null or an EVAL a = null (there is no way that ES returns a NULL type for an indexed field).

To make it really work, we'll need a slightly different solution, ie. if the first type if type NULL, also resolve the return type to the type of second field type.

I'm pushing a fix in this sense.

ivancea · 2024-05-22T16:28:07Z

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java

@@ -26,6 +26,11 @@ public class EsqlCapabilities {
     */
    private static final String FN_CBRT = "fn_cbrt";

+    /**
+     * Support for {@code MV_APPEND} funciton. #107001


Suggested change

* Support for {@code MV_APPEND} funciton. #107001

* Support for {@code MV_APPEND} function. #107001

ivancea · 2024-05-22T16:54:45Z

...c/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/multivalue/MvAppend.java

+            return resolution;
+        }
+        dataType = field1.dataType();
+        return isType(field2, t -> t == dataType, sourceText(), SECOND, dataType.typeName());


Checking isType(), it already checks for NULL from what I see: predicate.test(e.dataType()) || e.dataType() == NULL.

Anyway, if we consider nulls compatible with any type (Which I deduce from the isType() implementation?), and if the idea is that both fields are interchargeable, we should reassign dataType to field2 here if field1 is NULL (?)

Edit: After reconsidering the code: For a field to be of type "NULL", it means either a constant or a function returning null must have been used as the field right? If those are the cases, maybe it should be the opposite, if any field is null, set dataType to NULL. That way, we would shortcitcuit at plan time if there's a null chain (Correct me if I'm wrong somewhere there)

astefan

LGTM

luigidellaquila · 2024-06-04T13:34:45Z

@elasticmachine update branch

luigidellaquila · 2024-06-04T16:42:36Z

@elasticmachine update branch

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

@timestamp

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

@timestamp

* 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>

luigidellaquila added 7 commits March 14, 2024 10:03

Add mv_append function

22055b8

Fix tests

19e6de9

Merge branch 'main' into esql/mv_append_2

f491c85

Add tests

a2ce9a0

Generate docs

b5e808b

Better null testing

2be4d44

Merge branch 'main' into esql/mv_append_2

b9133ef

luigidellaquila added >non-issue :Analytics/ES|QL AKA ESQL labels Apr 2, 2024

luigidellaquila requested review from nik9000 and fang-xing-esql April 2, 2024 13:37

elasticsearchmachine added Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.14.0 labels Apr 2, 2024

luigidellaquila requested a review from costin April 2, 2024 13:37

luigidellaquila commented Apr 2, 2024

View reviewed changes

Merge branch 'main' into esql/mv_append_2

014384d

fang-xing-esql reviewed Apr 2, 2024

View reviewed changes

luigidellaquila added 3 commits April 3, 2024 09:48

Better nullability management + implement review suggestions

00b3101

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

d24af13

…esql/mv_append_2

Merge branch 'main' into esql/mv_append_2

19c8d8c

luigidellaquila mentioned this pull request Apr 3, 2024

ESQL: Missing MV_ (multi value) functions #105322

Closed

8 tasks

Merge branch 'main' into esql/mv_append_2

b640bfc

fang-xing-esql approved these changes Apr 5, 2024

View reviewed changes

costin requested a review from astefan April 8, 2024 02:14

luigidellaquila added 9 commits May 2, 2024 16:19

Do not extend OptionalArgument

b38d50e

Implement review suggestions

434502b

Make null inputs always return null

51a159f

Spotless

bfff500

Add validation tests

0331901

Merge branch 'main' into esql/mv_append_2

b111941

Fix tests and use capabilities

f9b78d1

Merge branch 'main' into esql/mv_append_2

1d6b9f2

Spotless

04c1ead

luigidellaquila requested review from astefan and bpintea May 22, 2024 09:06

bpintea approved these changes May 22, 2024

View reviewed changes

ivancea reviewed May 22, 2024

View reviewed changes

luigidellaquila added 5 commits May 24, 2024 10:16

Merge branch 'main' into esql/mv_append_2

7275340

Implement review suggestions

7e101cf

Merge branch 'main' into esql/mv_append_2

d5312c3

Fix conflicts

4dc127f

Fix test

a011137

astefan approved these changes Jun 4, 2024

View reviewed changes

Merge branch 'main' into esql/mv_append_2

ceb9297

luigidellaquila added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Jun 4, 2024

Merge branch 'main' into esql/mv_append_2

a8f4d9f

elasticsearchmachine merged commit 5f6e8f6 into elastic:main Jun 4, 2024
15 checks passed

luigidellaquila deleted the esql/mv_append_2 branch June 4, 2024 17:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ES|QL: add MV_APPEND function #107001

ES|QL: add MV_APPEND function #107001

luigidellaquila commented Apr 2, 2024 •

edited

Loading

github-actions bot commented Apr 2, 2024

elasticsearchmachine commented Apr 2, 2024

luigidellaquila Apr 2, 2024

luigidellaquila commented Apr 2, 2024

nik9000 commented Apr 2, 2024

nik9000 commented Apr 2, 2024

fang-xing-esql Apr 2, 2024

fang-xing-esql Apr 2, 2024

fang-xing-esql left a comment

bpintea left a comment

bpintea May 22, 2024

bpintea May 22, 2024

bpintea May 22, 2024

bpintea May 22, 2024

ivancea May 22, 2024 •

edited

Loading

bpintea May 22, 2024

luigidellaquila May 24, 2024

ivancea May 22, 2024

ivancea May 22, 2024 •

edited

Loading

astefan left a comment

luigidellaquila commented Jun 4, 2024

luigidellaquila commented Jun 4, 2024

	return isType(field2, t -> t == dataType, sourceText(), SECOND, dataType.typeName());
	return isType(field2, t -> t == NULL \|\| t == dataType, sourceText(), SECOND, dataType.typeName());

	* Support for {@code MV_APPEND} funciton. #107001
	* Support for {@code MV_APPEND} function. #107001

ES|QL: add MV_APPEND function #107001

ES|QL: add MV_APPEND function #107001

Conversation

luigidellaquila commented Apr 2, 2024 • edited Loading

github-actions bot commented Apr 2, 2024

elasticsearchmachine commented Apr 2, 2024

Choose a reason for hiding this comment

luigidellaquila commented Apr 2, 2024

nik9000 commented Apr 2, 2024

nik9000 commented Apr 2, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fang-xing-esql left a comment

Choose a reason for hiding this comment

bpintea left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivancea May 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivancea May 22, 2024 • edited Loading

Choose a reason for hiding this comment

astefan left a comment

Choose a reason for hiding this comment

luigidellaquila commented Jun 4, 2024

luigidellaquila commented Jun 4, 2024

luigidellaquila commented Apr 2, 2024 •

edited

Loading

ivancea May 22, 2024 •

edited

Loading

ivancea May 22, 2024 •

edited

Loading