Skip to content

ESQL: Fix validation NPE in Enrich and add extra @Nullable annotations #128260

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 2 commits into from
May 23, 2025
Merged

ESQL: Fix validation NPE in Enrich and add extra @Nullable annotations #128260

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b05734b
ESQL: Fix validation NPE in Enrich and add extra @Nullable annotations
ivancea May 21, 2025
539a8ed
Update docs/changelog/128260.yaml
ivancea May 21, 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
7 changes: 7 additions & 0 deletions docs/changelog/128260.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
pr: 128260
summary: Fix validation NPE in Enrich and add extra @Nullable annotations
area: ES|QL
type: bug
issues:
- 126297
- 126253
4 changes: 2 additions & 2 deletions .../plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/AbstractLookupService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ protected abstract QueryList queryList(
T request,
SearchExecutionContext context,
Block inputBlock,
DataType inputDataType,
@Nullable DataType inputDataType,
Warnings warnings
);

Expand All @@ -200,7 +200,7 @@ protected static QueryList termQueryList(
MappedFieldType field,
SearchExecutionContext searchExecutionContext,
Block block,
DataType inputDataType
@Nullable DataType inputDataType
) {
return switch (inputDataType) {
case IP -> QueryList.ipTermQueryList(field, searchExecutionContext, (BytesRefBlock) block);
Expand Down
9 changes: 5 additions & 4 deletions ...ck/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/EnrichLookupService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.elasticsearch.compute.data.Page;
import org.elasticsearch.compute.operator.Warnings;
import org.elasticsearch.compute.operator.lookup.QueryList;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.core.Releasables;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.mapper.RangeFieldMapper;
Expand Down Expand Up @@ -103,7 +104,7 @@ protected QueryList queryList(
TransportRequest request,
SearchExecutionContext context,
Block inputBlock,
DataType inputDataType,
@Nullable DataType inputDataType,
Warnings warnings
) {
MappedFieldType fieldType = context.getFieldType(request.matchField);
Expand All @@ -128,8 +129,8 @@ protected LookupResponse readLookupResponse(StreamInput in, BlockFactory blockFa
return new LookupResponse(in, blockFactory);
}

private static void validateTypes(DataType inputDataType, MappedFieldType fieldType) {
if (fieldType instanceof RangeFieldMapper.RangeFieldType rangeType) {
private static void validateTypes(@Nullable DataType inputDataType, MappedFieldType fieldType) {
Copy link
Contributor Author

@ivancea ivancea May 21, 2025

Choose a reason for hiding this comment

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

The @Nullable annotations are showing a warning here as expected. But they're not showing a warning on method calls. Not perfect, but well. I think they may still help (?)

if (fieldType instanceof RangeFieldMapper.RangeFieldType rangeType && inputDataType != null) {
// For range policy types, the ENRICH index field type will be one of a list of supported range types,
// which need to match the input data type (eg. ip-range -> ip, date-range -> date, etc.)
if (rangeTypesCompatible(rangeType.rangeType(), inputDataType) == false) {
Expand All @@ -142,7 +143,7 @@ private static void validateTypes(DataType inputDataType, MappedFieldType fieldT
// For geo_match, type validation is done earlier, in the Analyzer.
}

private static boolean rangeTypesCompatible(RangeType rangeType, DataType inputDataType) {
private static boolean rangeTypesCompatible(RangeType rangeType, @Nullable DataType inputDataType) {
if (inputDataType.noText() == DataType.KEYWORD) {
// We allow runtime parsing of string types to numeric types
return true;
Expand Down
3 changes: 2 additions & 1 deletion ...plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/LookupFromIndexService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.elasticsearch.compute.data.Page;
import org.elasticsearch.compute.operator.Warnings;
import org.elasticsearch.compute.operator.lookup.QueryList;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.core.Releasables;
import org.elasticsearch.index.query.SearchExecutionContext;
import org.elasticsearch.index.shard.ShardId;
Expand Down Expand Up @@ -83,7 +84,7 @@ protected QueryList queryList(
TransportRequest request,
SearchExecutionContext context,
Block inputBlock,
DataType inputDataType,
@Nullable DataType inputDataType,
Warnings warnings
) {
return termQueryList(context.getFieldType(request.matchField), context, inputBlock, inputDataType).onlySingleValues(
Expand Down
Loading