Skip to content

Commit 8bc990d

Browse files
carlosdelestcarlosdelest
committed
Validate parameters compatibility in match
1 parent 22dcdb4 commit 8bc990d

File tree

3 files changed

+82
-24
lines changed

3 files changed

+82
-24
lines changed

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

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
import org.elasticsearch.xpack.esql.core.expression.function.Function;
1515
import org.elasticsearch.xpack.esql.core.tree.Source;
1616
import org.elasticsearch.xpack.esql.core.type.DataType;
17-
import org.elasticsearch.xpack.esql.core.util.NumericUtils;
1817

1918
import java.util.List;
2019

@@ -47,7 +46,16 @@ protected final TypeResolution resolveType() {
4746
return new TypeResolution("Unresolved children");
4847
}
4948

50-
return resolveNonQueryParamTypes().and(resolveQueryParamType());
49+
return resolveNonQueryParamTypes().and(resolveQueryParamType().and(checkParamCompatibility()));
50+
}
51+
52+
/**
53+
* Checks parameter specific compatibility, to be overriden by subclasses
54+
*
55+
* @return TypeResolution for param compatibility
56+
*/
57+
protected TypeResolution checkParamCompatibility() {
58+
return TypeResolution.TYPE_RESOLVED;
5159
}
5260

5361
/**
@@ -77,12 +85,10 @@ public Expression query() {
7785
*
7886
* @return query expression as an object
7987
*/
80-
public final Object queryAsObject() {
88+
public Object queryAsObject() {
8189
Object queryAsObject = query().fold();
8290
if (queryAsObject instanceof BytesRef bytesRef) {
8391
return bytesRef.utf8ToString();
84-
} else if (query().dataType() == DataType.UNSIGNED_LONG) {
85-
return NumericUtils.unsignedLongAsBigInteger((Long) queryAsObject);
8692
}
8793

8894
return queryAsObject;

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

Lines changed: 58 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
package org.elasticsearch.xpack.esql.expression.function.fulltext;
99

10+
import org.apache.lucene.util.BytesRef;
1011
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
1112
import org.elasticsearch.common.io.stream.StreamInput;
1213
import org.elasticsearch.common.io.stream.StreamOutput;
@@ -20,10 +21,12 @@
2021
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
2122
import org.elasticsearch.xpack.esql.core.tree.Source;
2223
import org.elasticsearch.xpack.esql.core.type.DataType;
24+
import org.elasticsearch.xpack.esql.core.util.NumericUtils;
2325
import org.elasticsearch.xpack.esql.expression.function.Example;
2426
import org.elasticsearch.xpack.esql.expression.function.FunctionInfo;
2527
import org.elasticsearch.xpack.esql.expression.function.Param;
2628
import org.elasticsearch.xpack.esql.io.stream.PlanStreamInput;
29+
import org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter;
2730

2831
import java.io.IOException;
2932
import java.util.List;
@@ -46,6 +49,7 @@
4649
import static org.elasticsearch.xpack.esql.core.type.DataType.TEXT;
4750
import static org.elasticsearch.xpack.esql.core.type.DataType.UNSIGNED_LONG;
4851
import static org.elasticsearch.xpack.esql.core.type.DataType.VERSION;
52+
import static org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.EsqlBinaryComparison.formatIncompatibleTypesMessage;
4953

5054
/**
5155
* Full text function that performs a {@link QueryStringQuery} .
@@ -58,7 +62,7 @@ public class Match extends FullTextFunction implements Validatable {
5862

5963
private transient Boolean isOperator;
6064

61-
public static final Set<DataType> DATA_TYPES = Set.of(
65+
public static final Set<DataType> FIELD_DATA_TYPES = Set.of(
6266
KEYWORD,
6367
TEXT,
6468
BOOLEAN,
@@ -71,6 +75,18 @@ public class Match extends FullTextFunction implements Validatable {
7175
UNSIGNED_LONG,
7276
VERSION
7377
);
78+
public static final Set<DataType> QUERY_DATA_TYPES = Set.of(
79+
KEYWORD,
80+
BOOLEAN,
81+
DATETIME,
82+
DATE_NANOS,
83+
DOUBLE,
84+
INTEGER,
85+
IP,
86+
LONG,
87+
UNSIGNED_LONG,
88+
VERSION
89+
);
7490

7591
@FunctionInfo(
7692
returnType = "boolean",
@@ -87,7 +103,7 @@ public Match(
87103
) Expression field,
88104
@Param(
89105
name = "query",
90-
type = { "keyword", "text", "boolean", "date", "date_nanos", "double", "integer", "ip", "long", "unsigned_long", "version" },
106+
type = { "keyword", "boolean", "date", "date_nanos", "double", "integer", "ip", "long", "unsigned_long", "version" },
91107
description = "Text you wish to find in the provided field."
92108
) Expression matchQuery
93109
) {
@@ -119,7 +135,7 @@ protected TypeResolution resolveNonQueryParamTypes() {
119135
return isNotNull(field, sourceText(), FIRST).and(
120136
isType(
121137
field,
122-
DATA_TYPES::contains,
138+
FIELD_DATA_TYPES::contains,
123139
sourceText(),
124140
FIRST,
125141
"keyword, text, boolean, date, date_nanos, double, integer, ip, long, unsigned_long, version"
@@ -131,13 +147,32 @@ protected TypeResolution resolveNonQueryParamTypes() {
131147
protected TypeResolution resolveQueryParamType() {
132148
return isType(
133149
query(),
134-
DATA_TYPES::contains,
135-
functionName(),
150+
QUERY_DATA_TYPES::contains,
151+
sourceText(),
136152
queryParamOrdinal(),
137-
"keyword, text, boolean, date, date_nanos, double, integer, ip, long, unsigned_long, version"
153+
"keyword, boolean, date, date_nanos, double, integer, ip, long, unsigned_long, version"
138154
).and(isNotNullAndFoldable(query(), sourceText(), queryParamOrdinal()));
139155
}
140156

157+
@Override
158+
protected TypeResolution checkParamCompatibility() {
159+
DataType fieldType = field().dataType();
160+
DataType queryType = query().dataType();
161+
162+
if ((fieldType == queryType) || (queryType == KEYWORD)) {
163+
return TypeResolution.TYPE_RESOLVED;
164+
}
165+
166+
if (fieldType.isNumeric() && queryType.isNumeric()) {
167+
// When doing an unsigned long query, field must be an unsigned long
168+
if ((queryType == UNSIGNED_LONG && fieldType != UNSIGNED_LONG) == false) {
169+
return TypeResolution.TYPE_RESOLVED;
170+
}
171+
}
172+
173+
return new TypeResolution(formatIncompatibleTypesMessage(fieldType, queryType, sourceText()));
174+
}
175+
141176
@Override
142177
public void validate(Failures failures) {
143178
if (field instanceof FieldAttribute == false) {
@@ -153,6 +188,23 @@ public void validate(Failures failures) {
153188
}
154189
}
155190

191+
@Override
192+
public Object queryAsObject() {
193+
Object queryAsObject = query().fold();
194+
195+
if (queryAsObject instanceof BytesRef bytesRef) {
196+
return switch (query().dataType()) {
197+
case IP -> EsqlDataTypeConverter.ipToString(bytesRef);
198+
case VERSION -> EsqlDataTypeConverter.versionToString(bytesRef);
199+
default -> bytesRef.utf8ToString();
200+
};
201+
} else if (query().dataType() == DataType.UNSIGNED_LONG) {
202+
return NumericUtils.unsignedLongAsBigInteger((Long) queryAsObject);
203+
}
204+
205+
return queryAsObject;
206+
}
207+
156208
@Override
157209
public Expression replaceChildren(List<Expression> newChildren) {
158210
return new Match(source(), newChildren.get(0), newChildren.get(1));

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/EsqlBinaryComparison.java‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ protected TypeResolution checkCompatibility() {
220220
// Unsigned long is only interoperable with other unsigned longs
221221
if ((rightType == UNSIGNED_LONG && (false == (leftType == UNSIGNED_LONG || leftType == DataType.NULL)))
222222
|| (leftType == UNSIGNED_LONG && (false == (rightType == UNSIGNED_LONG || rightType == DataType.NULL)))) {
223-
return new TypeResolution(formatIncompatibleTypesMessage());
223+
return new TypeResolution(formatIncompatibleTypesMessage(left().dataType(), right().dataType(), sourceText()));
224224
}
225225

226226
if ((leftType.isNumeric() && rightType.isNumeric())
@@ -230,35 +230,35 @@ protected TypeResolution checkCompatibility() {
230230
|| DataType.isNull(rightType)) {
231231
return TypeResolution.TYPE_RESOLVED;
232232
}
233-
return new TypeResolution(formatIncompatibleTypesMessage());
233+
return new TypeResolution(formatIncompatibleTypesMessage(left().dataType(), right().dataType(), sourceText()));
234234
}
235235

236-
public String formatIncompatibleTypesMessage() {
237-
if (left().dataType().equals(UNSIGNED_LONG)) {
236+
public static String formatIncompatibleTypesMessage(DataType leftType, DataType rightType, String sourceText) {
237+
if (leftType.equals(UNSIGNED_LONG)) {
238238
return format(
239239
null,
240240
"first argument of [{}] is [unsigned_long] and second is [{}]. "
241241
+ "[unsigned_long] can only be operated on together with another [unsigned_long]",
242-
sourceText(),
243-
right().dataType().typeName()
242+
sourceText,
243+
rightType.typeName()
244244
);
245245
}
246-
if (right().dataType().equals(UNSIGNED_LONG)) {
246+
if (rightType.equals(UNSIGNED_LONG)) {
247247
return format(
248248
null,
249249
"first argument of [{}] is [{}] and second is [unsigned_long]. "
250250
+ "[unsigned_long] can only be operated on together with another [unsigned_long]",
251-
sourceText(),
252-
left().dataType().typeName()
251+
sourceText,
252+
leftType.typeName()
253253
);
254254
}
255255
return format(
256256
null,
257257
"first argument of [{}] is [{}] so second argument must also be [{}] but was [{}]",
258-
sourceText(),
259-
left().dataType().isNumeric() ? "numeric" : left().dataType().typeName(),
260-
left().dataType().isNumeric() ? "numeric" : left().dataType().typeName(),
261-
right().dataType().typeName()
258+
sourceText,
259+
leftType.isNumeric() ? "numeric" : leftType.typeName(),
260+
leftType.isNumeric() ? "numeric" : leftType.typeName(),
261+
rightType.typeName()
262262
);
263263
}
264264

0 commit comments

Comments
 (0)