Skip to content

Commit 03966b0

Browse files
smalyshevsmalyshev
committed
cleanups
1 parent 1e43515 commit 03966b0

File tree

3 files changed

+28
-37
lines changed

3 files changed

+28
-37
lines changed

‎x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClusterLookupJoinFailuresIT.java‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -66,78 +66,78 @@ public void sendResponse(Exception exception) {
6666
}
6767

6868
try {
69+
// FIXME: this should catch the error but fails instead
70+
/*
6971
try (
7072
EsqlQueryResponse resp = runQuery(
71-
"FROM logs-*,c*:logs-* | EVAL lookup_key = v | LOOKUP JOIN values_lookup ON lookup_key",
73+
"FROM c*:logs-* | EVAL lookup_key = v | LOOKUP JOIN values_lookup ON lookup_key",
7274
randomBoolean()
7375
)
7476
) {
7577
var columns = resp.columns().stream().map(ColumnInfoImpl::name).toList();
7678
assertThat(columns, hasItems("lookup_key", "lookup_name", "lookup_tag", "v", "tag"));
7779
7880
List<List<Object>> values = getValuesList(resp);
79-
assertThat(values, hasSize(10));
81+
assertThat(values, hasSize(0));
82+
8083
EsqlExecutionInfo executionInfo = resp.getExecutionInfo();
8184
82-
var localCluster = executionInfo.getCluster(LOCAL_CLUSTER);
83-
assertThat(localCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL));
8485
var remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER_1);
8586
assertThat(remoteCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SKIPPED));
8687
assertThat(remoteCluster.getFailures(), not(empty()));
8788
var failure = remoteCluster.getFailures().get(0);
88-
// FIXME: this produces a wrong message currently
89-
// assertThat(failure.reason(), containsString(simulatedFailure.getMessage()));
90-
assertThat(failure.reason(), containsString("lookup index [values_lookup] is not available in remote cluster [cluster-a]"));
91-
}
89+
assertThat(failure.reason(), containsString(simulatedFailure.getMessage()));
90+
} */
9291

9392
try (
9493
EsqlQueryResponse resp = runQuery(
95-
"FROM logs-*,*:logs-* | EVAL lookup_key = v | LOOKUP JOIN values_lookup ON lookup_key",
94+
"FROM logs-*,c*:logs-* | EVAL lookup_key = v | LOOKUP JOIN values_lookup ON lookup_key",
9695
randomBoolean()
9796
)
9897
) {
9998
var columns = resp.columns().stream().map(ColumnInfoImpl::name).toList();
10099
assertThat(columns, hasItems("lookup_key", "lookup_name", "lookup_tag", "v", "tag"));
101100

102101
List<List<Object>> values = getValuesList(resp);
103-
assertThat(values, hasSize(20));
102+
assertThat(values, hasSize(10));
104103
EsqlExecutionInfo executionInfo = resp.getExecutionInfo();
105104

106105
var localCluster = executionInfo.getCluster(LOCAL_CLUSTER);
107106
assertThat(localCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL));
108107
var remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER_1);
109108
assertThat(remoteCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SKIPPED));
110-
var remoteCluster2 = executionInfo.getCluster(REMOTE_CLUSTER_2);
111-
assertThat(remoteCluster2.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL));
112109
assertThat(remoteCluster.getFailures(), not(empty()));
113110
var failure = remoteCluster.getFailures().get(0);
114111
// FIXME: this produces a wrong message currently
115112
// assertThat(failure.reason(), containsString(simulatedFailure.getMessage()));
116113
assertThat(failure.reason(), containsString("lookup index [values_lookup] is not available in remote cluster [cluster-a]"));
117114
}
118115

119-
// FIXME: this should work but fails
120-
/*
121116
try (
122117
EsqlQueryResponse resp = runQuery(
123-
"FROM c*:logs-* | EVAL lookup_key = v | LOOKUP JOIN values_lookup ON lookup_key",
118+
"FROM logs-*,*:logs-* | EVAL lookup_key = v | LOOKUP JOIN values_lookup ON lookup_key",
124119
randomBoolean()
125120
)
126121
) {
127122
var columns = resp.columns().stream().map(ColumnInfoImpl::name).toList();
128123
assertThat(columns, hasItems("lookup_key", "lookup_name", "lookup_tag", "v", "tag"));
129124

130125
List<List<Object>> values = getValuesList(resp);
131-
assertThat(values, hasSize(0));
132-
126+
assertThat(values, hasSize(20));
133127
EsqlExecutionInfo executionInfo = resp.getExecutionInfo();
134128

129+
var localCluster = executionInfo.getCluster(LOCAL_CLUSTER);
130+
assertThat(localCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL));
135131
var remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER_1);
136132
assertThat(remoteCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SKIPPED));
133+
var remoteCluster2 = executionInfo.getCluster(REMOTE_CLUSTER_2);
134+
assertThat(remoteCluster2.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL));
137135
assertThat(remoteCluster.getFailures(), not(empty()));
138136
var failure = remoteCluster.getFailures().get(0);
139-
assertThat(failure.reason(), containsString(simulatedFailure.getMessage()));
140-
} */
137+
// FIXME: this produces a wrong message currently
138+
// assertThat(failure.reason(), containsString(simulatedFailure.getMessage()));
139+
assertThat(failure.reason(), containsString("lookup index [values_lookup] is not available in remote cluster [cluster-a]"));
140+
}
141141

142142
// now fail
143143
setSkipUnavailable(REMOTE_CLUSTER_1, false);

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/PhysicalVerifier.java‎

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -40,20 +40,15 @@ public Failures verify(PhysicalPlan plan) {
4040
return failures;
4141
}
4242
// Do the same for remote lookup joins
43-
// TODO: figure out why enrich does not need two sides?
44-
var joins = plan.collectFirstChildren(LookupJoinExec.class::isInstance);
45-
if (joins.isEmpty() == false) {
46-
return failures;
47-
}
48-
var fragment = plan.collectFirstChildren(FragmentExec.class::isInstance);
49-
if (fragment.isEmpty() == false) {
50-
// LookupJoin gets rewritten as Join by surrogate()
51-
FragmentExec f = (FragmentExec) fragment.get(0);
52-
var ljoins = f.fragment().collectFirstChildren(Join.class::isInstance);
53-
if (ljoins.isEmpty() == false) {
54-
return failures;
55-
}
56-
}
43+
// var fragment = plan.collectFirstChildren(FragmentExec.class::isInstance);
44+
// if (fragment.isEmpty() == false) {
45+
// // LookupJoin gets rewritten as Join by surrogate()
46+
// FragmentExec f = (FragmentExec) fragment.get(0);
47+
// var ljoins = f.fragment().collectFirstChildren(Join.class::isInstance);
48+
// if (ljoins.isEmpty() == false) {
49+
// return failures;
50+
// }
51+
// }
5752

5853
plan.forEachDown(p -> {
5954
if (p instanceof FieldExtractExec fieldExtractExec) {

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/Layout.java‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,10 +115,6 @@ public Layout build() {
115115
for (NameId id : set.nameIds) {
116116
// Duplicate name ids would mean that have 2 channels that are declared under the same id. That makes no sense - which
117117
// channel should subsequent operators use, then, when they want to refer to this id?
118-
// FIXME: temporry change for debugging
119-
if (layout.containsKey(id)) {
120-
throw new IllegalArgumentException("Duplicate name ids are not allowed in layouts: " + id);
121-
}
122118
assert (layout.containsKey(id) == false) : "Duplicate name ids are not allowed in layouts: " + id;
123119
ChannelAndType next = new ChannelAndType(channel, set.type);
124120
layout.put(id, next);

0 commit comments

Comments
 (0)