Skip to content

Commit adb9290

Browse files
julian-elasticjulian-elastic
committed
Fix non-snapshot UT errors
1 parent b10c360 commit adb9290

File tree

8 files changed

+103
-14
lines changed

8 files changed

+103
-14
lines changed

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/LogicalPlanBuilder.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -744,7 +744,7 @@ private JoinInfo processFieldBasedJoin(List<Expression> expressions) {
744744

745745
private JoinInfo processExpressionBasedJoin(List<Expression> expressions, EsqlBaseParser.JoinConditionContext ctx) {
746746
if (LOOKUP_JOIN_ON_BOOLEAN_EXPRESSION.isEnabled() == false) {
747-
throw new ParsingException(ctx.getText(), "JOIN ON clause only supports fields at the moment, found [{}]", ctx.getText());
747+
throw new ParsingException(source(ctx), "JOIN ON clause only supports fields at the moment.");
748748
}
749749
List<Attribute> joinFields = new ArrayList<>();
750750
List<Expression> joinExpressions = new ArrayList<>();

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,10 @@ public void testTooBigQuery() {
132132

133133
public void testJoinOnConstant() {
134134
assumeTrue("LOOKUP JOIN available as snapshot only", EsqlCapabilities.Cap.JOIN_LOOKUP_V12.isEnabled());
135+
assumeTrue(
136+
"requires LOOKUP JOIN ON boolean expression capability",
137+
EsqlCapabilities.Cap.LOOKUP_JOIN_ON_BOOLEAN_EXPRESSION.isEnabled()
138+
);
135139
assertEquals(
136140
"1:55: JOIN ON clause only supports fields or AND of Binary Expressions at the moment, found [123]",
137141
error("row languages = 1, gender = \"f\" | lookup join test on 123")

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2220,6 +2220,10 @@ public void testLookupJoinDataTypeMismatch() {
22202220

22212221
public void testLookupJoinExpressionAmbiguousRight() {
22222222
assumeTrue("requires LOOKUP JOIN capability", EsqlCapabilities.Cap.JOIN_LOOKUP_V12.isEnabled());
2223+
assumeTrue(
2224+
"requires LOOKUP JOIN ON boolean expression capability",
2225+
EsqlCapabilities.Cap.LOOKUP_JOIN_ON_BOOLEAN_EXPRESSION.isEnabled()
2226+
);
22232227
String queryString = """
22242228
from test
22252229
| rename languages as language_code
@@ -2234,6 +2238,10 @@ public void testLookupJoinExpressionAmbiguousRight() {
22342238

22352239
public void testLookupJoinExpressionAmbiguousLeft() {
22362240
assumeTrue("requires LOOKUP JOIN capability", EsqlCapabilities.Cap.JOIN_LOOKUP_V12.isEnabled());
2241+
assumeTrue(
2242+
"requires LOOKUP JOIN ON boolean expression capability",
2243+
EsqlCapabilities.Cap.LOOKUP_JOIN_ON_BOOLEAN_EXPRESSION.isEnabled()
2244+
);
22372245
String queryString = """
22382246
from test
22392247
| rename languages as language_name
@@ -2248,6 +2256,10 @@ public void testLookupJoinExpressionAmbiguousLeft() {
22482256

22492257
public void testLookupJoinExpressionAmbiguousBoth() {
22502258
assumeTrue("requires LOOKUP JOIN capability", EsqlCapabilities.Cap.JOIN_LOOKUP_V12.isEnabled());
2259+
assumeTrue(
2260+
"requires LOOKUP JOIN ON boolean expression capability",
2261+
EsqlCapabilities.Cap.LOOKUP_JOIN_ON_BOOLEAN_EXPRESSION.isEnabled()
2262+
);
22512263
String queryString = """
22522264
from test
22532265
| rename languages as language_code

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/enrich/LookupFromIndexOperatorTests.java‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
import org.elasticsearch.threadpool.TestThreadPool;
6464
import org.elasticsearch.threadpool.ThreadPool;
6565
import org.elasticsearch.transport.TransportService;
66+
import org.elasticsearch.xpack.esql.action.EsqlCapabilities;
6667
import org.elasticsearch.xpack.esql.core.expression.Expression;
6768
import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
6869
import org.elasticsearch.xpack.esql.core.expression.FoldContext;
@@ -114,10 +115,12 @@ public class LookupFromIndexOperatorTests extends AsyncOperatorTestCase {
114115
public static Iterable<Object[]> parametersFactory() {
115116
List<Object[]> operations = new ArrayList<>();
116117
operations.add(new Object[] { null });
117-
for (EsqlBinaryComparison.BinaryComparisonOperation operation : EsqlBinaryComparison.BinaryComparisonOperation.values()) {
118-
// we skip NEQ because there are too many matches and the test can timeout
119-
if (operation != EsqlBinaryComparison.BinaryComparisonOperation.NEQ) {
120-
operations.add(new Object[] { operation });
118+
if (EsqlCapabilities.Cap.LOOKUP_JOIN_ON_BOOLEAN_EXPRESSION.isEnabled()) {
119+
for (EsqlBinaryComparison.BinaryComparisonOperation operation : EsqlBinaryComparison.BinaryComparisonOperation.values()) {
120+
// we skip NEQ because there are too many matches and the test can timeout
121+
if (operation != EsqlBinaryComparison.BinaryComparisonOperation.NEQ) {
122+
operations.add(new Object[] { operation });
123+
}
121124
}
122125
}
123126
return operations;

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2520,6 +2520,10 @@ public void testPruneJoinOnNullMatchingField() {
25202520
* }</pre>
25212521
*/
25222522
public void testPruneJoinOnNullMatchingFieldExpressionJoin() {
2523+
assumeTrue(
2524+
"requires LOOKUP JOIN ON boolean expression capability",
2525+
EsqlCapabilities.Cap.LOOKUP_JOIN_ON_BOOLEAN_EXPRESSION.isEnabled()
2526+
);
25232527
var plan = optimizedPlan("""
25242528
from test
25252529
| eval language_code_left = null::integer
@@ -2571,6 +2575,10 @@ public void testPruneJoinOnNullAssignedMatchingField() {
25712575
* }</pre>
25722576
*/
25732577
public void testPruneJoinOnNullAssignedMatchingFieldExpr() {
2578+
assumeTrue(
2579+
"requires LOOKUP JOIN ON boolean expression capability",
2580+
EsqlCapabilities.Cap.LOOKUP_JOIN_ON_BOOLEAN_EXPRESSION.isEnabled()
2581+
);
25742582
var plan = optimizedPlan("""
25752583
from test
25762584
| eval my_null = null::integer
@@ -9150,6 +9158,10 @@ public void testDecayScaleMustBeLiteral() {
91509158
*
91519159
*/
91529160
public void testLookupJoinExpressionSwapped() {
9161+
assumeTrue(
9162+
"requires LOOKUP JOIN ON boolean expression capability",
9163+
EsqlCapabilities.Cap.LOOKUP_JOIN_ON_BOOLEAN_EXPRESSION.isEnabled()
9164+
);
91539165
LogicalPlan plan = optimizedPlan("""
91549166
from test
91559167
| keep languages
@@ -9183,6 +9195,10 @@ public void testLookupJoinExpressionSwapped() {
91839195
*
91849196
*/
91859197
public void testLookupJoinExpressionSameAttrsDifferentConditions() {
9198+
assumeTrue(
9199+
"requires LOOKUP JOIN ON boolean expression capability",
9200+
EsqlCapabilities.Cap.LOOKUP_JOIN_ON_BOOLEAN_EXPRESSION.isEnabled()
9201+
);
91869202
String query = """
91879203
from test
91889204
| rename languages as language_code_left

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownAndCombineFiltersTests.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
import org.elasticsearch.index.IndexMode;
1111
import org.elasticsearch.index.query.QueryBuilder;
12+
import org.elasticsearch.xpack.esql.action.EsqlCapabilities;
1213
import org.elasticsearch.xpack.esql.core.expression.Alias;
1314
import org.elasticsearch.xpack.esql.core.expression.Attribute;
1415
import org.elasticsearch.xpack.esql.core.expression.Expression;
@@ -931,6 +932,10 @@ public void testPushDownFilterPastLeftJoinExpressionWithPartiallyPushableAnd() {
931932
*
932933
*/
933934
public void testDoNotPushDownIsNullFilterPastLookupJoinExpression() {
935+
assumeTrue(
936+
"requires LOOKUP JOIN ON boolean expression capability",
937+
EsqlCapabilities.Cap.LOOKUP_JOIN_ON_BOOLEAN_EXPRESSION.isEnabled()
938+
);
934939
var plan = plan("""
935940
FROM test
936941
| LOOKUP JOIN languages_lookup ON languages > language_code
@@ -955,6 +960,10 @@ public void testDoNotPushDownIsNullFilterPastLookupJoinExpression() {
955960
* \_EsRelation[languages_lookup][LOOKUP][language_code{f}#20, language_name{f}#21]
956961
*/
957962
public void testPushDownLookupJoinExpressionMultipleWhere() {
963+
assumeTrue(
964+
"requires LOOKUP JOIN ON boolean expression capability",
965+
EsqlCapabilities.Cap.LOOKUP_JOIN_ON_BOOLEAN_EXPRESSION.isEnabled()
966+
);
958967
var plan = plan("""
959968
FROM test
960969
| LOOKUP JOIN languages_lookup ON languages <= language_code

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownJoinPastProjectTests.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,10 @@ public void testShadowingAfterPushdown2() {
250250
*/
251251
public void testShadowingAfterPushdownExpressionJoin() {
252252
assumeTrue("Requires LOOKUP JOIN", EsqlCapabilities.Cap.JOIN_LOOKUP_V12.isEnabled());
253+
assumeTrue(
254+
"requires LOOKUP JOIN ON boolean expression capability",
255+
EsqlCapabilities.Cap.LOOKUP_JOIN_ON_BOOLEAN_EXPRESSION.isEnabled()
256+
);
253257

254258
String query = """
255259
FROM test_lookup
@@ -295,6 +299,10 @@ public void testShadowingAfterPushdownExpressionJoin() {
295299
*/
296300
public void testShadowingAfterPushdownExpressionJoinKeepOrig() {
297301
assumeTrue("Requires LOOKUP JOIN", EsqlCapabilities.Cap.JOIN_LOOKUP_V12.isEnabled());
302+
assumeTrue(
303+
"requires LOOKUP JOIN ON boolean expression capability",
304+
EsqlCapabilities.Cap.LOOKUP_JOIN_ON_BOOLEAN_EXPRESSION.isEnabled()
305+
);
298306

299307
String query = """
300308
FROM test_lookup
@@ -344,6 +352,10 @@ public void testShadowingAfterPushdownExpressionJoinKeepOrig() {
344352
*/
345353
public void testShadowingAfterPushdownRenameExpressionJoin() {
346354
assumeTrue("Requires LOOKUP JOIN", EsqlCapabilities.Cap.JOIN_LOOKUP_V12.isEnabled());
355+
assumeTrue(
356+
"requires LOOKUP JOIN ON boolean expression capability",
357+
EsqlCapabilities.Cap.LOOKUP_JOIN_ON_BOOLEAN_EXPRESSION.isEnabled()
358+
);
347359

348360
String query = """
349361
FROM test_lookup
@@ -387,6 +399,10 @@ public void testShadowingAfterPushdownRenameExpressionJoin() {
387399
*/
388400
public void testShadowingAfterPushdownEvalExpressionJoin() {
389401
assumeTrue("Requires LOOKUP JOIN", EsqlCapabilities.Cap.JOIN_LOOKUP_V12.isEnabled());
402+
assumeTrue(
403+
"requires LOOKUP JOIN ON boolean expression capability",
404+
EsqlCapabilities.Cap.LOOKUP_JOIN_ON_BOOLEAN_EXPRESSION.isEnabled()
405+
);
390406

391407
String query = """
392408
FROM test_lookup

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java‎

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3169,8 +3169,17 @@ public void testValidJoinPatternFieldJoin() {
31693169
assertThat(joinType.coreJoin().joinName(), equalTo("LEFT OUTER"));
31703170
}
31713171

3172+
public void testExpressionJoinNonSnapshotBuild() {
3173+
assumeFalse("LOOKUP JOIN is not yet in non-snapshot builds", Build.current().isSnapshot());
3174+
expectThrows(
3175+
ParsingException.class,
3176+
startsWith("line 1:31: JOIN ON clause only supports fields at the moment."),
3177+
() -> statement("FROM test | LOOKUP JOIN test2 ON left_field >= right_field")
3178+
);
3179+
}
3180+
31723181
public void testValidJoinPatternExpressionJoin() {
3173-
assumeTrue("LOOKUP JOIN requires corresponding capability", EsqlCapabilities.Cap.JOIN_LOOKUP_V12.isEnabled());
3182+
assumeTrue("LOOKUP JOIN requires corresponding capability", EsqlCapabilities.Cap.LOOKUP_JOIN_ON_BOOLEAN_EXPRESSION.isEnabled());
31743183

31753184
var basePattern = randomIndexPatterns(without(CROSS_CLUSTER));
31763185
var joinPattern = randomIndexPattern(without(WILDCARD_PATTERN), without(CROSS_CLUSTER), without(INDEX_SELECTOR));
@@ -3200,7 +3209,9 @@ public void testValidJoinPatternExpressionJoin() {
32003209
}
32013210
}
32023211

3203-
var plan = statement("FROM " + basePattern + " | LOOKUP JOIN " + joinPattern + " ON " + onExpressionString);
3212+
// add a check that the feature is disabled on non-snaphsot build
3213+
String query = "FROM " + basePattern + " | LOOKUP JOIN " + joinPattern + " ON " + onExpressionString;
3214+
var plan = statement(query);
32043215

32053216
var join = as(plan, LookupJoin.class);
32063217
assertThat(as(join.left(), UnresolvedRelation.class).indexPattern().indexPattern(), equalTo(unquoteIndexPattern(basePattern)));
@@ -3356,6 +3367,10 @@ public void testValidJoinPatternWithRemoteFieldJoin() {
33563367
}
33573368

33583369
public void testValidJoinPatternWithRemoteExpressionJoin() {
3370+
assumeTrue(
3371+
"requires LOOKUP JOIN ON boolean expression capability",
3372+
EsqlCapabilities.Cap.LOOKUP_JOIN_ON_BOOLEAN_EXPRESSION.isEnabled()
3373+
);
33593374
testValidJoinPatternWithRemote(singleExpressionJoinClause());
33603375
}
33613376

@@ -3394,6 +3409,10 @@ public void testInvalidJoinPatternsExpressionJoinMixTwo() {
33943409
}
33953410

33963411
public void testInvalidLookupJoinOnClause() {
3412+
assumeTrue(
3413+
"requires LOOKUP JOIN ON boolean expression capability",
3414+
EsqlCapabilities.Cap.LOOKUP_JOIN_ON_BOOLEAN_EXPRESSION.isEnabled()
3415+
);
33973416
expectError(
33983417
"FROM test | LOOKUP JOIN test2 ON " + randomIdentifier() + " , " + singleExpressionJoinClause(),
33993418
"JOIN ON clause must be a comma separated list of fields or a single expression, found"
@@ -3699,7 +3718,6 @@ public void testForkAllReleasedCommands() {
36993718
( MV_EXPAND a )
37003719
( CHANGE_POINT a on b )
37013720
( LOOKUP JOIN idx2 ON f1 )
3702-
( LOOKUP JOIN idx2 ON f2 >= f3 )
37033721
( ENRICH idx2 on f1 with f2 = f3 )
37043722
( FORK ( WHERE a:"baz" ) ( EVAL x = [ 1, 2, 3 ] ) )
37053723
( COMPLETION a=b WITH { "inference_id": "c" } )
@@ -4154,6 +4172,10 @@ public void testInvalidFuse() {
41544172

41554173
public void testDoubleParamsForIdentifier() {
41564174
assumeTrue("double parameters markers for identifiers", EsqlCapabilities.Cap.DOUBLE_PARAMETER_MARKERS_FOR_IDENTIFIERS.isEnabled());
4175+
assumeTrue(
4176+
"requires LOOKUP JOIN ON boolean expression capability",
4177+
EsqlCapabilities.Cap.LOOKUP_JOIN_ON_BOOLEAN_EXPRESSION.isEnabled()
4178+
);
41574179
// There are three variations of double parameters - named, positional or anonymous, e.g. ??n, ??1 or ??, covered.
41584180
// Each query is executed three times with the three variations.
41594181

@@ -4656,6 +4678,10 @@ public void testDoubleParamsForIdentifier() {
46564678

46574679
public void testMixedSingleDoubleParams() {
46584680
assumeTrue("double parameters markers for identifiers", EsqlCapabilities.Cap.DOUBLE_PARAMETER_MARKERS_FOR_IDENTIFIERS.isEnabled());
4681+
assumeTrue(
4682+
"requires LOOKUP JOIN ON boolean expression capability",
4683+
EsqlCapabilities.Cap.LOOKUP_JOIN_ON_BOOLEAN_EXPRESSION.isEnabled()
4684+
);
46594685
// This is a subset of testDoubleParamsForIdentifier, with single and double parameter markers mixed in the queries
46604686
// Single parameter markers represent a constant value or pattern
46614687
// double parameter markers represent identifiers - field or function names
@@ -4959,7 +4985,9 @@ public void testBracketsInIndexNames() {
49594985
expectError("from te()st", "line 1:8: token recognition error at: '('");
49604986
expectError("from test | enrich foo)", "line -1:-1: Invalid query [from test | enrich foo)]");
49614987
expectError("from test | lookup join foo) on bar", "line 1:28: token recognition error at: ')'");
4962-
expectError("from test | lookup join foo) on bar1 > bar2", "line 1:28: token recognition error at: ')'");
4988+
if (EsqlCapabilities.Cap.LOOKUP_JOIN_ON_BOOLEAN_EXPRESSION.isEnabled()) {
4989+
expectError("from test | lookup join foo) on bar1 > bar2", "line 1:28: token recognition error at: ')'");
4990+
}
49634991
}
49644992

49654993
private void expectErrorForBracketsWithoutQuotes(String pattern) {
@@ -5001,11 +5029,12 @@ private void expectSuccessForBracketsWithinQuotes(String indexName) {
50015029
LookupJoin lookup = as(plan, LookupJoin.class);
50025030
UnresolvedRelation right = as(lookup.right(), UnresolvedRelation.class);
50035031
assertThat(right.indexPattern().indexPattern(), is(indexName));
5004-
5005-
plan = statement("from test | lookup join \"" + indexName + "\" on bar1 <= bar2");
5006-
lookup = as(plan, LookupJoin.class);
5007-
right = as(lookup.right(), UnresolvedRelation.class);
5008-
assertThat(right.indexPattern().indexPattern(), is(indexName));
5032+
if (EsqlCapabilities.Cap.LOOKUP_JOIN_ON_BOOLEAN_EXPRESSION.isEnabled()) {
5033+
plan = statement("from test | lookup join \"" + indexName + "\" on bar1 <= bar2");
5034+
lookup = as(plan, LookupJoin.class);
5035+
right = as(lookup.right(), UnresolvedRelation.class);
5036+
assertThat(right.indexPattern().indexPattern(), is(indexName));
5037+
}
50095038
}
50105039
}
50115040
}

0 commit comments

Comments
 (0)