From c65451115c40c16b49ebcada04ae3f6731dc212f Mon Sep 17 00:00:00 2001 From: Munendra S N Date: Fri, 18 Oct 2019 23:45:57 +0530 Subject: [PATCH] SOLR-13403: fix NPE in terms for DatePointField * This fixes NPE and adds support for DatePointField in terms component --- solr/CHANGES.txt | 2 ++ .../handler/component/TermsComponent.java | 9 +++---- .../org/apache/solr/search/PointMerger.java | 1 + .../solr/collection1/conf/schema.xml | 5 ++++ .../DistributedTermsComponentTest.java | 17 ++++++++----- .../handler/component/TermsComponentTest.java | 24 +++++++++++++++++++ .../org/apache/solr/common/util/Utils.java | 12 ++++++++-- 7 files changed, 58 insertions(+), 12 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 6861f79947f..17d55b35fee 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -51,6 +51,8 @@ Bug Fixes * SOLR-13827: Fail on unknown operation in Request Parameters API (Munendra S N, noble) +* SOLR-13403: Fix NPE in TermsComponent for DatePointField (yonik, Munendra S N) + Other Changes --------------------- diff --git a/solr/core/src/java/org/apache/solr/handler/component/TermsComponent.java b/solr/core/src/java/org/apache/solr/handler/component/TermsComponent.java index 7aeec0f7e1d..4a4841ad167 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/TermsComponent.java +++ b/solr/core/src/java/org/apache/solr/handler/component/TermsComponent.java @@ -50,6 +50,7 @@ import org.apache.solr.common.params.TermsParams; import org.apache.solr.common.util.NamedList; import org.apache.solr.common.util.SimpleOrderedMap; import org.apache.solr.common.util.StrUtils; +import org.apache.solr.common.util.Utils; import org.apache.solr.handler.component.HttpShardHandlerFactory.WhitelistHostChecker; import org.apache.solr.request.SimpleFacets.CountPair; import org.apache.solr.schema.FieldType; @@ -201,7 +202,7 @@ public class TermsComponent extends SearchComponent { SchemaField sf = rb.req.getSchema().getFieldOrNull(field); if (sf != null && sf.getType().isPointField()) { // FIXME: terms.ttf=true is not supported for pointFields - if (lowerStr!=null || upperStr!=null || prefix!=null || regexp!=null) { + if (lowerStr != null || upperStr != null || prefix != null || regexp != null) { throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, String.format(Locale.ROOT, "The terms component does not support Points-based fields with sorting or with parameters %s,%s,%s,%s ", TermsParams.TERMS_LOWER, TermsParams.TERMS_UPPER, TermsParams.TERMS_PREFIX_STR, TermsParams.TERMS_REGEXP_STR)); } @@ -221,7 +222,7 @@ public class TermsComponent extends SearchComponent { } for (CountPair item : queue) { - fieldTerms.add(item.key.toString(), item.val); + fieldTerms.add(Utils.OBJECT_TO_STRING.apply(item.key.toObject()), item.val); } continue; } else { @@ -237,7 +238,7 @@ public class TermsComponent extends SearchComponent { if (count < 0) break; if (count < freqmin || count > freqmax) continue; if (++num > limit) break; - ew.put(mv.toString(), (int)count); // match the numeric type of terms + ew.put(Utils.OBJECT_TO_STRING.apply(mv.toObject()), (int)count); // match the numeric type of terms } }); ***/ @@ -250,7 +251,7 @@ public class TermsComponent extends SearchComponent { if (count < 0) break; if (count < freqmin || count > freqmax) continue; if (++num > limit) break; - fieldTerms.add(mv.toString(), (int)count); // match the numeric type of terms + fieldTerms.add(Utils.OBJECT_TO_STRING.apply(mv.toObject()), (int)count); // match the numeric type of terms } continue; } diff --git a/solr/core/src/java/org/apache/solr/search/PointMerger.java b/solr/core/src/java/org/apache/solr/search/PointMerger.java index 59a6fb38cbc..a2d9ade0151 100644 --- a/solr/core/src/java/org/apache/solr/search/PointMerger.java +++ b/solr/core/src/java/org/apache/solr/search/PointMerger.java @@ -80,6 +80,7 @@ public class PointMerger { seg = new DoubleSeg(pv, capacity); break; case DATE: + seg = new DateSeg(pv, capacity); break; } int count = seg.setNextValue(); diff --git a/solr/core/src/test-files/solr/collection1/conf/schema.xml b/solr/core/src/test-files/solr/collection1/conf/schema.xml index 9f46cec836e..d5cf09035f4 100644 --- a/solr/core/src/test-files/solr/collection1/conf/schema.xml +++ b/solr/core/src/test-files/solr/collection1/conf/schema.xml @@ -841,6 +841,11 @@ + + + + + diff --git a/solr/core/src/test/org/apache/solr/handler/component/DistributedTermsComponentTest.java b/solr/core/src/test/org/apache/solr/handler/component/DistributedTermsComponentTest.java index 329cd80668b..53d074c3109 100644 --- a/solr/core/src/test/org/apache/solr/handler/component/DistributedTermsComponentTest.java +++ b/solr/core/src/test/org/apache/solr/handler/component/DistributedTermsComponentTest.java @@ -40,13 +40,13 @@ public class DistributedTermsComponentTest extends BaseDistributedSearchTestCase Random random = random(); del("*:*"); index(id, random.nextInt(), "b_t", "snake a,b spider shark snail slug seal", "foo_i", "1"); - index(id, random.nextInt(), "b_t", "snake spider shark snail slug", "foo_i", "2"); + index(id, random.nextInt(), "b_t", "snake spider shark snail slug", "foo_i", "2", "foo_date_p", "2015-01-03T14:30:00Z"); index(id, random.nextInt(), "b_t", "snake spider shark snail", "foo_i", "3"); - index(id, random.nextInt(), "b_t", "snake spider shark", "foo_i", "2"); - index(id, random.nextInt(), "b_t", "snake spider", "c_t", "snake spider"); - index(id, random.nextInt(), "b_t", "snake", "c_t", "snake"); - index(id, random.nextInt(), "b_t", "ant zebra", "c_t", "ant zebra"); - index(id, random.nextInt(), "b_t", "zebra", "c_t", "zebra"); + index(id, random.nextInt(), "b_t", "snake spider shark", "foo_i", "2", "foo_date_p", "2014-03-15T12:00:00Z"); + index(id, random.nextInt(), "b_t", "snake spider", "c_t", "snake spider", "foo_date_p", "2014-03-15T12:00:00Z"); + index(id, random.nextInt(), "b_t", "snake", "c_t", "snake", "foo_date_p", "2014-03-15T12:00:00Z"); + index(id, random.nextInt(), "b_t", "ant zebra", "c_t", "ant zebra", "foo_date_p", "2015-01-03T14:30:00Z"); + index(id, random.nextInt(), "b_t", "zebra", "c_t", "zebra", "foo_date_p", "2015-01-03T14:30:00Z"); commit(); handle.clear(); @@ -66,6 +66,11 @@ public class DistributedTermsComponentTest extends BaseDistributedSearchTestCase query("qt", "/terms", "shards.qt", "/terms", "terms", "true", "terms.fl", "foo_i", "terms.stats", "true","terms.list", "2,3,1"); query("qt", "/terms", "shards.qt", "/terms", "terms", "true", "terms.fl", "b_t", "terms.list", "snake,zebra", "terms.ttf", "true"); query("qt", "/terms", "shards.qt", "/terms", "terms", "true", "terms.fl", "b_t", "terms.fl", "c_t", "terms.list", "snake,ant,zebra", "terms.ttf", "true"); + + // for date point field + query("qt", "/terms", "shards.qt", "/terms", "terms", "true", "terms.fl", "foo_date_p"); + // terms.ttf=true doesn't work for point fields + //query("qt", "/terms", "shards.qt", "/terms", "terms", "true", "terms.fl", "foo_date_p", "terms.ttf", "true"); } protected QueryResponse query(Object... q) throws Exception { diff --git a/solr/core/src/test/org/apache/solr/handler/component/TermsComponentTest.java b/solr/core/src/test/org/apache/solr/handler/component/TermsComponentTest.java index aeb7273cc9f..5ad14298099 100644 --- a/solr/core/src/test/org/apache/solr/handler/component/TermsComponentTest.java +++ b/solr/core/src/test/org/apache/solr/handler/component/TermsComponentTest.java @@ -582,4 +582,28 @@ public class TermsComponentTest extends SolrTestCaseJ4 { private static String createShardQueryParamsString(ModifiableSolrParams params) { return TermsComponent.createShardQuery(params).params.toString(); } + + @Test + public void testDatePointField() throws Exception { + String[] dates = new String[]{"2015-01-03T14:30:00Z", "2014-03-15T12:00:00Z"}; + for (int i = 0; i < 100; i++) { + assertU(adoc("id", Integer.toString(100000+i), "foo_pdt", dates[i % 2]) ); + if (random().nextInt(10) == 0) assertU(commit()); // make multiple segments + } + assertU(commit()); + assertU(adoc("id", Integer.toString(100102), "foo_pdt", dates[1])); + assertU(commit()); + + try { + assertQ(req("indent","true", "qt","/terms", "terms","true", + "terms.fl","foo_pdt", "terms.sort","count"), + "count(//lst[@name='foo_pdt']/*)=2", + "//lst[@name='foo_pdt']/int[1][@name='" + dates[1] + "'][.='51']", + "//lst[@name='foo_pdt']/int[2][@name='" + dates[0] + "'][.='50']" + ); + } finally { + assertU(delQ("foo_pdt:[* TO *]")); + assertU(commit()); + } + } } diff --git a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java index db6ef378d99..477f68ae471 100644 --- a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java +++ b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java @@ -36,12 +36,14 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.Date; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.NoSuchElementException; +import java.util.Objects; import java.util.Set; import java.util.TreeMap; import java.util.TreeSet; @@ -324,6 +326,13 @@ public class Utils { } }; + /** + * Util function to convert {@link Object} to {@link String} + * Specially handles {@link Date} to string conversion + */ + public static final Function OBJECT_TO_STRING = + obj -> ((obj instanceof Date) ? Objects.toString(((Date) obj).toInstant()) : Objects.toString(obj)); + public static Object fromJSON(InputStream is, Function objBuilderProvider) { try { return objBuilderProvider.apply(getJSONParser((new InputStreamReader(is, StandardCharsets.UTF_8)))).getVal(); @@ -646,7 +655,6 @@ public class Utils { * @param input the json with new values * @return whether there was any change made to sink or not. */ - public static boolean mergeJson(Map sink, Map input) { boolean isModified = false; for (Map.Entry e : input.entrySet()) { @@ -728,7 +736,7 @@ public class Utils { } public static final InputStreamConsumer JAVABINCONSUMER = is -> new JavaBinCodec().unmarshal(is); - public static final InputStreamConsumer JSONCONSUMER = is -> Utils.fromJSON(is); + public static final InputStreamConsumer JSONCONSUMER = Utils::fromJSON; public static InputStreamConsumer newBytesConsumer(int maxSize) { return is -> {