Skip to content

Commit 744479f

Browse files
Bugfixes
1 parent e367e8c commit 744479f

File tree

4 files changed

+17
-15
lines changed

4 files changed

+17
-15
lines changed

x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/lookup/ExpressionQueryList.java

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,13 @@
1313

1414
import java.util.List;
1515

16+
/**
17+
* A {@link LookupEnrichQueryGenerator} that combines multiple {@link QueryList}s into a single query.
18+
* Each query in the resulting query will be a conjunction of all queries from the input lists at the same position.
19+
* In the future we can extend this to support more complex expressions, such as disjunctions or negations.
20+
*/
1621
public class ExpressionQueryList implements LookupEnrichQueryGenerator {
17-
List<QueryList> queryLists;
22+
private final List<QueryList> queryLists;
1823

1924
public ExpressionQueryList(List<QueryList> queryLists) {
2025
if (queryLists.size() < 2) {
@@ -25,13 +30,10 @@ public ExpressionQueryList(List<QueryList> queryLists) {
2530

2631
@Override
2732
public Query getQuery(int position) {
28-
// JULIAN: Do we have a guarantee that the positions will match across all QueryLists?
29-
// for now we only support AND of the queries in the lists
3033
BooleanQuery.Builder builder = new BooleanQuery.Builder();
3134
for (QueryList queryList : queryLists) {
3235
Query q = queryList.getQuery(position);
33-
builder.add(q, BooleanClause.Occur.MUST);
34-
36+
builder.add(q, BooleanClause.Occur.FILTER);
3537
}
3638
return builder.build();
3739
}
@@ -49,6 +51,6 @@ public int getPositionCount() {
4951
);
5052
}
5153
}
52-
return queryLists.get(0).getPositionCount();
54+
return positionCount;
5355
}
5456
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -224,16 +224,16 @@ private void runLookup(DataType keyType, PopulateIndices populateIndices) throws
224224
TEST_REQUEST_TIMEOUT
225225
);
226226
final String finalNodeWithShard = nodeWithShard;
227+
List<LookupFromIndexOperator.MatchConfig> matchFields = new ArrayList<>();
228+
matchFields.add(new LookupFromIndexOperator.MatchConfig(new FieldAttribute.FieldName("key"), 1, keyType));
227229
LookupFromIndexOperator.Factory lookup = new LookupFromIndexOperator.Factory(
230+
matchFields,
228231
"test",
229232
parentTask,
230233
QueryPragmas.ENRICH_MAX_WORKERS.get(Settings.EMPTY),
231-
1,
232234
ctx -> internalCluster().getInstance(TransportEsqlQueryAction.class, finalNodeWithShard).getLookupFromIndexService(),
233-
keyType,
234235
"lookup",
235236
"lookup",
236-
new FieldAttribute.FieldName("key"),
237237
List.of(new Alias(Source.EMPTY, "l", new ReferenceAttribute(Source.EMPTY, "l", DataType.LONG))),
238238
Source.EMPTY
239239
);

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,6 @@ private void doLookup(T request, CancellableTask task, ActionListener<List<Page>
309309
final int[] mergingChannels = IntStream.range(0, request.extractFields.size()).map(i -> i + 2).toArray();
310310
final Operator finishPages;
311311
final OrdinalBytesRefBlock ordinalsBytesRefBlock;
312-
// JULIAN: SHOULD WE DO THE NEXT CODE FOR EACH BLOCK IN THE PAGE?
313312
Block inputBlock = request.inputPage.getBlock(0);
314313
if (mergePages // TODO fix this optimization for Lookup.
315314
&& inputBlock instanceof BytesRefBlock bytesRefBlock

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ public Operator get(DriverContext driverContext) {
110110
private final String lookupIndex;
111111
private final List<NamedExpression> loadFields;
112112
private final Source source;
113-
private long totalTerms = 0L;
113+
private long totalRows = 0L;
114114
private List<MatchConfig> matchFields;
115115
/**
116116
* Total number of pages emitted by this {@link Operator}.
@@ -146,14 +146,15 @@ public LookupFromIndexOperator(
146146

147147
@Override
148148
protected void performAsync(Page inputPage, ActionListener<OngoingJoin> listener) {
149-
// JULIAN: should I be getting multiple blocks, and send them using the LookupFromIndexService.Request
150-
// is the totalTerms supposed to be the total number of terms in all blocks combined?
151149
Block[] inputBlockArray = new Block[matchFields.size()];
152150
for (int i = 0; i < matchFields.size(); i++) {
153151
MatchConfig matchField = matchFields.get(i);
154152
int inputChannel = matchField.channel;
155153
final Block inputBlock = inputPage.getBlock(inputChannel);
156-
totalTerms += inputBlock.getTotalValueCount();
154+
if (i == 0) {
155+
// we only add to the totalRows once, so we can use the first block
156+
totalRows += inputBlock.getTotalValueCount();
157+
}
157158
inputBlockArray[i] = inputBlock;
158159
}
159160
LookupFromIndexService.Request request = new LookupFromIndexService.Request(
@@ -246,7 +247,7 @@ protected void doClose() {
246247

247248
@Override
248249
protected Operator.Status status(long receivedPages, long completedPages, long processNanos) {
249-
return new LookupFromIndexOperator.Status(receivedPages, completedPages, processNanos, totalTerms, emittedPages);
250+
return new LookupFromIndexOperator.Status(receivedPages, completedPages, processNanos, totalRows, emittedPages);
250251
}
251252

252253
public static class Status extends AsyncOperator.Status {

0 commit comments

Comments
 (0)