Skip to content

Commit 7f8d663

Browse files
kderussokderusso
committed
PR feedback - refactor/consolidate code
1 parent c58c4a8 commit 7f8d663

File tree

4 files changed

+62
-106
lines changed

4 files changed

+62
-106
lines changed

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

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import org.elasticsearch.xpack.esql.core.InvalidArgumentException;
2222
import org.elasticsearch.xpack.esql.core.expression.EntryExpression;
2323
import org.elasticsearch.xpack.esql.core.expression.Expression;
24+
import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
2425
import org.elasticsearch.xpack.esql.core.expression.FoldContext;
2526
import org.elasticsearch.xpack.esql.core.expression.Literal;
2627
import org.elasticsearch.xpack.esql.core.expression.MapExpression;
@@ -31,7 +32,9 @@
3132
import org.elasticsearch.xpack.esql.core.tree.Source;
3233
import org.elasticsearch.xpack.esql.core.type.DataType;
3334
import org.elasticsearch.xpack.esql.core.type.DataTypeConverter;
35+
import org.elasticsearch.xpack.esql.core.type.MultiTypeEsField;
3436
import org.elasticsearch.xpack.esql.evaluator.mapper.EvaluatorMapper;
37+
import org.elasticsearch.xpack.esql.expression.function.scalar.convert.AbstractConvertFunction;
3538
import org.elasticsearch.xpack.esql.expression.predicate.logical.BinaryLogic;
3639
import org.elasticsearch.xpack.esql.expression.predicate.logical.Not;
3740
import org.elasticsearch.xpack.esql.optimizer.rules.physical.local.LucenePushdownPredicates;
@@ -56,7 +59,10 @@
5659
import static org.elasticsearch.common.logging.LoggerMessageFormat.format;
5760
import static org.elasticsearch.xpack.esql.common.Failure.fail;
5861
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.ParamOrdinal.DEFAULT;
62+
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.ParamOrdinal.THIRD;
5963
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isFoldable;
64+
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isMapExpression;
65+
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isNotNull;
6066
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isNotNullAndFoldable;
6167
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isString;
6268

@@ -389,4 +395,48 @@ protected static void populateOptionsMap(
389395
}
390396
}
391397
}
398+
399+
protected TypeResolution resolveOptions(Expression options) {
400+
if (options != null) {
401+
TypeResolution resolution = isNotNull(options, sourceText(), THIRD);
402+
if (resolution.unresolved()) {
403+
return resolution;
404+
}
405+
// MapExpression does not have a DataType associated with it
406+
resolution = isMapExpression(options, sourceText(), THIRD);
407+
if (resolution.unresolved()) {
408+
return resolution;
409+
}
410+
411+
try {
412+
resolvedOptions();
413+
} catch (InvalidArgumentException e) {
414+
return new TypeResolution(e.getMessage());
415+
}
416+
}
417+
return TypeResolution.TYPE_RESOLVED;
418+
}
419+
420+
protected Map<String, Object> resolvedOptions() throws InvalidArgumentException {
421+
return Map.of();
422+
}
423+
424+
public static String getNameFromFieldAttribute(FieldAttribute fieldAttribute) {
425+
String fieldName = fieldAttribute.name();
426+
if (fieldAttribute.field() instanceof MultiTypeEsField multiTypeEsField) {
427+
// If we have multiple field types, we allow the query to be done, but getting the underlying field name
428+
fieldName = multiTypeEsField.getName();
429+
}
430+
return fieldName;
431+
}
432+
433+
public static FieldAttribute fieldAsFieldAttribute(Expression field) {
434+
Expression fieldExpression = field;
435+
// Field may be converted to other data type (field_name :: data_type), so we need to check the original field
436+
if (fieldExpression instanceof AbstractConvertFunction convertFunction) {
437+
fieldExpression = convertFunction.field();
438+
}
439+
return fieldExpression instanceof FieldAttribute fieldAttribute ? fieldAttribute : null;
440+
}
441+
392442
}

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

Lines changed: 4 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
2727
import org.elasticsearch.xpack.esql.core.tree.Source;
2828
import org.elasticsearch.xpack.esql.core.type.DataType;
29-
import org.elasticsearch.xpack.esql.core.type.MultiTypeEsField;
3029
import org.elasticsearch.xpack.esql.core.util.Check;
3130
import org.elasticsearch.xpack.esql.core.util.NumericUtils;
3231
import org.elasticsearch.xpack.esql.expression.function.Example;
@@ -36,7 +35,6 @@
3635
import org.elasticsearch.xpack.esql.expression.function.MapParam;
3736
import org.elasticsearch.xpack.esql.expression.function.OptionalArgument;
3837
import org.elasticsearch.xpack.esql.expression.function.Param;
39-
import org.elasticsearch.xpack.esql.expression.function.scalar.convert.AbstractConvertFunction;
4038
import org.elasticsearch.xpack.esql.io.stream.PlanStreamInput;
4139
import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan;
4240
import org.elasticsearch.xpack.esql.planner.TranslatorHandler;
@@ -65,8 +63,6 @@
6563
import static org.elasticsearch.index.query.MatchQueryBuilder.ZERO_TERMS_QUERY_FIELD;
6664
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.ParamOrdinal.FIRST;
6765
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.ParamOrdinal.SECOND;
68-
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.ParamOrdinal.THIRD;
69-
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isMapExpression;
7066
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isNotNull;
7167
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isNotNullAndFoldable;
7268
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isType;
@@ -303,7 +299,7 @@ public final void writeTo(StreamOutput out) throws IOException {
303299

304300
@Override
305301
protected TypeResolution resolveParams() {
306-
return resolveField().and(resolveQuery()).and(resolveOptions()).and(checkParamCompatibility());
302+
return resolveField().and(resolveQuery()).and(resolveOptions(options())).and(checkParamCompatibility());
307303
}
308304

309305
private TypeResolution resolveField() {
@@ -347,25 +343,9 @@ private TypeResolution checkParamCompatibility() {
347343
return new TypeResolution(formatIncompatibleTypesMessage(fieldType, queryType, sourceText()));
348344
}
349345

350-
private TypeResolution resolveOptions() {
351-
if (options() != null) {
352-
TypeResolution resolution = isNotNull(options(), sourceText(), THIRD);
353-
if (resolution.unresolved()) {
354-
return resolution;
355-
}
356-
// MapExpression does not have a DataType associated with it
357-
resolution = isMapExpression(options(), sourceText(), THIRD);
358-
if (resolution.unresolved()) {
359-
return resolution;
360-
}
361-
362-
try {
363-
matchQueryOptions();
364-
} catch (InvalidArgumentException e) {
365-
return new TypeResolution(e.getMessage());
366-
}
367-
}
368-
return TypeResolution.TYPE_RESOLVED;
346+
@Override
347+
protected Map<String, Object> resolvedOptions() {
348+
return matchQueryOptions();
369349
}
370350

371351
private Map<String, Object> matchQueryOptions() throws InvalidArgumentException {
@@ -465,24 +445,6 @@ protected Query translate(TranslatorHandler handler) {
465445
return new MatchQuery(source(), fieldName, queryAsObject(), matchQueryOptions());
466446
}
467447

468-
public static String getNameFromFieldAttribute(FieldAttribute fieldAttribute) {
469-
String fieldName = fieldAttribute.name();
470-
if (fieldAttribute.field() instanceof MultiTypeEsField multiTypeEsField) {
471-
// If we have multiple field types, we allow the query to be done, but getting the underlying field name
472-
fieldName = multiTypeEsField.getName();
473-
}
474-
return fieldName;
475-
}
476-
477-
public static FieldAttribute fieldAsFieldAttribute(Expression field) {
478-
Expression fieldExpression = field;
479-
// Field may be converted to other data type (field_name :: data_type), so we need to check the original field
480-
if (fieldExpression instanceof AbstractConvertFunction convertFunction) {
481-
fieldExpression = convertFunction.field();
482-
}
483-
return fieldExpression instanceof FieldAttribute fieldAttribute ? fieldAttribute : null;
484-
}
485-
486448
private FieldAttribute fieldAsFieldAttribute() {
487449
return fieldAsFieldAttribute(field);
488450
}

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

Lines changed: 4 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
2525
import org.elasticsearch.xpack.esql.core.tree.Source;
2626
import org.elasticsearch.xpack.esql.core.type.DataType;
27-
import org.elasticsearch.xpack.esql.core.type.MultiTypeEsField;
2827
import org.elasticsearch.xpack.esql.core.util.Check;
2928
import org.elasticsearch.xpack.esql.expression.function.Example;
3029
import org.elasticsearch.xpack.esql.expression.function.FunctionAppliesTo;
@@ -33,7 +32,6 @@
3332
import org.elasticsearch.xpack.esql.expression.function.MapParam;
3433
import org.elasticsearch.xpack.esql.expression.function.OptionalArgument;
3534
import org.elasticsearch.xpack.esql.expression.function.Param;
36-
import org.elasticsearch.xpack.esql.expression.function.scalar.convert.AbstractConvertFunction;
3735
import org.elasticsearch.xpack.esql.io.stream.PlanStreamInput;
3836
import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan;
3937
import org.elasticsearch.xpack.esql.planner.TranslatorHandler;
@@ -55,8 +53,6 @@
5553
import static org.elasticsearch.index.query.MatchQueryBuilder.ANALYZER_FIELD;
5654
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.ParamOrdinal.FIRST;
5755
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.ParamOrdinal.SECOND;
58-
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.ParamOrdinal.THIRD;
59-
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isMapExpression;
6056
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isNotNull;
6157
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isNotNullAndFoldable;
6258
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isType;
@@ -199,7 +195,7 @@ public final void writeTo(StreamOutput out) throws IOException {
199195

200196
@Override
201197
protected TypeResolution resolveParams() {
202-
return resolveField().and(resolveQuery()).and(resolveOptions()).and(checkParamCompatibility());
198+
return resolveField().and(resolveQuery()).and(resolveOptions(options())).and(checkParamCompatibility());
203199
}
204200

205201
private TypeResolution resolveField() {
@@ -226,25 +222,9 @@ private TypeResolution checkParamCompatibility() {
226222
return new TypeResolution(formatIncompatibleTypesMessage(fieldType, queryType, sourceText()));
227223
}
228224

229-
private TypeResolution resolveOptions() {
230-
if (options() != null) {
231-
TypeResolution resolution = isNotNull(options(), sourceText(), THIRD);
232-
if (resolution.unresolved()) {
233-
return resolution;
234-
}
235-
// MapExpression does not have a DataType associated with it
236-
resolution = isMapExpression(options(), sourceText(), THIRD);
237-
if (resolution.unresolved()) {
238-
return resolution;
239-
}
240-
241-
try {
242-
matchPhraseQueryOptions();
243-
} catch (InvalidArgumentException e) {
244-
return new TypeResolution(e.getMessage());
245-
}
246-
}
247-
return TypeResolution.TYPE_RESOLVED;
225+
@Override
226+
protected Map<String, Object> resolvedOptions() throws InvalidArgumentException {
227+
return matchPhraseQueryOptions();
248228
}
249229

250230
private Map<String, Object> matchPhraseQueryOptions() throws InvalidArgumentException {
@@ -338,24 +318,6 @@ protected Query translate(TranslatorHandler handler) {
338318
return new MatchPhraseQuery(source(), fieldName, queryAsObject(), matchPhraseQueryOptions());
339319
}
340320

341-
public static String getNameFromFieldAttribute(FieldAttribute fieldAttribute) {
342-
String fieldName = fieldAttribute.name();
343-
if (fieldAttribute.field() instanceof MultiTypeEsField multiTypeEsField) {
344-
// If we have multiple field types, we allow the query to be done, but getting the underlying field name
345-
fieldName = multiTypeEsField.getName();
346-
}
347-
return fieldName;
348-
}
349-
350-
public static FieldAttribute fieldAsFieldAttribute(Expression field) {
351-
Expression fieldExpression = field;
352-
// Field may be converted to other data type (field_name :: data_type), so we need to check the original field
353-
if (fieldExpression instanceof AbstractConvertFunction convertFunction) {
354-
fieldExpression = convertFunction.field();
355-
}
356-
return fieldExpression instanceof FieldAttribute fieldAttribute ? fieldAttribute : null;
357-
}
358-
359321
private FieldAttribute fieldAsFieldAttribute() {
360322
return fieldAsFieldAttribute(field);
361323
}

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

Lines changed: 4 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,6 @@
6060
import static org.elasticsearch.index.query.QueryStringQueryBuilder.TIME_ZONE_FIELD;
6161
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.ParamOrdinal.FIRST;
6262
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.ParamOrdinal.SECOND;
63-
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isMapExpression;
64-
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isNotNull;
6563
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isNotNullAndFoldable;
6664
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isType;
6765
import static org.elasticsearch.xpack.esql.core.type.DataType.BOOLEAN;
@@ -328,30 +326,14 @@ private Map<String, Object> queryStringOptions() throws InvalidArgumentException
328326
return matchOptions;
329327
}
330328

331-
private TypeResolution resolveOptions() {
332-
if (options() != null) {
333-
TypeResolution resolution = isNotNull(options(), sourceText(), SECOND);
334-
if (resolution.unresolved()) {
335-
return resolution;
336-
}
337-
// MapExpression does not have a DataType associated with it
338-
resolution = isMapExpression(options(), sourceText(), SECOND);
339-
if (resolution.unresolved()) {
340-
return resolution;
341-
}
342-
343-
try {
344-
queryStringOptions();
345-
} catch (InvalidArgumentException e) {
346-
return new TypeResolution(e.getMessage());
347-
}
348-
}
349-
return TypeResolution.TYPE_RESOLVED;
329+
@Override
330+
protected Map<String, Object> resolvedOptions() {
331+
return queryStringOptions();
350332
}
351333

352334
@Override
353335
protected TypeResolution resolveParams() {
354-
return resolveQuery().and(resolveOptions());
336+
return resolveQuery().and(resolveOptions(options()));
355337
}
356338

357339
@Override

0 commit comments

Comments
 (0)