SOLR-12096: Fixed inconsistent results format of subquery transformer for distributed search (multi-shard)

This commit is contained in:
Ishan Chattopadhyaya 2018-04-09 16:36:07 +05:30
parent a7a3c0a282
commit ea08bd3b67
6 changed files with 289 additions and 23 deletions

217
SOLR-12096.patch Normal file
View File

@ -0,0 +1,217 @@
diff --git a/solr/core/src/java/org/apache/solr/response/GeoJSONResponseWriter.java b/solr/core/src/java/org/apache/solr/response/GeoJSONResponseWriter.java
index 43fd7b4..012290e 100644
--- a/solr/core/src/java/org/apache/solr/response/GeoJSONResponseWriter.java
+++ b/solr/core/src/java/org/apache/solr/response/GeoJSONResponseWriter.java
@@ -166,7 +166,8 @@ class GeoJSONWriter extends JSONWriter {
// SolrDocument will now have multiValued fields represented as a Collection,
// even if only a single value is returned for this document.
- if (val instanceof List) {
+ // For SolrDocumentList, use writeVal instead of writeArray
+ if (!(val instanceof SolrDocumentList) && val instanceof List) {
// shortcut this common case instead of going through writeVal again
writeArray(name,((Iterable)val).iterator());
} else {
diff --git a/solr/core/src/java/org/apache/solr/response/JSONResponseWriter.java b/solr/core/src/java/org/apache/solr/response/JSONResponseWriter.java
index 513df4e..5f6e2f2 100644
--- a/solr/core/src/java/org/apache/solr/response/JSONResponseWriter.java
+++ b/solr/core/src/java/org/apache/solr/response/JSONResponseWriter.java
@@ -25,10 +25,11 @@ import java.util.Map;
import java.util.Set;
import org.apache.solr.common.IteratorWriter;
+import org.apache.solr.common.MapWriter;
import org.apache.solr.common.MapWriter.EntryWriter;
import org.apache.solr.common.PushWriter;
import org.apache.solr.common.SolrDocument;
-import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.SolrDocumentList;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.common.util.NamedList;
import org.apache.solr.common.util.SimpleOrderedMap;
@@ -367,7 +368,8 @@ class JSONWriter extends TextResponseWriter {
// SolrDocument will now have multiValued fields represented as a Collection,
// even if only a single value is returned for this document.
- if (val instanceof List) {
+ // For SolrDocumentList, use writeVal instead of writeArray
+ if (!(val instanceof SolrDocumentList) && val instanceof List) {
// shortcut this common case instead of going through writeVal again
writeArray(name,((Iterable)val).iterator());
} else {
diff --git a/solr/core/src/test/org/apache/solr/response/JSONWriterTest.java b/solr/core/src/test/org/apache/solr/response/JSONWriterTest.java
index 1b53150..68cebd2 100644
--- a/solr/core/src/test/org/apache/solr/response/JSONWriterTest.java
+++ b/solr/core/src/test/org/apache/solr/response/JSONWriterTest.java
@@ -22,7 +22,10 @@ import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.List;
+
+import org.apache.solr.JSONTestUtil;
import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.common.SolrDocument;
import org.apache.solr.common.SolrDocumentList;
@@ -130,9 +133,9 @@ public class JSONWriterTest extends SolrTestCaseJ4 {
}
@Test
- public void testJSONSolrDocument() throws IOException {
+ public void testJSONSolrDocument() throws Exception {
SolrQueryRequest req = req(CommonParams.WT,"json",
- CommonParams.FL,"id,score");
+ CommonParams.FL,"id,score,_children_,path");
SolrQueryResponse rsp = new SolrQueryResponse();
JSONResponseWriter w = new JSONResponseWriter();
@@ -141,11 +144,22 @@ public class JSONWriterTest extends SolrTestCaseJ4 {
StringWriter buf = new StringWriter();
+ SolrDocument childDoc = new SolrDocument();
+ childDoc.addField("id", "2");
+ childDoc.addField("score", "0.4");
+ childDoc.addField("path", Arrays.asList("a>b", "a>b>c"));
+
+ SolrDocumentList childList = new SolrDocumentList();
+ childList.setNumFound(1);
+ childList.setStart(0);
+ childList.add(childDoc);
+
SolrDocument solrDoc = new SolrDocument();
solrDoc.addField("id", "1");
solrDoc.addField("subject", "hello2");
solrDoc.addField("title", "hello3");
solrDoc.addField("score", "0.7");
+ solrDoc.setField("_children_", childList);
SolrDocumentList list = new SolrDocumentList();
list.setNumFound(1);
@@ -163,8 +177,12 @@ public class JSONWriterTest extends SolrTestCaseJ4 {
result.contains("\"title\""));
assertTrue("response doesn't contain expected fields: " + result,
result.contains("\"id\"") &&
- result.contains("\"score\""));
+ result.contains("\"score\"") && result.contains("_children_"));
+ String expectedResult = "{'response':{'numFound':1,'start':0,'maxScore':0.7,'docs':[{'id':'1', 'score':'0.7'," +
+ " '_children_':{'numFound':1,'start':0,'docs':[{'id':'2', 'score':'0.4', 'path':['a>b', 'a>b>c']}] }}] }}";
+ String error = JSONTestUtil.match(result, "=="+expectedResult);
+ assertNull("response validation failed with error: " + error, error);
req.close();
}
diff --git a/solr/core/src/test/org/apache/solr/response/transform/TestSubQueryTransformerDistrib.java b/solr/core/src/test/org/apache/solr/response/transform/TestSubQueryTransformerDistrib.java
index 620cac0..f6d0a38 100644
--- a/solr/core/src/test/org/apache/solr/response/transform/TestSubQueryTransformerDistrib.java
+++ b/solr/core/src/test/org/apache/solr/response/transform/TestSubQueryTransformerDistrib.java
@@ -16,7 +16,11 @@
*/
package org.apache.solr.response.transform;
+import java.io.ByteArrayOutputStream;
import java.io.IOException;
+import java.io.InputStream;
+import java.net.URL;
+import java.nio.charset.Charset;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
@@ -26,6 +30,8 @@ import java.util.List;
import java.util.Map;
import java.util.Random;
+import org.apache.commons.io.IOUtils;
+import org.apache.solr.JSONTestUtil;
import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.client.solrj.impl.CloudSolrClient;
import org.apache.solr.client.solrj.request.CollectionAdminRequest;
@@ -37,10 +43,12 @@ import org.apache.solr.cloud.SolrCloudTestCase;
import org.apache.solr.common.SolrDocument;
import org.apache.solr.common.SolrDocumentList;
import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.util.ContentStreamBase;
import org.junit.BeforeClass;
import org.junit.Test;
+@org.apache.solr.SolrTestCaseJ4.SuppressSSL()
public class TestSubQueryTransformerDistrib extends SolrCloudTestCase {
private static final String support = "These guys help customers";
@@ -92,7 +100,7 @@ public class TestSubQueryTransformerDistrib extends SolrCloudTestCase {
@SuppressWarnings("serial")
@Test
- public void test() throws SolrServerException, IOException {
+ public void test() throws Exception {
int peopleMultiplier = atLeast(1);
int deptMultiplier = atLeast(1);
@@ -100,24 +108,26 @@ public class TestSubQueryTransformerDistrib extends SolrCloudTestCase {
Random random1 = random();
+ final ModifiableSolrParams params = params(
+ new String[]{"q","name_s:dave", "indent","true",
+ "fl","*,depts:[subquery "+((random1.nextBoolean() ? "" : "separator=,"))+"]",
+ "rows","" + peopleMultiplier,
+ "depts.q","{!terms f=dept_id_s v=$row.dept_ss_dv "+((random1.nextBoolean() ? "" : "separator=,"))+"}",
+ "depts.fl","text_t"+(differentUniqueId?",id:notid":""),
+ "depts.indent","true",
+ "depts.collection","departments",
+ differentUniqueId ? "depts.distrib.singlePass":"notnecessary","true",
+ "depts.rows",""+(deptMultiplier*2),
+ "depts.logParamsList","q,fl,rows,row.dept_ss_dv",
+ random().nextBoolean()?"depts.wt":"whatever",anyWt(),
+ random().nextBoolean()?"wt":"whatever",anyWt()});
+
+ final SolrDocumentList hits;
{
-
- final QueryRequest qr = new QueryRequest(params(
- new String[]{"q","name_s:dave", "indent","true",
- "fl","*,depts:[subquery "+((random1.nextBoolean() ? "" : "separator=,"))+"]",
- "rows","" + peopleMultiplier,
- "depts.q","{!terms f=dept_id_s v=$row.dept_ss_dv "+((random1.nextBoolean() ? "" : "separator=,"))+"}",
- "depts.fl","text_t"+(differentUniqueId?",id:notid":""),
- "depts.indent","true",
- "depts.collection","departments",
- differentUniqueId ? "depts.distrib.singlePass":"notnecessary","true",
- "depts.rows",""+(deptMultiplier*2),
- "depts.logParamsList","q,fl,rows,row.dept_ss_dv",
- random().nextBoolean()?"depts.wt":"whatever",anyWt(),
- random().nextBoolean()?"wt":"whatever",anyWt()}));
+ final QueryRequest qr = new QueryRequest(params);
final QueryResponse rsp = new QueryResponse();
- rsp.setResponse(cluster.getSolrClient().request(qr, people));
- final SolrDocumentList hits = rsp.getResults();
+ rsp.setResponse(cluster.getSolrClient().request(qr, people+","+depts));
+ hits = rsp.getResults();
assertEquals(peopleMultiplier, hits.getNumFound());
@@ -140,6 +150,21 @@ public class TestSubQueryTransformerDistrib extends SolrCloudTestCase {
}
assertEquals(hits.toString(), engineerCount, supportCount);
}
+
+ params.set("wt", "json");
+ final URL node = new URL(cluster.getRandomJetty(random()).getBaseUrl().toString()
+ +"/"+people+"/select"+params.toQueryString());
+
+ try(final InputStream jsonResponse = node.openStream()){
+ final ByteArrayOutputStream outBuffer = new ByteArrayOutputStream();
+ IOUtils.copy(jsonResponse, outBuffer);
+
+ final Object expected = ((SolrDocumentList) hits.get(0).getFieldValue("depts")).get(0).get("text_t");
+ final String err = JSONTestUtil.match("/response/docs/[0]/depts/docs/[0]/text_t"
+ ,outBuffer.toString(Charset.forName("UTF-8").toString()),
+ "\""+expected+"\"");
+ assertNull(err,err);
+ }
}

View File

@ -114,6 +114,9 @@ Bug Fixes
* SOLR-12199: TestReplicationHandler.doTestRepeater(): TEST_PORT interpolation failure:
Server refused connection at: http://127.0.0.1:TEST_PORT/solr (Mikhail Khludnev, Dawid Weiss, Steve Rowe)
* SOLR-12096: Fixed inconsistent results format of subquery transformer for distributed search (multi-shard).
(Munendra S N, Mikhail Khludnev via Ishan Chattopadhyaya)
Optimizations
----------------------

View File

@ -166,7 +166,8 @@ class GeoJSONWriter extends JSONWriter {
// SolrDocument will now have multiValued fields represented as a Collection,
// even if only a single value is returned for this document.
if (val instanceof List) {
// For SolrDocumentList, use writeVal instead of writeArray
if (!(val instanceof SolrDocumentList) && val instanceof List) {
// shortcut this common case instead of going through writeVal again
writeArray(name,((Iterable)val).iterator());
} else {

View File

@ -25,10 +25,11 @@ import java.util.Map;
import java.util.Set;
import org.apache.solr.common.IteratorWriter;
import org.apache.solr.common.MapWriter;
import org.apache.solr.common.MapWriter.EntryWriter;
import org.apache.solr.common.PushWriter;
import org.apache.solr.common.SolrDocument;
import org.apache.solr.common.MapWriter;
import org.apache.solr.common.SolrDocumentList;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.common.util.NamedList;
import org.apache.solr.common.util.SimpleOrderedMap;
@ -367,7 +368,8 @@ class JSONWriter extends TextResponseWriter {
// SolrDocument will now have multiValued fields represented as a Collection,
// even if only a single value is returned for this document.
if (val instanceof List) {
// For SolrDocumentList, use writeVal instead of writeArray
if (!(val instanceof SolrDocumentList) && val instanceof List) {
// shortcut this common case instead of going through writeVal again
writeArray(name,((Iterable)val).iterator());
} else {

View File

@ -22,7 +22,10 @@ import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import org.apache.solr.JSONTestUtil;
import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.common.SolrDocument;
import org.apache.solr.common.SolrDocumentList;
@ -130,9 +133,9 @@ public class JSONWriterTest extends SolrTestCaseJ4 {
}
@Test
public void testJSONSolrDocument() throws IOException {
public void testJSONSolrDocument() throws Exception {
SolrQueryRequest req = req(CommonParams.WT,"json",
CommonParams.FL,"id,score");
CommonParams.FL,"id,score,_children_,path");
SolrQueryResponse rsp = new SolrQueryResponse();
JSONResponseWriter w = new JSONResponseWriter();
@ -141,11 +144,22 @@ public class JSONWriterTest extends SolrTestCaseJ4 {
StringWriter buf = new StringWriter();
SolrDocument childDoc = new SolrDocument();
childDoc.addField("id", "2");
childDoc.addField("score", "0.4");
childDoc.addField("path", Arrays.asList("a>b", "a>b>c"));
SolrDocumentList childList = new SolrDocumentList();
childList.setNumFound(1);
childList.setStart(0);
childList.add(childDoc);
SolrDocument solrDoc = new SolrDocument();
solrDoc.addField("id", "1");
solrDoc.addField("subject", "hello2");
solrDoc.addField("title", "hello3");
solrDoc.addField("score", "0.7");
solrDoc.setField("_children_", childList);
SolrDocumentList list = new SolrDocumentList();
list.setNumFound(1);
@ -163,8 +177,12 @@ public class JSONWriterTest extends SolrTestCaseJ4 {
result.contains("\"title\""));
assertTrue("response doesn't contain expected fields: " + result,
result.contains("\"id\"") &&
result.contains("\"score\""));
result.contains("\"score\"") && result.contains("_children_"));
String expectedResult = "{'response':{'numFound':1,'start':0,'maxScore':0.7,'docs':[{'id':'1', 'score':'0.7'," +
" '_children_':{'numFound':1,'start':0,'docs':[{'id':'2', 'score':'0.4', 'path':['a>b', 'a>b>c']}] }}] }}";
String error = JSONTestUtil.match(result, "=="+expectedResult);
assertNull("response validation failed with error: " + error, error);
req.close();
}

View File

@ -16,7 +16,11 @@
*/
package org.apache.solr.response.transform;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.net.URL;
import java.nio.charset.Charset;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
@ -26,6 +30,8 @@ import java.util.List;
import java.util.Map;
import java.util.Random;
import org.apache.commons.io.IOUtils;
import org.apache.solr.JSONTestUtil;
import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.client.solrj.impl.CloudSolrClient;
import org.apache.solr.client.solrj.request.CollectionAdminRequest;
@ -37,10 +43,12 @@ import org.apache.solr.cloud.SolrCloudTestCase;
import org.apache.solr.common.SolrDocument;
import org.apache.solr.common.SolrDocumentList;
import org.apache.solr.common.cloud.ZkStateReader;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.util.ContentStreamBase;
import org.junit.BeforeClass;
import org.junit.Test;
@org.apache.solr.SolrTestCaseJ4.SuppressSSL()
public class TestSubQueryTransformerDistrib extends SolrCloudTestCase {
private static final String support = "These guys help customers";
@ -92,7 +100,7 @@ public class TestSubQueryTransformerDistrib extends SolrCloudTestCase {
@SuppressWarnings("serial")
@Test
public void test() throws SolrServerException, IOException {
public void test() throws Exception {
int peopleMultiplier = atLeast(1);
int deptMultiplier = atLeast(1);
@ -100,24 +108,26 @@ public class TestSubQueryTransformerDistrib extends SolrCloudTestCase {
Random random1 = random();
final ModifiableSolrParams params = params(
new String[]{"q","name_s:dave", "indent","true",
"fl","*,depts:[subquery "+((random1.nextBoolean() ? "" : "separator=,"))+"]",
"rows","" + peopleMultiplier,
"depts.q","{!terms f=dept_id_s v=$row.dept_ss_dv "+((random1.nextBoolean() ? "" : "separator=,"))+"}",
"depts.fl","text_t"+(differentUniqueId?",id:notid":""),
"depts.indent","true",
"depts.collection","departments",
differentUniqueId ? "depts.distrib.singlePass":"notnecessary","true",
"depts.rows",""+(deptMultiplier*2),
"depts.logParamsList","q,fl,rows,row.dept_ss_dv",
random().nextBoolean()?"depts.wt":"whatever",anyWt(),
random().nextBoolean()?"wt":"whatever",anyWt()});
final SolrDocumentList hits;
{
final QueryRequest qr = new QueryRequest(params(
new String[]{"q","name_s:dave", "indent","true",
"fl","*,depts:[subquery "+((random1.nextBoolean() ? "" : "separator=,"))+"]",
"rows","" + peopleMultiplier,
"depts.q","{!terms f=dept_id_s v=$row.dept_ss_dv "+((random1.nextBoolean() ? "" : "separator=,"))+"}",
"depts.fl","text_t"+(differentUniqueId?",id:notid":""),
"depts.indent","true",
"depts.collection","departments",
differentUniqueId ? "depts.distrib.singlePass":"notnecessary","true",
"depts.rows",""+(deptMultiplier*2),
"depts.logParamsList","q,fl,rows,row.dept_ss_dv",
random().nextBoolean()?"depts.wt":"whatever",anyWt(),
random().nextBoolean()?"wt":"whatever",anyWt()}));
final QueryRequest qr = new QueryRequest(params);
final QueryResponse rsp = new QueryResponse();
rsp.setResponse(cluster.getSolrClient().request(qr, people));
final SolrDocumentList hits = rsp.getResults();
rsp.setResponse(cluster.getSolrClient().request(qr, people+","+depts));
hits = rsp.getResults();
assertEquals(peopleMultiplier, hits.getNumFound());
@ -140,6 +150,21 @@ public class TestSubQueryTransformerDistrib extends SolrCloudTestCase {
}
assertEquals(hits.toString(), engineerCount, supportCount);
}
params.set("wt", "json");
final URL node = new URL(cluster.getRandomJetty(random()).getBaseUrl().toString()
+"/"+people+"/select"+params.toQueryString());
try(final InputStream jsonResponse = node.openStream()){
final ByteArrayOutputStream outBuffer = new ByteArrayOutputStream();
IOUtils.copy(jsonResponse, outBuffer);
final Object expected = ((SolrDocumentList) hits.get(0).getFieldValue("depts")).get(0).get("text_t");
final String err = JSONTestUtil.match("/response/docs/[0]/depts/docs/[0]/text_t"
,outBuffer.toString(Charset.forName("UTF-8").toString()),
"\""+expected+"\"");
assertNull(err,err);
}
}