Skip to content

Commit

Permalink
Date Range: Inclusive upper range does not round up properly, closes #…
Browse files Browse the repository at this point in the history 
…1463.
  • Loading branch information
@kimchy
kimchy committed Nov 14, 2011
1 parent ed281fb commit 73ba30b
Show file tree
Hide file tree
Showing 8 changed files with 117 additions and 29 deletions.
15 changes: 11 additions & 4 deletions modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java
View file
Expand Up @@ -140,14 +140,18 @@ public static class Builder {

private final String index;

@Nullable private final Settings indexSettings;

private final RootObjectMapper rootObjectMapper;

private ImmutableMap<String, Object> meta = ImmutableMap.of();

private Mapper.BuilderContext builderContext = new Mapper.BuilderContext(new ContentPath(1));
private final Mapper.BuilderContext builderContext;

public Builder(String index, @Nullable Settings indexSettings, RootObjectMapper.Builder builder) {
this.index = index;
this.indexSettings = indexSettings;
this.builderContext = new Mapper.BuilderContext(indexSettings, new ContentPath(1));
this.rootObjectMapper = builder.build(builderContext);
IdFieldMapper idFieldMapper = new IdFieldMapper();
if (indexSettings != null) {
Expand Down Expand Up @@ -203,7 +207,7 @@ public boolean hasSearchAnalyzer() {

public DocumentMapper build(DocumentMapperParser docMapperParser) {
Preconditions.checkNotNull(rootObjectMapper, "Mapper builder must have the root object mapper set");
return new DocumentMapper(index, docMapperParser, rootObjectMapper, meta,
return new DocumentMapper(index, indexSettings, docMapperParser, rootObjectMapper, meta,
indexAnalyzer, searchAnalyzer,
rootMappers);
}
Expand All @@ -212,12 +216,14 @@ public DocumentMapper build(DocumentMapperParser docMapperParser) {

private ThreadLocal<ParseContext> cache = new ThreadLocal<ParseContext>() {
@Override protected ParseContext initialValue() {
return new ParseContext(index, docMapperParser, DocumentMapper.this, new ContentPath(0));
return new ParseContext(index, indexSettings, docMapperParser, DocumentMapper.this, new ContentPath(0));
}
};

private final String index;

private final Settings indexSettings;

private final String type;

private final DocumentMapperParser docMapperParser;
Expand Down Expand Up @@ -250,12 +256,13 @@ public DocumentMapper build(DocumentMapperParser docMapperParser) {

private final Object mutex = new Object();

public DocumentMapper(String index, DocumentMapperParser docMapperParser,
public DocumentMapper(String index, @Nullable Settings indexSettings, DocumentMapperParser docMapperParser,
RootObjectMapper rootObjectMapper,
ImmutableMap<String, Object> meta,
NamedAnalyzer indexAnalyzer, NamedAnalyzer searchAnalyzer,
Map<Class<? extends RootMapper>, RootMapper> rootMappers) {
this.index = index;
this.indexSettings = indexSettings;
this.type = rootObjectMapper.name();
this.docMapperParser = docMapperParser;
this.meta = meta;
Expand Down
10 changes: 9 additions & 1 deletion modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/Mapper.java
View file
Expand Up @@ -19,8 +19,10 @@

package org.elasticsearch.index.mapper;

import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.collect.ImmutableMap;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.NotThreadSafe;
import org.elasticsearch.common.util.concurrent.ThreadSafe;
import org.elasticsearch.common.xcontent.ToXContent;
Expand All @@ -39,15 +41,21 @@ public interface Mapper extends ToXContent {

@NotThreadSafe
public static class BuilderContext {
private final Settings indexSettings;
private final ContentPath contentPath;

public BuilderContext(ContentPath contentPath) {
public BuilderContext(@Nullable Settings indexSettings, ContentPath contentPath) {
this.contentPath = contentPath;
this.indexSettings = indexSettings;
}

public ContentPath path() {
return this.contentPath;
}

@Nullable public Settings indexSettings() {
return this.indexSettings;
}
}

@NotThreadSafe
Expand Down
13 changes: 11 additions & 2 deletions modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/ParseContext.java
View file
Expand Up @@ -22,7 +22,9 @@
import org.apache.lucene.analysis.Analyzer;
import org.apache.lucene.document.Document;
import org.apache.lucene.document.Field;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.lucene.all.AllEntries;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.NotThreadSafe;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.analysis.AnalysisService;
Expand Down Expand Up @@ -53,7 +55,9 @@ public class ParseContext {

private Analyzer analyzer;

private String index;
private final String index;

@Nullable private final Settings indexSettings;

private SourceToParse sourceToParse;
private byte[] source;
Expand All @@ -78,8 +82,9 @@ public class ParseContext {

private AllEntries allEntries = new AllEntries();

public ParseContext(String index, DocumentMapperParser docMapperParser, DocumentMapper docMapper, ContentPath path) {
public ParseContext(String index, @Nullable Settings indexSettings, DocumentMapperParser docMapperParser, DocumentMapper docMapper, ContentPath path) {
this.index = index;
this.indexSettings = indexSettings;
this.docMapper = docMapper;
this.docMapperParser = docMapperParser;
this.path = path;
Expand Down Expand Up @@ -128,6 +133,10 @@ public String index() {
return this.index;
}

@Nullable public Settings indexSettings() {
return this.indexSettings;
}

public String type() {
return sourceToParse.type();
}
Expand Down
38 changes: 33 additions & 5 deletions modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/core/DateFieldMapper.java
View file
Expand Up @@ -30,6 +30,8 @@
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.joda.FormatDateTimeFormatter;
import org.elasticsearch.common.joda.Joda;
import org.elasticsearch.common.joda.time.DateTimeZone;
import org.elasticsearch.common.joda.time.MutableDateTime;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
Expand Down Expand Up @@ -64,6 +66,7 @@ public static class Defaults extends NumberFieldMapper.Defaults {
public static final String NULL_VALUE = null;

public static final TimeUnit TIME_UNIT = TimeUnit.MILLISECONDS;
public static final boolean PARSE_UPPER_INCLUSIVE = true;
}

public static class Builder extends NumberFieldMapper.Builder<Builder, DateFieldMapper> {
Expand Down Expand Up @@ -95,8 +98,12 @@ public Builder dateTimeFormatter(FormatDateTimeFormatter dateTimeFormatter) {
}

@Override public DateFieldMapper build(BuilderContext context) {
boolean parseUpperInclusive = Defaults.PARSE_UPPER_INCLUSIVE;
if (context.indexSettings() != null) {
parseUpperInclusive = context.indexSettings().getAsBoolean("index.mapping.date.parse_upper_inclusive", Defaults.PARSE_UPPER_INCLUSIVE);
}
DateFieldMapper fieldMapper = new DateFieldMapper(buildNames(context), dateTimeFormatter,
precisionStep, fuzzyFactor, index, store, boost, omitNorms, omitTermFreqAndPositions, nullValue, timeUnit);
precisionStep, fuzzyFactor, index, store, boost, omitNorms, omitTermFreqAndPositions, nullValue, timeUnit, parseUpperInclusive);
fieldMapper.includeInAll(includeInAll);
return fieldMapper;
}
Expand All @@ -123,20 +130,23 @@ public static class TypeParser implements Mapper.TypeParser {

protected final FormatDateTimeFormatter dateTimeFormatter;

private final boolean parseUpperInclusive;

private String nullValue;

protected final TimeUnit timeUnit;

protected DateFieldMapper(Names names, FormatDateTimeFormatter dateTimeFormatter, int precisionStep, String fuzzyFactor,
Field.Index index, Field.Store store,
float boost, boolean omitNorms, boolean omitTermFreqAndPositions,
String nullValue, TimeUnit timeUnit) {
String nullValue, TimeUnit timeUnit, boolean parseUpperInclusive) {
super(names, precisionStep, fuzzyFactor, index, store, boost, omitNorms, omitTermFreqAndPositions,
new NamedAnalyzer("_date/" + precisionStep, new NumericDateAnalyzer(precisionStep, dateTimeFormatter.parser())),
new NamedAnalyzer("_date/max", new NumericDateAnalyzer(Integer.MAX_VALUE, dateTimeFormatter.parser())));
this.dateTimeFormatter = dateTimeFormatter;
this.nullValue = nullValue;
this.timeUnit = timeUnit;
this.parseUpperInclusive = parseUpperInclusive;
}

@Override protected double parseFuzzyFactor(String fuzzyFactor) {
Expand Down Expand Up @@ -212,21 +222,21 @@ protected DateFieldMapper(Names names, FormatDateTimeFormatter dateTimeFormatter
@Override public Query rangeQuery(String lowerTerm, String upperTerm, boolean includeLower, boolean includeUpper) {
return NumericRangeQuery.newLongRange(names.indexName(), precisionStep,
lowerTerm == null ? null : parseStringValue(lowerTerm),
upperTerm == null ? null : parseStringValue(upperTerm),
upperTerm == null ? null : includeUpper ? parseUpperInclusiveStringValue(upperTerm) : parseStringValue(upperTerm),
includeLower, includeUpper);
}

@Override public Filter rangeFilter(String lowerTerm, String upperTerm, boolean includeLower, boolean includeUpper) {
return NumericRangeFilter.newLongRange(names.indexName(), precisionStep,
lowerTerm == null ? null : parseStringValue(lowerTerm),
upperTerm == null ? null : parseStringValue(upperTerm),
upperTerm == null ? null : includeUpper ? parseUpperInclusiveStringValue(upperTerm) : parseStringValue(upperTerm),
includeLower, includeUpper);
}

@Override public Filter rangeFilter(FieldDataCache fieldDataCache, String lowerTerm, String upperTerm, boolean includeLower, boolean includeUpper) {
return NumericRangeFieldDataFilter.newLongRange(fieldDataCache, names.indexName(),
lowerTerm == null ? null : parseStringValue(lowerTerm),
upperTerm == null ? null : parseStringValue(upperTerm),
upperTerm == null ? null : includeUpper ? parseUpperInclusiveStringValue(upperTerm) : parseStringValue(upperTerm),
includeLower, includeUpper);
}

Expand Down Expand Up @@ -363,4 +373,22 @@ protected long parseStringValue(String value) {
}
}
}

protected long parseUpperInclusiveStringValue(String value) {
if (!parseUpperInclusive) {
return parseStringValue(value);
}
try {
MutableDateTime dateTime = new MutableDateTime(3000, 12, 31, 23, 59, 59, 999, DateTimeZone.UTC);
dateTimeFormatter.parser().parseInto(dateTime, value, 0);
return dateTime.getMillis();
} catch (RuntimeException e) {
try {
long time = Long.parseLong(value);
return timeUnit.toMillis(time);
} catch (NumberFormatException e1) {
throw new MapperParsingException("failed to parse date field, tried both date format [" + dateTimeFormatter.format() + "], and timestamp number", e);
}
}
}
}
12 changes: 8 additions & 4 deletions ...ticsearch/src/main/java/org/elasticsearch/index/mapper/internal/TimestampFieldMapper.java
View file
Expand Up @@ -90,7 +90,11 @@ public Builder dateTimeFormatter(FormatDateTimeFormatter dateTimeFormatter) {
}

@Override public TimestampFieldMapper build(BuilderContext context) {
return new TimestampFieldMapper(store, index, enabled, path, dateTimeFormatter);
boolean parseUpperInclusive = Defaults.PARSE_UPPER_INCLUSIVE;
if (context.indexSettings() != null) {
parseUpperInclusive = context.indexSettings().getAsBoolean("index.mapping.date.parse_upper_inclusive", Defaults.PARSE_UPPER_INCLUSIVE);
}
return new TimestampFieldMapper(store, index, enabled, path, dateTimeFormatter, parseUpperInclusive);
}
}

Expand Down Expand Up @@ -119,13 +123,13 @@ public static class TypeParser implements Mapper.TypeParser {
private final String path;

public TimestampFieldMapper() {
this(Defaults.STORE, Defaults.INDEX, Defaults.ENABLED, Defaults.PATH, Defaults.DATE_TIME_FORMATTER);
this(Defaults.STORE, Defaults.INDEX, Defaults.ENABLED, Defaults.PATH, Defaults.DATE_TIME_FORMATTER, Defaults.PARSE_UPPER_INCLUSIVE);
}

protected TimestampFieldMapper(Field.Store store, Field.Index index, boolean enabled, String path, FormatDateTimeFormatter dateTimeFormatter) {
protected TimestampFieldMapper(Field.Store store, Field.Index index, boolean enabled, String path, FormatDateTimeFormatter dateTimeFormatter, boolean parseUpperInclusive) {
super(new Names(Defaults.NAME, Defaults.NAME, Defaults.NAME, Defaults.NAME), dateTimeFormatter,
Defaults.PRECISION_STEP, Defaults.FUZZY_FACTOR, index, store, Defaults.BOOST, Defaults.OMIT_NORMS,
Defaults.OMIT_TERM_FREQ_AND_POSITIONS, Defaults.NULL_VALUE, TimeUnit.MILLISECONDS /*always milliseconds*/);
Defaults.OMIT_TERM_FREQ_AND_POSITIONS, Defaults.NULL_VALUE, TimeUnit.MILLISECONDS /*always milliseconds*/, parseUpperInclusive);
this.enabled = enabled;
this.path = path;
}
Expand Down
4 changes: 2 additions & 2 deletions modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/object/ObjectMapper.java
View file
Expand Up @@ -510,7 +510,7 @@ private void serializeObject(final ParseContext context, String currentFieldName
}
// remove the current field name from path, since the object builder adds it as well...
context.path().remove();
BuilderContext builderContext = new BuilderContext(context.path());
BuilderContext builderContext = new BuilderContext(context.indexSettings(), context.path());
objectMapper = builder.build(builderContext);
putMapper(objectMapper);
// now re add it
Expand Down Expand Up @@ -595,7 +595,7 @@ private void serializeValue(final ParseContext context, String currentFieldName,
mapper = mappers.get(currentFieldName);
if (mapper == null) {
newMapper = true;
BuilderContext builderContext = new BuilderContext(context.path());
BuilderContext builderContext = new BuilderContext(context.indexSettings(), context.path());
if (token == XContentParser.Token.VALUE_STRING) {
boolean resolved = false;

Expand Down
10 changes: 10 additions & 0 deletions modules/elasticsearch/src/test/java/org/elasticsearch/deps/joda/SimpleJodaTests.java
View file
Expand Up @@ -61,6 +61,16 @@ public class SimpleJodaTests {
assertThat(millis, equalTo(0l));
}

@Test public void testUpperBound() {
MutableDateTime dateTime = new MutableDateTime(3000, 12, 31, 23, 59, 59, 999, DateTimeZone.UTC);
DateTimeFormatter formatter = ISODateTimeFormat.dateOptionalTimeParser().withZone(DateTimeZone.UTC);

String value = "2000-01-01";
int i = formatter.parseInto(dateTime, value, 0);
assertThat(i, equalTo(value.length()));
assertThat(dateTime.toString(), equalTo("2000-01-01T23:59:59.999Z"));
}

@Test public void testIsoDateFormatDateOptionalTimeUTC() {
DateTimeFormatter formatter = ISODateTimeFormat.dateOptionalTimeParser().withZone(DateTimeZone.UTC);
long millis = formatter.parseMillis("1970-01-01T00:00:00Z");
Expand Down
44 changes: 33 additions & 11 deletions .../integration/search/ip/IpSearchTests.java → ...tion/search/simple/SimpleSearchTests.java
View file
Expand Up @@ -17,12 +17,13 @@
* under the License.
*/

package org.elasticsearch.test.integration.search.ip;
package org.elasticsearch.test.integration.search.simple;

import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.client.Client;
import org.elasticsearch.common.settings.ImmutableSettings;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.test.integration.AbstractNodesTests;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
Expand All @@ -32,10 +33,7 @@
import static org.hamcrest.MatcherAssert.*;
import static org.hamcrest.Matchers.*;

/**
* @author kimchy (shay.banon)
*/
public class IpSearchTests extends AbstractNodesTests {
public class SimpleSearchTests extends AbstractNodesTests {

private Client client;

Expand All @@ -53,12 +51,8 @@ protected Client getClient() {
return client("node1");
}

@Test public void filterExistsMissingTests() throws Exception {
try {
client.admin().indices().prepareDelete("test").execute().actionGet();
} catch (Exception e) {
// ignore
}
@Test public void simpleIpTests() throws Exception {
client.admin().indices().prepareDelete().execute().actionGet();

client.admin().indices().prepareCreate("test").setSettings(ImmutableSettings.settingsBuilder().put("number_of_shards", 1)).execute().actionGet();

Expand All @@ -77,4 +71,32 @@ protected Client getClient() {

assertThat(search.hits().totalHits(), equalTo(1l));
}

@Test public void simpleDateRangeWithUpperInclusiveEnabledTests() throws Exception {
client.admin().indices().prepareDelete().execute().actionGet();
client.admin().indices().prepareCreate("test").setSettings(ImmutableSettings.settingsBuilder()).execute().actionGet();
client.prepareIndex("test", "type1", "1").setSource("field", "2010-01-05T02:00").execute().actionGet();
client.prepareIndex("test", "type1", "2").setSource("field", "2010-01-06T02:00").execute().actionGet();
client.admin().indices().prepareRefresh().execute().actionGet();

// test include upper on ranges to include the full day on the upper bound
SearchResponse searchResponse = client.prepareSearch("test").setQuery(QueryBuilders.rangeQuery("field").gte("2010-01-05").lte("2010-01-06")).execute().actionGet();
assertThat(searchResponse.hits().totalHits(), equalTo(2l));
searchResponse = client.prepareSearch("test").setQuery(QueryBuilders.rangeQuery("field").gte("2010-01-05").lt("2010-01-06")).execute().actionGet();
assertThat(searchResponse.hits().totalHits(), equalTo(1l));
}

@Test public void simpleDateRangeWithUpperInclusiveDisabledTests() throws Exception {
client.admin().indices().prepareDelete().execute().actionGet();
client.admin().indices().prepareCreate("test").setSettings(ImmutableSettings.settingsBuilder().put("index.mapping.date.parse_upper_inclusive", false)).execute().actionGet();
client.prepareIndex("test", "type1", "1").setSource("field", "2010-01-05T02:00").execute().actionGet();
client.prepareIndex("test", "type1", "2").setSource("field", "2010-01-06T02:00").execute().actionGet();
client.admin().indices().prepareRefresh().execute().actionGet();

// test include upper on ranges to include the full day on the upper bound (disabled here though...)
SearchResponse searchResponse = client.prepareSearch("test").setQuery(QueryBuilders.rangeQuery("field").gte("2010-01-05").lte("2010-01-06")).execute().actionGet();
assertThat(searchResponse.hits().totalHits(), equalTo(1l));
searchResponse = client.prepareSearch("test").setQuery(QueryBuilders.rangeQuery("field").gte("2010-01-05").lt("2010-01-06")).execute().actionGet();
assertThat(searchResponse.hits().totalHits(), equalTo(1l));
}
}

0 comments on commit 73ba30b

Please sign in to comment.