Skip to content

ESQL - full text functions verifier tests refactor #128775

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 18 commits into from
Jun 3, 2025
Merged

ESQL - full text functions verifier tests refactor #128775

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
28c00b2
Refactor verifier tests
carlosdelest May 30, 2025
cd7a468
Refactor verifier tests
carlosdelest May 30, 2025
c417c01
Simplify tests
carlosdelest Jun 2, 2025
ba53a4d
Simplify tests
carlosdelest Jun 2, 2025
e35cb96
Simplify tests
carlosdelest Jun 2, 2025
6580832
Fix merge
carlosdelest Jun 2, 2025
b35e06f
Merge branch 'main' into non-issue/esql-full-text-functions-verifier-…
carlosdelest Jun 3, 2025
43997d8
Add capabilities checks
carlosdelest Jun 3, 2025
0dda9d2
Merge remote-tracking branch 'carlosdelest/non-issue/esql-full-text-f…
carlosdelest Jun 3, 2025
3cfdccb
Merge branch 'main' into non-issue/esql-full-text-functions-verifier-…
carlosdelest Jun 3, 2025
713de2b
Add new full text functions data set and modify VerifierTests
carlosdelest Jun 3, 2025
1774cb0
Don't use data with the CSV data loader, just the mapping
carlosdelest Jun 3, 2025
0742884
Spotless
carlosdelest Jun 3, 2025
ac0982a
Fix naming and visibility of helper methods
carlosdelest Jun 3, 2025
f3da4da
Fix merge
carlosdelest Jun 3, 2025
94bb978
Merge remote-tracking branch 'carlosdelest/non-issue/esql-full-text-f…
carlosdelest Jun 3, 2025
2818298
Fix naming of helper methods
carlosdelest Jun 3, 2025
ede0896
Remove colors dataset from knn
carlosdelest Jun 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion ...n/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/CsvTestsDataLoader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ public class CsvTestsDataLoader {
private static final TestDataset LOGS = new TestDataset("logs");
private static final TestDataset MV_TEXT = new TestDataset("mv_text");
private static final TestDataset DENSE_VECTOR = new TestDataset("dense_vector");
private static final TestDataset COLORS = new TestDataset("colors");

public static final Map<String, TestDataset> CSV_DATASET_MAP = Map.ofEntries(
Map.entry(EMPLOYEES.indexName, EMPLOYEES),
Expand Down Expand Up @@ -204,7 +205,8 @@ public class CsvTestsDataLoader {
Map.entry(SEMANTIC_TEXT.indexName, SEMANTIC_TEXT),
Map.entry(LOGS.indexName, LOGS),
Map.entry(MV_TEXT.indexName, MV_TEXT),
Map.entry(DENSE_VECTOR.indexName, DENSE_VECTOR)
Map.entry(DENSE_VECTOR.indexName, DENSE_VECTOR),
Map.entry(COLORS.indexName, COLORS)
);

private static final EnrichConfig LANGUAGES_ENRICH = new EnrichConfig("languages_policy", "enrich-policy-languages.json");
Expand Down
26 changes: 26 additions & 0 deletions x-pack/plugin/esql/qa/testFixtures/src/main/resources/mapping-full_text_search.json
View file
Open in desktop
Copy link
Member Author

@carlosdelest carlosdelest Jun 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added this new mapping, as I found easier when working on KNN to have a separate mapping for full text functions than to modify the default mapping - which impacted a lot of other tests.

This way we can be the owners of this mapping in case we want to add later for example sparse_vector field types, and not impact other test cases.

Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
{
"properties": {
"id": {
"type": "integer"
},
"title": {
"type": "text"
},
"body": {
"type": "text"
},
"tags": {
"type": "keyword"
},
"category": {
"type": "integer"
},
"published_date": {
"type": "date"
},
"vector": {
"type": "dense_vector",
"similarity": "l2_norm"
}
}
}
18 changes: 1 addition & 17 deletions ...main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/FullTextFunction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,23 +204,7 @@ private static void checkFullTextQueryFunctions(LogicalPlan plan, Failures failu
checkCommandsBeforeExpression(
plan,
condition,
Match.class,
lp -> (lp instanceof Limit == false) && (lp instanceof Aggregate == false),
m -> "[" + m.functionName() + "] " + m.functionType(),
failures
);
checkCommandsBeforeExpression(
plan,
condition,
MultiMatch.class,
lp -> (lp instanceof Limit == false) && (lp instanceof Aggregate == false),
m -> "[" + m.functionName() + "] " + m.functionType(),
failures
);
checkCommandsBeforeExpression(
plan,
condition,
Term.class,
FullTextFunction.class,
Copy link
Member Author

@carlosdelest carlosdelest Jun 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplified this to just run on all FullTextFunctions

lp -> (lp instanceof Limit == false) && (lp instanceof Aggregate == false),
m -> "[" + m.functionName() + "] " + m.functionType(),
failures
Expand Down
2 changes: 1 addition & 1 deletion ...l/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/MultiMatch.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ public String functionName() {
private TypeResolution resolveFields() {
return fields.stream()
.map(
(Expression field) -> isNotNull(field, sourceText(), FIRST).and(
(Expression field) -> isNotNull(field, sourceText(), SECOND).and(
Copy link
Member Author

@carlosdelest carlosdelest Jun 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed a bug in MultiMatch when refactoring the tests

isType(
field,
FIELD_DATA_TYPES::contains,
Expand Down
Loading