diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index d7e1b3902bf..d00972b4d0e 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -84,6 +84,10 @@ Bug Fixes that led to IllegalStateException being thrown when nothing was wrong. (David Smiley, yonik) +* LUCENE-7219: Make queryparser/xml (Point|LegacyNumeric)RangeQuery builders + match the underlying queries' (lower|upper)Term optionality logic. + (Kaneshanathan Srivisagan, Christine Poerschke) + Documentation * LUCENE-7223: Improve XXXPoint javadocs to make it clear that you diff --git a/lucene/core/src/test/org/apache/lucene/index/Test4GBStoredFields.java b/lucene/core/src/test/org/apache/lucene/index/Test4GBStoredFields.java index 0129bf9ad5f..3c147b6f29f 100644 --- a/lucene/core/src/test/org/apache/lucene/index/Test4GBStoredFields.java +++ b/lucene/core/src/test/org/apache/lucene/index/Test4GBStoredFields.java @@ -18,6 +18,7 @@ package org.apache.lucene.index; import org.apache.lucene.analysis.MockAnalyzer; +import org.apache.lucene.codecs.compressing.CompressingCodec; import org.apache.lucene.document.Document; import org.apache.lucene.document.Field; import org.apache.lucene.document.FieldType; @@ -34,7 +35,7 @@ import com.carrotsearch.randomizedtesting.generators.RandomInts; /** * This test creates an index with one segment that is a little larger than 4GB. */ -@SuppressCodecs({ "SimpleText" }) +@SuppressCodecs({ "SimpleText", "Compressing" }) @TimeoutSuite(millis = 4 * TimeUnits.HOUR) public class Test4GBStoredFields extends LuceneTestCase { @@ -43,13 +44,20 @@ public class Test4GBStoredFields extends LuceneTestCase { MockDirectoryWrapper dir = new MockDirectoryWrapper(random(), new MMapDirectory(createTempDir("4GBStoredFields"))); dir.setThrottling(MockDirectoryWrapper.Throttling.NEVER); - IndexWriter w = new IndexWriter(dir, - new IndexWriterConfig(new MockAnalyzer(random())) - .setMaxBufferedDocs(IndexWriterConfig.DISABLE_AUTO_FLUSH) - .setRAMBufferSizeMB(256.0) - .setMergeScheduler(new ConcurrentMergeScheduler()) - .setMergePolicy(newLogMergePolicy(false, 10)) - .setOpenMode(IndexWriterConfig.OpenMode.CREATE)); + IndexWriterConfig iwc = new IndexWriterConfig(new MockAnalyzer(random())); + iwc.setMaxBufferedDocs(IndexWriterConfig.DISABLE_AUTO_FLUSH); + iwc.setRAMBufferSizeMB(256.0); + iwc.setMergeScheduler(new ConcurrentMergeScheduler()); + iwc.setMergePolicy(newLogMergePolicy(false, 10)); + iwc.setOpenMode(IndexWriterConfig.OpenMode.CREATE); + + // TODO: we disable "Compressing" since it likes to pick very extreme values which will be too slow for this test. + // maybe we should factor out crazy cases to ExtremeCompressing? then annotations can handle this stuff... + if (random().nextBoolean()) { + iwc.setCodec(CompressingCodec.reasonableInstance(random())); + } + + IndexWriter w = new IndexWriter(dir, iwc); MergePolicy mp = w.getConfig().getMergePolicy(); if (mp instanceof LogByteSizeMergePolicy) { diff --git a/lucene/queryparser/src/java/org/apache/lucene/queryparser/xml/builders/LegacyNumericRangeQueryBuilder.java b/lucene/queryparser/src/java/org/apache/lucene/queryparser/xml/builders/LegacyNumericRangeQueryBuilder.java index e19596430e2..f7aef3f477b 100644 --- a/lucene/queryparser/src/java/org/apache/lucene/queryparser/xml/builders/LegacyNumericRangeQueryBuilder.java +++ b/lucene/queryparser/src/java/org/apache/lucene/queryparser/xml/builders/LegacyNumericRangeQueryBuilder.java @@ -45,14 +45,14 @@ import org.w3c.dom.Element; * * lowerTerm * Specified by type - * Yes - * N/A + * No + * Null * * * upperTerm * Specified by type - * Yes - * N/A + * No + * Null * * * type @@ -91,8 +91,8 @@ public class LegacyNumericRangeQueryBuilder implements QueryBuilder { @Override public Query getQuery(Element e) throws ParserException { String field = DOMUtils.getAttributeWithInheritanceOrFail(e, "fieldName"); - String lowerTerm = DOMUtils.getAttributeOrFail(e, "lowerTerm"); - String upperTerm = DOMUtils.getAttributeOrFail(e, "upperTerm"); + final String lowerTerm = DOMUtils.getAttribute(e, "lowerTerm", null); + final String upperTerm = DOMUtils.getAttribute(e, "upperTerm", null); boolean lowerInclusive = DOMUtils.getAttribute(e, "includeLower", true); boolean upperInclusive = DOMUtils.getAttribute(e, "includeUpper", true); int precisionStep = DOMUtils.getAttribute(e, "precisionStep", LegacyNumericUtils.PRECISION_STEP_DEFAULT); @@ -101,20 +101,28 @@ public class LegacyNumericRangeQueryBuilder implements QueryBuilder { try { Query filter; if (type.equalsIgnoreCase("int")) { - filter = LegacyNumericRangeQuery.newIntRange(field, precisionStep, Integer - .valueOf(lowerTerm), Integer.valueOf(upperTerm), lowerInclusive, + filter = LegacyNumericRangeQuery.newIntRange(field, precisionStep, + (lowerTerm == null ? null : Integer.valueOf(lowerTerm)), + (upperTerm == null ? null : Integer.valueOf(upperTerm)), + lowerInclusive, upperInclusive); } else if (type.equalsIgnoreCase("long")) { - filter = LegacyNumericRangeQuery.newLongRange(field, precisionStep, Long - .valueOf(lowerTerm), Long.valueOf(upperTerm), lowerInclusive, + filter = LegacyNumericRangeQuery.newLongRange(field, precisionStep, + (lowerTerm == null ? null : Long.valueOf(lowerTerm)), + (upperTerm == null ? null : Long.valueOf(upperTerm)), + lowerInclusive, upperInclusive); } else if (type.equalsIgnoreCase("double")) { - filter = LegacyNumericRangeQuery.newDoubleRange(field, precisionStep, Double - .valueOf(lowerTerm), Double.valueOf(upperTerm), lowerInclusive, + filter = LegacyNumericRangeQuery.newDoubleRange(field, precisionStep, + (lowerTerm == null ? null : Double.valueOf(lowerTerm)), + (upperTerm == null ? null : Double.valueOf(upperTerm)), + lowerInclusive, upperInclusive); } else if (type.equalsIgnoreCase("float")) { - filter = LegacyNumericRangeQuery.newFloatRange(field, precisionStep, Float - .valueOf(lowerTerm), Float.valueOf(upperTerm), lowerInclusive, + filter = LegacyNumericRangeQuery.newFloatRange(field, precisionStep, + (lowerTerm == null ? null : Float.valueOf(lowerTerm)), + (upperTerm == null ? null : Float.valueOf(upperTerm)), + lowerInclusive, upperInclusive); } else { throw new ParserException("type attribute must be one of: [long, int, double, float]"); diff --git a/lucene/queryparser/src/java/org/apache/lucene/queryparser/xml/builders/PointRangeQueryBuilder.java b/lucene/queryparser/src/java/org/apache/lucene/queryparser/xml/builders/PointRangeQueryBuilder.java index 45483168bac..82f7039f0be 100644 --- a/lucene/queryparser/src/java/org/apache/lucene/queryparser/xml/builders/PointRangeQueryBuilder.java +++ b/lucene/queryparser/src/java/org/apache/lucene/queryparser/xml/builders/PointRangeQueryBuilder.java @@ -46,14 +46,14 @@ import org.w3c.dom.Element; * * lowerTerm * Specified by type - * Yes - * N/A + * No + * Integer.MIN_VALUE Long.MIN_VALUE Float.NEGATIVE_INFINITY Double.NEGATIVE_INFINITY * * * upperTerm * Specified by type - * Yes - * N/A + * No + * Integer.MAX_VALUE Long.MAX_VALUE Float.POSITIVE_INFINITY Double.POSITIVE_INFINITY * * * type @@ -72,19 +72,27 @@ public class PointRangeQueryBuilder implements QueryBuilder { @Override public Query getQuery(Element e) throws ParserException { String field = DOMUtils.getAttributeWithInheritanceOrFail(e, "fieldName"); - String lowerTerm = DOMUtils.getAttributeOrFail(e, "lowerTerm"); - String upperTerm = DOMUtils.getAttributeOrFail(e, "upperTerm"); + final String lowerTerm = DOMUtils.getAttribute(e, "lowerTerm", null); + final String upperTerm = DOMUtils.getAttribute(e, "upperTerm", null); String type = DOMUtils.getAttribute(e, "type", "int"); try { if (type.equalsIgnoreCase("int")) { - return IntPoint.newRangeQuery(field, Integer.valueOf(lowerTerm), Integer.valueOf(upperTerm)); + return IntPoint.newRangeQuery(field, + (lowerTerm == null ? Integer.MIN_VALUE : Integer.valueOf(lowerTerm)), + (upperTerm == null ? Integer.MAX_VALUE : Integer.valueOf(upperTerm))); } else if (type.equalsIgnoreCase("long")) { - return LongPoint.newRangeQuery(field, Long.valueOf(lowerTerm), Long.valueOf(upperTerm)); + return LongPoint.newRangeQuery(field, + (lowerTerm == null ? Long.MIN_VALUE : Long.valueOf(lowerTerm)), + (upperTerm == null ? Long.MAX_VALUE : Long.valueOf(upperTerm))); } else if (type.equalsIgnoreCase("double")) { - return DoublePoint.newRangeQuery(field, Double.valueOf(lowerTerm), Double.valueOf(upperTerm)); + return DoublePoint.newRangeQuery(field, + (lowerTerm == null ? Double.NEGATIVE_INFINITY : Double.valueOf(lowerTerm)), + (upperTerm == null ? Double.POSITIVE_INFINITY : Double.valueOf(upperTerm))); } else if (type.equalsIgnoreCase("float")) { - return FloatPoint.newRangeQuery(field, Float.valueOf(lowerTerm), Float.valueOf(upperTerm)); + return FloatPoint.newRangeQuery(field, + (lowerTerm == null ? Float.NEGATIVE_INFINITY : Float.valueOf(lowerTerm)), + (upperTerm == null ? Float.POSITIVE_INFINITY : Float.valueOf(upperTerm))); } else { throw new ParserException("type attribute must be one of: [long, int, double, float]"); } diff --git a/lucene/queryparser/src/test/org/apache/lucene/queryparser/xml/LegacyNumericRangeQueryWithoutLowerTerm.xml b/lucene/queryparser/src/test/org/apache/lucene/queryparser/xml/LegacyNumericRangeQueryWithoutLowerTerm.xml new file mode 100644 index 00000000000..54de55fd0f3 --- /dev/null +++ b/lucene/queryparser/src/test/org/apache/lucene/queryparser/xml/LegacyNumericRangeQueryWithoutLowerTerm.xml @@ -0,0 +1,31 @@ + + + + + merger + + + sumitomo + + + bank + + + + + diff --git a/lucene/queryparser/src/test/org/apache/lucene/queryparser/xml/LegacyNumericRangeQueryWithoutRange.xml b/lucene/queryparser/src/test/org/apache/lucene/queryparser/xml/LegacyNumericRangeQueryWithoutRange.xml new file mode 100644 index 00000000000..764f6548d76 --- /dev/null +++ b/lucene/queryparser/src/test/org/apache/lucene/queryparser/xml/LegacyNumericRangeQueryWithoutRange.xml @@ -0,0 +1,31 @@ + + + + + merger + + + sumitomo + + + bank + + + + + diff --git a/lucene/queryparser/src/test/org/apache/lucene/queryparser/xml/LegacyNumericRangeQueryWithoutUpperTerm.xml b/lucene/queryparser/src/test/org/apache/lucene/queryparser/xml/LegacyNumericRangeQueryWithoutUpperTerm.xml new file mode 100644 index 00000000000..d3a61c183b3 --- /dev/null +++ b/lucene/queryparser/src/test/org/apache/lucene/queryparser/xml/LegacyNumericRangeQueryWithoutUpperTerm.xml @@ -0,0 +1,31 @@ + + + + + merger + + + sumitomo + + + bank + + + + + diff --git a/lucene/queryparser/src/test/org/apache/lucene/queryparser/xml/PointRangeQueryWithoutLowerTerm.xml b/lucene/queryparser/src/test/org/apache/lucene/queryparser/xml/PointRangeQueryWithoutLowerTerm.xml new file mode 100644 index 00000000000..2159c2cee05 --- /dev/null +++ b/lucene/queryparser/src/test/org/apache/lucene/queryparser/xml/PointRangeQueryWithoutLowerTerm.xml @@ -0,0 +1,31 @@ + + + + + merger + + + sumitomo + + + bank + + + + + diff --git a/lucene/queryparser/src/test/org/apache/lucene/queryparser/xml/PointRangeQueryWithoutRange.xml b/lucene/queryparser/src/test/org/apache/lucene/queryparser/xml/PointRangeQueryWithoutRange.xml new file mode 100644 index 00000000000..dc18953358f --- /dev/null +++ b/lucene/queryparser/src/test/org/apache/lucene/queryparser/xml/PointRangeQueryWithoutRange.xml @@ -0,0 +1,31 @@ + + + + + merger + + + sumitomo + + + bank + + + + + diff --git a/lucene/queryparser/src/test/org/apache/lucene/queryparser/xml/PointRangeQueryWithoutUpperTerm.xml b/lucene/queryparser/src/test/org/apache/lucene/queryparser/xml/PointRangeQueryWithoutUpperTerm.xml new file mode 100644 index 00000000000..eca8573265f --- /dev/null +++ b/lucene/queryparser/src/test/org/apache/lucene/queryparser/xml/PointRangeQueryWithoutUpperTerm.xml @@ -0,0 +1,31 @@ + + + + + merger + + + sumitomo + + + bank + + + + + diff --git a/lucene/queryparser/src/test/org/apache/lucene/queryparser/xml/TestCoreParser.java b/lucene/queryparser/src/test/org/apache/lucene/queryparser/xml/TestCoreParser.java index 82426d06076..4cf5fcffe2f 100644 --- a/lucene/queryparser/src/test/org/apache/lucene/queryparser/xml/TestCoreParser.java +++ b/lucene/queryparser/src/test/org/apache/lucene/queryparser/xml/TestCoreParser.java @@ -132,12 +132,42 @@ public class TestCoreParser extends LuceneTestCase { Query q = parse("LegacyNumericRangeQuery.xml"); dumpResults("LegacyNumericRangeQuery", q, 5); } + + public void testNumericRangeQueryXMLWithoutLowerTerm() throws ParserException, IOException { + Query q = parse("LegacyNumericRangeQueryWithoutLowerTerm.xml"); + dumpResults("LegacyNumericRangeQueryWithoutLowerTerm", q, 5); + } + + public void testNumericRangeQueryXMLWithoutUpperTerm() throws ParserException, IOException { + Query q = parse("LegacyNumericRangeQueryWithoutUpperTerm.xml"); + dumpResults("LegacyNumericRangeQueryWithoutUpperTerm", q, 5); + } + + public void testNumericRangeQueryXMLWithoutRange() throws ParserException, IOException { + Query q = parse("LegacyNumericRangeQueryWithoutRange.xml"); + dumpResults("LegacyNumericRangeQueryWithoutRange", q, 5); + } public void testPointRangeQuery() throws ParserException, IOException { Query q = parse("PointRangeQuery.xml"); dumpResults("PointRangeQuery", q, 5); } + public void testPointRangeQueryWithoutLowerTerm() throws ParserException, IOException { + Query q = parse("PointRangeQueryWithoutLowerTerm.xml"); + dumpResults("PointRangeQueryWithoutLowerTerm", q, 5); + } + + public void testPointRangeQueryWithoutUpperTerm() throws ParserException, IOException { + Query q = parse("PointRangeQueryWithoutUpperTerm.xml"); + dumpResults("PointRangeQueryWithoutUpperTerm", q, 5); + } + + public void testPointRangeQueryWithoutRange() throws ParserException, IOException { + Query q = parse("PointRangeQueryWithoutRange.xml"); + dumpResults("PointRangeQueryWithoutRange", q, 5); + } + //================= Helper methods =================================== protected String defaultField() { diff --git a/lucene/spatial/src/test/org/apache/lucene/spatial/geopoint/search/TestLegacyGeoPointQuery.java b/lucene/spatial/src/test/org/apache/lucene/spatial/geopoint/search/TestLegacyGeoPointQuery.java index e56e28b0fd4..8766c0e1ce4 100644 --- a/lucene/spatial/src/test/org/apache/lucene/spatial/geopoint/search/TestLegacyGeoPointQuery.java +++ b/lucene/spatial/src/test/org/apache/lucene/spatial/geopoint/search/TestLegacyGeoPointQuery.java @@ -24,12 +24,14 @@ import org.apache.lucene.geo.Polygon; import org.apache.lucene.geo.Rectangle; import org.apache.lucene.spatial.geopoint.document.GeoPointField; import org.apache.lucene.spatial.geopoint.document.GeoPointField.TermEncoding; +import org.apache.lucene.util.LuceneTestCase.SuppressCodecs; /** * random testing for GeoPoint query logic (with deprecated numeric encoding) * @deprecated remove this when TermEncoding.NUMERIC is removed */ @Deprecated +@SuppressCodecs("Direct") // can easily create too many postings and blow direct sky high public class TestLegacyGeoPointQuery extends BaseGeoPointTestCase { @Override diff --git a/lucene/test-framework/src/java/org/apache/lucene/codecs/compressing/CompressingCodec.java b/lucene/test-framework/src/java/org/apache/lucene/codecs/compressing/CompressingCodec.java index 7db02337540..ca42881e8e5 100644 --- a/lucene/test-framework/src/java/org/apache/lucene/codecs/compressing/CompressingCodec.java +++ b/lucene/test-framework/src/java/org/apache/lucene/codecs/compressing/CompressingCodec.java @@ -60,6 +60,19 @@ public abstract class CompressingCodec extends FilterCodec { final int blockSize = random.nextBoolean() ? RandomInts.randomIntBetween(random, 1, 10) : RandomInts.randomIntBetween(random, 1, 1024); return randomInstance(random, chunkSize, chunkDocs, false, blockSize); } + + /** + * Creates a random {@link CompressingCodec} with more reasonable parameters for big tests. + */ + public static CompressingCodec reasonableInstance(Random random) { + // e.g. defaults use 2^14 for FAST and ~ 2^16 for HIGH + final int chunkSize = TestUtil.nextInt(random, 1<<13, 1<<17); + // e.g. defaults use 128 for FAST and 512 for HIGH + final int chunkDocs = TestUtil.nextInt(random, 1<<6, 1<<10); + // e.g. defaults use 1024 for both cases + final int blockSize = TestUtil.nextInt(random, 1<<9, 1<<11); + return randomInstance(random, chunkSize, chunkDocs, false, blockSize); + } /** * Creates a random {@link CompressingCodec} that is using a segment suffix diff --git a/lucene/test-framework/src/java/org/apache/lucene/geo/GeoTestUtil.java b/lucene/test-framework/src/java/org/apache/lucene/geo/GeoTestUtil.java index eb48a8fd89d..98e296684ac 100644 --- a/lucene/test-framework/src/java/org/apache/lucene/geo/GeoTestUtil.java +++ b/lucene/test-framework/src/java/org/apache/lucene/geo/GeoTestUtil.java @@ -195,6 +195,10 @@ public class GeoTestUtil { } else { double x = nextLongitudeBetween(minX, maxX); double y = (y1 - y2) / (x1 - x2) * (x-x1) + y1; + if (Double.isFinite(y) == false) { + // this can happen due to underflow when delta between x values is wonderfully tiny! + y = Math.copySign(90, x1); + } double delta = (maxY - minY) * 0.01; // our formula may put the targeted Y out of bounds y = Math.min(90, y);