Skip to content
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.

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

QL: Eliminate internal type DATETIME_NANOS (#68220) #68850

Merged
merged 2 commits into from
Feb 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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 = unmodifiableMap(TYPES.stream()
.collect(toMap(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 @@ -138,7 +137,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 @@ -169,7 +168,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 @@ -209,7 +208,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 @@ -258,7 +257,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 @@ -300,7 +299,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 @@ -324,7 +323,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 @@ -348,7 +347,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 @@ -390,9 +389,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