Skip to content

Commit 1af4bb8

Browse files
Bugfix
1 parent b6ab940 commit 1af4bb8

File tree

5 files changed

+45
-58
lines changed

5 files changed

+45
-58
lines changed

x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/analysis/MutableAnalyzerContext.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,14 @@ public TransportVersion minimumVersion() {
5252
public RestoreTransportVersion setTemporaryTransportVersionOnOrAfter(TransportVersion minVersion) {
5353
TransportVersion oldVersion = this.currentVersion;
5454
// Set to a random version between minVersion and current
55-
this.currentVersion = TransportVersionUtils.randomVersionBetween(ESTestCase.random(), minVersion, TransportVersion.current());
55+
// have 50+% probability of being the current version, to improve probability of catching a change
56+
// that only affects the version we are trying to check in
57+
boolean useCurrent = ESTestCase.randomBoolean();
58+
if (useCurrent) {
59+
this.currentVersion = TransportVersion.current();
60+
} else {
61+
this.currentVersion = TransportVersionUtils.randomVersionBetween(ESTestCase.random(), minVersion, TransportVersion.current());
62+
}
5663
return new RestoreTransportVersion(oldVersion);
5764
}
5865

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

Lines changed: 6 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -439,35 +439,15 @@ private PhysicalPlan buildRightPreJoinPlan(List<DataType> keyTypes, Expression f
439439
* This ensures consistent NameId usage across MatchConfig and join conditions.
440440
*/
441441
private FieldAttribute getLeftSideAttribute(int index, DataType keyType) {
442-
String name = "key" + index;
443-
// Handle special case for key3 which can be INTEGER or KEYWORD
444-
if (index == 3 && keyType == DataType.INTEGER) {
445-
return getAttribute(name, DataType.INTEGER);
446-
}
447-
// Default to KEYWORD or LONG based on keyType
448-
if (keyType == DataType.KEYWORD) {
449-
return getAttribute(name, DataType.KEYWORD);
450-
} else {
451-
return getAttribute(name, DataType.LONG);
452-
}
442+
return getAttribute("key" + index, keyType);
453443
}
454444

455445
/**
456446
* Gets the right-side attribute for a given index and type.
457447
* This ensures consistent NameId usage across join conditions and rightPreJoinPlan.
458448
*/
459449
private FieldAttribute getRightSideAttribute(int index, DataType keyType) {
460-
String name = "rkey" + index;
461-
// Handle special case for rkey3 which can be INTEGER or KEYWORD
462-
if (index == 3 && keyType == DataType.INTEGER) {
463-
return getAttribute(name, DataType.INTEGER);
464-
}
465-
// Default to KEYWORD or LONG based on keyType
466-
if (keyType == DataType.KEYWORD) {
467-
return getAttribute(name, DataType.KEYWORD);
468-
} else {
469-
return getAttribute(name, DataType.LONG);
470-
}
450+
return getAttribute("rkey" + index, keyType);
471451
}
472452

473453
private List<Attribute> buildRightSideAttributes(List<DataType> keyTypes) {
@@ -599,7 +579,7 @@ private void runLookup(List<DataType> keyTypes, PopulateIndices populateIndices,
599579
TEST_REQUEST_TIMEOUT
600580
);
601581
final String finalNodeWithShard = nodeWithShard;
602-
boolean expressionJoin = true;// EsqlCapabilities.Cap.LOOKUP_JOIN_ON_BOOLEAN_EXPRESSION.isEnabled() ? randomBoolean() : false;
582+
boolean expressionJoin = EsqlCapabilities.Cap.LOOKUP_JOIN_ON_BOOLEAN_EXPRESSION.isEnabled() ? randomBoolean() : false;
603583
List<MatchConfig> matchFields = new ArrayList<>();
604584
List<Expression> joinOnConditions = new ArrayList<>();
605585
if (expressionJoin) {
@@ -609,9 +589,9 @@ private void runLookup(List<DataType> keyTypes, PopulateIndices populateIndices,
609589
FieldAttribute rightAttr = getRightSideAttribute(i, keyTypes.get(i));
610590
joinOnConditions.add(new Equals(Source.EMPTY, leftAttr, rightAttr));
611591
// randomly decide to apply the filter as additional join on filter instead of pushed down filter
612-
boolean applyAsJoinOnCondition = true;// EsqlCapabilities.Cap.LOOKUP_JOIN_WITH_FULL_TEXT_FUNCTION.isEnabled()
613-
// ? randomBoolean()
614-
// : false;
592+
boolean applyAsJoinOnCondition = EsqlCapabilities.Cap.LOOKUP_JOIN_WITH_FULL_TEXT_FUNCTION.isEnabled()
593+
? randomBoolean()
594+
: false;
615595
if (applyAsJoinOnCondition
616596
&& pushedDownFilter instanceof FragmentExec fragmentExec
617597
&& fragmentExec.fragment() instanceof Filter filter) {

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/MatchConfig.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,8 @@ public MatchConfig(StreamInput in) throws IOException {
8181
@Override
8282
public void writeTo(StreamOutput out) throws IOException {
8383
if (out.getTransportVersion().onOrAfter(ESQL_LOOKUP_JOIN_GENERAL_EXPRESSION)) {
84-
// New format: write NamedExpression directly, same as Enrich and EnrichExec
8584
out.writeNamedWriteable(fieldName);
8685
} else {
87-
// Old format: write field name as string
8886
out.writeString(fieldName.name());
8987
}
9088
out.writeInt(channel);

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -784,11 +784,11 @@ private PhysicalOperation planLookupJoin(LookupJoinExec join, LocalExecutionPlan
784784

785785
// TODO: Using exactAttribute was supposed to handle TEXT fields with KEYWORD subfields - but we don't allow these in lookup
786786
// indices, so the call to exactAttribute looks redundant now.
787-
Attribute fieldName = right.exactAttribute();
787+
Attribute matchFieldsAttribute = right.exactAttribute();
788788

789789
// we support 2 types of joins: Field name joins and Expression joins
790790
// for Field name join, we do not ship any join on expression.
791-
// we built the Lucene query on the field name that is passed in the MatchConfig.fieldName
791+
// we built the Lucene query on the field name that is passed in the MatchConfig.matchFieldsAttribute
792792
// so for Field name we need to pass the attribute name from the right side, because that is needed to build the query
793793
// For expression joins, we pass an expression such as left_id > right_id.
794794
// So in this case we pass in left_id as the field name, because that is what we are shipping to the lookup node
@@ -798,9 +798,9 @@ private PhysicalOperation planLookupJoin(LookupJoinExec join, LocalExecutionPlan
798798
// e.g. LOOKUP JOIN ON left_id < right_id_1 and left_id >= right_id_2
799799
// we want to be able to optimize this in the future and only ship the left_id once
800800
if (join.isOnJoinExpression()) {
801-
fieldName = left;
801+
matchFieldsAttribute = left;
802802
}
803-
matchFields.add(new MatchConfig(fieldName, input));
803+
matchFields.add(new MatchConfig(matchFieldsAttribute, input));
804804
matchFieldIds.add(left.id());
805805
}
806806
// Extract additional left-side fields referenced in joinOnConditions that aren't already in matchFields

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

Lines changed: 27 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
import static org.elasticsearch.xpack.esql.EsqlTestUtils.testAnalyzerContext;
5454
import static org.elasticsearch.xpack.esql.EsqlTestUtils.withDefaultLimitWarning;
5555
import static org.elasticsearch.xpack.esql.analysis.Analyzer.ESQL_LOOKUP_JOIN_FULL_TEXT_FUNCTION;
56+
import static org.elasticsearch.xpack.esql.analysis.Analyzer.ESQL_LOOKUP_JOIN_GENERAL_EXPRESSION;
5657
import static org.elasticsearch.xpack.esql.analysis.AnalyzerTestUtils.TEXT_EMBEDDING_INFERENCE_ID;
5758
import static org.elasticsearch.xpack.esql.analysis.AnalyzerTestUtils.defaultLookupResolution;
5859
import static org.elasticsearch.xpack.esql.analysis.AnalyzerTestUtils.loadMapping;
@@ -2340,7 +2341,10 @@ public void testLookupJoinDataTypeMismatch() {
23402341

23412342
assertEquals(
23422343
"1:87: JOIN left field [language_code] of type [KEYWORD] is incompatible with right field [language_code] of type [INTEGER]",
2343-
error("FROM test | EVAL language_code = languages::keyword | LOOKUP JOIN languages_lookup ON language_code")
2344+
error(
2345+
"FROM test | EVAL language_code = languages::keyword | LOOKUP JOIN languages_lookup ON language_code",
2346+
ESQL_LOOKUP_JOIN_GENERAL_EXPRESSION
2347+
)
23442348
);
23452349
}
23462350

@@ -2364,66 +2368,59 @@ public void testLookupJoinExpressionAmbiguousRight() {
23642368
public void testLookupJoinExpressionRightNotPushable() {
23652369
assumeTrue(
23662370
"requires LOOKUP JOIN ON boolean expression capability",
2367-
EsqlCapabilities.Cap.LOOKUP_JOIN_WITH_FULL_TEXT_FUNCTION.isEnabled()
2371+
EsqlCapabilities.Cap.LOOKUP_JOIN_WITH_GENERAL_EXPRESSION.isEnabled()
23682372
);
23692373
String queryString = """
23702374
from test
23712375
| rename languages as languages_left
23722376
| lookup join languages_lookup ON languages_left == language_code and abs(salary) > 1000
23732377
""";
23742378

2375-
assertEquals(
2376-
"3:71: Unsupported join filter expression:abs(salary) > 1000",
2377-
error(queryString, ESQL_LOOKUP_JOIN_FULL_TEXT_FUNCTION)
2378-
);
2379+
query(queryString, ESQL_LOOKUP_JOIN_GENERAL_EXPRESSION);
23792380
}
23802381

23812382
public void testLookupJoinExpressionConstant() {
23822383
assumeTrue(
23832384
"requires LOOKUP JOIN ON boolean expression capability",
2384-
EsqlCapabilities.Cap.LOOKUP_JOIN_WITH_FULL_TEXT_FUNCTION.isEnabled()
2385+
EsqlCapabilities.Cap.LOOKUP_JOIN_WITH_GENERAL_EXPRESSION.isEnabled()
23852386
);
23862387
String queryString = """
23872388
from test
23882389
| rename languages as languages_left
23892390
| lookup join languages_lookup ON false and languages_left == language_code
23902391
""";
23912392

2392-
assertEquals("3:35: Unsupported join filter expression:false", error(queryString, ESQL_LOOKUP_JOIN_FULL_TEXT_FUNCTION));
2393+
query(queryString, ESQL_LOOKUP_JOIN_GENERAL_EXPRESSION);
23932394
}
23942395

23952396
public void testLookupJoinExpressionTranslatableButFromLeft() {
23962397
assumeTrue(
23972398
"requires LOOKUP JOIN ON boolean expression capability",
2398-
EsqlCapabilities.Cap.LOOKUP_JOIN_WITH_FULL_TEXT_FUNCTION.isEnabled()
2399+
EsqlCapabilities.Cap.LOOKUP_JOIN_WITH_GENERAL_EXPRESSION.isEnabled()
23992400
);
24002401
String queryString = """
24012402
from test
24022403
| rename languages as languages_left
2403-
| lookup join languages_lookup ON languages_left == language_code and languages_left == "English"
2404+
| eval languages_left_str = to_string(languages_left)
2405+
| lookup join languages_lookup ON languages_left == language_code and languages_left_str == language_name
24042406
""";
24052407

2406-
assertEquals(
2407-
"3:71: Unsupported join filter expression:languages_left == \"English\"",
2408-
error(queryString, ESQL_LOOKUP_JOIN_FULL_TEXT_FUNCTION)
2409-
);
2408+
query(queryString, ESQL_LOOKUP_JOIN_GENERAL_EXPRESSION);
24102409
}
24112410

24122411
public void testLookupJoinExpressionTranslatableButMixedLeftRight() {
24132412
assumeTrue(
24142413
"requires LOOKUP JOIN ON boolean expression capability",
2415-
EsqlCapabilities.Cap.LOOKUP_JOIN_WITH_FULL_TEXT_FUNCTION.isEnabled()
2414+
EsqlCapabilities.Cap.LOOKUP_JOIN_WITH_GENERAL_EXPRESSION.isEnabled()
24162415
);
24172416
String queryString = """
24182417
from test
24192418
| rename languages as languages_left
2420-
| lookup join languages_lookup ON languages_left == language_code and CONCAT(languages_left, language_code) == "English"
2419+
| eval language_name_left = "English"
2420+
| lookup join languages_lookup ON languages_left == language_code and CONCAT(language_name_left, language_name) == "English"
24212421
""";
24222422

2423-
assertEquals(
2424-
"3:71: Unsupported join filter expression:CONCAT(languages_left, language_code) == \"English\"",
2425-
error(queryString, ESQL_LOOKUP_JOIN_FULL_TEXT_FUNCTION)
2426-
);
2423+
query(queryString, ESQL_LOOKUP_JOIN_GENERAL_EXPRESSION);
24272424
}
24282425

24292426
public void testLookupJoinExpressionComplexFormula() {
@@ -2434,13 +2431,11 @@ public void testLookupJoinExpressionComplexFormula() {
24342431
String queryString = """
24352432
from test
24362433
| rename languages as languages_left
2437-
| lookup join languages_lookup ON languages_left == language_code AND STARTSWITH(languages_left, language_code)
2434+
| eval languages_left_str = to_string(languages_left)
2435+
| lookup join languages_lookup ON languages_left == language_code AND starts_with(languages_left_str, language_name)
24382436
""";
24392437

2440-
assertEquals(
2441-
"3:71: Unsupported join filter expression:STARTSWITH(languages_left, language_code)",
2442-
error(queryString, ESQL_LOOKUP_JOIN_FULL_TEXT_FUNCTION)
2443-
);
2438+
query(queryString, ESQL_LOOKUP_JOIN_GENERAL_EXPRESSION);
24442439
}
24452440

24462441
public void testLookupJoinExpressionAmbiguousLeft() {
@@ -3348,6 +3343,13 @@ private void query(String query, Analyzer analyzer) {
33483343
analyzer.analyze(parser.createStatement(query));
33493344
}
33503345

3346+
private void query(String query, TransportVersion transportVersion) {
3347+
MutableAnalyzerContext mutableContext = (MutableAnalyzerContext) defaultAnalyzer.context();
3348+
try (var restore = mutableContext.setTemporaryTransportVersionOnOrAfter(transportVersion)) {
3349+
query(query);
3350+
}
3351+
}
3352+
33513353
private String error(String query) {
33523354
return error(query, defaultAnalyzer);
33533355
}

0 commit comments

Comments
 (0)