Skip to content

Commit 06da8a4

Browse files
Fix FORK with union-types (elastic#134033)
--------- Co-authored-by: ncordon <[email protected]> Co-authored-by: Nacho Cordón <[email protected]>
1 parent 5c70b47 commit 06da8a4

File tree

6 files changed

+64
-9
lines changed

6 files changed

+64
-9
lines changed

docs/changelog/134033.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 134033
2+
summary: Fix FORK with union-types
3+
area: ES|QL
4+
type: bug
5+
issues:
6+
- 133973

x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/GenerativeForkRestTest.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,6 @@ protected void shouldSkipTest(String testName) throws IOException {
5555
testCase.requiredCapabilities.contains(UNMAPPED_FIELDS.capabilityName())
5656
);
5757

58-
assumeFalse(
59-
"Tests using implicit_casting_date_and_date_nanos are not supported for now",
60-
testCase.requiredCapabilities.contains(IMPLICIT_CASTING_DATE_AND_DATE_NANOS.capabilityName())
61-
);
62-
6358
assumeTrue("Cluster needs to support FORK", hasCapabilities(adminClient(), List.of(FORK_V9.capabilityName())));
6459
}
6560
}

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1397,6 +1397,23 @@ event_duration:long | _index:keyword | ts:date | ts_str:
13971397
8268153 | sample_data_ts_nanos | 2023-10-23T13:52:55.015Z | 2023-10-23T13:52:55.015123456Z | 1698069175015123456 | 172.21.3.15 | 172.21.3.15
13981398
;
13991399

1400+
multiIndexInlinestatsOfMultiTypedField
1401+
required_capability: inlinestats_v11
1402+
required_capability: fork_union_types
1403+
// https://github.com/elastic/elasticsearch/issues/133973
1404+
FROM apps, apps_short
1405+
| INLINESTATS s = sum(id::integer)
1406+
| SORT name, version
1407+
| LIMIT 5
1408+
;
1409+
1410+
id:unsupported | name:keyword | version:version | s:long
1411+
null | aaaaa | 1 | 210
1412+
null | aaaaa | 1 | 210
1413+
null | aaaaa | 1.2.3.4 | 210
1414+
null | aaaaa | 1.2.3.4 | 210
1415+
null | bbbbb | 2.1 | 210
1416+
;
14001417

14011418
multiIndexIndirectUseOfUnionTypesInSort
14021419
required_capability: union_types
@@ -1652,6 +1669,35 @@ id:integer | name:keyword | count:long
16521669
14 | mmmmm | 2
16531670
;
16541671

1672+
shortIntegerWideningFork
1673+
required_capability: union_types
1674+
required_capability: fork_union_types
1675+
required_capability: casting_operator
1676+
required_capability: union_types_numeric_widening
1677+
required_capability: fork_v9
1678+
1679+
FROM apps, apps_short
1680+
| EVAL x = id::integer
1681+
| FORK (WHERE true) (WHERE true)
1682+
| DROP _fork
1683+
| SORT x ASC, name ASC, version ASC
1684+
| EVAL x = x + 1
1685+
| LIMIT 10
1686+
;
1687+
1688+
id:unsupported | name:keyword | version:version | x:integer
1689+
null | aaaaa | 1 | 2
1690+
null | aaaaa | 1 | 2
1691+
null | aaaaa | 1 | 2
1692+
null | aaaaa | 1 | 2
1693+
null | bbbbb | 2.1 | 3
1694+
null | bbbbb | 2.1 | 3
1695+
null | bbbbb | 2.1 | 3
1696+
null | bbbbb | 2.1 | 3
1697+
null | ccccc | 2.3.4 | 4
1698+
null | ccccc | 2.3.4 | 4
1699+
;
1700+
16551701
ImplicitCastingMultiTypedFieldsKeepSort
16561702
required_capability: date_nanos_type
16571703
required_capability: implicit_casting_date_and_date_nanos

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1100,6 +1100,11 @@ public enum Cap {
11001100
*/
11011101
FORK_V9,
11021102

1103+
/**
1104+
* Support for union types in FORK
1105+
*/
1106+
FORK_UNION_TYPES,
1107+
11031108
/**
11041109
* Support for the {@code leading_zeros} named parameter.
11051110
*/

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -809,7 +809,9 @@ private LogicalPlan resolveFork(Fork fork, AnalyzerContext context) {
809809
// We don't want to keep the same attributes that are outputted by the FORK branches.
810810
// Keeping the same attributes can have unintended side effects when applying optimizations like constant folding.
811811
for (Attribute attr : outputUnion) {
812-
newOutput.add(new ReferenceAttribute(attr.source(), null, attr.name(), attr.dataType()));
812+
newOutput.add(
813+
new ReferenceAttribute(attr.source(), null, attr.name(), attr.dataType(), Nullability.FALSE, null, attr.synthetic())
814+
);
813815
}
814816

815817
return changed ? new Fork(fork.source(), newSubPlans, newOutput) : fork;
@@ -1946,10 +1948,9 @@ static Attribute checkUnresolved(FieldAttribute fa) {
19461948
private static LogicalPlan planWithoutSyntheticAttributes(LogicalPlan plan) {
19471949
List<Attribute> output = plan.output();
19481950
List<Attribute> newOutput = new ArrayList<>(output.size());
1949-
19501951
for (Attribute attr : output) {
19511952
// Do not let the synthetic union type field attributes end up in the final output.
1952-
if (attr.synthetic() && attr instanceof FieldAttribute) {
1953+
if (attr.synthetic() && attr != NO_FIELDS.getFirst()) {
19531954
continue;
19541955
}
19551956
newOutput.add(attr);

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/Fork.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
import java.util.function.BiConsumer;
3030
import java.util.stream.Collectors;
3131

32+
import static org.elasticsearch.xpack.esql.analysis.Analyzer.NO_FIELDS;
33+
3234
/**
3335
* A Fork is a n-ary {@code Plan} where each child is a sub plan, e.g.
3436
* {@code FORK [WHERE content:"fox" ] [WHERE content:"dog"] }
@@ -121,7 +123,7 @@ public static List<Attribute> outputUnion(List<LogicalPlan> subplans) {
121123
continue;
122124
}
123125

124-
if (names.contains(attr.name()) == false && attr.name().equals(Analyzer.NO_FIELDS_NAME) == false) {
126+
if (names.contains(attr.name()) == false && attr != NO_FIELDS.getFirst()) {
125127
names.add(attr.name());
126128
output.add(attr);
127129
}

0 commit comments

Comments
 (0)