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: Remove the unnecessary DateTimeFunction.dateTimeFormat() #68788

Merged
merged 3 commits into from
Feb 17, 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
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@
import org.elasticsearch.xpack.ql.type.DataTypes;
import org.elasticsearch.xpack.ql.util.Check;
import org.elasticsearch.xpack.ql.util.CollectionUtils;
import org.elasticsearch.xpack.ql.util.Holder;

import java.time.OffsetTime;
import java.time.ZoneId;
Expand Down Expand Up @@ -273,12 +272,12 @@ private static Query translate(BinaryComparison bc, TranslatorHandler handler) {
Source source = bc.source();
String name = handler.nameOf(bc.left());
Object value = valueOf(bc.right());
String format = handler.dateFormat(bc.left());
String format = null;
boolean isDateLiteralComparison = false;

// for a date constant comparison, we need to use a format for the date, to make sure that the format is the same
// no matter the timezone provided by the user
if ((value instanceof ZonedDateTime || value instanceof OffsetTime) && format == null) {
if (value instanceof ZonedDateTime || value instanceof OffsetTime) {
DateFormatter formatter;
if (value instanceof ZonedDateTime) {
formatter = DateFormatter.forPattern(DATE_FORMAT);
Expand Down Expand Up @@ -345,36 +344,33 @@ public static Query doTranslate(Range r, TranslatorHandler handler) {
Expression val = r.value();

Query query = null;
Holder<Object> lower = new Holder<>(valueOf(r.lower()));
Holder<Object> upper = new Holder<>(valueOf(r.upper()));
Holder<String> format = new Holder<>(handler.dateFormat(val));
Object lower = valueOf(r.lower());
Object upper = valueOf(r.upper());
String format = null;

// for a date constant comparison, we need to use a format for the date, to make sure that the format is the same
// no matter the timezone provided by the user
if (format.get() == null) {
DateFormatter formatter = null;
if (lower.get() instanceof ZonedDateTime || upper.get() instanceof ZonedDateTime) {
formatter = DateFormatter.forPattern(DATE_FORMAT);
} else if (lower.get() instanceof OffsetTime || upper.get() instanceof OffsetTime) {
formatter = DateFormatter.forPattern(TIME_FORMAT);
DateFormatter formatter = null;
if (lower instanceof ZonedDateTime || upper instanceof ZonedDateTime) {
formatter = DateFormatter.forPattern(DATE_FORMAT);
} else if (lower instanceof OffsetTime || upper instanceof OffsetTime) {
formatter = DateFormatter.forPattern(TIME_FORMAT);
}
if (formatter != null) {
// RangeQueryBuilder accepts an Object as its parameter, but it will call .toString() on the ZonedDateTime
// instance which can have a slightly different format depending on the ZoneId used to create the ZonedDateTime
// Since RangeQueryBuilder can handle date as String as well, we'll format it as String and provide the format.
if (lower instanceof ZonedDateTime || lower instanceof OffsetTime) {
lower = formatter.format((TemporalAccessor) lower);
}
if (formatter != null) {
// RangeQueryBuilder accepts an Object as its parameter, but it will call .toString() on the ZonedDateTime
// instance which can have a slightly different format depending on the ZoneId used to create the ZonedDateTime
// Since RangeQueryBuilder can handle date as String as well, we'll format it as String and provide the format.
if (lower.get() instanceof ZonedDateTime || lower.get() instanceof OffsetTime) {
lower.set(formatter.format((TemporalAccessor) lower.get()));
}
if (upper.get() instanceof ZonedDateTime || upper.get() instanceof OffsetTime) {
upper.set(formatter.format((TemporalAccessor) upper.get()));
}
format.set(formatter.pattern());
if (upper instanceof ZonedDateTime || upper instanceof OffsetTime) {
upper = formatter.format((TemporalAccessor) upper);
}
format = formatter.pattern();
}

query = handler.wrapFunctionQuery(r, val, new RangeQuery(r.source(), handler.nameOf(val), lower.get(), r.includeLower(),
upper.get(), r.includeUpper(), format.get(), r.zoneId()));

query = handler.wrapFunctionQuery(r, val,
new RangeQuery(r.source(), handler.nameOf(val), lower, r.includeLower(), upper, r.includeUpper(), format, r.zoneId()));

return query;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,6 @@ public String nameOf(Expression e) {
}
}

@Override
public String dateFormat(Expression e) {
return null;
}

@Override
public Object convert(Object value, DataType dataType) {
return DataTypeConverter.convert(value, dataType);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,5 @@ public interface TranslatorHandler {

String nameOf(Expression e);

String dateFormat(Expression e);

Object convert(Object value, DataType dataType);
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,6 @@ public DataType dataType() {
return DataTypes.INTEGER;
}

// used for applying ranges
public abstract String dateTimeFormat();

protected DateTimeExtractor extractor() {
return extractor;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,4 @@ protected DayOfMonth replaceChild(Expression newChild) {
return new DayOfMonth(source(), newChild, zoneId());
}

@Override
public String dateTimeFormat() {
return "d";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,4 @@ protected UnaryScalarFunction replaceChild(Expression newChild) {
return new DayOfYear(source(), newChild, zoneId());
}

@Override
public String dateTimeFormat() {
return "D";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,4 @@ protected HourOfDay replaceChild(Expression newChild) {
return new HourOfDay(source(), newChild, zoneId());
}

@Override
public String dateTimeFormat() {
return "hour";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,4 @@ protected IsoDayOfWeek replaceChild(Expression newChild) {
return new IsoDayOfWeek(source(), newChild, zoneId());
}

@Override
public String dateTimeFormat() {
return "e";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,4 @@ protected IsoWeekOfYear replaceChild(Expression newChild) {
return new IsoWeekOfYear(source(), newChild, zoneId());
}

@Override
public String dateTimeFormat() {
return "w";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,4 @@ protected MinuteOfDay replaceChild(Expression newChild) {
return new MinuteOfDay(source(), newChild, zoneId());
}

@Override
public String dateTimeFormat() {
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,4 @@ protected MinuteOfHour replaceChild(Expression newChild) {
return new MinuteOfHour(source(), newChild, zoneId());
}

@Override
public String dateTimeFormat() {
return "m";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,4 @@ protected MonthOfYear replaceChild(Expression newChild) {
return new MonthOfYear(source(), newChild, zoneId());
}

@Override
public String dateTimeFormat() {
return "M";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,4 @@ protected SecondOfMinute replaceChild(Expression newChild) {
return new SecondOfMinute(source(), newChild, zoneId());
}

@Override
public String dateTimeFormat() {
return "s";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,6 @@ protected Year replaceChild(Expression newChild) {
return new Year(source(), newChild, zoneId());
}

@Override
public String dateTimeFormat() {
return "year";
}

@Override
public String calendarInterval() {
return Histogram.YEAR_INTERVAL;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import org.elasticsearch.xpack.ql.querydsl.query.Query;
import org.elasticsearch.xpack.ql.querydsl.query.ScriptQuery;
import org.elasticsearch.xpack.ql.type.DataType;
import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DateTimeFunction;
import org.elasticsearch.xpack.sql.expression.function.scalar.geo.StDistance;
import org.elasticsearch.xpack.sql.type.SqlDataTypeConverter;

Expand Down Expand Up @@ -49,14 +48,6 @@ public String nameOf(Expression e) {
return QueryTranslator.nameOf(e);
}

@Override
public String dateFormat(Expression e) {
if (e instanceof DateTimeFunction) {
return ((DateTimeFunction) e).dateTimeFormat();
}
return null;
}

@Override
public Object convert(Object value, DataType dataType) {
return SqlDataTypeConverter.convert(value, dataType);
Expand Down