Skip to content

Commit

Permalink
[ESQL] Minor refactor for DataType (elastic#111016)
Browse files Browse the repository at this point in the history
* Remove duplicate functions from EsqlDataTypes

* Remove unused methods from DataType
  • Loading branch information
ioanatia authored and salvatore-campagna committed Jul 23, 2024
1 parent 2699682 commit 12dda74
Show file tree
Hide file tree
Showing 14 changed files with 17 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -229,25 +229,10 @@ public static boolean isNullOrNumeric(DataType t) {
return t.isNumeric() || isNull(t);
}

public static boolean isSigned(DataType t) {
return t.isNumeric() && t.equals(UNSIGNED_LONG) == false;
}

public static boolean isDateTime(DataType type) {
return type == DATETIME;
}

public static boolean areCompatible(DataType left, DataType right) {
if (left == right) {
return true;
} else {
return (left == NULL || right == NULL)
|| (isString(left) && isString(right))
|| (left.isNumeric() && right.isNumeric())
|| (isDateTime(left) && isDateTime(right));
}
}

public String nameUpper() {
return name;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import org.elasticsearch.core.Releasables;
import org.elasticsearch.xpack.esql.core.type.DataType;
import org.elasticsearch.xpack.esql.planner.PlannerUtils;
import org.elasticsearch.xpack.esql.type.EsqlDataTypes;

import java.io.IOException;

Expand All @@ -28,7 +27,7 @@ public record Column(DataType type, Block values) implements Releasable, Writeab
}

public Column(BlockStreamInput in) throws IOException {
this(EsqlDataTypes.fromTypeName(in.readString()), in.readNamedWriteable(Block.class));
this(DataType.fromTypeName(in.readString()), in.readNamedWriteable(Block.class));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ private static void checkRegexExtractOnlyOnStrings(LogicalPlan p, Set<Failure> f
if (p instanceof RegexExtract re) {
Expression expr = re.input();
DataType type = expr.dataType();
if (EsqlDataTypes.isString(type) == false) {
if (DataType.isString(type) == false) {
failures.add(
fail(
expr,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@
import org.elasticsearch.xpack.esql.planner.EsPhysicalOperationProviders;
import org.elasticsearch.xpack.esql.planner.PlannerUtils;
import org.elasticsearch.xpack.esql.plugin.EsqlPlugin;
import org.elasticsearch.xpack.esql.type.EsqlDataTypes;

import java.io.IOException;
import java.util.ArrayList;
Expand Down Expand Up @@ -467,7 +466,7 @@ private static class LookupRequest extends TransportRequest implements IndicesRe
String inputDataType = (in.getTransportVersion().onOrAfter(TransportVersions.ESQL_EXTENDED_ENRICH_INPUT_TYPE))
? in.readString()
: "unknown";
this.inputDataType = EsqlDataTypes.fromTypeName(inputDataType);
this.inputDataType = DataType.fromTypeName(inputDataType);
this.matchType = in.readString();
this.matchField = in.readString();
try (BlockStreamInput bsi = new BlockStreamInput(in, blockFactory)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@
import org.elasticsearch.xpack.core.enrich.EnrichPolicy;
import org.elasticsearch.xpack.esql.analysis.EnrichResolution;
import org.elasticsearch.xpack.esql.core.index.EsIndex;
import org.elasticsearch.xpack.esql.core.type.DataType;
import org.elasticsearch.xpack.esql.core.type.EsField;
import org.elasticsearch.xpack.esql.core.util.StringUtils;
import org.elasticsearch.xpack.esql.plan.logical.Enrich;
import org.elasticsearch.xpack.esql.session.IndexResolver;
import org.elasticsearch.xpack.esql.type.EsqlDataTypes;

import java.io.IOException;
import java.util.ArrayList;
Expand Down Expand Up @@ -192,7 +192,7 @@ private Tuple<ResolvedEnrichPolicy, String> mergeLookupResults(
EsField field = m.getValue();
field = new EsField(
field.getName(),
EsqlDataTypes.fromTypeName(field.getDataType().typeName()),
DataType.fromTypeName(field.getDataType().typeName()),
field.getProperties(),
field.isAggregatable(),
field.isAlias()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import org.elasticsearch.xpack.esql.expression.function.Warnings;
import org.elasticsearch.xpack.esql.expression.function.scalar.UnaryScalarFunction;
import org.elasticsearch.xpack.esql.io.stream.PlanStreamInput;
import org.elasticsearch.xpack.esql.type.EsqlDataTypes;

import java.io.IOException;
import java.util.ArrayList;
Expand All @@ -50,7 +49,7 @@ public abstract class AbstractConvertFunction extends UnaryScalarFunction {

// the numeric types convert functions need to handle; the other numeric types are converted upstream to one of these
private static final List<DataType> NUMERIC_TYPES = List.of(DataType.INTEGER, DataType.LONG, DataType.UNSIGNED_LONG, DataType.DOUBLE);
public static final List<DataType> STRING_TYPES = DataType.types().stream().filter(EsqlDataTypes::isString).toList();
public static final List<DataType> STRING_TYPES = DataType.types().stream().filter(DataType::isString).toList();

protected AbstractConvertFunction(Source source, Expression field) {
super(source, field);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import org.elasticsearch.xpack.esql.expression.function.Param;
import org.elasticsearch.xpack.esql.expression.function.scalar.EsqlConfigurationFunction;
import org.elasticsearch.xpack.esql.io.stream.PlanStreamInput;
import org.elasticsearch.xpack.esql.type.EsqlDataTypes;

import java.io.IOException;
import java.time.ZoneId;
Expand Down Expand Up @@ -129,7 +128,7 @@ private ChronoField chronoField() {
if (chronoField == null) {
Expression field = children().get(0);
try {
if (field.foldable() && EsqlDataTypes.isString(field.dataType())) {
if (field.foldable() && DataType.isString(field.dataType())) {
chronoField = (ChronoField) STRING_TO_CHRONO_FIELD.convert(field.fold());
}
} catch (Exception e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import org.elasticsearch.xpack.esql.expression.function.scalar.EsqlConfigurationFunction;
import org.elasticsearch.xpack.esql.io.stream.PlanStreamInput;
import org.elasticsearch.xpack.esql.session.EsqlConfiguration;
import org.elasticsearch.xpack.esql.type.EsqlDataTypes;

import java.io.IOException;
import java.util.List;
Expand Down Expand Up @@ -146,7 +145,7 @@ public ExpressionEvaluator.Factory toEvaluator(Function<Expression, ExpressionEv
if (format == null) {
return new DateFormatConstantEvaluator.Factory(source(), fieldEvaluator, DEFAULT_DATE_TIME_FORMATTER);
}
if (EsqlDataTypes.isString(format.dataType()) == false) {
if (DataType.isString(format.dataType()) == false) {
throw new IllegalArgumentException("unsupported data type for format [" + format.dataType() + "]");
}
if (format.foldable()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import org.elasticsearch.xpack.esql.expression.function.Param;
import org.elasticsearch.xpack.esql.expression.function.scalar.EsqlScalarFunction;
import org.elasticsearch.xpack.esql.io.stream.PlanStreamInput;
import org.elasticsearch.xpack.esql.type.EsqlDataTypes;

import java.io.IOException;
import java.time.ZoneId;
Expand Down Expand Up @@ -143,7 +142,7 @@ public ExpressionEvaluator.Factory toEvaluator(Function<Expression, ExpressionEv
if (format == null) {
return new DateParseConstantEvaluator.Factory(source(), fieldEvaluator, DEFAULT_DATE_TIME_FORMATTER);
}
if (EsqlDataTypes.isString(format.dataType()) == false) {
if (DataType.isString(format.dataType()) == false) {
throw new IllegalArgumentException("unsupported data type for date_parse [" + format.dataType() + "]");
}
if (format.foldable()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import org.elasticsearch.xpack.esql.evaluator.mapper.ExpressionMapper;
import org.elasticsearch.xpack.esql.expression.function.scalar.math.Cast;
import org.elasticsearch.xpack.esql.planner.Layout;
import org.elasticsearch.xpack.esql.type.EsqlDataTypes;

import static org.elasticsearch.xpack.esql.evaluator.EvalMapper.toEvaluator;

Expand All @@ -36,7 +35,7 @@ public final ExpressionEvaluator.Factory map(InsensitiveEquals bc, Layout layout
var leftEval = toEvaluator(bc.left(), layout);
var rightEval = toEvaluator(bc.right(), layout);
if (leftType == DataType.KEYWORD || leftType == DataType.TEXT) {
if (bc.right().foldable() && EsqlDataTypes.isString(rightType)) {
if (bc.right().foldable() && DataType.isString(rightType)) {
BytesRef rightVal = BytesRefs.toBytesRef(bc.right().fold());
Automaton automaton = InsensitiveEquals.automaton(rightVal);
return dvrCtx -> new InsensitiveEqualsConstantEvaluator(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,17 @@

import org.elasticsearch.xpack.esql.core.type.DataType;

import java.util.Locale;

import static org.elasticsearch.xpack.esql.core.type.DataType.BYTE;
import static org.elasticsearch.xpack.esql.core.type.DataType.DATE_PERIOD;
import static org.elasticsearch.xpack.esql.core.type.DataType.FLOAT;
import static org.elasticsearch.xpack.esql.core.type.DataType.HALF_FLOAT;
import static org.elasticsearch.xpack.esql.core.type.DataType.KEYWORD;
import static org.elasticsearch.xpack.esql.core.type.DataType.NESTED;
import static org.elasticsearch.xpack.esql.core.type.DataType.NULL;
import static org.elasticsearch.xpack.esql.core.type.DataType.OBJECT;
import static org.elasticsearch.xpack.esql.core.type.DataType.PARTIAL_AGG;
import static org.elasticsearch.xpack.esql.core.type.DataType.SCALED_FLOAT;
import static org.elasticsearch.xpack.esql.core.type.DataType.SHORT;
import static org.elasticsearch.xpack.esql.core.type.DataType.SOURCE;
import static org.elasticsearch.xpack.esql.core.type.DataType.TEXT;
import static org.elasticsearch.xpack.esql.core.type.DataType.TIME_DURATION;
import static org.elasticsearch.xpack.esql.core.type.DataType.UNSUPPORTED;
import static org.elasticsearch.xpack.esql.core.type.DataType.isNull;
Expand All @@ -31,14 +27,6 @@ public final class EsqlDataTypes {

private EsqlDataTypes() {}

public static DataType fromTypeName(String name) {
return DataType.fromTypeName(name.toLowerCase(Locale.ROOT));
}

public static boolean isString(DataType t) {
return t == KEYWORD || t == TEXT;
}

public static boolean isPrimitive(DataType t) {
return t != OBJECT && t != NESTED;
}
Expand Down Expand Up @@ -98,7 +86,9 @@ public static boolean areCompatible(DataType left, DataType right) {
if (left == right) {
return true;
} else {
return (left == NULL || right == NULL) || (isString(left) && isString(right)) || (left.isNumeric() && right.isNumeric());
return (left == NULL || right == NULL)
|| (DataType.isString(left) && DataType.isString(right))
|| (left.isNumeric() && right.isNumeric());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import org.elasticsearch.xpack.esql.core.type.DataType;
import org.elasticsearch.xpack.esql.expression.function.AbstractScalarFunctionTestCase;
import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier;
import org.elasticsearch.xpack.esql.type.EsqlDataTypes;

import java.util.ArrayList;
import java.util.List;
Expand All @@ -33,11 +32,11 @@ public MvConcatTests(@Name("TestCase") Supplier<TestCaseSupplier.TestCase> testC
public static Iterable<Object[]> parameters() {
List<TestCaseSupplier> suppliers = new ArrayList<>();
for (DataType fieldType : DataType.types()) {
if (EsqlDataTypes.isString(fieldType) == false) {
if (DataType.isString(fieldType) == false) {
continue;
}
for (DataType delimType : DataType.types()) {
if (EsqlDataTypes.isString(delimType) == false) {
if (DataType.isString(delimType) == false) {
continue;
}
for (int l = 1; l < 10; l++) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@
import java.util.stream.Collectors;
import java.util.stream.IntStream;

import static org.elasticsearch.xpack.esql.core.type.DataType.isString;
import static org.elasticsearch.xpack.esql.expression.function.scalar.spatial.SpatialRelatesFunction.compatibleTypeNames;
import static org.elasticsearch.xpack.esql.type.EsqlDataTypes.isSpatial;
import static org.elasticsearch.xpack.esql.type.EsqlDataTypes.isSpatialGeo;
import static org.elasticsearch.xpack.esql.type.EsqlDataTypes.isString;
import static org.hamcrest.Matchers.equalTo;

public abstract class BinarySpatialFunctionTestCase extends AbstractScalarFunctionTestCase {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
import java.util.List;
import java.util.Map;

import static org.elasticsearch.xpack.esql.type.EsqlDataTypes.isString;
import static org.elasticsearch.xpack.esql.core.type.DataType.isString;

/**
* This test was originally based on the tests for sub-classes of EsField, like InvalidMappedFieldTests.
Expand Down

0 comments on commit 12dda74

Please sign in to comment.