Skip to content

Commit

Permalink
QL: Eliminate internal type DATETIME_NANOS (elastic#68220)
Browse files Browse the repository at this point in the history
Moving towards grouping of data types in the field caps API
the internal data type `DATETIME_NANOS` introduced for `date_nanos`
support is eliminated.

Relates: elastic#67722
Follows: elastic#67666
  • Loading branch information
matriv authored Feb 10, 2021
1 parent 4a19708 commit 45677a3
Show file tree
Hide file tree
Showing 25 changed files with 65 additions and 175 deletions.
2 changes: 1 addition & 1 deletion docs/reference/sql/endpoints/translate.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ Which returns:
},
{
"field": "release_date",
"format": "epoch_millis"
"format": "strict_date_optional_time_nanos"
}
],
"sort": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,13 @@
import org.elasticsearch.xpack.ql.execution.search.extractor.AbstractFieldHitExtractor;
import org.elasticsearch.xpack.ql.type.DataType;
import org.elasticsearch.xpack.ql.util.DateUtils;

import java.io.IOException;
import java.time.Instant;
import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.util.List;

import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME;
import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME_NANOS;

public class FieldHitExtractor extends AbstractFieldHitExtractor {

Expand Down Expand Up @@ -50,20 +48,17 @@ protected Object unwrapCustomValue(Object values) {

if (dataType == DATETIME) {
if (values instanceof String) {
return parseDateString(values);
}
}
if (dataType == DATETIME_NANOS) {
if (values instanceof String) {
return DateUtils.asDateTimeWithNanos(values.toString(), zoneId());
// We ask @timestamp (or the defined alternative field) to be returned as `epoch_millis`
// when matching sequence to avoid parsing into ZonedDateTime objects for performance reasons.
return parseEpochMillisAsString(values.toString());
}
}

return null;
}

protected Object parseDateString(Object values) {
return ZonedDateTime.ofInstant(Instant.ofEpochMilli(Long.parseLong(values.toString())), zoneId());
protected Object parseEpochMillisAsString(String str) {
return ZonedDateTime.ofInstant(Instant.ofEpochMilli(Long.parseLong(str)), zoneId());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public TimestampFieldHitExtractor(FieldHitExtractor target) {
}

@Override
protected Object parseDateString(Object values) {
return Long.parseLong(values.toString());
protected Object parseEpochMillisAsString(String str) {
return Long.parseLong(str);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import org.elasticsearch.xpack.ql.type.DataType;

import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME;
import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME_NANOS;

// NB: this class is taken from SQL - it hasn't been ported over to QL
// since at this stage is unclear whether the whole FieldExtraction infrastructure
Expand Down Expand Up @@ -65,9 +64,8 @@ public String toString() {
}

private static String format(DataType dataType) {
if (dataType == DATETIME_NANOS) {
return "strict_date_optional_time_nanos";
}
// We need epoch_millis for the tiebreaker timestamp field, because parsing timestamp strings
// can have a negative performance impact
return dataType == DATETIME ? "epoch_millis" : null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,11 @@
import org.elasticsearch.xpack.ql.expression.gen.script.ScriptTemplate;
import org.elasticsearch.xpack.ql.expression.gen.script.Scripts;
import org.elasticsearch.xpack.ql.tree.Source;
import org.elasticsearch.xpack.ql.type.DataTypes;
import org.elasticsearch.xpack.ql.util.DateUtils;

import static java.util.Collections.emptyList;
import static org.elasticsearch.xpack.ql.expression.gen.script.ParamsBuilder.paramsBuilder;
import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME_NANOS;
import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME;

/**
* A {@code ScalarFunction} is a {@code Function} that takes values from some
Expand Down Expand Up @@ -128,9 +127,9 @@ protected ScriptTemplate scriptWithGrouping(GroupingFunction grouping) {

// FIXME: this needs to be refactored to account for different datatypes in different projects (ie DATE from SQL)
private String basicTemplate(Function function) {
if (function.dataType().name().equals("DATE") || function.dataType() == DataTypes.DATETIME ||
// Aggregations on date_nanos are returned as double
(function instanceof AggregateFunction && ((AggregateFunction) function).field().dataType() == DATETIME_NANOS)) {
if (function.dataType().name().equals("DATE") || function.dataType() == DATETIME ||
// Aggregations on date_nanos are returned as string
(function instanceof AggregateFunction && ((AggregateFunction) function).field().dataType() == DATETIME)) {

return "{sql}.asDateTime({})";
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@
import static java.util.Collections.emptySet;
import static org.elasticsearch.action.ActionListener.wrap;
import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME;
import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME_NANOS;
import static org.elasticsearch.xpack.ql.type.DataTypes.KEYWORD;
import static org.elasticsearch.xpack.ql.type.DataTypes.OBJECT;
import static org.elasticsearch.xpack.ql.type.DataTypes.TEXT;
Expand Down Expand Up @@ -450,9 +449,6 @@ private static EsField createField(DataTypeRegistry typeRegistry, String fieldNa
if (esType == DATETIME) {
return DateEsField.dateEsField(fieldName, props, isAggregateable);
}
if (esType == DATETIME_NANOS) {
return DateEsField.dateNanosEsField(fieldName, props, isAggregateable);
}
if (esType == UNSUPPORTED) {
return new UnsupportedEsField(fieldName, typeName, null, props);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ public final class DataTypes {
public static final DataType TEXT = new DataType("text", Integer.MAX_VALUE, false, false, false);
// date
public static final DataType DATETIME = new DataType("DATETIME", "date", Long.BYTES, false, false, true);
public static final DataType DATETIME_NANOS = new DataType("DATETIME", "date_nanos", Long.BYTES, false, false, true);
// ip
public static final DataType IP = new DataType("ip", 45, false, false, true);
// binary
Expand Down Expand Up @@ -77,11 +76,11 @@ public final class DataTypes {
private static final Map<String, DataType> NAME_TO_TYPE = TYPES.stream()
.collect(toUnmodifiableMap(DataType::typeName, t -> t));

private static final Map<String, DataType> ES_TO_TYPE;
private static Map<String, DataType> ES_TO_TYPE;

static {
Map<String, DataType> map = TYPES.stream().filter(e -> e.esType() != null).collect(toMap(DataType::esType, t -> t));
map.put(DATETIME_NANOS.esType(), DATETIME_NANOS);
map.put("date_nanos", DATETIME);
ES_TO_TYPE = Collections.unmodifiableMap(map);
}

Expand Down Expand Up @@ -160,7 +159,7 @@ public static boolean isSigned(DataType t) {
}

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

public static boolean areCompatible(DataType left, DataType right) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,6 @@ public static DateEsField dateEsField(String name, Map<String, EsField> properti
return new DateEsField(name, DataTypes.DATETIME, properties, hasDocValues);
}

public static DateEsField dateNanosEsField(String name, Map<String, EsField> properties, boolean hasDocValues) {
return new DateEsField(name, DataTypes.DATETIME_NANOS, properties, hasDocValues);
}

private DateEsField(String name, DataType dataType, Map<String, EsField> properties, boolean hasDocValues) {
super(name, dataType, properties, hasDocValues);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

import static java.util.Collections.emptyMap;
import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME;
import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME_NANOS;
import static org.elasticsearch.xpack.ql.type.DataTypes.KEYWORD;
import static org.elasticsearch.xpack.ql.type.DataTypes.NESTED;
import static org.elasticsearch.xpack.ql.type.DataTypes.OBJECT;
Expand Down Expand Up @@ -97,8 +96,6 @@ private static void walkMapping(DataTypeRegistry typeRegistry, String name, Obje
field = new KeywordEsField(name, properties, docValues, length, normalized);
} else if (esDataType == DATETIME) {
field = DateEsField.dateEsField(name, properties, docValues);
} else if (esDataType == DATETIME_NANOS) {
field = DateEsField.dateNanosEsField(name, properties, docValues);
} else if (esDataType == UNSUPPORTED) {
String type = content.get("type").toString();
field = new UnsupportedEsField(name, type, null, properties);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,9 @@
import java.util.BitSet;
import java.util.Iterator;

import static org.elasticsearch.test.ESTestCase.randomFrom;
import static org.elasticsearch.xpack.ql.tree.Source.EMPTY;
import static org.elasticsearch.xpack.ql.type.DataTypes.BOOLEAN;
import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME;
import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME_NANOS;
import static org.elasticsearch.xpack.ql.type.DataTypes.INTEGER;
import static org.elasticsearch.xpack.ql.type.DataTypes.KEYWORD;

Expand All @@ -48,8 +46,7 @@ public static Literal randomBooleanLiteral() {
}

public static Literal randomDatetimeLiteral() {
return l(ZonedDateTime.ofInstant(Instant.ofEpochMilli(ESTestCase.randomLong()), ESTestCase.randomZone()),
randomFrom(DATETIME, DATETIME_NANOS));
return l(ZonedDateTime.ofInstant(Instant.ofEpochMilli(ESTestCase.randomLong()), ESTestCase.randomZone()), DATETIME);
}

public static class Combinations implements Iterable<BitSet> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import static org.elasticsearch.xpack.ql.type.DataTypes.BOOLEAN;
import static org.elasticsearch.xpack.ql.type.DataTypes.BYTE;
import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME;
import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME_NANOS;
import static org.elasticsearch.xpack.ql.type.DataTypes.DOUBLE;
import static org.elasticsearch.xpack.ql.type.DataTypes.FLOAT;
import static org.elasticsearch.xpack.ql.type.DataTypes.INTEGER;
Expand All @@ -42,7 +41,7 @@ public void testConversionToString() {
assertEquals("10.0", conversion.convert(10.0));
}
{
Converter conversion = converterFor(randomFrom(DATETIME, DATETIME_NANOS), to);
Converter conversion = converterFor(DATETIME, to);
assertNull(conversion.convert(null));
assertEquals("1973-11-29T21:33:09.101Z", conversion.convert(asDateTime(123456789101L)));
assertEquals("1966-02-02T02:26:50.899Z", conversion.convert(asDateTime(-123456789101L)));
Expand Down Expand Up @@ -77,7 +76,7 @@ public void testConversionToLong() {
assertEquals(0L, conversion.convert(false));
}
{
Converter conversion = converterFor(randomFrom(DATETIME, DATETIME_NANOS), to);
Converter conversion = converterFor(DATETIME, to);
assertNull(conversion.convert(null));
assertEquals(123456789101L, conversion.convert(asDateTime(123456789101L)));
assertEquals(-123456789101L, conversion.convert(asDateTime(-123456789101L)));
Expand Down Expand Up @@ -141,7 +140,7 @@ public void testConversionToDateTime() {

// double check back and forth conversion
ZonedDateTime dt = org.elasticsearch.common.time.DateUtils.nowWithMillisResolution();
Converter forward = converterFor(randomFrom(DATETIME, DATETIME_NANOS), KEYWORD);
Converter forward = converterFor(DATETIME, KEYWORD);
Converter back = converterFor(KEYWORD, DATETIME);
assertEquals(dt, back.convert(forward.convert(dt)));
Exception e = expectThrows(QlIllegalArgumentException.class, () -> conversion.convert("0xff"));
Expand Down Expand Up @@ -172,7 +171,7 @@ public void testConversionToFloat() {
assertEquals(0.0f, (float) conversion.convert(false), 0);
}
{
Converter conversion = converterFor(randomFrom(DATETIME, DATETIME_NANOS), to);
Converter conversion = converterFor(DATETIME, to);
assertNull(conversion.convert(null));
assertEquals(1.23456789101E11f, (float) conversion.convert(asDateTime(123456789101L)), 0);
assertEquals(-1.23456789101E11f, (float) conversion.convert(asDateTime(-123456789101L)), 0);
Expand Down Expand Up @@ -212,7 +211,7 @@ public void testConversionToDouble() {
assertEquals(0.0, (double) conversion.convert(false), 0);
}
{
Converter conversion = converterFor(randomFrom(DATETIME, DATETIME_NANOS), to);
Converter conversion = converterFor(DATETIME, to);
assertNull(conversion.convert(null));
assertEquals(1.23456789101E11, (double) conversion.convert(asDateTime(123456789101L)), 0);
assertEquals(-1.23456789101E11, (double) conversion.convert(asDateTime(-123456789101L)), 0);
Expand Down Expand Up @@ -261,7 +260,7 @@ public void testConversionToBoolean() {
assertEquals(false, conversion.convert(0.0d));
}
{
Converter conversion = converterFor(randomFrom(DATETIME, DATETIME_NANOS), to);
Converter conversion = converterFor(DATETIME, to);
assertNull(conversion.convert(null));
assertEquals(true, conversion.convert(asDateTime(123456789101L)));
assertEquals(true, conversion.convert(asDateTime(-123456789101L)));
Expand Down Expand Up @@ -303,7 +302,7 @@ public void testConversionToInt() {
assertEquals("[" + Long.MAX_VALUE + "] out of [integer] range", e.getMessage());
}
{
Converter conversion = converterFor(randomFrom(DATETIME, DATETIME_NANOS), to);
Converter conversion = converterFor(DATETIME, to);
assertNull(conversion.convert(null));
assertEquals(12345678, conversion.convert(asDateTime(12345678L)));
assertEquals(223456789, conversion.convert(asDateTime(223456789L)));
Expand All @@ -327,7 +326,7 @@ public void testConversionToShort() {
assertEquals("[" + Integer.MAX_VALUE + "] out of [short] range", e.getMessage());
}
{
Converter conversion = converterFor(randomFrom(DATETIME, DATETIME_NANOS), to);
Converter conversion = converterFor(DATETIME, to);
assertNull(conversion.convert(null));
assertEquals((short) 12345, conversion.convert(asDateTime(12345L)));
assertEquals((short) -12345, conversion.convert(asDateTime(-12345L)));
Expand All @@ -351,7 +350,7 @@ public void testConversionToByte() {
assertEquals("[" + Short.MAX_VALUE + "] out of [byte] range", e.getMessage());
}
{
Converter conversion = converterFor(randomFrom(DATETIME, DATETIME_NANOS), to);
Converter conversion = converterFor(DATETIME, to);
assertNull(conversion.convert(null));
assertEquals((byte) 123, conversion.convert(asDateTime(123L)));
assertEquals((byte) -123, conversion.convert(asDateTime(-123L)));
Expand Down Expand Up @@ -393,9 +392,6 @@ public void testCommonType() {
assertEquals(FLOAT, commonType(FLOAT, INTEGER));
assertEquals(DOUBLE, commonType(DOUBLE, FLOAT));

assertEquals(DATETIME, commonType(DATETIME, DATETIME_NANOS));
assertEquals(DATETIME, commonType(DATETIME_NANOS, DATETIME));

// strings
assertEquals(TEXT, commonType(TEXT, KEYWORD));
assertEquals(TEXT, commonType(KEYWORD, TEXT));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

import static java.util.Collections.emptyMap;
import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME;
import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME_NANOS;
import static org.elasticsearch.xpack.ql.type.DataTypes.INTEGER;
import static org.elasticsearch.xpack.ql.type.DataTypes.KEYWORD;
import static org.elasticsearch.xpack.ql.type.DataTypes.NESTED;
Expand Down Expand Up @@ -109,7 +108,7 @@ public void testDateNanosField() {

assertThat(mapping.size(), is(1));
EsField field = mapping.get("date_nanos");
assertThat(field.getDataType(), is(DATETIME_NANOS));
assertThat(field.getDataType(), is(DATETIME));
assertThat(field.isAggregatable(), is(true));
assertThat(field, is(instanceOf(DateEsField.class)));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@
*/
package org.elasticsearch.xpack.sql.action;

import java.util.ArrayList;
import java.util.List;

import org.elasticsearch.action.index.IndexRequest;
import org.elasticsearch.action.support.WriteRequest;
import org.elasticsearch.search.builder.SearchSourceBuilder;
import org.elasticsearch.search.fetch.subphase.FieldAndFormat;
import org.elasticsearch.search.sort.SortBuilders;

import java.util.ArrayList;
import java.util.List;

import static java.util.Collections.singletonList;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;

Expand All @@ -39,13 +39,12 @@ public void testSqlTranslateAction() {
if (columnOrder) {
expectedFields.add(new FieldAndFormat("data", null));
expectedFields.add(new FieldAndFormat("count", null));
expectedFields.add(new FieldAndFormat("date", "epoch_millis"));
expectedFields.add(new FieldAndFormat("date", "strict_date_optional_time_nanos"));
} else {
expectedFields.add(new FieldAndFormat("date", "epoch_millis"));
expectedFields.add(new FieldAndFormat("date", "strict_date_optional_time_nanos"));
expectedFields.add(new FieldAndFormat("data", null));
expectedFields.add(new FieldAndFormat("count", null));
}

assertEquals(expectedFields, actualFields);
assertEquals(singletonList(SortBuilders.fieldSort("count").missing("_last").unmappedType("long")), source.sorts());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,12 @@
import org.elasticsearch.xpack.sql.expression.literal.geo.GeoShape;
import org.elasticsearch.xpack.sql.type.SqlDataTypes;
import org.elasticsearch.xpack.sql.util.DateUtils;

import java.io.IOException;
import java.time.ZoneId;
import java.util.List;
import java.util.Map;

import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME;
import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME_NANOS;
import static org.elasticsearch.xpack.sql.type.SqlDataTypes.GEO_POINT;
import static org.elasticsearch.xpack.sql.type.SqlDataTypes.GEO_SHAPE;
import static org.elasticsearch.xpack.sql.type.SqlDataTypes.SHAPE;
Expand Down Expand Up @@ -123,15 +121,6 @@ protected Object unwrapCustomValue(Object values) {
throw new SqlIllegalArgumentException("Objects (returned by [{}]) are not supported", fieldName());
}
if (dataType == DATETIME) {
if (values instanceof String) {
try {
return DateUtils.asDateTimeWithMillis(Long.parseLong(values.toString()), zoneId());
} catch (NumberFormatException e) {
return DateUtils.asDateTimeWithNanos(values.toString()).withZoneSameInstant(zoneId());
}
}
}
if (dataType == DATETIME_NANOS) {
if (values instanceof String) {
return DateUtils.asDateTimeWithNanos(values.toString()).withZoneSameInstant(zoneId());
}
Expand Down
Loading

0 comments on commit 45677a3

Please sign in to comment.