From 0eb9cbc0084415706240cb3b81d14eac31d7a206 Mon Sep 17 00:00:00 2001 From: Moshe Date: Thu, 30 Aug 2018 10:23:20 +0300 Subject: [PATCH 01/10] SOLR-12722: add fl param to ChildDocTransformer --- .../transform/ChildDocTransformer.java | 9 +++--- .../transform/ChildDocTransformerFactory.java | 6 +++- .../TestChildDocTransformerHierarchy.java | 29 +++++++++++++++++++ 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformer.java b/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformer.java index bffbaf2b9729..97fc3573f50d 100644 --- a/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformer.java +++ b/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformer.java @@ -54,17 +54,16 @@ class ChildDocTransformer extends DocTransformer { private final DocSet childDocSet; private final int limit; private final boolean isNestedSchema; + private final SolrReturnFields childReturnFields; - //TODO ought to be provided/configurable - private final SolrReturnFields childReturnFields = new SolrReturnFields(); - - ChildDocTransformer(String name, BitSetProducer parentsFilter, - DocSet childDocSet, boolean isNestedSchema, int limit) { + ChildDocTransformer(String name, BitSetProducer parentsFilter, DocSet childDocSet, + SolrReturnFields returnFields, boolean isNestedSchema, int limit) { this.name = name; this.parentsFilter = parentsFilter; this.childDocSet = childDocSet; this.limit = limit; this.isNestedSchema = isNestedSchema; + this.childReturnFields = returnFields!=null? returnFields: new SolrReturnFields(); } @Override diff --git a/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformerFactory.java b/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformerFactory.java index 2478c480a22d..df1cb2c422dd 100644 --- a/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformerFactory.java +++ b/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformerFactory.java @@ -33,6 +33,7 @@ import org.apache.solr.schema.SchemaField; import org.apache.solr.search.DocSet; import org.apache.solr.search.QParser; +import org.apache.solr.search.SolrReturnFields; import org.apache.solr.search.SyntaxError; import static org.apache.solr.schema.IndexSchema.NEST_PATH_FIELD_NAME; @@ -106,9 +107,12 @@ public DocTransformer create(String field, SolrParams params, SolrQueryRequest r } } + String childReturnFields = params.get("fl"); + SolrReturnFields childSolrReturnFields = childReturnFields==null? new SolrReturnFields(): new SolrReturnFields(childReturnFields ,req); + int limit = params.getInt( "limit", 10 ); - return new ChildDocTransformer(field, parentsFilter, childDocSet, buildHierarchy, limit); + return new ChildDocTransformer(field, parentsFilter, childDocSet, childSolrReturnFields, buildHierarchy, limit); } private static Query parseQuery(String qstr, SolrQueryRequest req, String param) { diff --git a/solr/core/src/test/org/apache/solr/response/transform/TestChildDocTransformerHierarchy.java b/solr/core/src/test/org/apache/solr/response/transform/TestChildDocTransformerHierarchy.java index b3d93729cb95..acedd0363a60 100644 --- a/solr/core/src/test/org/apache/solr/response/transform/TestChildDocTransformerHierarchy.java +++ b/solr/core/src/test/org/apache/solr/response/transform/TestChildDocTransformerHierarchy.java @@ -297,6 +297,35 @@ public void testNoChildren() throws Exception { "/response/docs/[0]/type_s==cake"); } + @Test + public void testChildReturnFields() throws Exception { + indexSampleData(numberOfDocsPerNestedTest); + + assertJQ(req("q", "test_s:testing", + "sort", "id asc", + "fl", "*, [child childFilter='lonely/lonelyGrandChild/test2_s:secondTest' fl='*, _nest_path_']", + "fq", fqToExcludeNonTestedDocs), + "/response/docs/[0]/test_s==testing", + "/response/docs/[0]/lonelyGrandChild/test2_s==secondTest", + "/response/docs/[0]/lonelyGrandChild/_nest_path_=='lonely#/lonelyGrandChild#'"); + + try(SolrQueryRequest req = req("q", "test_s:testing", "sort", "id asc", + "fl", "id, [child childFilter='lonely/lonelyGrandChild/test2_s:secondTest' fl='_nest_path_']", + "fq", fqToExcludeNonTestedDocs)) { + BasicResultContext res = (BasicResultContext) h.queryAndResponse("/select", req).getResponse(); + Iterator docsStreamer = res.getProcessedDocuments(); + while (docsStreamer.hasNext()) { + SolrDocument doc = docsStreamer.next(); + assertFalse("root docs should not contain fields specified in child return fields", doc.containsKey("_nest_path_")); + SolrDocument childDoc = (SolrDocument) doc.getFieldValue("lonelyGrandChild"); + assertEquals("child doc should only have 1 key", 1, childDoc.keySet().size()); + assertTrue("child docs should contain fields specified in child return fields", childDoc.containsKey("_nest_path_")); + assertEquals("child docs should contain fields specified in child return fields", + "lonely#/lonelyGrandChild#", childDoc.getFieldValue("_nest_path_")); + } + } + } + private void indexSampleData(int numDocs) throws Exception { for(int i = 0; i < numDocs; ++i) { updateJ(generateDocHierarchy(i), params("update.chain", "nested")); From b6369f6dfb5b832d64734c7aabb6fa1ffc73eaa6 Mon Sep 17 00:00:00 2001 From: Moshe Date: Thu, 30 Aug 2018 16:56:39 +0300 Subject: [PATCH 02/10] SOLR-12722: move tests to TestChildDocTransformer --- .../transform/TestChildDocTransformer.java | 33 +++++++++++++++++++ .../TestChildDocTransformerHierarchy.java | 29 ---------------- 2 files changed, 33 insertions(+), 29 deletions(-) diff --git a/solr/core/src/test/org/apache/solr/response/transform/TestChildDocTransformer.java b/solr/core/src/test/org/apache/solr/response/transform/TestChildDocTransformer.java index 385ca4fa02d1..69018b159123 100644 --- a/solr/core/src/test/org/apache/solr/response/transform/TestChildDocTransformer.java +++ b/solr/core/src/test/org/apache/solr/response/transform/TestChildDocTransformer.java @@ -16,9 +16,15 @@ */ package org.apache.solr.response.transform; +import java.util.Collection; +import java.util.Iterator; + import org.apache.lucene.util.TestUtil; import org.apache.solr.SolrTestCaseJ4; +import org.apache.solr.common.SolrDocument; import org.apache.solr.common.SolrInputDocument; +import org.apache.solr.request.SolrQueryRequest; +import org.apache.solr.response.BasicResultContext; import org.junit.After; import org.junit.BeforeClass; import org.junit.Test; @@ -61,6 +67,7 @@ public void testAllParams() throws Exception { testSubQueryJSON(); testChildDocNonStoredDVFields(); + testChildReturnFields(); } private void testChildDoctransformerXML() throws Exception { @@ -250,6 +257,32 @@ private void testChildDocNonStoredDVFields() throws Exception { } + private void testChildReturnFields() throws Exception { + + assertJQ(req("q", "*:*", "fq", "subject:\"parentDocument\" ", + "fl", "*,[child parentFilter=\"subject:parentDocument\" fl=\"intDvoDefault\"]"), + "/response/docs/[0]/intDefault==42", + "/response/docs/[0]/_childDocuments_/[0]/intDvoDefault==42"); + + try(SolrQueryRequest req = req("q", "*:*", "fq", "subject:\"parentDocument\" ", + "fl", "intDefault,[child parentFilter=\"subject:parentDocument\" fl=\"intDvoDefault\"]")) { + BasicResultContext res = (BasicResultContext) h.queryAndResponse("/select", req).getResponse(); + Iterator docsStreamer = res.getProcessedDocuments(); + while (docsStreamer.hasNext()) { + SolrDocument doc = docsStreamer.next(); + assertFalse("root docs should not contain fields specified in child return fields", doc.containsKey("intDvoDefault")); + assertTrue("root docs should contain fields specified in query return fields", doc.containsKey("intDefault")); + Collection childDocs = doc.getChildDocuments(); + for(SolrDocument childDoc: childDocs) { + assertEquals("child doc should only have 1 key", 1, childDoc.keySet().size()); + assertTrue("child docs should contain fields specified in child return fields", childDoc.containsKey("intDvoDefault")); + assertEquals("child docs should contain fields specified in child return fields", + 42, childDoc.getFieldValue("intDvoDefault")); + } + } + } + } + private void createSimpleIndex() { SolrInputDocument parentDocument = new SolrInputDocument(); diff --git a/solr/core/src/test/org/apache/solr/response/transform/TestChildDocTransformerHierarchy.java b/solr/core/src/test/org/apache/solr/response/transform/TestChildDocTransformerHierarchy.java index acedd0363a60..b3d93729cb95 100644 --- a/solr/core/src/test/org/apache/solr/response/transform/TestChildDocTransformerHierarchy.java +++ b/solr/core/src/test/org/apache/solr/response/transform/TestChildDocTransformerHierarchy.java @@ -297,35 +297,6 @@ public void testNoChildren() throws Exception { "/response/docs/[0]/type_s==cake"); } - @Test - public void testChildReturnFields() throws Exception { - indexSampleData(numberOfDocsPerNestedTest); - - assertJQ(req("q", "test_s:testing", - "sort", "id asc", - "fl", "*, [child childFilter='lonely/lonelyGrandChild/test2_s:secondTest' fl='*, _nest_path_']", - "fq", fqToExcludeNonTestedDocs), - "/response/docs/[0]/test_s==testing", - "/response/docs/[0]/lonelyGrandChild/test2_s==secondTest", - "/response/docs/[0]/lonelyGrandChild/_nest_path_=='lonely#/lonelyGrandChild#'"); - - try(SolrQueryRequest req = req("q", "test_s:testing", "sort", "id asc", - "fl", "id, [child childFilter='lonely/lonelyGrandChild/test2_s:secondTest' fl='_nest_path_']", - "fq", fqToExcludeNonTestedDocs)) { - BasicResultContext res = (BasicResultContext) h.queryAndResponse("/select", req).getResponse(); - Iterator docsStreamer = res.getProcessedDocuments(); - while (docsStreamer.hasNext()) { - SolrDocument doc = docsStreamer.next(); - assertFalse("root docs should not contain fields specified in child return fields", doc.containsKey("_nest_path_")); - SolrDocument childDoc = (SolrDocument) doc.getFieldValue("lonelyGrandChild"); - assertEquals("child doc should only have 1 key", 1, childDoc.keySet().size()); - assertTrue("child docs should contain fields specified in child return fields", childDoc.containsKey("_nest_path_")); - assertEquals("child docs should contain fields specified in child return fields", - "lonely#/lonelyGrandChild#", childDoc.getFieldValue("_nest_path_")); - } - } - } - private void indexSampleData(int numDocs) throws Exception { for(int i = 0; i < numDocs; ++i) { updateJ(generateDocHierarchy(i), params("update.chain", "nested")); From f06c951d356476771725a571de08dca25432d4ac Mon Sep 17 00:00:00 2001 From: Moshe Date: Thu, 30 Aug 2018 16:59:59 +0300 Subject: [PATCH 03/10] SOLR-12722: update ChildDocTransformer in solr ref guide --- solr/solr-ref-guide/src/transforming-result-documents.adoc | 1 + 1 file changed, 1 insertion(+) diff --git a/solr/solr-ref-guide/src/transforming-result-documents.adoc b/solr/solr-ref-guide/src/transforming-result-documents.adoc index f9fe80583695..7f656f27ce80 100644 --- a/solr/solr-ref-guide/src/transforming-result-documents.adoc +++ b/solr/solr-ref-guide/src/transforming-result-documents.adoc @@ -137,6 +137,7 @@ When using this transformer, the `parentFilter` parameter must be specified, and * `childFilter` - query to filter which child documents should be included, this can be particularly useful when you have multiple levels of hierarchical documents (default: all children) * `limit` - the maximum number of child documents to be returned per parent document (default: 10) +* `fl` - the field list which the transformer is to return. === [shard] - ShardAugmenterFactory From da0856f1dc41b7ea85ce71801900d63c06422db2 Mon Sep 17 00:00:00 2001 From: Moshe Date: Thu, 30 Aug 2018 17:16:49 +0300 Subject: [PATCH 04/10] SOLR-12722: update ChildDocTransformer default to top query fl --- .../solr/response/transform/ChildDocTransformerFactory.java | 2 +- solr/solr-ref-guide/src/transforming-result-documents.adoc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformerFactory.java b/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformerFactory.java index df1cb2c422dd..8b86b86dc89d 100644 --- a/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformerFactory.java +++ b/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformerFactory.java @@ -108,7 +108,7 @@ public DocTransformer create(String field, SolrParams params, SolrQueryRequest r } String childReturnFields = params.get("fl"); - SolrReturnFields childSolrReturnFields = childReturnFields==null? new SolrReturnFields(): new SolrReturnFields(childReturnFields ,req); + SolrReturnFields childSolrReturnFields = childReturnFields==null? new SolrReturnFields(req): new SolrReturnFields(childReturnFields, req); int limit = params.getInt( "limit", 10 ); diff --git a/solr/solr-ref-guide/src/transforming-result-documents.adoc b/solr/solr-ref-guide/src/transforming-result-documents.adoc index 7f656f27ce80..df9bff4d33a2 100644 --- a/solr/solr-ref-guide/src/transforming-result-documents.adoc +++ b/solr/solr-ref-guide/src/transforming-result-documents.adoc @@ -137,7 +137,7 @@ When using this transformer, the `parentFilter` parameter must be specified, and * `childFilter` - query to filter which child documents should be included, this can be particularly useful when you have multiple levels of hierarchical documents (default: all children) * `limit` - the maximum number of child documents to be returned per parent document (default: 10) -* `fl` - the field list which the transformer is to return. +* `fl` - the field list which the transformer is to return (default: top level "fl"). There is a further limitation in which the fields here should be a subset of those specified by the top level fl param === [shard] - ShardAugmenterFactory From cf6dd2bf1fa98416d0efef34f2cbc1c105a85847 Mon Sep 17 00:00:00 2001 From: Moshe Date: Thu, 30 Aug 2018 17:54:11 +0300 Subject: [PATCH 05/10] SOLR-12722: update ChildDocTransformer default to top query fl --- .../transform/ChildDocTransformerFactory.java | 25 ++++++++++++++++++- .../transform/TestChildDocTransformer.java | 16 ++++++------ .../solr/client/solrj/SolrExampleTests.java | 4 +-- 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformerFactory.java b/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformerFactory.java index 8b86b86dc89d..ecd6c88109fb 100644 --- a/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformerFactory.java +++ b/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformerFactory.java @@ -17,6 +17,8 @@ package org.apache.solr.response.transform; import java.io.IOException; +import java.util.List; +import java.util.stream.Collectors; import org.apache.lucene.search.BooleanClause; import org.apache.lucene.search.BooleanQuery; @@ -29,6 +31,8 @@ import org.apache.solr.common.SolrException; import org.apache.solr.common.SolrException.ErrorCode; import org.apache.solr.common.params.SolrParams; +import org.apache.solr.common.util.StrUtils; +import org.apache.solr.request.LocalSolrQueryRequest; import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.schema.SchemaField; import org.apache.solr.search.DocSet; @@ -108,7 +112,16 @@ public DocTransformer create(String field, SolrParams params, SolrQueryRequest r } String childReturnFields = params.get("fl"); - SolrReturnFields childSolrReturnFields = childReturnFields==null? new SolrReturnFields(req): new SolrReturnFields(childReturnFields, req); + SolrReturnFields childSolrReturnFields; + String[] topLevelFieldsArr = null; + if(childReturnFields == null) { + List topLevelFields = StrUtils.splitSmart(req.getOriginalParams().get("fl", "*"), ',').stream().filter(x -> (!(x.trim().startsWith("[")))).collect(Collectors.toList()); + topLevelFieldsArr = new String[topLevelFields.size()]; + topLevelFields.toArray(topLevelFieldsArr); + childSolrReturnFields = new SolrReturnFields(topLevelFieldsArr, req); + } else { + childSolrReturnFields = new SolrReturnFields(childReturnFields, req); + } int limit = params.getInt( "limit", 10 ); @@ -123,6 +136,16 @@ private static Query parseQuery(String qstr, SolrQueryRequest req, String param) } } +// private static computeChildReturnFields(SolrParams childDocTransFormerParams, SolrQueryRequest req) { +// String childReturnFields = childDocTransFormerParams.get("fl"); +// if(childReturnFields == null) { +// List topLevelFls = StrUtils.splitSmart(req.getParams().get("fl"), '['); +// if(topLevelFls) { +// +// } +// } +// } + // NOTE: THIS FEATURE IS PRESENTLY EXPERIMENTAL; WAIT TO SEE IT IN THE REF GUIDE. FINAL SYNTAX IS TBD. protected static String processPathHierarchyQueryString(String queryString) { // if the filter includes a path string, build a lucene query string to match those specific child documents. diff --git a/solr/core/src/test/org/apache/solr/response/transform/TestChildDocTransformer.java b/solr/core/src/test/org/apache/solr/response/transform/TestChildDocTransformer.java index 69018b159123..3c0644d2c293 100644 --- a/solr/core/src/test/org/apache/solr/response/transform/TestChildDocTransformer.java +++ b/solr/core/src/test/org/apache/solr/response/transform/TestChildDocTransformer.java @@ -98,10 +98,10 @@ private void testChildDoctransformerXML() throws Exception { "fl", "*,[child parentFilter=\"subject:parentDocument\"]"), test1); assertQ(req("q", "*:*", "fq", "subject:\"parentDocument\" ", - "fl", "subject,[child parentFilter=\"subject:parentDocument\" childFilter=\"title:foo\"]"), test2); + "fl", "id, subject,[child parentFilter=\"subject:parentDocument\" childFilter=\"title:foo\"]"), test2); assertQ(req("q", "*:*", "fq", "subject:\"parentDocument\" ", - "fl", "subject,[child parentFilter=\"subject:parentDocument\" childFilter=\"title:bar\" limit=2]"), test3); + "fl", "id, subject,[child parentFilter=\"subject:parentDocument\" childFilter=\"title:bar\" limit=2]"), test3); } private void testSubQueryXML() { @@ -219,10 +219,10 @@ private void testChildDoctransformerJSON() throws Exception { "fl", "*,[child parentFilter=\"subject:parentDocument\"]"), test1); assertJQ(req("q", "*:*", "fq", "subject:\"parentDocument\" ", - "fl", "subject,[child parentFilter=\"subject:parentDocument\" childFilter=\"title:foo\"]"), test2); + "fl", "id, subject,[child parentFilter=\"subject:parentDocument\" childFilter=\"title:foo\"]"), test2); assertJQ(req("q", "*:*", "fq", "subject:\"parentDocument\" ", - "fl", "subject,[child parentFilter=\"subject:parentDocument\" childFilter=\"title:bar\" limit=3]"), test3); + "fl", "id, subject,[child parentFilter=\"subject:parentDocument\" childFilter=\"title:bar\" limit=3]"), test3); } private void testChildDocNonStoredDVFields() throws Exception { @@ -250,10 +250,10 @@ private void testChildDocNonStoredDVFields() throws Exception { "fl", "*,[child parentFilter=\"subject:parentDocument\"]"), test1); assertJQ(req("q", "*:*", "fq", "subject:\"parentDocument\" ", - "fl", "subject,[child parentFilter=\"subject:parentDocument\" childFilter=\"title:foo\"]"), test2); + "fl", "intDvoDefault, subject,[child parentFilter=\"subject:parentDocument\" childFilter=\"title:foo\"]"), test2); assertJQ(req("q", "*:*", "fq", "subject:\"parentDocument\" ", - "fl", "subject,[child parentFilter=\"subject:parentDocument\" childFilter=\"title:bar\" limit=2]"), test3); + "fl", "intDvoDefault, subject,[child parentFilter=\"subject:parentDocument\" childFilter=\"title:bar\" limit=2]"), test3); } @@ -372,7 +372,7 @@ private void testParentFilterJSON() throws Exception { assertJQ(req("q", "*:*", "sort", "id asc", "fq", "subject:\"parentDocument\" ", - "fl", "id,[child childFilter='cat:childDocument' parentFilter=\"subject:parentDocument\"]"), + "fl", "id, cat, title, [child childFilter='cat:childDocument' parentFilter=\"subject:parentDocument\"]"), tests); } @@ -431,7 +431,7 @@ private void testParentFilterXML() { assertQ(req("q", "*:*", "sort", "id asc", "fq", "subject:\"parentDocument\" ", - "fl", "id,[child childFilter='cat:childDocument' parentFilter=\"subject:parentDocument\"]"), + "fl", "id, cat, title, [child childFilter='cat:childDocument' parentFilter=\"subject:parentDocument\"]"), tests); } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java b/solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java index f6628da5c1dd..1dabe5de97c2 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java @@ -1831,7 +1831,7 @@ public void testChildDoctransformer() throws IOException, SolrServerException { q = new SolrQuery("q", "*:*", "indent", "true"); q.setFilterQueries(parentFilter); - q.setFields("id,[child parentFilter=\"" + parentFilter + + q.setFields("id, level_i, [child parentFilter=\"" + parentFilter + "\" childFilter=\"" + childFilter + "\" limit=\"" + maxKidCount + "\"], name"); resp = client.query(q); @@ -1916,7 +1916,7 @@ public void testChildDoctransformer() throws IOException, SolrServerException { q = new SolrQuery("q", "name:" + name, "indent", "true"); } q.setFilterQueries(parentFilter); - q.setFields("id,[child parentFilter=\"" + parentFilter + + q.setFields("id, level_i, [child parentFilter=\"" + parentFilter + "\" childFilter=\"" + childFilter + "\" limit=\"" + maxKidCount + "\"],name"); resp = client.query(q); From 0cf214e849b28958465db6807f529c5fe50eb269 Mon Sep 17 00:00:00 2001 From: Moshe Date: Sun, 2 Sep 2018 08:23:43 +0300 Subject: [PATCH 06/10] SOLR-12722: add threadlocal to ensure StackOverflow doesnot occur --- .../transform/ChildDocTransformerFactory.java | 47 +++++++++---------- .../response/transform/DocTransformer.java | 23 +++++++++ .../transform/RawValueTransformerFactory.java | 20 +------- 3 files changed, 47 insertions(+), 43 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformerFactory.java b/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformerFactory.java index ecd6c88109fb..7c53b9ccd86d 100644 --- a/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformerFactory.java +++ b/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformerFactory.java @@ -17,8 +17,6 @@ package org.apache.solr.response.transform; import java.io.IOException; -import java.util.List; -import java.util.stream.Collectors; import org.apache.lucene.search.BooleanClause; import org.apache.lucene.search.BooleanQuery; @@ -31,8 +29,6 @@ import org.apache.solr.common.SolrException; import org.apache.solr.common.SolrException.ErrorCode; import org.apache.solr.common.params.SolrParams; -import org.apache.solr.common.util.StrUtils; -import org.apache.solr.request.LocalSolrQueryRequest; import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.schema.SchemaField; import org.apache.solr.search.DocSet; @@ -62,12 +58,34 @@ public class ChildDocTransformerFactory extends TransformerFactory { static final char PATH_SEP_CHAR = '/'; static final char NUM_SEP_CHAR = '#'; + private static final ThreadLocal transformerInitialized = new ThreadLocal(){ + @Override + protected Boolean initialValue() { + this.set(false); + return false; + } + }; private static final BooleanQuery rootFilter = new BooleanQuery.Builder() .add(new BooleanClause(new MatchAllDocsQuery(), BooleanClause.Occur.MUST)) .add(new BooleanClause(new DocValuesFieldExistsQuery(NEST_PATH_FIELD_NAME), BooleanClause.Occur.MUST_NOT)).build(); @Override public DocTransformer create(String field, SolrParams params, SolrQueryRequest req) { + if(transformerInitialized.get()) { + // this is a recursive call by SolrReturnFields see #createChildDocTransformer + return new DocTransformer.NoopFieldTransformer(); + } else { + try { + // transformer is yet to be initialized in this thread, create it + transformerInitialized.set(true); + return createChildDocTransformer(field, params, req); + } finally { + transformerInitialized.remove(); + } + } + } + + private DocTransformer createChildDocTransformer(String field, SolrParams params, SolrQueryRequest req) { SchemaField uniqueKeyField = req.getSchema().getUniqueKeyField(); if (uniqueKeyField == null) { throw new SolrException( ErrorCode.BAD_REQUEST, @@ -112,16 +130,7 @@ public DocTransformer create(String field, SolrParams params, SolrQueryRequest r } String childReturnFields = params.get("fl"); - SolrReturnFields childSolrReturnFields; - String[] topLevelFieldsArr = null; - if(childReturnFields == null) { - List topLevelFields = StrUtils.splitSmart(req.getOriginalParams().get("fl", "*"), ',').stream().filter(x -> (!(x.trim().startsWith("[")))).collect(Collectors.toList()); - topLevelFieldsArr = new String[topLevelFields.size()]; - topLevelFields.toArray(topLevelFieldsArr); - childSolrReturnFields = new SolrReturnFields(topLevelFieldsArr, req); - } else { - childSolrReturnFields = new SolrReturnFields(childReturnFields, req); - } + SolrReturnFields childSolrReturnFields = childReturnFields==null? new SolrReturnFields(req): new SolrReturnFields(childReturnFields, req); int limit = params.getInt( "limit", 10 ); @@ -136,16 +145,6 @@ private static Query parseQuery(String qstr, SolrQueryRequest req, String param) } } -// private static computeChildReturnFields(SolrParams childDocTransFormerParams, SolrQueryRequest req) { -// String childReturnFields = childDocTransFormerParams.get("fl"); -// if(childReturnFields == null) { -// List topLevelFls = StrUtils.splitSmart(req.getParams().get("fl"), '['); -// if(topLevelFls) { -// -// } -// } -// } - // NOTE: THIS FEATURE IS PRESENTLY EXPERIMENTAL; WAIT TO SEE IT IN THE REF GUIDE. FINAL SYNTAX IS TBD. protected static String processPathHierarchyQueryString(String queryString) { // if the filter includes a path string, build a lucene query string to match those specific child documents. diff --git a/solr/core/src/java/org/apache/solr/response/transform/DocTransformer.java b/solr/core/src/java/org/apache/solr/response/transform/DocTransformer.java index 7665acd560f4..f1f6bf3ea371 100644 --- a/solr/core/src/java/org/apache/solr/response/transform/DocTransformer.java +++ b/solr/core/src/java/org/apache/solr/response/transform/DocTransformer.java @@ -114,4 +114,27 @@ public String[] getExtraRequestFields() { public String toString() { return getName(); } + + /** + * Trivial Impl that ensure that the specified field is requested as an "extra" field, + * but then does nothing during the transformation phase. + */ + public static final class NoopFieldTransformer extends DocTransformer { + final String field; + + public NoopFieldTransformer() { + this.field = null; + } + + public NoopFieldTransformer(String field ) { + this.field = field; + } + public String getName() { return "noop"; } + public String[] getExtraRequestFields() { + return this.field==null? null: new String[] { field }; + } + public void transform(SolrDocument doc, int docid) { + // No-Op + } + } } diff --git a/solr/core/src/java/org/apache/solr/response/transform/RawValueTransformerFactory.java b/solr/core/src/java/org/apache/solr/response/transform/RawValueTransformerFactory.java index 25fe56ce9a28..55216e5b8ecb 100644 --- a/solr/core/src/java/org/apache/solr/response/transform/RawValueTransformerFactory.java +++ b/solr/core/src/java/org/apache/solr/response/transform/RawValueTransformerFactory.java @@ -84,28 +84,10 @@ public DocTransformer create(String display, SolrParams params, SolrQueryRequest if (field.equals(display)) { // we have to ensure the field is returned - return new NoopFieldTransformer(field); + return new DocTransformer.NoopFieldTransformer(field); } return new RenameFieldTransformer( field, display, false ); } - - /** - * Trivial Impl that ensure that the specified field is requested as an "extra" field, - * but then does nothing during the transformation phase. - */ - private static final class NoopFieldTransformer extends DocTransformer { - final String field; - public NoopFieldTransformer(String field ) { - this.field = field; - } - public String getName() { return "noop"; } - public String[] getExtraRequestFields() { - return new String[] { field }; - } - public void transform(SolrDocument doc, int docid) { - // No-Op - } - } static class RawTransformer extends DocTransformer { From 3f85fa67f57a480f2bcadf2de1de8193799e9745 Mon Sep 17 00:00:00 2001 From: Moshe Date: Sun, 2 Sep 2018 10:47:18 +0300 Subject: [PATCH 07/10] SOLR-12722: run transformer provided by childdoctransformer fl --- .../solr/response/transform/ChildDocTransformer.java | 6 ++++++ .../response/transform/TestChildDocTransformer.java | 10 ++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformer.java b/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformer.java index 97fc3573f50d..566f605b5171 100644 --- a/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformer.java +++ b/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformer.java @@ -131,6 +131,12 @@ public void transform(SolrDocument rootDoc, int rootDocId) { // load the doc SolrDocument doc = searcher.getDocFetcher().solrDoc(docId, childReturnFields); + if(childReturnFields.getTransformer() != null) { + if(childReturnFields.getTransformer().context != this.context) { + childReturnFields.getTransformer().setContext(context); + } + childReturnFields.getTransformer().transform(doc, docId); + } if (isAncestor) { // if this path has pending child docs, add them. diff --git a/solr/core/src/test/org/apache/solr/response/transform/TestChildDocTransformer.java b/solr/core/src/test/org/apache/solr/response/transform/TestChildDocTransformer.java index 3c0644d2c293..85b476a2bea7 100644 --- a/solr/core/src/test/org/apache/solr/response/transform/TestChildDocTransformer.java +++ b/solr/core/src/test/org/apache/solr/response/transform/TestChildDocTransformer.java @@ -260,12 +260,13 @@ private void testChildDocNonStoredDVFields() throws Exception { private void testChildReturnFields() throws Exception { assertJQ(req("q", "*:*", "fq", "subject:\"parentDocument\" ", - "fl", "*,[child parentFilter=\"subject:parentDocument\" fl=\"intDvoDefault\"]"), + "fl", "*,[child parentFilter=\"subject:parentDocument\" fl=\"intDvoDefault,child_fl:[value v='child_fl_test']\"]"), "/response/docs/[0]/intDefault==42", - "/response/docs/[0]/_childDocuments_/[0]/intDvoDefault==42"); + "/response/docs/[0]/_childDocuments_/[0]/intDvoDefault==42", + "/response/docs/[0]/_childDocuments_/[0]/child_fl=='child_fl_test'"); try(SolrQueryRequest req = req("q", "*:*", "fq", "subject:\"parentDocument\" ", - "fl", "intDefault,[child parentFilter=\"subject:parentDocument\" fl=\"intDvoDefault\"]")) { + "fl", "intDefault,[child parentFilter=\"subject:parentDocument\" fl=\"intDvoDefault, [docid]\"]")) { BasicResultContext res = (BasicResultContext) h.queryAndResponse("/select", req).getResponse(); Iterator docsStreamer = res.getProcessedDocuments(); while (docsStreamer.hasNext()) { @@ -274,10 +275,11 @@ private void testChildReturnFields() throws Exception { assertTrue("root docs should contain fields specified in query return fields", doc.containsKey("intDefault")); Collection childDocs = doc.getChildDocuments(); for(SolrDocument childDoc: childDocs) { - assertEquals("child doc should only have 1 key", 1, childDoc.keySet().size()); + assertEquals("child doc should only have 2 keys", 2, childDoc.keySet().size()); assertTrue("child docs should contain fields specified in child return fields", childDoc.containsKey("intDvoDefault")); assertEquals("child docs should contain fields specified in child return fields", 42, childDoc.getFieldValue("intDvoDefault")); + assertTrue("child docs should contain fields specified in child return fields", childDoc.containsKey("[docid]")); } } } From 23c105fb3d7daa2c3e017986e03c485302cfe83d Mon Sep 17 00:00:00 2001 From: Moshe Date: Sun, 2 Sep 2018 13:19:43 +0300 Subject: [PATCH 08/10] SOLR-12722: ChildDocTransformer needsSolrIndexSearcher --- .../apache/solr/response/transform/ChildDocTransformer.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformer.java b/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformer.java index 566f605b5171..8b5a5774a52a 100644 --- a/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformer.java +++ b/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformer.java @@ -71,6 +71,9 @@ public String getName() { return name; } + @Override + public boolean needsSolrIndexSearcher() { return true; } + @Override public void transform(SolrDocument rootDoc, int rootDocId) { // note: this algorithm works if both if we have have _nest_path_ and also if we don't! From bf6d1a66f491fc933f448056f7f78f7627234af9 Mon Sep 17 00:00:00 2001 From: Moshe Date: Mon, 3 Sep 2018 07:44:50 +0300 Subject: [PATCH 09/10] SOLR-12722: Change default fl param for ChildDocTransformer based on lucene version --- .../transform/ChildDocTransformerFactory.java | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformerFactory.java b/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformerFactory.java index 7c53b9ccd86d..82be49dfa339 100644 --- a/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformerFactory.java +++ b/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformerFactory.java @@ -58,29 +58,23 @@ public class ChildDocTransformerFactory extends TransformerFactory { static final char PATH_SEP_CHAR = '/'; static final char NUM_SEP_CHAR = '#'; - private static final ThreadLocal transformerInitialized = new ThreadLocal(){ - @Override - protected Boolean initialValue() { - this.set(false); - return false; - } - }; + private static final ThreadLocal recursionCheckThreadLocal = ThreadLocal.withInitial(() -> Boolean.FALSE); private static final BooleanQuery rootFilter = new BooleanQuery.Builder() .add(new BooleanClause(new MatchAllDocsQuery(), BooleanClause.Occur.MUST)) .add(new BooleanClause(new DocValuesFieldExistsQuery(NEST_PATH_FIELD_NAME), BooleanClause.Occur.MUST_NOT)).build(); @Override public DocTransformer create(String field, SolrParams params, SolrQueryRequest req) { - if(transformerInitialized.get()) { - // this is a recursive call by SolrReturnFields see #createChildDocTransformer + if(recursionCheckThreadLocal.get()) { + // this is a recursive call by SolrReturnFields, see ChildDocTransformerFactory#createChildDocTransformer return new DocTransformer.NoopFieldTransformer(); } else { try { // transformer is yet to be initialized in this thread, create it - transformerInitialized.set(true); + recursionCheckThreadLocal.set(true); return createChildDocTransformer(field, params, req); } finally { - transformerInitialized.remove(); + recursionCheckThreadLocal.set(false); } } } @@ -130,7 +124,15 @@ private DocTransformer createChildDocTransformer(String field, SolrParams params } String childReturnFields = params.get("fl"); - SolrReturnFields childSolrReturnFields = childReturnFields==null? new SolrReturnFields(req): new SolrReturnFields(childReturnFields, req); + SolrReturnFields childSolrReturnFields; + if(childReturnFields != null) { + childSolrReturnFields = new SolrReturnFields(childReturnFields, req); + } else if(req.getSchema().getDefaultLuceneMatchVersion().major < 8) { + // ensure backwards for versions prior to SOLR 8 + childSolrReturnFields = new SolrReturnFields(); + } else { + childSolrReturnFields = new SolrReturnFields(req); + } int limit = params.getInt( "limit", 10 ); From 407337e8dc475468ffe8fc38cc36d7317285345b Mon Sep 17 00:00:00 2001 From: Moshe Date: Tue, 4 Sep 2018 15:23:37 +0300 Subject: [PATCH 10/10] SOLR-12722: set transformer context if is null for child fl --- .../org/apache/solr/response/transform/ChildDocTransformer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformer.java b/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformer.java index 8b5a5774a52a..2628f75cf295 100644 --- a/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformer.java +++ b/solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformer.java @@ -135,7 +135,7 @@ public void transform(SolrDocument rootDoc, int rootDocId) { // load the doc SolrDocument doc = searcher.getDocFetcher().solrDoc(docId, childReturnFields); if(childReturnFields.getTransformer() != null) { - if(childReturnFields.getTransformer().context != this.context) { + if(childReturnFields.getTransformer().context == null) { childReturnFields.getTransformer().setContext(context); } childReturnFields.getTransformer().transform(doc, docId);