From c8b02e88f8c3b485e68dca583277eda4db4df710 Mon Sep 17 00:00:00 2001 From: Bereng Date: Mon, 4 Apr 2016 14:13:27 +0200 Subject: [PATCH] SOLR-8939 Date millisecond resolution lost on distributed queries --- solr/CHANGES.txt | 2 + .../handler/component/QueryComponent.java | 19 +- .../org/apache/solr/schema/IndexSchema.java | 44 +- .../StoredFieldsShardRequestFactory.java | 4 +- .../StoredFieldsShardResponseProcessor.java | 7 +- .../solr/collection1/conf/TSid-schema.xml | 687 ++++++++++++++++++ .../apache/solr/TestTSDistributedSearch.java | 64 ++ .../solr/BaseDistributedSearchTestCase.java | 14 +- 8 files changed, 824 insertions(+), 17 deletions(-) create mode 100644 solr/core/src/test-files/solr/collection1/conf/TSid-schema.xml create mode 100644 solr/core/src/test/org/apache/solr/TestTSDistributedSearch.java diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index d79e997e0022..2da4ee20df18 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -67,6 +67,8 @@ New Features Bug Fixes ---------------------- +* SOLR-8939 Date millisecond resolution lost on distributed queries + * SOLR-8857: HdfsUpdateLog does not use configured or new default number of version buckets and is hard coded to 256. (Mark Miller, yonik, Gregory Chanan) diff --git a/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java b/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java index 9cb7b1da50af..bdf1cdb72e27 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java +++ b/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java @@ -1115,9 +1115,9 @@ protected void mergeIds(ResponseBuilder rb, ShardRequest sreq) { for (int i=resultSize-1; i>=0; i--) { ShardDoc shardDoc = queue.pop(); shardDoc.positionInResponse = i; - // Need the toString() for correlation with other lists that must + // Need the IndexSchema#printableUniqueKey() for correlation with other lists that must // be strings (like keys in highlighting, explain, etc) - resultIds.put(shardDoc.id.toString(), shardDoc); + resultIds.put(rb.req.getSchema().printableUniqueKey(uniqueKeyField, shardDoc.id), shardDoc); } // Add hits for distributed requests @@ -1290,8 +1290,9 @@ protected void createRetrieveDocs(ResponseBuilder rb) { ArrayList ids = new ArrayList<>(shardDocs.size()); for (ShardDoc shardDoc : shardDocs) { - // TODO: depending on the type, we may need more tha a simple toString()? - ids.add(shardDoc.id.toString()); + // Need the IndexSchema#printableUniqueKey() for correlation with other lists that must + // be strings (like keys in highlighting, explain, etc) + ids.add(rb.req.getSchema().printableUniqueKey(uniqueField, shardDoc.id)); } sreq.params.add(ShardParams.IDS, StrUtils.join(ids, ',')); @@ -1310,8 +1311,8 @@ protected void returnFields(ResponseBuilder rb, ShardRequest sreq) { if ((sreq.purpose & ShardRequest.PURPOSE_GET_FIELDS) != 0) { boolean returnScores = (rb.getFieldFlags() & SolrIndexSearcher.GET_SCORES) != 0; - String keyFieldName = rb.req.getSchema().getUniqueKeyField().getName(); - boolean removeKeyField = !rb.rsp.getReturnFields().wantsField(keyFieldName); + SchemaField uniqueKeyField = rb.req.getSchema().getUniqueKeyField(); + boolean removeKeyField = !rb.rsp.getReturnFields().wantsField(uniqueKeyField.getName()); for (ShardResponse srsp : sreq.responses) { if (srsp.getException() != null) { @@ -1339,7 +1340,9 @@ protected void returnFields(ResponseBuilder rb, ShardRequest sreq) { SolrDocumentList docs = (SolrDocumentList) srsp.getSolrResponse().getResponse().get("response"); for (SolrDocument doc : docs) { - Object id = doc.getFieldValue(keyFieldName); + // Need the IndexSchema#printableUniqueKey() for correlation with other lists that must + // be strings (like keys in highlighting, explain, etc) + Object id = rb.req.getSchema().printableUniqueKey(uniqueKeyField, doc.getFieldValue(uniqueKeyField.getName())); ShardDoc sdoc = rb.resultIds.get(id.toString()); if (sdoc != null) { if (returnScores) { @@ -1350,7 +1353,7 @@ protected void returnFields(ResponseBuilder rb, ShardRequest sreq) { doc.remove("score"); } if (removeKeyField) { - doc.removeFields(keyFieldName); + doc.removeFields(uniqueKeyField.getName()); } rb.getResponseDocs().set(sdoc.positionInResponse, doc); } diff --git a/solr/core/src/java/org/apache/solr/schema/IndexSchema.java b/solr/core/src/java/org/apache/solr/schema/IndexSchema.java index 4319c3ebba33..713352f6eff4 100644 --- a/solr/core/src/java/org/apache/solr/schema/IndexSchema.java +++ b/solr/core/src/java/org/apache/solr/schema/IndexSchema.java @@ -328,7 +328,49 @@ public IndexableField getUniqueKeyField(org.apache.lucene.document.Document doc) */ public String printableUniqueKey(org.apache.lucene.document.Document doc) { IndexableField f = doc.getField(uniqueKeyFieldName); - return f==null ? null : uniqueKeyFieldType.toExternal(f); + return printableUniqueKey(f); + } + + /** + * The printable value of the Unique Key field for + * the specified IndexableField + * @throws SolrException if the field is not the unique key field + * @return null if the field is null or the printable representation of the field + */ + public String printableUniqueKey(IndexableField f) { + String res = null; + + if (f!=null){ + if (f.name().equals(uniqueKeyFieldName)) { + res = uniqueKeyFieldType.toExternal(f); + } + else { + throw new SolrException(ErrorCode.BAD_REQUEST, "IndexableField [" + f.toString() + "] is not the unique key field [" + uniqueKeyFieldName + "]"); + } + } + + return res; + } + + /** + * The printable value of the Unique Key field for + * the specified SchemaField + * @throws SolrException if the field is not the unique key field + * @return null if the field is null or the printable representation of the field + */ + public String printableUniqueKey(SchemaField f, Object value) { + String res = null; + + if (f!=null){ + if (f.getName().equals(uniqueKeyFieldName)) { + res = printableUniqueKey(f.createField(value, 1.0f)); + } + else { + throw new SolrException(ErrorCode.BAD_REQUEST, "SchemaField [" + f.toString() + "] is not the unique key field [" + uniqueKeyFieldName + "]"); + } + } + + return res; } private SchemaField getIndexedField(String fname) { diff --git a/solr/core/src/java/org/apache/solr/search/grouping/distributed/requestfactory/StoredFieldsShardRequestFactory.java b/solr/core/src/java/org/apache/solr/search/grouping/distributed/requestfactory/StoredFieldsShardRequestFactory.java index 497d0e1d6adb..0eece782d768 100644 --- a/solr/core/src/java/org/apache/solr/search/grouping/distributed/requestfactory/StoredFieldsShardRequestFactory.java +++ b/solr/core/src/java/org/apache/solr/search/grouping/distributed/requestfactory/StoredFieldsShardRequestFactory.java @@ -76,7 +76,9 @@ public ShardRequest[] constructRequest(ResponseBuilder rb) { List ids = new ArrayList<>(shardDocs.size()); for (ShardDoc shardDoc : shardDocs) { - ids.add(shardDoc.id.toString()); + // Need the IndexSchema#printableUniqueKey() for correlation with other lists that must + // be strings (like keys in highlighting, explain, etc) + ids.add(rb.req.getSchema().printableUniqueKey(uniqueField, shardDoc.id)); } sreq.params.add(ShardParams.IDS, StrUtils.join(ids, ',')); shardRequests[i++] = sreq; diff --git a/solr/core/src/java/org/apache/solr/search/grouping/distributed/responseprocessor/StoredFieldsShardResponseProcessor.java b/solr/core/src/java/org/apache/solr/search/grouping/distributed/responseprocessor/StoredFieldsShardResponseProcessor.java index dcb3c617ee9c..d8d5ecc02c1b 100644 --- a/solr/core/src/java/org/apache/solr/search/grouping/distributed/responseprocessor/StoredFieldsShardResponseProcessor.java +++ b/solr/core/src/java/org/apache/solr/search/grouping/distributed/responseprocessor/StoredFieldsShardResponseProcessor.java @@ -23,6 +23,7 @@ import org.apache.solr.handler.component.ShardDoc; import org.apache.solr.handler.component.ShardRequest; import org.apache.solr.handler.component.ShardResponse; +import org.apache.solr.schema.SchemaField; import org.apache.solr.search.SolrIndexSearcher; import org.apache.solr.search.grouping.distributed.ShardResponseProcessor; @@ -39,10 +40,12 @@ public void process(ResponseBuilder rb, ShardRequest shardRequest) { boolean returnScores = (rb.getFieldFlags() & SolrIndexSearcher.GET_SCORES) != 0; ShardResponse srsp = shardRequest.responses.get(0); SolrDocumentList docs = (SolrDocumentList)srsp.getSolrResponse().getResponse().get("response"); - String uniqueIdFieldName = rb.req.getSchema().getUniqueKeyField().getName(); + SchemaField uniqueKeyField = rb.req.getSchema().getUniqueKeyField(); for (SolrDocument doc : docs) { - Object id = doc.getFieldValue(uniqueIdFieldName).toString(); + // Need the IndexSchema#printableUniqueKey() for correlation with other lists that must + // be strings (like keys in highlighting, explain, etc) + Object id = rb.req.getSchema().printableUniqueKey(uniqueKeyField, doc.getFieldValue(uniqueKeyField.getName())); ShardDoc shardDoc = rb.resultIds.get(id); FieldDoc fieldDoc = (FieldDoc) shardDoc; if (shardDoc != null) { diff --git a/solr/core/src/test-files/solr/collection1/conf/TSid-schema.xml b/solr/core/src/test-files/solr/collection1/conf/TSid-schema.xml new file mode 100644 index 000000000000..9c2c6f4d40e9 --- /dev/null +++ b/solr/core/src/test-files/solr/collection1/conf/TSid-schema.xml @@ -0,0 +1,687 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + text + id + + + + + + + + + + + + + + + + + + + + + + + + + + + + I am your default sim + + diff --git a/solr/core/src/test/org/apache/solr/TestTSDistributedSearch.java b/solr/core/src/test/org/apache/solr/TestTSDistributedSearch.java new file mode 100644 index 000000000000..e9d3683243fc --- /dev/null +++ b/solr/core/src/test/org/apache/solr/TestTSDistributedSearch.java @@ -0,0 +1,64 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr; + +import org.junit.Test; + +import org.apache.lucene.util.LuceneTestCase.Slow; +import org.apache.solr.client.solrj.response.QueryResponse; +import org.apache.solr.common.SolrDocumentList; +import org.apache.solr.common.params.ModifiableSolrParams; + +/** + * Tests uniqueKey Date docs work down to milliseconds + */ +@Slow +public class TestTSDistributedSearch extends BaseDistributedSearchTestCase { + + @Test + public void test() throws Exception { + del("*:*"); + indexr(id,"2010-05-02T11:00:00.222Z"); + indexr(id,"2010-05-02T11:00:00.333Z"); + commit(); + + // We should return 2 docs, millisecond resolution should be preserved + ModifiableSolrParams q = new ModifiableSolrParams(); + q.set("q", "*:*"); + setDistributedParams(q); + QueryResponse rsp = queryServer(q); + assertEquals(2, rsp.getResults().size()); + assertEquals(2, ((SolrDocumentList)rsp.getResponse().get("response")).getNumFound()); + + // Both distributed and non distributed paths should behave the same + handle.clear(); + handle.put("maxScore", SKIPVAL); + handle.put("QTime", SKIPVAL); + handle.put("timestamp", SKIPVAL); + handle.put("_version_", SKIPVAL); + query("q","*:*", "sort", "id asc"); + } + + @Override + protected String getSchemaFileOverride() { + return "TSid-schema.xml"; + } + +} + + diff --git a/solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java b/solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java index a751459e5964..c400dc2fcc94 100644 --- a/solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java +++ b/solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java @@ -314,12 +314,12 @@ public void distribTearDown() throws Exception { destroyServers(); } - protected JettySolrRunner createControlJetty() throws Exception { + protected JettySolrRunner createControlJetty(String schemaOverride) throws Exception { Path jettyHome = testDir.toPath().resolve("control"); File jettyHomeFile = jettyHome.toFile(); seedSolrHome(jettyHomeFile); seedCoreRootDirWithDefaultTestCore(jettyHome.resolve("cores")); - JettySolrRunner jetty = createJetty(jettyHomeFile, null, null, getSolrConfigFile(), getSchemaFile()); + JettySolrRunner jetty = createJetty(jettyHomeFile, null, null, getSolrConfigFile(), schemaOverride); return jetty; } @@ -327,7 +327,8 @@ protected void createServers(int numShards) throws Exception { System.setProperty("configSetBaseDir", getSolrHome()); - controlJetty = createControlJetty(); + String schemaFile = getSchemaFileOverride() == null ? getSchemaFile() : getSchemaFileOverride(); + controlJetty = createControlJetty(schemaFile); controlClient = createNewSolrClient(controlJetty.getLocalPort()); shardsArr = new String[numShards]; @@ -339,7 +340,7 @@ protected void createServers(int numShards) throws Exception { File jettyHomeFile = jettyHome.toFile(); seedSolrHome(jettyHomeFile); seedCoreRootDirWithDefaultTestCore(jettyHome.resolve("cores")); - JettySolrRunner j = createJetty(jettyHomeFile, null, null, getSolrConfigFile(), getSchemaFile()); + JettySolrRunner j = createJetty(jettyHomeFile, null, null, getSolrConfigFile(), schemaFile); jettys.add(j); clients.add(createNewSolrClient(j.getLocalPort())); String shardStr = buildUrl(j.getLocalPort()) + "/" + DEFAULT_TEST_CORENAME; @@ -349,7 +350,10 @@ protected void createServers(int numShards) throws Exception { shards = sb.toString(); } - + + protected String getSchemaFileOverride() { + return null; + } protected void setDistributedParams(ModifiableSolrParams params) { params.set("shards", getShardsString());