Skip to content

Commit 64d081c

Browse files
carlosdelestcarlosdelest
committed
Make first tests pass
1 parent adb7e3e commit 64d081c

File tree

5 files changed

+123
-89
lines changed

5 files changed

+123
-89
lines changed

‎x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/LuceneQueryExpressionEvaluator.java‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,7 @@ public Block eval(Page page) {
8585

8686
@Override
8787
public DoubleBlock score(Page page, BlockFactory blockFactory) {
88-
if (scoreVector == null) {
89-
Releasables.closeExpectNoException(eval(page));
90-
}
88+
assert scoreVector != null : "eval() should be invoked before calling score()";
9189
return scoreVector.asBlock();
9290
}
9391

@@ -184,7 +182,6 @@ private Tuple<BooleanVector, DoubleVector> evalSlow(DocVector docs) throws IOExc
184182

185183
@Override
186184
public void close() {
187-
Releasables.closeExpectNoException(scoreVector);
188185
}
189186

190187
private ShardState shardState(int shard) throws IOException {
@@ -285,9 +282,9 @@ Tuple<BooleanVector, DoubleVector> scoreDense(int min, int max) throws IOExcepti
285282

286283
final DenseCollector collector;
287284
if (usesScoring) {
288-
collector = new DenseCollector(blockFactory, min, max);
289-
} else {
290285
collector = new ScoringDenseCollector(blockFactory, min, max);
286+
} else {
287+
collector = new DenseCollector(blockFactory, min, max);
291288
}
292289
try (collector) {
293290
bulkScorer.score(collector, ctx.reader().getLiveDocs(), min, max + 1);

‎x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/FilterOperator.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,14 +74,13 @@ protected Page process(Page page) {
7474
page.releaseBlocks();
7575
return null;
7676
}
77-
final DoubleBlock scoreBlock;
77+
DoubleBlock scoreBlock = null;
7878
if (usesScoring) {
7979
scoreBlock = evaluator.score(page, blockFactory);
80-
} else {
81-
scoreBlock = null;
80+
assert scoreBlock != null : "score block is when using scoring";
8281
}
8382

84-
if (rowCount == page.getPositionCount() && (usesScoring == false || scoreBlock.asVector().isConstant())) {
83+
if (rowCount == page.getPositionCount() && usesScoring == false) {
8584
return page;
8685
}
8786
positions = Arrays.copyOf(positions, rowCount);
@@ -98,8 +97,8 @@ protected Page process(Page page) {
9897
}
9998
success = true;
10099
} finally {
101-
Releasables.closeExpectNoException(scoreBlock);
102100
page.releaseBlocks();
101+
Releasables.closeExpectNoException(scoreBlock);
103102
if (success == false) {
104103
Releasables.closeExpectNoException(filteredBlocks);
105104
}
@@ -114,7 +113,7 @@ private Block createScoresBlock(int rowCount, DoubleBlock originalScoreBlock, Do
114113
for (int j = 0; j < rowCount; j++) {
115114
updatedScoresBuilder.appendDouble(originalScoreBlock.getDouble(positions[j]) + newScoreBlock.getDouble(positions[j]));
116115
}
117-
return updatedScoresBuilder.build().asBlock().filter(positions);
116+
return updatedScoresBuilder.build().asBlock();
118117
}
119118

120119
@Override

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

Lines changed: 115 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,11 @@
1919
import java.util.List;
2020

2121
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
22+
import static org.elasticsearch.xpack.esql.EsqlTestUtils.getValuesList;
2223
import static org.hamcrest.CoreMatchers.containsString;
24+
import static org.hamcrest.Matchers.equalTo;
25+
import static org.hamcrest.Matchers.greaterThan;
26+
import static org.hamcrest.Matchers.lessThan;
2327

2428
//@TestLogging(value = "org.elasticsearch.xpack.esql:TRACE,org.elasticsearch.compute:TRACE", reason = "debug")
2529
public class MatchFunctionIT extends AbstractEsqlIntegTestCase {
@@ -260,20 +264,124 @@ public void testMatchWithinEval() {
260264
assertThat(error.getMessage(), containsString("[MATCH] function is only supported in WHERE commands"));
261265
}
262266

267+
public void testDisjunctionScoring() {
268+
var query = """
269+
FROM test METADATA _score
270+
| WHERE match(content, "fox") OR length(content) < 20
271+
| KEEP id, _score
272+
| SORT _score DESC, id ASC
273+
""";
274+
275+
try (var resp = run(query)) {
276+
assertColumnNames(resp.columns(), List.of("id", "_score"));
277+
assertColumnTypes(resp.columns(), List.of("integer", "double"));
278+
List<List<Object>> values = getValuesList(resp);
279+
assertThat(values.size(), equalTo(3));
280+
281+
assertThat(values.get(0).get(0), equalTo(1));
282+
assertThat(values.get(1).get(0), equalTo(6));
283+
assertThat(values.get(2).get(0), equalTo(2));
284+
285+
// Matches full text query and non pushable query
286+
assertThat((Double) values.get(0).get(1), greaterThan(1.0));
287+
// Matches just non pushable query
288+
assertThat((Double) values.get(1).get(1), greaterThan(1.0));
289+
// Matches just full text query
290+
assertThat((Double) values.get(2).get(1), equalTo(0.0));
291+
}
292+
}
293+
294+
public void testDisjunctionScoringMultipleNonPushableFunctions() {
295+
var query = """
296+
FROM test METADATA _score
297+
| WHERE match(content, "fox") OR length(content) < 20 AND id > 2
298+
| KEEP id, _score
299+
| SORT _score DESC
300+
""";
301+
302+
try (var resp = run(query)) {
303+
assertColumnNames(resp.columns(), List.of("id", "_score"));
304+
assertColumnTypes(resp.columns(), List.of("integer", "double"));
305+
List<List<Object>> values = getValuesList(resp);
306+
assertThat(values.size(), equalTo(2));
307+
308+
assertThat(values.get(0).get(0), equalTo(1));
309+
assertThat(values.get(1).get(0), equalTo(6));
310+
311+
// Matches the full text query and a non pushable query
312+
assertThat((Double) values.get(0).get(1), greaterThan(1.0));
313+
assertThat((Double) values.get(0).get(1), lessThan(2.0));
314+
// Matches just the match function
315+
assertThat((Double) values.get(1).get(1), lessThan(1.0));
316+
assertThat((Double) values.get(1).get(1), greaterThan(0.0));
317+
}
318+
}
319+
320+
public void testDisjunctionScoringWithNot() {
321+
var query = """
322+
FROM test METADATA _score
323+
| WHERE NOT(match(content, "dog")) OR length(content) > 50
324+
| KEEP id, _score
325+
| SORT _score DESC, id ASC
326+
""";
327+
328+
try (var resp = run(query)) {
329+
assertColumnNames(resp.columns(), List.of("id", "_score"));
330+
assertColumnTypes(resp.columns(), List.of("integer", "double"));
331+
List<List<Object>> values = getValuesList(resp);
332+
assertThat(values.size(), equalTo(3));
333+
334+
assertThat(values.get(0).get(0), equalTo(1));
335+
assertThat(values.get(1).get(0), equalTo(4));
336+
assertThat(values.get(2).get(0), equalTo(5));
337+
338+
// Matches NOT and non pushable query gets score of 0.0
339+
assertThat((Double) values.get(0).get(1), equalTo(0.0));
340+
assertThat((Double) values.get(1).get(1), equalTo(0.0));
341+
assertThat((Double) values.get(2).get(1), equalTo(0.0));
342+
}
343+
}
344+
345+
public void testScoringWithNoFullTextFunction() {
346+
var query = """
347+
FROM test METADATA _score
348+
| WHERE length(content) > 50
349+
| KEEP id, _score
350+
| SORT _score DESC, id ASC
351+
""";
352+
353+
try (var resp = run(query)) {
354+
assertColumnNames(resp.columns(), List.of("id", "_score"));
355+
assertColumnTypes(resp.columns(), List.of("integer", "double"));
356+
List<List<Object>> values = getValuesList(resp);
357+
assertThat(values.size(), equalTo(1));
358+
359+
assertThat(values.get(0).get(0), equalTo(4));
360+
361+
// Non pushable query gets score of 0.0, summed with 1.0 coming from Lucene
362+
assertThat((Double) values.get(0).get(1), equalTo(1.0));
363+
}
364+
}
365+
263366
private void createAndPopulateIndex() {
264367
var indexName = "test";
265368
var client = client().admin().indices();
266369
var CreateRequest = client.prepareCreate(indexName)
267370
.setSettings(Settings.builder().put("index.number_of_shards", 1))
268-
.setMapping("id", "type=integer", "content", "type=text");
371+
.setMapping("id", "type=integer", "content", "type=text", "length", "type=integer");
269372
assertAcked(CreateRequest);
270373
client().prepareBulk()
271-
.add(new IndexRequest(indexName).id("1").source("id", 1, "content", "This is a brown fox"))
272-
.add(new IndexRequest(indexName).id("2").source("id", 2, "content", "This is a brown dog"))
273-
.add(new IndexRequest(indexName).id("3").source("id", 3, "content", "This dog is really brown"))
274-
.add(new IndexRequest(indexName).id("4").source("id", 4, "content", "The dog is brown but this document is very very long"))
275-
.add(new IndexRequest(indexName).id("5").source("id", 5, "content", "There is also a white cat"))
276-
.add(new IndexRequest(indexName).id("6").source("id", 6, "content", "The quick brown fox jumps over the lazy dog"))
374+
.add(new IndexRequest(indexName).id("1").source("id", 1, "content", "This is a brown fox", "length", 19))
375+
.add(new IndexRequest(indexName).id("2").source("id", 2, "content", "This is a brown dog", "length", 19))
376+
.add(new IndexRequest(indexName).id("3").source("id", 3, "content", "This dog is really brown", "length", 25))
377+
.add(
378+
new IndexRequest(indexName).id("4")
379+
.source("id", 4, "content", "The dog is brown but this document is very very long", "length", 52)
380+
)
381+
.add(new IndexRequest(indexName).id("5").source("id", 5, "content", "There is also a white cat", "length", 25))
382+
.add(
383+
new IndexRequest(indexName).id("6").source("id", 6, "content", "The quick brown fox jumps over the lazy dog", "length", 43)
384+
)
277385
.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)
278386
.get();
279387
ensureYellow(indexName);

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/EvalMapper.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,6 @@ private Block eval(BooleanVector lhs, BooleanVector rhs) {
181181
@Override
182182
public DoubleBlock score(Page page, BlockFactory blockFactory) {
183183
try (DoubleBlock lhs = leftEval.score(page, blockFactory); DoubleBlock rhs = rightEval.score(page, blockFactory)) {
184-
185184
int positionCount = lhs.getPositionCount();
186185
// TODO We could optimize for constant vectors
187186
try (var result = lhs.blockFactory().newDoubleVectorFixedBuilder(positionCount)) {

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/FullTextFunction.java‎

Lines changed: 0 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,15 @@
1717
import org.elasticsearch.xpack.esql.common.Failures;
1818
import org.elasticsearch.xpack.esql.core.expression.Expression;
1919
import org.elasticsearch.xpack.esql.core.expression.FoldContext;
20-
import org.elasticsearch.xpack.esql.core.expression.MetadataAttribute;
2120
import org.elasticsearch.xpack.esql.core.expression.Nullability;
2221
import org.elasticsearch.xpack.esql.core.expression.TypeResolutions;
2322
import org.elasticsearch.xpack.esql.core.expression.function.Function;
2423
import org.elasticsearch.xpack.esql.core.querydsl.query.Query;
2524
import org.elasticsearch.xpack.esql.core.tree.Source;
2625
import org.elasticsearch.xpack.esql.core.type.DataType;
27-
import org.elasticsearch.xpack.esql.core.util.Holder;
2826
import org.elasticsearch.xpack.esql.evaluator.mapper.EvaluatorMapper;
2927
import org.elasticsearch.xpack.esql.expression.predicate.logical.BinaryLogic;
3028
import org.elasticsearch.xpack.esql.expression.predicate.logical.Not;
31-
import org.elasticsearch.xpack.esql.expression.predicate.logical.Or;
3229
import org.elasticsearch.xpack.esql.optimizer.rules.physical.local.LucenePushdownPredicates;
3330
import org.elasticsearch.xpack.esql.plan.logical.Aggregate;
3431
import org.elasticsearch.xpack.esql.plan.logical.EsRelation;
@@ -208,79 +205,13 @@ private static void checkFullTextQueryFunctions(LogicalPlan plan, Failures failu
208205
failures
209206
);
210207
checkFullTextFunctionsParents(condition, failures);
211-
212-
boolean usesScore = plan.output()
213-
.stream()
214-
.anyMatch(attr -> attr instanceof MetadataAttribute ma && ma.name().equals(MetadataAttribute.SCORE));
215-
if (usesScore) {
216-
checkFullTextSearchDisjunctions(condition, failures);
217-
}
218208
} else {
219209
plan.forEachExpression(FullTextFunction.class, ftf -> {
220210
failures.add(fail(ftf, "[{}] {} is only supported in WHERE commands", ftf.functionName(), ftf.functionType()));
221211
});
222212
}
223213
}
224214

225-
/**
226-
* Checks whether a condition contains a disjunction with a full text search.
227-
* If it does, check that every element of the disjunction is a full text search or combinations (AND, OR, NOT) of them.
228-
* If not, add a failure to the failures collection.
229-
*
230-
* @param condition condition to check for disjunctions of full text searches
231-
* @param failures failures collection to add to
232-
*/
233-
private static void checkFullTextSearchDisjunctions(Expression condition, Failures failures) {
234-
Holder<Boolean> isInvalid = new Holder<>(false);
235-
condition.forEachDown(Or.class, or -> {
236-
if (isInvalid.get()) {
237-
// Exit early if we already have a failures
238-
return;
239-
}
240-
if (checkDisjunctionPushable(or) == false) {
241-
isInvalid.set(true);
242-
failures.add(
243-
fail(
244-
or,
245-
"Invalid condition when using METADATA _score [{}]. Full text functions can be used in an OR condition, "
246-
+ "but only if just full text functions are used in the OR condition",
247-
or.sourceText()
248-
)
249-
);
250-
}
251-
});
252-
}
253-
254-
/**
255-
* Checks if a disjunction is pushable from the point of view of FullTextFunctions. Either it has no FullTextFunctions or
256-
* all it contains are FullTextFunctions.
257-
*
258-
* @param or disjunction to check
259-
* @return true if the disjunction is pushable, false otherwise
260-
*/
261-
private static boolean checkDisjunctionPushable(Or or) {
262-
boolean hasFullText = or.anyMatch(FullTextFunction.class::isInstance);
263-
return hasFullText == false || onlyFullTextFunctionsInExpression(or);
264-
}
265-
266-
/**
267-
* Checks whether an expression contains just full text functions or negations (NOT) and combinations (AND, OR) of full text functions
268-
*
269-
* @param expression expression to check
270-
* @return true if all children are full text functions or negations of full text functions, false otherwise
271-
*/
272-
private static boolean onlyFullTextFunctionsInExpression(Expression expression) {
273-
if (expression instanceof FullTextFunction) {
274-
return true;
275-
} else if (expression instanceof Not) {
276-
return onlyFullTextFunctionsInExpression(expression.children().get(0));
277-
} else if (expression instanceof BinaryLogic binaryLogic) {
278-
return onlyFullTextFunctionsInExpression(binaryLogic.left()) && onlyFullTextFunctionsInExpression(binaryLogic.right());
279-
}
280-
281-
return false;
282-
}
283-
284215
/**
285216
* Checks all commands that exist before a specific type satisfy conditions.
286217
*

0 commit comments

Comments
 (0)